On Fri, May 12, 2017 at 3:22 PM, Paul Moore [off-list ref] wrote:

On Thu, May 11, 2017 at 4:45 PM, Casey Schaufler [off-list ref] wrote:

quoted

On 5/11/2017 1:22 PM, Stephen Smalley wrote:

quoted

On Thu, 2017-05-11 at 08:56 -0700, Casey Schaufler wrote:

quoted

On 5/11/2017 5:59 AM, Sebastien Buisson wrote:

quoted

Add policybrief field to struct policydb. It holds a brief info
of the policydb, in the following form:
<0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
Policy brief is computed every time the policy is loaded, and when
enforce or checkreqprot are changed.

Add security_policy_brief hook to give access to policy brief to
the rest of the kernel. Lustre client makes use of this information
to detect changes to the policy, and forward it to Lustre servers.
Depending on how the policy is enforced on Lustre client side,
Lustre servers can refuse connection.

Signed-off-by: Sebastien Buisson <redacted>
---
 include/linux/lsm_hooks.h           | 16 ++++++++
 include/linux/security.h            |  7 ++++
 security/security.c                 |  6 +++
 security/selinux/hooks.c            |  7 ++++
 security/selinux/include/security.h |  2 +
 security/selinux/selinuxfs.c        |  2 +
 security/selinux/ss/policydb.c      | 76
+++++++++++++++++++++++++++++++++++++
 security/selinux/ss/policydb.h      |  2 +
 security/selinux/ss/services.c      | 62
++++++++++++++++++++++++++++++
 9 files changed, 180 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 080f34e..9cac282 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -1336,6 +1336,20 @@
  * @inode we wish to get the security context of.
  * @ctx is a pointer in which to place the allocated

security context.
  * @ctxlen points to the place to put the length of @ctx.
+ *
+ * Security hooks for policy brief
+ *
+ * @policy_brief:
+ *
+ * Returns a string containing a brief info of the
policydb, in the
+ * following form:
+ * <0 or 1 for enforce>:<0 or 1 for
checkreqprot>:<hashalg>=<checksum>

This sure looks like SELinux specific information. If the Spiffy
security module has multiple values for enforcement (e.g. off,
soft and hard) this interface definition does not work. What is a
"checkreqprot", and what is it for?

I expect that you have no interest (or incentive) in supporting
security modules other than SELinux, and that's OK. What's I'm
after is an interface that another security module could use if
someone where interested (or inspired) to do so.

Rather than a string with predefined positional values (something
I was taught not to do when 1 MIPS and 1 MEG was a big computer)
you might use
"enforce=<value>:checkreqprot=<value>:hashalg=<checksum>"

No objection to the above, although it makes his updating code for
enforce/checkreqprot a bit uglier.

Sure, but can you imagine trying to use find(1) if the
options where positional?

Perhaps I'm suffering from audit induced PTSD, but I think we need to
operate under the assumption that we are going to need to augment this
at some point in the future (no good feature goes un-abused) and I'd
much rather have some sort of name-value pairing to keep my sanity.  I
also feel rather strongly that we should make it very explicit in the
comments that the ordering of the fields in the string may change.

quoted

quoted

quoted

for SELinux and define @policy_brief as

     A string containing colon separated name and value pairs
     that will be parsed and interpreted by the security module
     or modules.

Actually, I'm not clear it will be parsed or interpreted by the
security module(s).  I think he is just fetching it and then doing a
simple comparison to check for inconsistencies between clients and the
server, and optionally admins/users can read it and interpret it as
they see fit.

OK, in which case human eyes *need* the name as well as the value.
That, and strcmp(value, "enforce=0") is no harder than strcmp(value, "0").

The initial use case may not require parsing the string, but who knows
what will end up using this five years from now.  Once again, I agree
with Casey, let's make sure it easily parsed and readable by admins;
imagine this as the output of a file under /proc or /sys.

quoted

quoted

quoted

You already have it right for the "hashalg" field. If you want to
be really forward looking you could use names field names that
identify the security module that uses them, such as

     "selinux/enforce=1:selinux/checkreqprot=0:selinux/MD5=8675309"

That seems a bit verbose, particularly the duplicated selinux/ bit.

True that, on the other hand

        "selinux(enforce=1:checkreqprot=0:MD5=8675309)"

would be harder to parse. Either works for me.

I'm not sure I care too much about how the name-value pairs get
namespaced, but considering the LSM stacking work that is happening,
we should namespace it somehow.

--
paul moore
www.paul-moore.com

(Take 2, clicks plain text mode) Darn, I cleaned my inbox out, and
forgot to comment. Couldn't you replace the hex string conversion and
kzalloc
with asprintf and the kernel format specifier extension phN as
documented under Documentation/printk-formats.txt.

It looks like asprintf flows down into vsnprintf and into the
pointer() routine properly.

asprintf prototype:
http://elixir.free-electrons.com/linux/latest/source/include/linux/kernel.h#L426

As an aside, I wonder if audit can ditch there hex escaping routine
and use something like *pE
as mentioned in that Documentation under "Raw buffer as an escaped string:"
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html