From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935732AbdEKOdz (ORCPT ); Thu, 11 May 2017 10:33:55 -0400 Received: from smtp.nsa.gov ([8.44.101.8]:21663 "EHLO emsm-gh1-uea10.nsa.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935677AbdEKOdv (ORCPT ); Thu, 11 May 2017 10:33:51 -0400 X-IronPort-AV: E=Sophos;i="5.38,324,1491264000"; d="scan'208";a="6921316" IronPort-PHdr: =?us-ascii?q?9a23=3AztuJbBECDAG9T30WB7o5QZ1GYnF86YWxBRYc798d?= =?us-ascii?q?s5kLTJ76pMi6bnLW6fgltlLVR4KTs6sC0LuJ9fi9EjNeqb+681k6OKRWUBEEjc?= =?us-ascii?q?hE1ycBO+WiTXPBEfjxciYhF95DXlI2t1uyMExSBdqsLwaK+i764jEdAAjwOhRo?= =?us-ascii?q?LerpBIHSk9631+ev8JHPfglEnjSwbLdwIRmssQnctsYajIljJ60s1hbHv3xEdv?= =?us-ascii?q?hMy2h1P1yThRH85smx/J5n7Stdvu8q+tBDX6vnYak2VKRUAzs6PW874s3rrgTD?= =?us-ascii?q?QhCU5nQASGUWkwFHDBbD4RrnQ5r+qCr6tu562CmHIc37SK0/VDq+46t3ThLjlT?= =?us-ascii?q?wKPCAl/m7JlsNwjbpboBO/qBx5347Ue5yeOP5ncq/AYd8WWW9NU8BfWCxbBoO3?= =?us-ascii?q?cpUBAewPM+1Fq4XxvkUCoQe7CQSqGejhyCJHhmXu0KM5zuovER/I0gIiENIAt3?= =?us-ascii?q?TbsNL7O6gdX+2u0KnFzi/OY+9M1Dvh6oXFdA0qr/GWXbJ3dMrc0VQhFx/bgVWI?= =?us-ascii?q?qYzqITWV3fkQvWie9eVgUeavhHAnqgpspTWv3dojipLSi4IJylHL6SV5wIEvKd?= =?us-ascii?q?2+U050e8SoEJRXtyGELoZ7RN4pTWJwuCsi17ELtpG2cDIKxZg63RLTdfOKf5aS?= =?us-ascii?q?7h7+UuuaPC12i2h/eL2lgha/6U2gyurhWcaqyFtKtS9FksXUtnAKyhzT9tCLSv?= =?us-ascii?q?tj8Uel3jaCzxzT5fteIUA1iKrbMIQtwqIwl5UPsUTDGTX6mEPqg6+Nakoo4O2o?= =?us-ascii?q?6+XjYrn+p5+cMZF7ih3mP6gzlcGyDv40PwgTU2SB5+ix26Pv8VfkTLlSi/05iK?= =?us-ascii?q?jZsJTUJcQBoa65BhdY0p0+5BakFDqmzNQZkmUHLFJCYh6HiZPpNEvULPD3Cve/?= =?us-ascii?q?nUygkC13yPDeIr3hHpLNI2DYkLj6YLZ96lVcyBE0zdBZ/J9bF6wOIPTpVkDts9?= =?us-ascii?q?zYCwczMxaozOb/FNV9yoQeVHqXAqCDLaPStUSF5vo1LOmRYI8ZoTP9K/8i5/70?= =?us-ascii?q?k3A1g0MSfa6s3ZEPcnC3AuxmI1mFYXrrmtoOD38KsRAkTOzrk12PSiZTaGyoX6?= =?us-ascii?q?I9/TE7EIamAp3fSY+zmrCB2z27HpJObGBcFl+MCWvod5mDW/oUaiKdOMphnSIf?= =?us-ascii?q?VbS7T48tzxSutAjgy7p9L+rU4TYVtZX51Ndv++LTkQ89+SZoAMSa1mGHV3t0kX?= =?us-ascii?q?8QRz8qwKB/plRwykyd3qhijPxXC8de5/NTXQc+MZ7dz+p6B8ruVQLGe9eDUEym?= =?us-ascii?q?Tcm+ATEtUtIxxMcDY0J8G9WkkxDC0DOmA7wLmLyRApw77Kbc0mPvJ8Zy1XnGzr?= =?us-ascii?q?Mtj1o4TctVM22pmKp/+xLUB47TnEWTj7yqergE3C7R6GeDynKDvEVZUA52TKXE?= =?us-ascii?q?UmkTZlDIotvl+0PCVb6uCagnMwdYzM6CLbZFasDtjVpYX/rjJtvebHyrm2uqBh?= =?us-ascii?q?aH2KmMbIz0dGUZxindD1IEkw8L93acKQc+Hjuho37ZDDF2DlLgeF7s8ehlqHOg?= =?us-ascii?q?SU80yRuGYFB82Lqz4RMVivmcROkS3rIAoisutzJ0HFPul+7RXuGNrQN6YKRRZ5?= =?us-ascii?q?sX/VZczmXf/1hmNIGhNLtlgBgSfwJfsEbn1hExAYJFx4xiqHIs0Ro3Mq+TzUlA?= =?us-ascii?q?ayLd2Jf8J7naAnf98QrpaKPM3FzaltGM9eNH7PU+tkWmvwyzEEcm22to3sMT0H?= =?us-ascii?q?aG4JjOSg0IXta5SUsz9h5nt5nGcyI94MXSznQqPq6q4RHY3Nd8P/co0hateZ9k?= =?us-ascii?q?NaqAEALjW5kBC9OGNP0hm1/vaAkNeu9V6vhnbIuda/Ka1fvzb65blzW8gDECud?= =?us-ascii?q?ol3w=3D=3D?= X-IPAS-Result: =?us-ascii?q?A2F3AwBldRRZ/wHyM5BdGwEBAQMBAQEJAQEBFgEBAQMBAQE?= =?us-ascii?q?JAQEBgwEpgW6DaZpCAQEBAQEBBoEmcpcRhiQChQxXAQEBAQEBAQECAQJoKIIzI?= =?us-ascii?q?gGCQAEFIwQLAUYQCQINAQoCAiYCAlcGARKIB4IODZM8nWCBbDomAopOAQEBAQE?= =?us-ascii?q?BBAEBAQEBASKBC4UOgiSCZzSHdYJgAQSJPYg+jA+TG4IEiRUMhkaIf4tEWFkxJ?= =?us-ascii?q?gkCHggfD4U7ARyBfyQ2iFwBAQE?= Message-ID: <1494513466.10447.3.camel@tycho.nsa.gov> Subject: Re: [PATCH v3 1/2] selinux: add brief info to policydb From: Stephen Smalley To: Sebastien Buisson , linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, selinux@tycho.nsa.gov Cc: serge@hallyn.com, james.l.morris@oracle.com, eparis@parisplace.org, paul@paul-moore.com, Sebastien Buisson Date: Thu, 11 May 2017 10:37:46 -0400 In-Reply-To: <1494507551-4643-1-git-send-email-sbuisson@ddn.com> References: <1494507551-4643-1-git-send-email-sbuisson@ddn.com> Organization: National Security Agency Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.6 (3.22.6-2.fc25) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2017-05-11 at 21:59 +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>:= > 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 > --- >  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>:= > + * > + * @brief: pointer to buffer holding brief > + * @len: in: brief buffer length if no alloc, out: brief > string len > + * @alloc: whether to allocate buffer for brief or not > + * On success 0 is returned , or negative value on error. > + * >   * This is the main security structure. >   */ >   > @@ -1568,6 +1582,7 @@ >   int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 > ctxlen); >   int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 > *ctxlen); >   > + int (*policy_brief)(char **brief, size_t *len, bool alloc); >  #ifdef CONFIG_SECURITY_NETWORK >   int (*unix_stream_connect)(struct sock *sock, struct sock > *other, >   struct sock *newsk); > @@ -1813,6 +1828,7 @@ struct security_hook_heads { >   struct list_head inode_notifysecctx; >   struct list_head inode_setsecctx; >   struct list_head inode_getsecctx; > + struct list_head policy_brief; >  #ifdef CONFIG_SECURITY_NETWORK >   struct list_head unix_stream_connect; >   struct list_head unix_may_send; > diff --git a/include/linux/security.h b/include/linux/security.h > index af675b5..3b72053 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -377,6 +377,8 @@ int security_sem_semop(struct sem_array *sma, > struct sembuf *sops, >  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 > ctxlen); >  int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 > ctxlen); >  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 > *ctxlen); > + > +int security_policy_brief(char **brief, size_t *len, bool alloc); >  #else /* CONFIG_SECURITY */ >  struct security_mnt_opts { >  }; > @@ -1166,6 +1168,11 @@ static inline int > security_inode_getsecctx(struct inode *inode, void **ctx, u32 >  { >   return -EOPNOTSUPP; >  } > + > +static inline int security_policy_brief(char **brief, size_t *len, > bool alloc) > +{ > + return -EOPNOTSUPP; > +} >  #endif /* CONFIG_SECURITY */ >   >  #ifdef CONFIG_SECURITY_NETWORK > diff --git a/security/security.c b/security/security.c > index b9fea39..954b391 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1285,6 +1285,12 @@ int security_inode_getsecctx(struct inode > *inode, void **ctx, u32 *ctxlen) >  } >  EXPORT_SYMBOL(security_inode_getsecctx); >   > +int security_policy_brief(char **brief, size_t *len, bool alloc) > +{ > + return call_int_hook(policy_brief, -EOPNOTSUPP, brief, len, > alloc); > +} > +EXPORT_SYMBOL(security_policy_brief); > + >  #ifdef CONFIG_SECURITY_NETWORK >   >  int security_unix_stream_connect(struct sock *sock, struct sock > *other, struct sock *newsk) > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index e67a526..da245e8 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -6063,6 +6063,11 @@ static int selinux_inode_getsecctx(struct > inode *inode, void **ctx, u32 *ctxlen) >   *ctxlen = len; >   return 0; >  } > + > +static int selinux_policy_brief(char **brief, size_t *len, bool > alloc) > +{ > + return security_policydb_brief(brief, len, alloc); > +} >  #ifdef CONFIG_KEYS >   >  static int selinux_key_alloc(struct key *k, const struct cred *cred, > @@ -6277,6 +6282,8 @@ static int selinux_key_getsecurity(struct key > *key, char **_buffer) >   LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), >   LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx), >   > + LSM_HOOK_INIT(policy_brief, selinux_policy_brief), > + >   LSM_HOOK_INIT(unix_stream_connect, > selinux_socket_unix_stream_connect), >   LSM_HOOK_INIT(unix_may_send, selinux_socket_unix_may_send), >   > diff --git a/security/selinux/include/security.h > b/security/selinux/include/security.h > index f979c35..a0d4d7d 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -97,6 +97,8 @@ enum { >  int security_load_policy(void *data, size_t len); >  int security_read_policy(void **data, size_t *len); >  size_t security_policydb_len(void); > +int security_policydb_brief(char **brief, size_t *len, bool alloc); > +void security_policydb_update_info(u32 requested); >   >  int security_policycap_supported(unsigned int req_cap); >   > diff --git a/security/selinux/selinuxfs.c > b/security/selinux/selinuxfs.c > index 50062e7..8c9f5b7 100644 > --- a/security/selinux/selinuxfs.c > +++ b/security/selinux/selinuxfs.c > @@ -159,6 +159,7 @@ static ssize_t sel_write_enforce(struct file > *file, const char __user *buf, >   from_kuid(&init_user_ns, > audit_get_loginuid(current)), >   audit_get_sessionid(current)); >   selinux_enforcing = new_value; > + security_policydb_update_info(SECURITY__SETENFORCE); >   if (selinux_enforcing) >   avc_ss_reset(0); >   selnl_notify_setenforce(selinux_enforcing); > @@ -621,6 +622,7 @@ static ssize_t sel_write_checkreqprot(struct file > *file, const char __user *buf, >   goto out; >   >   selinux_checkreqprot = new_value ? 1 : 0; > + security_policydb_update_info(SECURITY__SETCHECKREQPROT); >   length = count; >  out: >   kfree(page); > diff --git a/security/selinux/ss/policydb.c > b/security/selinux/ss/policydb.c > index 0080122..58e73f5 100644 > --- a/security/selinux/ss/policydb.c > +++ b/security/selinux/ss/policydb.c > @@ -32,11 +32,13 @@ >  #include >  #include >  #include > +#include >  #include "security.h" >   >  #include "policydb.h" >  #include "conditional.h" >  #include "mls.h" > +#include "objsec.h" >  #include "services.h" >   >  #define _DEBUG_HASHES > @@ -879,6 +881,8 @@ void policydb_destroy(struct policydb *p) >   ebitmap_destroy(&p->filename_trans_ttypes); >   ebitmap_destroy(&p->policycaps); >   ebitmap_destroy(&p->permissive_map); > + > + kfree(p->policybrief); >  } >   >  /* > @@ -2220,6 +2224,73 @@ static int ocontext_read(struct policydb *p, > struct policydb_compat_info *info, >  } >   >  /* > + * Compute summary of a policy database binary representation file, > + * and store it into a policy database structure. > + */ > +static int policydb_brief(struct policydb *policydb, void *ptr) > +{ > + struct policy_file *fp = ptr; > + struct crypto_shash *tfm; > + char hashalg[] = "sha256"; const > + size_t hashsize; > + u8 *hashval; > + 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); > + } > + > + hashsize = crypto_shash_digestsize(tfm); Let's do the crypto_alloc_shash() and crypto_shash_digestsize() once during init, e.g. see init_profile_hash() in security/apparmor/crypto.c. > + hashval = kmalloc(hashsize, GFP_KERNEL); > + if (hashval == NULL) { > + crypto_free_shash(tfm); > + return -ENOMEM; > + } > + > + { > + int rc; > + > + SHASH_DESC_ON_STACK(desc, tfm); > + desc->tfm = tfm; > + desc->flags = 0; > + rc = crypto_shash_digest(desc, fp->data, fp->len, > hashval); > + crypto_free_shash(tfm); > + if (rc) { > + printk(KERN_ERR "Failed crypto_shash_digest: > %d\n", rc); > + kfree(hashval); > + return rc; > + } > + } > + > + /* policy brief is in the form: > +  * <0 or 1 for enforce>:<0 or 1 for > checkreqprot>:= > +  */ > + policydb->policybrief = kzalloc(5 + strlen(hashalg) + > 2*hashsize + 1, We can also compute and save this size once during init; it won't ever change at runtime. > + GFP_KERNEL); > + if (policydb->policybrief == NULL) { > + kfree(hashval); > + return -ENOMEM; > + } > + > + sprintf(policydb->policybrief, "%d:%d:%s=", > + selinux_enforcing, selinux_checkreqprot, hashalg); > + p = policydb->policybrief + strlen(policydb->policybrief); > + for (idx = 0; idx < hashsize; idx++) { > + snprintf(p, 3, "%02x", hashval[idx]); > + p += 2; > + } > + kfree(hashval); > + > + return 0; > +} > + > +/* >   * Read the configuration data from a policy database binary >   * representation file into a policy database structure. >   */ > @@ -2238,6 +2309,11 @@ int policydb_read(struct policydb *p, void > *fp) >   if (rc) >   return rc; >   > + /* Compute summary of policy, and store it in policydb */ > + 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) > diff --git a/security/selinux/ss/policydb.h > b/security/selinux/ss/policydb.h > index 725d594..31689d2f 100644 > --- a/security/selinux/ss/policydb.h > +++ b/security/selinux/ss/policydb.h > @@ -293,6 +293,8 @@ struct policydb { >   size_t len; >   >   unsigned int policyvers; > + /* summary computed on the policy */ > + unsigned char *policybrief; >   >   unsigned int reject_unknown : 1; >   unsigned int allow_unknown : 1; > diff --git a/security/selinux/ss/services.c > b/security/selinux/ss/services.c > index 60d9b02..3bbe649 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -2170,6 +2170,68 @@ size_t security_policydb_len(void) >  } >   >  /** > + * security_policydb_brief - Get policydb brief > + * @brief: pointer to buffer holding brief > + * @len: in: brief buffer length if no alloc, out: brief string len > + * @alloc: whether to allocate buffer for brief or not > + * > + * On success 0 is returned , or negative value on error. > + **/ > +int security_policydb_brief(char **brief, size_t *len, bool alloc) > +{ > + size_t policybrief_len; > + > + if (!ss_initialized || brief == NULL) > + return -EINVAL; > + > + read_lock(&policy_rwlock); > + policybrief_len = strlen(policydb.policybrief); > + read_unlock(&policy_rwlock); We don't need to compute this via strlen(); we can precompute during init. Then we don't need to fetch anything here. > + > + 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); > + *len = strlen(policydb.policybrief); > + strncpy(*brief, policydb.policybrief, *len); If in fact the length could change at runtime, then this would be unsafe, since it could have changed between dropping the lock and re- acquiring it. But this doesn't matter; we don't need to compute this at runtime. > + read_unlock(&policy_rwlock); > + > + return 0; > +} > + > +void security_policydb_update_info(u32 requested) > +{ > + /* policy brief is in the form: > +  * <0 or 1 for enforce>:<0 or 1 for > checkreqprot>:= > +  */ > + > + if (!ss_initialized) > + return; > + > + /* update global policydb, needs write lock */ > + write_lock_irq(&policy_rwlock); > + if (requested == SECURITY__SETENFORCE) > + policydb.policybrief[0] = '0' + selinux_enforcing; > + else if (requested == SECURITY__SETCHECKREQPROT) > + policydb.policybrief[2] = '0' + > selinux_checkreqprot; > + write_unlock_irq(&policy_rwlock); > +} > + > +/** >   * security_port_sid - Obtain the SID for a port. >   * @protocol: protocol number >   * @port: port number