From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753169AbdEPUgz (ORCPT ); Tue, 16 May 2017 16:36:55 -0400 Received: from emsm-gh1-uea10.nsa.gov ([8.44.101.8]:65163 "EHLO emsm-gh1-uea10.nsa.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750799AbdEPUgw (ORCPT ); Tue, 16 May 2017 16:36:52 -0400 X-IronPort-AV: E=Sophos;i="5.38,350,1491264000"; d="scan'208";a="7125719" IronPort-PHdr: =?us-ascii?q?9a23=3ANyf6Bx1hvf4SxEPasmDT+DRfVm0co7zxezQtwd8Z?= =?us-ascii?q?sesVLv/xwZ3uMQTl6Ol3ixeRBMOAuq0C0red6vyxEUU7or+5+EgYd5JNUxJXwe?= =?us-ascii?q?43pCcHRPC/NEvgMfTxZDY7FskRHHVs/nW8LFQHUJ2mPw6arXK99yMdFQviPgRp?= =?us-ascii?q?OOv1BpTSj8Oq3Oyu5pHfeQtFiT6/bL9oMRm7rQrdutQZjIZmN6081gbHrnxUdu?= =?us-ascii?q?pM2GhmP0iTnxHy5sex+J5s7SFdsO8/+sBDTKv3Yb02QaRXAzo6PW814tbrtQTY?= =?us-ascii?q?QguU+nQcSGQWnQFWDAXD8Rr3Q43+sir+tup6xSmaIcj7Rq06VDi+86tmTgLjhT?= =?us-ascii?q?wZPDAl7m7Yls1wjLpaoB2/oRx/35XUa5yROPZnY6/RYc8WSW9HU8lfTSxBBp63?= =?us-ascii?q?YZUJAeQPIO1Uq5Dxq0USoRe7AwSnGeHhxSJShnLu3qM0zuQvHx/I0gMiEdIOt2?= =?us-ascii?q?jbotL6O6kdSu210KrFwC/fY/5MxTvw6o7FeQ0hr/GWWrJwdNLcx1QzFwzbllWQ?= =?us-ascii?q?qZLqPzWI3eoQtmiU9e5gVeaxhG8ntgp8pSOvydo3ioTSmoIUykzL9SV+wIovI9?= =?us-ascii?q?24U1R0bcSrEJtXqSGXLo17Sd4hTWFwoCs217ILtJGhcCUK1Zgr3QDTZvOZf4SS?= =?us-ascii?q?/x7uUvuaLy1ii3J/Yr2/gg6/8U2nyuLhSMa5yE1Kri9ZktnUsXANygDT5tCHSv?= =?us-ascii?q?Rj+keh3i6C1xzJ5eFeIEA0iLHbJ4Q9wr8wipUTsUPDEjXwmErql6+Zal8o+u2p?= =?us-ascii?q?6+Tjernmp5mcOJFoigzmL6gjlcOyDf44PwQTRWSX5+ux2KP58UHkWLlKi+c5kq?= =?us-ascii?q?jdsJDUP8Qboau5DhdO0ok+8BayFCum0dQEknkHK1JJYhSHj5PzNF3UL/D4Cum/?= =?us-ascii?q?j0y2kDh33/DGIqHhApLVI3jYi7jhYLd961VHyAo0y9BS/I9bBawHIP7pRkDxs9?= =?us-ascii?q?nYBAcjMwOo2+bnFMl91oQGVGKXGKCZLafSvESQ5u01PumMYJYZuDP6K/gi/f7h?= =?us-ascii?q?k2U1lkMafamsxZEXcmy3Hux6I0WFZnrhmskOEX8QsQokTezqk0aPUSZJaHaoXq?= =?us-ascii?q?I8/Sk7CIa8AojfWI+hmruB3D20HpdOfGBJFkiMEWv0d4WDQ/oMajidIsp/nTwf?= =?us-ascii?q?T7ShT5Ut1RSptA/g0bpnL/HU9zYftZL5ztd6++nTmg8o+TNoCMSd1nmHT3tokW?= =?us-ascii?q?MQWz82wKd/rFRmylidy6h4jOJXGMdT5/xTVgc6MZ7dz+pgB9/uXQLBe8+DSEy6?= =?us-ascii?q?TdW+HTExUtUxzscKY0Z8HdWikx/C0zOpA7ALjbyLAoI78qbH0njvKMZy1WzG2L?= =?us-ascii?q?Mij1Y4WMtPM3Ophqpl+wjUHY7JnF2Tl7y2eqQEwC7N6GCDwHKKvEFZVg5wTKrE?= =?us-ascii?q?UWkEZkTIsdv5+1nCT76yCbUnKwdBzMmCJbZXat3tk1pLX+njONvAbGKrgWuwBg?= =?us-ascii?q?iHxqmKbIX0f2URxiLdCFILkwoL53aJKRA+Bju9o2LZFDFuDkngY17t8ells3O7?= =?us-ascii?q?SUk0wxuXYEJ80bq44REVhfmGRPMS2rIIojsuqzJxHAX149WDMNeKrhF9fahaKf?= =?us-ascii?q?kg4Uxc0mSR4xd3I527NKdkwFISdSx4ukrv01N8DYAW1YAurXU33E9pJKmFylJd?= =?us-ascii?q?Znad2pztPrD/NGb/5lasZrTQ11WY18yZvu8L6fIluxDgsRuvG04K7XpqyZ9W3m?= =?us-ascii?q?Ga65GMCxAdFdrqX0I28QVqj63LaSk6oYXP3DtjNrfnnCXF3ocSGOY9yhumN+xa?= =?us-ascii?q?OaeAGR66R9YWHOCyOecqnB6vdRtCM+dMov1nd/i6fueLjfb4dN1rmyir2CEeut?= =?us-ascii?q?hw?= X-IPAS-Result: =?us-ascii?q?A2GoAgCWYhtZ/wHyM5BcGgEBAQECAQEBAQgBAQEBFQEBAQE?= =?us-ascii?q?CAQEBAQgBAQEBgwEpgW6DbJpKAQEBAQEBBoEmcpcShiQChU9XAQEBAQEBAQECA?= =?us-ascii?q?QJoKIIzJAGCQQEFIwQLAUYQCQINAQoCAiYCAlcGARKIB4IPDY5rnWCBbDomAop?= =?us-ascii?q?ZAQEBAQEFAQEBAQEBIoELhQ6CJIJnNIRNFoMSgmABBIk9hmWBWYwPkxuCBIkVD?= =?us-ascii?q?IZGiH+LRFhZMSYJAh4IHw+FPAEcgX8kNohDAQEB?= Message-ID: <1494967240.21557.18.camel@tycho.nsa.gov> Subject: Re: [PATCH v5 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: Tue, 16 May 2017 16:40:40 -0400 In-Reply-To: <1494928281-11128-1-git-send-email-sbuisson@ddn.com> References: <1494928281-11128-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 Tue, 2017-05-16 at 18:51 +0900, Sebastien Buisson wrote: > Add policybrief field to struct policydb. It holds a brief info > of the policydb, made of colon separated name and value pairs > that give information about how the policy is applied in the > security module(s). > Note that the ordering of the fields in the string may change. > > 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. It is useful for any network or > distributed file system that cares about how SELinux is enforced > on its client nodes. This information is used to detect changes > to the policy on file system client nodes, and can be forwarded > to file system server nodes. Depending on how the policy is > enforced on client side, server can refuse connection. > > Signed-off-by: Sebastien Buisson > --- >  include/linux/lsm_hooks.h           | 20 +++++++++ >  include/linux/security.h            |  7 +++ >  security/security.c                 |  8 ++++ >  security/selinux/hooks.c            |  7 +++ >  security/selinux/include/security.h |  2 + >  security/selinux/selinuxfs.c        |  2 + >  security/selinux/ss/policydb.c      | 85 > +++++++++++++++++++++++++++++++++++++ >  security/selinux/ss/policydb.h      |  3 ++ >  security/selinux/ss/services.c      | 67 > +++++++++++++++++++++++++++++ >  9 files changed, 201 insertions(+) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 1aa6333..e4dc2b4 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -1331,6 +1331,24 @@ >   * @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. > The string > + * contains colon separated name and value pairs that give > information > + * about how the policy is applied in the security module(s). > + * Note that the ordering of the fields in the string may > change. > + * > + * @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 > + * If @alloc, *brief must be kfreed by caller. > + * If not @alloc, caller must pass a buffer that can hold > policy brief > + * info (including terminating NUL). > + * On success 0 is returned , or negative value on error. > + * >   * This is the main security structure. >   */ >   > @@ -1562,6 +1580,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); > @@ -1806,6 +1825,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 97df7ba..35ac97f 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -376,6 +376,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 { >  }; > @@ -1159,6 +1161,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 d6d18a3..46052d3 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1269,6 +1269,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) > @@ -1868,6 +1874,8 @@ struct security_hook_heads security_hook_heads > __lsm_ro_after_init = { >   LIST_HEAD_INIT(security_hook_heads.inode_setsecctx), >   .inode_getsecctx = >   LIST_HEAD_INIT(security_hook_heads.inode_getsecctx), > + .policy_brief = > + LIST_HEAD_INIT(security_hook_heads.policy_brief), >  #ifdef CONFIG_SECURITY_NETWORK >   .unix_stream_connect = >   LIST_HEAD_INIT(security_hook_heads.unix_stream_conne > ct), > 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 ce71718..e8fe914 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..83309a4 100644 > --- a/security/selinux/ss/policydb.c > +++ b/security/selinux/ss/policydb.c > @@ -32,13 +32,20 @@ >  #include >  #include >  #include > +#include >  #include "security.h" >   >  #include "policydb.h" >  #include "conditional.h" >  #include "mls.h" > +#include "objsec.h" >  #include "services.h" >   > +static unsigned int policybrief_hash_size; > +static struct crypto_shash *policybrief_tfm; > +static const char policybrief_hash_alg[] = "sha256"; > +unsigned int policybrief_len; > + >  #define _DEBUG_HASHES >   >  #ifdef DEBUG_HASHES > @@ -879,6 +886,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 +2229,49 @@ 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) > +{ > + SHASH_DESC_ON_STACK(desc, policybrief_tfm); > + struct policy_file *fp = ptr; > + u8 *hashval; > + int rc = 0; > + > + BUG_ON(policydb->policybrief); > + > + hashval = kmalloc(policybrief_hash_size, GFP_KERNEL); > + if (hashval == NULL) > + return -ENOMEM; > + > + desc->tfm = policybrief_tfm; > + desc->flags = 0; > + rc = crypto_shash_digest(desc, fp->data, fp->len, hashval); > + if (rc) { > + printk(KERN_ERR "Failed crypto_shash_digest: %d\n", > rc); > + goto out_free; > + } > + > + /* policy brief is in the form: > +  * selinux(enforce=<0 or 1>:checkreqprot=<0 or > 1>:=) > +  */ > + policydb->policybrief = kasprintf(GFP_KERNEL, > +   "selinux(enforce=%d:checkr > eqprot=%d:%s=%*phN)", > +   selinux_enforcing, > +   selinux_checkreqprot, > +   policybrief_hash_alg, > +   policybrief_hash_size, > hashval); kasprintf() calls vsnprintf() twice, once to compute the length and once to output the string. But you already precomputed the length during init in policybrief_len. So kmalloc() + snprintf() should suffice. You can also then check that snprintf() returns the expected value. > + if (policydb->policybrief == NULL) > + rc = -ENOMEM; > + > +out_free: > + kfree(hashval); > + > + return rc; > +} > + > +/* >   * Read the configuration data from a policy database binary >   * representation file into a policy database structure. >   */ > @@ -2238,6 +2290,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) > @@ -3456,3 +3513,31 @@ int policydb_write(struct policydb *p, void > *fp) >   >   return 0; >  } > + > +static int __init init_policybrief_hash(void) > +{ > + struct crypto_shash *tfm; > + > + if (!selinux_enabled) > + return 0; > + > + tfm = crypto_alloc_shash(policybrief_hash_alg, 0, 0); > + if (IS_ERR(tfm)) { > + printk(KERN_ERR "Failed to alloc crypto hash %s\n", > +        policybrief_hash_alg); > + return PTR_ERR(tfm); > + } > + > + policybrief_tfm = tfm; > + policybrief_hash_size = > crypto_shash_digestsize(policybrief_tfm); > + > + /* policy brief is in the form: > +  * selinux(enforce=<0 or 1>:checkreqprot=<0 or > 1>:=) > +  */ > + policybrief_len = 35 + strlen(policybrief_hash_alg) + > + 2*policybrief_hash_size + 1; > + > + return 0; > +} > + > +late_initcall(init_policybrief_hash); > diff --git a/security/selinux/ss/policydb.h > b/security/selinux/ss/policydb.h > index 725d594..2e5048c 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; > @@ -309,6 +311,7 @@ struct policydb { >  extern int policydb_role_isvalid(struct policydb *p, unsigned int > role); >  extern int policydb_read(struct policydb *p, void *fp); >  extern int policydb_write(struct policydb *p, void *fp); > +extern unsigned int policybrief_len; >   >  #define PERM_SYMTAB_SIZE 32 >   > diff --git a/security/selinux/ss/services.c > b/security/selinux/ss/services.c > index 60d9b02..67eb80d 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -2170,6 +2170,73 @@ 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 > + * > + * If @alloc, *brief must be kfreed by caller. > + * If not @alloc, caller must pass a buffer that can hold > policybrief_len > + * chars (including terminating NUL). > + * On success 0 is returned , or negative value on error. > + **/ > +int security_policydb_brief(char **brief, size_t *len, bool alloc) > +{ > + if (!ss_initialized || brief == NULL) > + return -EINVAL; > + > + if (alloc) { > + *brief = kzalloc(policybrief_len, GFP_KERNEL); > + } else if (*len < policybrief_len) { > + /* put in *len the string size we need to write */ > + *len = policybrief_len; > + return -ENAMETOOLONG; > + } > + > + if (*brief == NULL) > + return -ENOMEM; > + > + read_lock(&policy_rwlock); > + strcpy(*brief, policydb.policybrief); > + /* *len is the length of the output string */ > + *len = policybrief_len - 1; Is there a particular reason to not just return policybrief_len here as well, for consistency in the interface? How do you intend to use this value in the caller? > + read_unlock(&policy_rwlock); > + > + return 0; > +} > + > +void security_policydb_update_info(u32 requested) > +{ > + /* policy brief is in the form: > +  * selinux(enforce=<0 or 1>:checkreqprot=<0 or > 1>:=) > +  */ > + char enforce[] = "enforce="; > + char checkreqprot[] = "checkreqprot="; > + char *p, *str; > + int val; > + > + if (!ss_initialized) > + return; > + > + if (requested == SECURITY__SETENFORCE) { > + str = enforce; > + val = selinux_enforcing; > + } else if (requested == SECURITY__SETCHECKREQPROT) { > + str = checkreqprot; > + val = selinux_checkreqprot; > + } > + > + /* update global policydb, needs write lock */ > + write_lock_irq(&policy_rwlock); > + p = strstr(policydb.policybrief, str); > + if (p) { > + p += strlen(str); > + *p = '0' + val; > + } > + write_unlock_irq(&policy_rwlock); > +} > + > +/** >   * security_port_sid - Obtain the SID for a port. >   * @protocol: protocol number >   * @port: port number