On Fri, 2017-05-05 at 19:10 +0900, Sebastien Buisson wrote:

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.

Signed-off-by: Sebastien Buisson <redacted>
---
?include/linux/lsm_hooks.h???????????|??2 +
?include/linux/security.h????????????|??7 ++++
?security/security.c?????????????????|??6 +++
?security/selinux/hooks.c????????????| 11 +++++-
?security/selinux/include/security.h |??2 +
?security/selinux/selinuxfs.c????????|??6 ++-
?security/selinux/ss/policydb.c??????| 70
+++++++++++++++++++++++++++++++++++
?security/selinux/ss/policydb.h??????|??3 ++
?security/selinux/ss/services.c??????| 73
+++++++++++++++++++++++++++++++++++++
?9 files changed, 178 insertions(+), 2 deletions(-)

I don't think you need this.  You are unlikely to request the policy
brief until after policy has been loaded (and if you do, you'll only
get a partial result), so you can just defer setting up the enforcing
field until the initial policy load, and then update it on subsequent
writes to /sys/fs/selinux/enforce.

+	}
?	return 1;
?}
?__setup("enforcing=", enforcing_setup);

Ditto.  Just initialize it with the rest of the info on initial policy
load, and update it on writes to checkreqprot.

?	return 1;
?}
?__setup("checkreqprot=", checkreqprot_setup);

size_t

+	char hashval[hashsize];

unsigned char or u8

+	int idx;
+	unsigned char *p;
+
+	if (policydb->policybrief)
+		return -EINVAL;
+
+	tfm = crypto_alloc_shash(hashalg, 0, 0);
+	if (IS_ERR(tfm)) {
+		printk(KERN_ERR "Failed to alloc crypto hash %s\n",
hashalg);
+		return PTR_ERR(tfm);
+	}

Should you be checking crypto_shash_digestsize(tfm) against sizeof
hashval, or allocating hashval based on it instead? Not a problem now,
but if you ever change the algorithm and forget to update the
hashsize...

+
+	{
+		int rc;
+
+		SHASH_DESC_ON_STACK(desc, tfm);
+		desc->tfm = tfm;
+		desc->flags = 0;
+		rc = crypto_shash_init(desc);
+		if (rc) {
+			printk(KERN_ERR "Failed to init shash\n");
+			crypto_free_shash(tfm);
+			return rc;
+		}
+
+		crypto_shash_update(desc, fp->data, fp->len);
+		crypto_shash_final(desc, hashval);

Just use crypto_shash_digest(); handles _init, _update, and _final for
you in one call.

+		crypto_free_shash(tfm);
+	}
+
+	/* policy brief is in the form:
+	?* <0 or 1 for enforce>:<0 or 1 for
checkreqprot>:<hashalg>=<checksum>
+	?*/
+	policydb->policybrief = kzalloc(5 + strlen(hashalg) +
2*hashsize + 1,
+					GFP_KERNEL);
+	if (policydb->policybrief == NULL)
+		return -ENOMEM;
+
+	sprintf(policydb->policybrief, "x:x:%s=", hashalg);

Couldn't you just directly set the enforcing and checkreqprot fields
above?  No need to call another function.

+	security_policydb_update_info(policydb);
+	p = policydb->policybrief + strlen(policydb->policybrief);
+	for (idx = 0; idx < hashsize; idx++) {
+		snprintf(p, 3, "%02x", (unsigned
char)(hashval[idx]));

No need to cast if you fix the type above.

+		p += 2;
+	}
+	policydb->policybrief_len = (size_t)(p - policydb-

quoted

policybrief);

This length is actually computable at build time, right?  It isn't
variant based on policy, just on hash algorithm.  No need to store it
in the policydb.

summary

+	rc = policydb_brief(p, fp);
+	if (rc)
+		goto bad;
+
?	/* Read the magic number and string length. */
?	rc = next_entry(buf, fp, sizeof(u32) * 2);
?	if (rc)

You can return immediately if !ss_initialized.

+
+	read_lock(&policy_rwlock);
+	policybrief_len = policydb.policybrief_len;

The length is fixed by the hash algorithm.  No need to fetch it.

+	if (policydb.policybrief == NULL)
+		rc = -EAGAIN;

This shouldn't ever be possible if ss_initialized, right?

+	read_unlock(&policy_rwlock);
+
+	if (rc)
+		return rc;
+
+	if (alloc)
+		/* *brief must be kfreed by caller in this case */
+		*brief = kzalloc(policybrief_len + 1, GFP_KERNEL);
+	else
+		/*
+		?* if !alloc, caller must pass a buffer that
+		?* can hold policybrief_len+1 chars
+		?*/
+		if (*len < policybrief_len + 1) {
+			/* put in *len the string size we need to
write */
+			*len = policybrief_len;
+			return -ENAMETOOLONG;
+		}
+
+	if (*brief == NULL)
+		return -ENOMEM;
+
+	read_lock(&policy_rwlock);
+	strncpy(*brief, policydb.policybrief,
policydb.policybrief_len);
+	*len = policydb.policybrief_len;
+	read_unlock(&policy_rwlock);
+
+	return rc;
+}
+
+void security_policydb_update_info(void *p)
+{
+	/* policy brief is in the form:
+	?* <0 or 1 for enforce>:<0 or 1 for
checkreqprot>:<hashalg>=<checksum>
+	?*/

if (!ss_initialized)
	return;

+	if (p) {
+		struct policydb *poldb = p;
+		/* update policydb given as parameter if possible */
+		if (poldb->policybrief) {
+			poldb->policybrief[0] = '0' +
selinux_enforcing;
+			poldb->policybrief[2] = '0' +
selinux_checkreqprot;

This case could be handled directly in the caller.

+		}
+	} else {
+		/* update global policydb, needs write lock */
+		write_lock_irq(&policy_rwlock);
+		if (policydb.policybrief) {

Don't need this once ss_initialized is set.

+			policydb.policybrief[0] = '0' +
selinux_enforcing;
+			policydb.policybrief[2] = '0' +
selinux_checkreqprot;
+		}

Technically only need to update one of the two fields at any given
time, and the caller can specify which one.  But maybe it isn't worth
it.

+		write_unlock_irq(&policy_rwlock);
+	}
+}
+
+/**
? * security_port_sid - Obtain the SID for a port.
? * @protocol: protocol number
? * @port: port number

--
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