Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH v3 24/24] AppArmor: Remove the exclusive flag
From: John Johansen @ 2019-06-24 21:45 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-25-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> With the inclusion of the "display" process attribute
> mechanism AppArmor no longer needs to be treated as an
> "exclusive" security module. Remove the flag that indicates
> it is exclusive. Remove the stub getpeersec_dgram AppArmor
> hook as it has no effect in the single LSM case and
> interferes in the multiple LSM case.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  security/apparmor/lsm.c | 20 +-------------------
>  1 file changed, 1 insertion(+), 19 deletions(-)
> 
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index dcbbefbd95ff..c4365434f10b 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1082,22 +1082,6 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock,
>  	return error;
>  }
>  
> -/**
> - * apparmor_socket_getpeersec_dgram - get security label of packet
> - * @sock: the peer socket
> - * @skb: packet data
> - * @secid: pointer to where to put the secid of the packet
> - *
> - * Sets the netlabel socket state on sk from parent
> - */
> -static int apparmor_socket_getpeersec_dgram(struct socket *sock,
> -					    struct sk_buff *skb, u32 *secid)
> -
> -{
> -	/* TODO: requires secid support */
> -	return -ENOPROTOOPT;
> -}
> -
>  /**
>   * apparmor_sock_graft - Initialize newly created socket
>   * @sk: child sock
> @@ -1196,8 +1180,6 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
>  #endif
>  	LSM_HOOK_INIT(socket_getpeersec_stream,
>  		      apparmor_socket_getpeersec_stream),
> -	LSM_HOOK_INIT(socket_getpeersec_dgram,
> -		      apparmor_socket_getpeersec_dgram),
>  	LSM_HOOK_INIT(sock_graft, apparmor_sock_graft),
>  #ifdef CONFIG_NETWORK_SECMARK
>  	LSM_HOOK_INIT(inet_conn_request, apparmor_inet_conn_request),
> @@ -1709,7 +1691,7 @@ static int __init apparmor_init(void)
>  
>  DEFINE_LSM(apparmor) = {
>  	.name = "apparmor",
> -	.flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
> +	.flags = LSM_FLAG_LEGACY_MAJOR,
>  	.enabled = &apparmor_enabled,
>  	.blobs = &apparmor_blob_sizes,
>  	.init = apparmor_init,
> 


^ permalink raw reply

* Re: [PATCH v3 23/24] NET: Store LSM netlabel data in a lsmblob
From: John Johansen @ 2019-06-24 21:44 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-24-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Netlabel uses LSM interfaces requiring an lsmblob and
> the internal storage is used to pass information between
> these interfaces, so change the internal data from a secid
> to a lsmblob. Update the netlabel interfaces and their
> callers to accomodate the change.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canomical.com>

> ---
>  include/net/netlabel.h              |  8 ++--
>  net/ipv4/cipso_ipv4.c               |  6 ++-
>  net/netlabel/netlabel_kapi.c        |  6 +--
>  net/netlabel/netlabel_unlabeled.c   | 57 +++++++++++------------------
>  net/netlabel/netlabel_unlabeled.h   |  2 +-
>  security/selinux/include/security.h |  1 +
>  security/selinux/netlabel.c         |  2 +-
>  security/selinux/ss/services.c      |  4 +-
>  security/smack/smack.h              |  1 +
>  security/smack/smack_lsm.c          |  3 +-
>  security/smack/smackfs.c            | 10 +++--
>  11 files changed, 48 insertions(+), 52 deletions(-)
> 
> diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> index 72d6435fc16c..6c550455e69f 100644
> --- a/include/net/netlabel.h
> +++ b/include/net/netlabel.h
> @@ -180,7 +180,7 @@ struct netlbl_lsm_catmap {
>   * @attr.mls: MLS sensitivity label
>   * @attr.mls.cat: MLS category bitmap
>   * @attr.mls.lvl: MLS sensitivity level
> - * @attr.secid: LSM specific secid token
> + * @attr.lsmblob: LSM specific data
>   *
>   * Description:
>   * This structure is used to pass security attributes between NetLabel and the
> @@ -215,7 +215,7 @@ struct netlbl_lsm_secattr {
>  			struct netlbl_lsm_catmap *cat;
>  			u32 lvl;
>  		} mls;
> -		u32 secid;
> +		struct lsmblob lsmblob;
>  	} attr;
>  };
>  
> @@ -429,7 +429,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
>  				const void *addr,
>  				const void *mask,
>  				u16 family,
> -				u32 secid,
> +				struct lsmblob *lsmblob,
>  				struct netlbl_audit *audit_info);
>  int netlbl_cfg_unlbl_static_del(struct net *net,
>  				const char *dev_name,
> @@ -537,7 +537,7 @@ static inline int netlbl_cfg_unlbl_static_add(struct net *net,
>  					      const void *addr,
>  					      const void *mask,
>  					      u16 family,
> -					      u32 secid,
> +					      struct lsmblob *lsmblob,
>  					      struct netlbl_audit *audit_info)
>  {
>  	return -ENOSYS;
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index f0165c5f376b..eb4939f38a14 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -1481,7 +1481,8 @@ static int cipso_v4_gentag_loc(const struct cipso_v4_doi *doi_def,
>  
>  	buffer[0] = CIPSO_V4_TAG_LOCAL;
>  	buffer[1] = CIPSO_V4_TAG_LOC_BLEN;
> -	*(u32 *)&buffer[2] = secattr->attr.secid;
> +	/* only one netlabel user - the first */
> +	*(u32 *)&buffer[2] = secattr->attr.lsmblob.secid[0];
>  
>  	return CIPSO_V4_TAG_LOC_BLEN;
>  }
> @@ -1501,7 +1502,8 @@ static int cipso_v4_parsetag_loc(const struct cipso_v4_doi *doi_def,
>  				 const unsigned char *tag,
>  				 struct netlbl_lsm_secattr *secattr)
>  {
> -	secattr->attr.secid = *(u32 *)&tag[2];
> +	/* only one netlabel user - the first */
> +	secattr->attr.lsmblob.secid[0] = *(u32 *)&tag[2];
>  	secattr->flags |= NETLBL_SECATTR_SECID;
>  
>  	return 0;
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index ee3e5b6471a6..724d44943543 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -210,7 +210,7 @@ int netlbl_cfg_unlbl_map_add(const char *domain,
>   * @addr: IP address in network byte order (struct in[6]_addr)
>   * @mask: address mask in network byte order (struct in[6]_addr)
>   * @family: address family
> - * @secid: LSM secid value for the entry
> + * @lsmblob: LSM data value for the entry
>   * @audit_info: NetLabel audit information
>   *
>   * Description:
> @@ -224,7 +224,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
>  				const void *addr,
>  				const void *mask,
>  				u16 family,
> -				u32 secid,
> +				struct lsmblob *lsmblob,
>  				struct netlbl_audit *audit_info)
>  {
>  	u32 addr_len;
> @@ -244,7 +244,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
>  
>  	return netlbl_unlhsh_add(net,
>  				 dev_name, addr, mask, addr_len,
> -				 secid, audit_info);
> +				 lsmblob, audit_info);
>  }
>  
>  /**
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 4716e0011ba5..57ede7781c8f 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -80,7 +80,7 @@ struct netlbl_unlhsh_tbl {
>  #define netlbl_unlhsh_addr4_entry(iter) \
>  	container_of(iter, struct netlbl_unlhsh_addr4, list)
>  struct netlbl_unlhsh_addr4 {
> -	u32 secid;
> +	struct lsmblob lsmblob;
>  
>  	struct netlbl_af4list list;
>  	struct rcu_head rcu;
> @@ -88,7 +88,7 @@ struct netlbl_unlhsh_addr4 {
>  #define netlbl_unlhsh_addr6_entry(iter) \
>  	container_of(iter, struct netlbl_unlhsh_addr6, list)
>  struct netlbl_unlhsh_addr6 {
> -	u32 secid;
> +	struct lsmblob lsmblob;
>  
>  	struct netlbl_af6list list;
>  	struct rcu_head rcu;
> @@ -233,7 +233,7 @@ static struct netlbl_unlhsh_iface *netlbl_unlhsh_search_iface(int ifindex)
>   * @iface: the associated interface entry
>   * @addr: IPv4 address in network byte order
>   * @mask: IPv4 address mask in network byte order
> - * @secid: LSM secid value for entry
> + * @lsmblob: LSM data value for entry
>   *
>   * Description:
>   * Add a new address entry into the unlabeled connection hash table using the
> @@ -244,7 +244,7 @@ static struct netlbl_unlhsh_iface *netlbl_unlhsh_search_iface(int ifindex)
>  static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
>  				   const struct in_addr *addr,
>  				   const struct in_addr *mask,
> -				   u32 secid)
> +				   struct lsmblob *lsmblob)
>  {
>  	int ret_val;
>  	struct netlbl_unlhsh_addr4 *entry;
> @@ -256,7 +256,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
>  	entry->list.addr = addr->s_addr & mask->s_addr;
>  	entry->list.mask = mask->s_addr;
>  	entry->list.valid = 1;
> -	entry->secid = secid;
> +	entry->lsmblob = *lsmblob;
>  
>  	spin_lock(&netlbl_unlhsh_lock);
>  	ret_val = netlbl_af4list_add(&entry->list, &iface->addr4_list);
> @@ -273,7 +273,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
>   * @iface: the associated interface entry
>   * @addr: IPv6 address in network byte order
>   * @mask: IPv6 address mask in network byte order
> - * @secid: LSM secid value for entry
> + * @lsmblob: LSM data value for entry
>   *
>   * Description:
>   * Add a new address entry into the unlabeled connection hash table using the
> @@ -284,7 +284,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
>  static int netlbl_unlhsh_add_addr6(struct netlbl_unlhsh_iface *iface,
>  				   const struct in6_addr *addr,
>  				   const struct in6_addr *mask,
> -				   u32 secid)
> +				   struct lsmblob *lsmblob)
>  {
>  	int ret_val;
>  	struct netlbl_unlhsh_addr6 *entry;
> @@ -300,7 +300,7 @@ static int netlbl_unlhsh_add_addr6(struct netlbl_unlhsh_iface *iface,
>  	entry->list.addr.s6_addr32[3] &= mask->s6_addr32[3];
>  	entry->list.mask = *mask;
>  	entry->list.valid = 1;
> -	entry->secid = secid;
> +	entry->lsmblob = *lsmblob;
>  
>  	spin_lock(&netlbl_unlhsh_lock);
>  	ret_val = netlbl_af6list_add(&entry->list, &iface->addr6_list);
> @@ -379,7 +379,7 @@ int netlbl_unlhsh_add(struct net *net,
>  		      const void *addr,
>  		      const void *mask,
>  		      u32 addr_len,
> -		      u32 secid,
> +		      struct lsmblob *lsmblob,
>  		      struct netlbl_audit *audit_info)
>  {
>  	int ret_val;
> @@ -388,7 +388,6 @@ int netlbl_unlhsh_add(struct net *net,
>  	struct netlbl_unlhsh_iface *iface;
>  	struct audit_buffer *audit_buf = NULL;
>  	struct lsmcontext context;
> -	struct lsmblob blob;
>  
>  	if (addr_len != sizeof(struct in_addr) &&
>  	    addr_len != sizeof(struct in6_addr))
> @@ -421,7 +420,7 @@ int netlbl_unlhsh_add(struct net *net,
>  		const struct in_addr *addr4 = addr;
>  		const struct in_addr *mask4 = mask;
>  
> -		ret_val = netlbl_unlhsh_add_addr4(iface, addr4, mask4, secid);
> +		ret_val = netlbl_unlhsh_add_addr4(iface, addr4, mask4, lsmblob);
>  		if (audit_buf != NULL)
>  			netlbl_af4list_audit_addr(audit_buf, 1,
>  						  dev_name,
> @@ -434,7 +433,7 @@ int netlbl_unlhsh_add(struct net *net,
>  		const struct in6_addr *addr6 = addr;
>  		const struct in6_addr *mask6 = mask;
>  
> -		ret_val = netlbl_unlhsh_add_addr6(iface, addr6, mask6, secid);
> +		ret_val = netlbl_unlhsh_add_addr6(iface, addr6, mask6, lsmblob);
>  		if (audit_buf != NULL)
>  			netlbl_af6list_audit_addr(audit_buf, 1,
>  						  dev_name,
> @@ -451,8 +450,7 @@ int netlbl_unlhsh_add(struct net *net,
>  unlhsh_add_return:
>  	rcu_read_unlock();
>  	if (audit_buf != NULL) {
> -		lsmblob_init(&blob, secid);
> -		if (security_secid_to_secctx(&blob, &context) == 0) {
> +		if (security_secid_to_secctx(lsmblob, &context) == 0) {
>  			audit_log_format(audit_buf, " sec_obj=%s",
>  					 context.context);
>  			security_release_secctx(&context);
> @@ -487,7 +485,6 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
>  	struct audit_buffer *audit_buf;
>  	struct net_device *dev;
>  	struct lsmcontext context;
> -	struct lsmblob blob;
>  
>  	spin_lock(&netlbl_unlhsh_lock);
>  	list_entry = netlbl_af4list_remove(addr->s_addr, mask->s_addr,
> @@ -507,10 +504,8 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
>  					  addr->s_addr, mask->s_addr);
>  		if (dev != NULL)
>  			dev_put(dev);
> -		if (entry != NULL)
> -			lsmblob_init(&blob, entry->secid);
>  		if (entry != NULL &&
> -		    security_secid_to_secctx(&blob, &context) == 0) {
> +		    security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
>  			audit_log_format(audit_buf, " sec_obj=%s",
>  					 context.context);
>  			security_release_secctx(&context);
> @@ -551,7 +546,6 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
>  	struct audit_buffer *audit_buf;
>  	struct net_device *dev;
>  	struct lsmcontext context;
> -	struct lsmblob blob;
>  
>  	spin_lock(&netlbl_unlhsh_lock);
>  	list_entry = netlbl_af6list_remove(addr, mask, &iface->addr6_list);
> @@ -570,10 +564,8 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
>  					  addr, mask);
>  		if (dev != NULL)
>  			dev_put(dev);
> -		if (entry != NULL)
> -			lsmblob_init(&blob, entry->secid);
>  		if (entry != NULL &&
> -		    security_secid_to_secctx(&blob, &context) == 0) {
> +		    security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
>  			audit_log_format(audit_buf, " sec_obj=%s",
>  					 context.context);
>  			security_release_secctx(&context);
> @@ -927,9 +919,8 @@ static int netlbl_unlabel_staticadd(struct sk_buff *skb,
>  	if (ret_val != 0)
>  		return ret_val;
>  
> -	/* scaffolding with the [0] */
>  	return netlbl_unlhsh_add(&init_net,
> -				 dev_name, addr, mask, addr_len, blob.secid[0],
> +				 dev_name, addr, mask, addr_len, &blob,
>  				 &audit_info);
>  }
>  
> @@ -977,10 +968,8 @@ static int netlbl_unlabel_staticadddef(struct sk_buff *skb,
>  	if (ret_val != 0)
>  		return ret_val;
>  
> -	/* scaffolding with the [0] */
>  	return netlbl_unlhsh_add(&init_net,
> -				 NULL, addr, mask, addr_len, blob.secid[0],
> -				 &audit_info);
> +				 NULL, addr, mask, addr_len, &blob, &audit_info);
>  }
>  
>  /**
> @@ -1092,8 +1081,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
>  	struct net_device *dev;
>  	struct lsmcontext context;
>  	void *data;
> -	u32 secid;
> -	struct lsmblob blob;
> +	struct lsmblob *lsmb;
>  
>  	data = genlmsg_put(cb_arg->skb, NETLINK_CB(cb_arg->nl_cb->skb).portid,
>  			   cb_arg->seq, &netlbl_unlabel_gnl_family,
> @@ -1131,7 +1119,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
>  		if (ret_val != 0)
>  			goto list_cb_failure;
>  
> -		secid = addr4->secid;
> +		lsmb = (struct lsmblob *)&addr4->lsmblob;
>  	} else {
>  		ret_val = nla_put_in6_addr(cb_arg->skb,
>  					   NLBL_UNLABEL_A_IPV6ADDR,
> @@ -1145,11 +1133,10 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
>  		if (ret_val != 0)
>  			goto list_cb_failure;
>  
> -		secid = addr6->secid;
> +		lsmb = (struct lsmblob *)&addr6->lsmblob;
>  	}
>  
> -	lsmblob_init(&blob, secid);
> -	ret_val = security_secid_to_secctx(&blob, &context);
> +	ret_val = security_secid_to_secctx(lsmb, &context);
>  	if (ret_val != 0)
>  		goto list_cb_failure;
>  	ret_val = nla_put(cb_arg->skb,
> @@ -1500,7 +1487,7 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
>  					      &iface->addr4_list);
>  		if (addr4 == NULL)
>  			goto unlabel_getattr_nolabel;
> -		secattr->attr.secid = netlbl_unlhsh_addr4_entry(addr4)->secid;
> +		secattr->attr.lsmblob = netlbl_unlhsh_addr4_entry(addr4)->lsmblob;
>  		break;
>  	}
>  #if IS_ENABLED(CONFIG_IPV6)
> @@ -1513,7 +1500,7 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
>  					      &iface->addr6_list);
>  		if (addr6 == NULL)
>  			goto unlabel_getattr_nolabel;
> -		secattr->attr.secid = netlbl_unlhsh_addr6_entry(addr6)->secid;
> +		secattr->attr.lsmblob = netlbl_unlhsh_addr6_entry(addr6)->lsmblob;
>  		break;
>  	}
>  #endif /* IPv6 */
> diff --git a/net/netlabel/netlabel_unlabeled.h b/net/netlabel/netlabel_unlabeled.h
> index 3a9e5dc9511b..dcff99695c97 100644
> --- a/net/netlabel/netlabel_unlabeled.h
> +++ b/net/netlabel/netlabel_unlabeled.h
> @@ -225,7 +225,7 @@ int netlbl_unlhsh_add(struct net *net,
>  		      const void *addr,
>  		      const void *mask,
>  		      u32 addr_len,
> -		      u32 secid,
> +		      struct lsmblob *lsmblob,
>  		      struct netlbl_audit *audit_info);
>  int netlbl_unlhsh_remove(struct net *net,
>  			 const char *dev_name,
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index b5b7c5aade8c..94787988c8fb 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -68,6 +68,7 @@
>  struct netlbl_lsm_secattr;
>  
>  extern int selinux_enabled;
> +extern int selinux_lsmblob_slot;
>  
>  /* Policy capabilities */
>  enum {
> diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
> index c40914a157b7..320a4cdc657e 100644
> --- a/security/selinux/netlabel.c
> +++ b/security/selinux/netlabel.c
> @@ -122,7 +122,7 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_getattr(
>  		return NULL;
>  
>  	if ((secattr->flags & NETLBL_SECATTR_SECID) &&
> -	    (secattr->attr.secid == sid))
> +	    (secattr->attr.lsmblob.secid[selinux_lsmblob_slot] == sid))
>  		return secattr;
>  
>  	return NULL;
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index e3f5d6aece66..a3be1afafd7f 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -3593,7 +3593,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state,
>  	if (secattr->flags & NETLBL_SECATTR_CACHE)
>  		*sid = *(u32 *)secattr->cache->data;
>  	else if (secattr->flags & NETLBL_SECATTR_SECID)
> -		*sid = secattr->attr.secid;
> +		*sid = secattr->attr.lsmblob.secid[selinux_lsmblob_slot];
>  	else if (secattr->flags & NETLBL_SECATTR_MLS_LVL) {
>  		rc = -EIDRM;
>  		ctx = sidtab_search(sidtab, SECINITSID_NETMSG);
> @@ -3666,7 +3666,7 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
>  	if (secattr->domain == NULL)
>  		goto out;
>  
> -	secattr->attr.secid = sid;
> +	secattr->attr.lsmblob.secid[selinux_lsmblob_slot] = sid;
>  	secattr->flags |= NETLBL_SECATTR_DOMAIN_CPY | NETLBL_SECATTR_SECID;
>  	mls_export_netlbl_lvl(policydb, ctx, secattr);
>  	rc = mls_export_netlbl_cat(policydb, ctx, secattr);
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 7cc3a3382fee..097ffde7f202 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -320,6 +320,7 @@ void smk_destroy_label_list(struct list_head *list);
>   * Shared data.
>   */
>  extern int smack_enabled;
> +extern int smack_lsmblob_slot;
>  extern int smack_cipso_direct;
>  extern int smack_cipso_mapped;
>  extern struct smack_known *smack_net_ambient;
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 273f311fb153..b83aba0f2013 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3742,7 +3742,8 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap,
>  		/*
>  		 * Looks like a fallback, which gives us a secid.
>  		 */
> -		return smack_from_secid(sap->attr.secid);
> +		return smack_from_secid(
> +				sap->attr.lsmblob.secid[smack_lsmblob_slot]);
>  	/*
>  	 * Without guidance regarding the smack value
>  	 * for the packet fall back on the network
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index faf2ea3968b3..066d53c29ed4 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -1150,6 +1150,7 @@ static void smk_net4addr_insert(struct smk_net4addr *new)
>  static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
>  				size_t count, loff_t *ppos)
>  {
> +	struct lsmblob lsmblob;
>  	struct smk_net4addr *snp;
>  	struct sockaddr_in newname;
>  	char *smack;
> @@ -1281,10 +1282,13 @@ static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
>  	 * this host so that incoming packets get labeled.
>  	 * but only if we didn't get the special CIPSO option
>  	 */
> -	if (rc == 0 && skp != NULL)
> +	if (rc == 0 && skp != NULL) {
> +		lsmblob_init(&lsmblob, 0);
> +		lsmblob.secid[smack_lsmblob_slot] = snp->smk_label->smk_secid;
>  		rc = netlbl_cfg_unlbl_static_add(&init_net, NULL,
> -			&snp->smk_host, &snp->smk_mask, PF_INET,
> -			snp->smk_label->smk_secid, &audit_info);
> +			&snp->smk_host, &snp->smk_mask, PF_INET, &lsmblob,
> +			&audit_info);
> +	}
>  
>  	if (rc == 0)
>  		rc = count;
> 


^ permalink raw reply

* Re: [PATCH v3 22/24] LSM: Return the lsmblob slot on initialization
From: John Johansen @ 2019-06-24 21:39 UTC (permalink / raw)
  To: Kees Cook, Casey Schaufler
  Cc: casey.schaufler, jmorris, linux-security-module, selinux,
	penguin-kernel, paul, sds
In-Reply-To: <201906221613.3443FA528B@keescook>

On 6/22/19 4:13 PM, Kees Cook wrote:
> On Fri, Jun 21, 2019 at 11:52:31AM -0700, Casey Schaufler wrote:
>> Return the slot allocated to the calling LSM in the lsmblob
>> structure. This can be used to set lsmblobs explicitly for
>> netlabel interfaces.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> (I have some thoughts on refactoring the slot assignment, but that
> should happen after this series -- it's nothing more than a storage
> optimization.)
> 
> -Kees

haha so do I, now I am curious as to how close they align


> 
>> ---
>>  include/linux/lsm_hooks.h  | 4 ++--
>>  security/apparmor/lsm.c    | 8 ++++++--
>>  security/security.c        | 9 +++++++--
>>  security/selinux/hooks.c   | 5 ++++-
>>  security/smack/smack_lsm.c | 5 ++++-
>>  5 files changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 4d1ddf1a2aa6..ce341bcbce5d 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -2068,8 +2068,8 @@ struct lsm_blob_sizes {
>>  extern struct security_hook_heads security_hook_heads;
>>  extern char *lsm_names;
>>  
>> -extern void security_add_hooks(struct security_hook_list *hooks, int count,
>> -				char *lsm);
>> +extern int security_add_hooks(struct security_hook_list *hooks, int count,
>> +			      char *lsm);
>>  
>>  #define LSM_FLAG_LEGACY_MAJOR	BIT(0)
>>  #define LSM_FLAG_EXCLUSIVE	BIT(1)
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index 2716e7731279..dcbbefbd95ff 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -47,6 +47,9 @@
>>  /* Flag indicating whether initialization completed */
>>  int apparmor_initialized;
>>  
>> +/* Slot for the AppArmor secid in the lsmblob structure */
>> +int apparmor_lsmblob_slot;
>> +
>>  DEFINE_PER_CPU(struct aa_buffers, aa_buffers);
>>  
>>  
>> @@ -1678,8 +1681,9 @@ static int __init apparmor_init(void)
>>  		aa_free_root_ns();
>>  		goto buffers_out;
>>  	}
>> -	security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
>> -				"apparmor");
>> +	apparmor_lsmblob_slot = security_add_hooks(apparmor_hooks,
>> +						   ARRAY_SIZE(apparmor_hooks),
>> +						   "apparmor");
>>  
>>  	/* Report that AppArmor successfully initialized */
>>  	apparmor_initialized = 1;
>> diff --git a/security/security.c b/security/security.c
>> index b2ffcd1f3057..c93a368b697b 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -437,9 +437,12 @@ static int lsm_slot __initdata;
>>   * Each LSM has to register its hooks with the infrastructure.
>>   * If the LSM is using hooks that export secids allocate a slot
>>   * for it in the lsmblob.
>> + *
>> + * Returns the slot number in the lsmblob structure if one is
>> + * allocated or LSMBLOB_INVALID if one was not allocated.
>>   */
>> -void __init security_add_hooks(struct security_hook_list *hooks, int count,
>> -				char *lsm)
>> +int __init security_add_hooks(struct security_hook_list *hooks, int count,
>> +			      char *lsm)
>>  {
>>  	int slot = LSMBLOB_INVALID;
>>  	int i;
>> @@ -479,6 +482,8 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>  	}
>>  	if (lsm_append(lsm, &lsm_names) < 0)
>>  		panic("%s - Cannot get early memory.\n", __func__);
>> +
>> +	return slot;
>>  }
>>  
>>  int call_lsm_notifier(enum lsm_event event, void *data)
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index ee840fecfebb..1e09acbf9630 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -103,6 +103,7 @@
>>  #include "avc_ss.h"
>>  
>>  struct selinux_state selinux_state;
>> +int selinux_lsmblob_slot;
>>  
>>  /* SECMARK reference count */
>>  static atomic_t selinux_secmark_refcount = ATOMIC_INIT(0);
>> @@ -6877,7 +6878,9 @@ static __init int selinux_init(void)
>>  
>>  	hashtab_cache_init();
>>  
>> -	security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
>> +	selinux_lsmblob_slot = security_add_hooks(selinux_hooks,
>> +						  ARRAY_SIZE(selinux_hooks),
>> +						  "selinux");
>>  
>>  	if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
>>  		panic("SELinux: Unable to register AVC netcache callback\n");
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 3834b751d1e9..273f311fb153 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -60,6 +60,7 @@ static LIST_HEAD(smk_ipv6_port_list);
>>  #endif
>>  static struct kmem_cache *smack_inode_cache;
>>  int smack_enabled;
>> +int smack_lsmblob_slot;
>>  
>>  #define A(s) {"smack"#s, sizeof("smack"#s) - 1, Opt_##s}
>>  static struct {
>> @@ -4749,7 +4750,9 @@ static __init int smack_init(void)
>>  	/*
>>  	 * Register with LSM
>>  	 */
>> -	security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
>> +	smack_lsmblob_slot = security_add_hooks(smack_hooks,
>> +						ARRAY_SIZE(smack_hooks),
>> +						"smack");
>>  	smack_enabled = 1;
>>  
>>  	pr_info("Smack:  Initializing.\n");
>> -- 
>> 2.20.1
>>
> 


^ permalink raw reply

* Re: [PATCH v3 21/24] Audit: Store LSM audit information in an lsmblob
From: John Johansen @ 2019-06-24 21:33 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds, Eric Paris
In-Reply-To: <20190621185233.6766-22-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change the audit code to store full lsmblob data instead of
> a single u32 secid. This allows for multiple security modules
> to use the audit system at the same time. It also allows the
> removal of scaffolding code that was included during the
> revision of LSM interfaces.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

I know Kees raised this too, but I haven't seen a reply

Eric (Paul is already CCed): I have directly added you because of
the question below.

In summary there isn't necessarily a single secid any more, and
we need to know whether dropping the logging of the secid or
logging all secids is the correct action.

> ---
>  kernel/audit.h   |  6 +++---
>  kernel/auditsc.c | 38 +++++++++++---------------------------
>  2 files changed, 14 insertions(+), 30 deletions(-)
> 
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 29e29c6f4afb..a8dd479e9556 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -91,7 +91,7 @@ struct audit_names {
>  	kuid_t			uid;
>  	kgid_t			gid;
>  	dev_t			rdev;
> -	u32			osid;
> +	struct lsmblob		olsm;
>  	struct audit_cap_data	fcap;
>  	unsigned int		fcap_ver;
>  	unsigned char		type;		/* record type */
> @@ -148,7 +148,7 @@ struct audit_context {
>  	kuid_t		    target_auid;
>  	kuid_t		    target_uid;
>  	unsigned int	    target_sessionid;
> -	struct lsmblob   target_lsm;
> +	struct lsmblob	    target_lsm;
>  	char		    target_comm[TASK_COMM_LEN];
>  
>  	struct audit_tree_refs *trees, *first_trees;
> @@ -165,7 +165,7 @@ struct audit_context {
>  			kuid_t			uid;
>  			kgid_t			gid;
>  			umode_t			mode;
> -			u32			osid;
> +			struct lsmblob		olsm;
>  			int			has_perm;
>  			uid_t			perm_uid;
>  			gid_t			perm_gid;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 0478680cd0a8..d3ad13f11788 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -646,17 +646,15 @@ static int audit_filter_rules(struct task_struct *tsk,
>  			if (f->lsm_rule) {
>  				/* Find files that match */
>  				if (name) {
> -					lsmblob_init(&blob, name->osid);
>  					result = security_audit_rule_match(
> -								&blob,
> +								&name->olsm,
>  								f->type,
>  								f->op,
>  								f->lsm_rule);
>  				} else if (ctx) {
>  					list_for_each_entry(n, &ctx->names_list, list) {
> -						lsmblob_init(&blob, n->osid);
>  						if (security_audit_rule_match(
> -								&blob,
> +								&n->olsm,
>  								f->type,
>  								f->op,
>  								f->lsm_rule)) {
> @@ -668,8 +666,7 @@ static int audit_filter_rules(struct task_struct *tsk,
>  				/* Find ipc objects that match */
>  				if (!ctx || ctx->type != AUDIT_IPC)
>  					break;
> -				lsmblob_init(&blob, ctx->ipc.osid);
> -				if (security_audit_rule_match(&blob,
> +				if (security_audit_rule_match(&ctx->ipc.olsm,
>  							      f->type, f->op,
>  							      f->lsm_rule))
>  					++result;
> @@ -1187,21 +1184,18 @@ static void show_special(struct audit_context *context, int *call_panic)
>  				context->socketcall.args[i]);
>  		break; }
>  	case AUDIT_IPC: {
> -		u32 osid = context->ipc.osid;
> +		struct lsmblob *olsm = &context->ipc.olsm;
>  
>  		audit_log_format(ab, "ouid=%u ogid=%u mode=%#ho",
>  				 from_kuid(&init_user_ns, context->ipc.uid),
>  				 from_kgid(&init_user_ns, context->ipc.gid),
>  				 context->ipc.mode);
> -		if (osid) {
> +		if (lsmblob_is_set(olsm)) {
>  			struct lsmcontext lsmcxt;
> -			struct lsmblob blob;
>  
> -			lsmblob_init(&blob, osid);
> -			if (security_secid_to_secctx(&blob, &lsmcxt)) {
> -				audit_log_format(ab, " osid=%u", osid);

I am not comfortable just dropping this I would think logging all secids is the
correct action here.


> +			if (security_secid_to_secctx(olsm, &lsmcxt))
>  				*call_panic = 1;
> -			} else {
> +			else {
>  				audit_log_format(ab, " obj=%s", lsmcxt.context);
>  				security_release_secctx(&lsmcxt);
>  			}
> @@ -1346,13 +1340,10 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
>  				 from_kgid(&init_user_ns, n->gid),
>  				 MAJOR(n->rdev),
>  				 MINOR(n->rdev));
> -	if (n->osid != 0) {
> -		struct lsmblob blob;
> +	if (lsmblob_is_set(&n->olsm)) {
>  		struct lsmcontext lsmctx;
>  
> -		lsmblob_init(&blob, n->osid);
> -		if (security_secid_to_secctx(&blob, &lsmctx)) {
> -			audit_log_format(ab, " osid=%u", n->osid);

and here


> +		if (security_secid_to_secctx(&n->olsm, &lsmctx)) {
>  			if (call_panic)
>  				*call_panic = 2;
>  		} else {
> @@ -1906,17 +1897,13 @@ static inline int audit_copy_fcaps(struct audit_names *name,
>  void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
>  		      struct inode *inode, unsigned int flags)
>  {
> -	struct lsmblob blob;
> -
>  	name->ino   = inode->i_ino;
>  	name->dev   = inode->i_sb->s_dev;
>  	name->mode  = inode->i_mode;
>  	name->uid   = inode->i_uid;
>  	name->gid   = inode->i_gid;
>  	name->rdev  = inode->i_rdev;
> -	security_inode_getsecid(inode, &blob);
> -	/* scaffolding until osid is updated */
> -	name->osid = blob.secid[0];
> +	security_inode_getsecid(inode, &name->olsm);
>  	if (flags & AUDIT_INODE_NOEVAL) {
>  		name->fcap_ver = -1;
>  		return;
> @@ -2266,14 +2253,11 @@ void __audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat)
>  void __audit_ipc_obj(struct kern_ipc_perm *ipcp)
>  {
>  	struct audit_context *context = audit_context();
> -	struct lsmblob blob;
>  	context->ipc.uid = ipcp->uid;
>  	context->ipc.gid = ipcp->gid;
>  	context->ipc.mode = ipcp->mode;
>  	context->ipc.has_perm = 0;
> -	security_ipc_getsecid(ipcp, &blob);
> -	/* scaffolding on the [0] - change "osid" to a lsmblob */
> -	context->ipc.osid = blob.secid[0];
> +	security_ipc_getsecid(ipcp, &context->ipc.olsm);
>  	context->type = AUDIT_IPC;
>  }
>  
> 


^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-24 21:30 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andy Lutomirski, James Morris, LSM List,
	Linux Kernel Mailing List, Linux API, David Howells,
	Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
	bpf
In-Reply-To: <7f36edf7-3120-975e-b643-3c0fa470bafd@iogearbox.net>

On Mon, Jun 24, 2019 at 2:22 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> Agree, for example, bpf_probe_write_user() can never write into
> kernel memory (only user one). Just thinking out loud, wouldn't it
> be cleaner and more generic to perform this check at the actual function
> which performs the kernel memory without faulting? All three of these
> are in mm/maccess.c, and the very few occasions that override the
> probe_kernel_read symbol are calling eventually into __probe_kernel_read(),
> so this would catch all of them wrt lockdown restrictions. Otherwise
> you'd need to keep tracking every bit of new code being merged that
> calls into one of these, no? That way you only need to do it once like
> below and are guaranteed that the check catches these in future as well.

Not all paths into probe_kernel_read/write are from entry points that
need to be locked down (eg, as far as I can tell ftrace can't leak
anything interesting here).

^ permalink raw reply

* Re: [PATCH V31 07/25] kexec_file: Restrict at runtime if the kernel is locked down
From: Mimi Zohar @ 2019-06-24 21:27 UTC (permalink / raw)
  To: Matthew Garrett, Dave Young
  Cc: James Morris, Jiri Bohac, Linux API, kexec,
	Linux Kernel Mailing List, David Howells, LSM List,
	Andy Lutomirski
In-Reply-To: <CACdnJusPtYLdg7ZPhBo=Y5EsBz6B+5M2zYscBrLcc89oNnPkdQ@mail.gmail.com>

Hi Matthew,

On Mon, 2019-06-24 at 14:06 -0700, Matthew Garrett wrote:
> On Sun, Jun 23, 2019 at 6:52 PM Dave Young <dyoung@redhat.com> wrote:
> >
> > On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> > > I don't think so - we want it to be possible to load images if they
> > > have a valid signature.
> >
> > I know it works like this way because of the previous patch.  But from
> > the patch log "When KEXEC_SIG is not enabled, kernel should not load
> > images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
> > kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)  instead of depending
> > on the late code to verify signature.  In that way, easier to
> > understand the logic, no?
> 
> But that combination doesn't enforce signature validation? We can't
> depend on !IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) because then it'll
> enforce signature validation even if lockdown is disabled.

I agree with Dave.  There should be a stub lockdown function to
prevent enforcing lockdown when it isn't enabled.

Mimi


^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Daniel Borkmann @ 2019-06-24 20:59 UTC (permalink / raw)
  To: Andy Lutomirski, Matthew Garrett
  Cc: James Morris, LSM List, Linux Kernel Mailing List, Linux API,
	David Howells, Alexei Starovoitov, Network Development,
	Chun-Yi Lee, Jann Horn, bpf
In-Reply-To: <CALCETrWmZX3R1L88Gz9vLY68gcK8zSXL4cA4GqAzQoyqSR7rRQ@mail.gmail.com>

On 06/24/2019 10:08 PM, Andy Lutomirski wrote:
> On Mon, Jun 24, 2019 at 12:54 PM Matthew Garrett <mjg59@google.com> wrote:
>> On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>> On 06/22/2019 02:03 AM, Matthew Garrett wrote:
>>>> From: David Howells <dhowells@redhat.com>
>>>>
>>>> There are some bpf functions can be used to read kernel memory:
>>>
>>> Nit: that
>>
>> Fixed.
>>
>>>> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
>>>
>>> Please explain how bpf_probe_write_user reads kernel memory ... ?!
>>
>> Ha.
>>
>>>> private keys in kernel memory (e.g. the hibernation image signing key) to
>>>> be read by an eBPF program and kernel memory to be altered without
>>>
>>> ... and while we're at it, also how they allow "kernel memory to be
>>> altered without restriction". I've been pointing this false statement
>>> out long ago.
>>
>> Yup. How's the following description:
>>
>>     bpf: Restrict bpf when kernel lockdown is in confidentiality mode
>>
>>     There are some bpf functions that can be used to read kernel memory and
>>     exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
>>     bpf_trace_printk.  These could be abused to (eg) allow private
>> keys in kernel
>>     memory to be leaked. Disable them if the kernel has been locked
>> down in confidentiality
>>     mode.
> 
> I'm confused.  I understand why we're restricting bpf_probe_read().
> Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
> though?

Agree, for example, bpf_probe_write_user() can never write into
kernel memory (only user one). Just thinking out loud, wouldn't it
be cleaner and more generic to perform this check at the actual function
which performs the kernel memory without faulting? All three of these
are in mm/maccess.c, and the very few occasions that override the
probe_kernel_read symbol are calling eventually into __probe_kernel_read(),
so this would catch all of them wrt lockdown restrictions. Otherwise
you'd need to keep tracking every bit of new code being merged that
calls into one of these, no? That way you only need to do it once like
below and are guaranteed that the check catches these in future as well.

Thanks,
Daniel

diff --git a/mm/maccess.c b/mm/maccess.c
index 482d4d6..2c8220f 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -29,6 +29,9 @@ long __probe_kernel_read(void *dst, const void *src, size_t size)
 	long ret;
 	mm_segment_t old_fs = get_fs();

+	if (security_locked_down(LOCKDOWN_KERNEL_READ))
+		return -EFAULT;
+
 	set_fs(KERNEL_DS);
 	pagefault_disable();
 	ret = __copy_from_user_inatomic(dst,
@@ -57,6 +60,9 @@ long __probe_kernel_write(void *dst, const void *src, size_t size)
 	long ret;
 	mm_segment_t old_fs = get_fs();

+	if (security_locked_down(LOCKDOWN_KERNEL_WRITE))
+		return -EFAULT;
+
 	set_fs(KERNEL_DS);
 	pagefault_disable();
 	ret = __copy_to_user_inatomic((__force void __user *)dst, src, size);
@@ -90,6 +96,9 @@ long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count)
 	const void *src = unsafe_addr;
 	long ret;

+	if (security_locked_down(LOCKDOWN_KERNEL_READ))
+		return -EFAULT;
+
 	if (unlikely(count <= 0))
 		return 0;


^ permalink raw reply related

* Re: [PATCH v3 10/24] Use lsmblob in security_ipc_getsecid
From: Kees Cook @ 2019-06-24 21:09 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: casey.schaufler, jmorris, linux-security-module, selinux,
	john.johansen, penguin-kernel, paul, sds
In-Reply-To: <a70ad13e-bc69-ef03-1f9a-3378c38cae23@schaufler-ca.com>

On Mon, Jun 24, 2019 at 09:39:05AM -0700, Casey Schaufler wrote:
> On 6/22/2019 3:48 PM, Kees Cook wrote:
> > On Fri, Jun 21, 2019 at 11:52:19AM -0700, Casey Schaufler wrote:
> >> +	struct security_hook_list *hp;
> >> +
> >> +	lsmblob_init(blob, 0);
> >> +	hlist_for_each_entry(hp, &security_hook_heads.ipc_getsecid, list)
> >> +		hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]);
> > Just for sanity when using hp->slot, it might be good to do something
> > like this in the places it gets used. Like for here:
> >
> > 	if (!WARN_ON(hp->slot < 0 || hp->slot >= LSMBLOB_COUNT))
> > 		hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]);
> >
> > This _should_ be overkill, but since lists of hooks that trigger slot
> > assignment is hardcoded, it seems nice to cover any future problems or
> > mismatches.
> 
> How about a CONFIG_LSM_SLOT_CHECK around a function lsm_slot_check()?
> If configured, it does the WARN_ON, and if not it's a static inline
> true return. As you say, it's probably overkill, but it would be available
> for the paranoid/debug/bringup situation.

No, this doesn't need another CONFIG. The test is nearly free and WARN
means it's wrapped in an unlikely already. I think just adding it
outright would be fine.

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH V31 07/25] kexec_file: Restrict at runtime if the kernel is locked down
From: Matthew Garrett @ 2019-06-24 21:06 UTC (permalink / raw)
  To: Dave Young
  Cc: James Morris, Jiri Bohac, Linux API, kexec,
	Linux Kernel Mailing List, David Howells, LSM List,
	Andy Lutomirski
In-Reply-To: <20190624015206.GB2976@dhcp-128-65.nay.redhat.com>

On Sun, Jun 23, 2019 at 6:52 PM Dave Young <dyoung@redhat.com> wrote:
>
> On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> > I don't think so - we want it to be possible to load images if they
> > have a valid signature.
>
> I know it works like this way because of the previous patch.  But from
> the patch log "When KEXEC_SIG is not enabled, kernel should not load
> images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
> kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)  instead of depending
> on the late code to verify signature.  In that way, easier to
> understand the logic, no?

But that combination doesn't enforce signature validation? We can't
depend on !IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) because then it'll
enforce signature validation even if lockdown is disabled.

^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-24 20:15 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Daniel Borkmann, James Morris, LSM List,
	Linux Kernel Mailing List, Linux API, David Howells,
	Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
	bpf
In-Reply-To: <CALCETrWmZX3R1L88Gz9vLY68gcK8zSXL4cA4GqAzQoyqSR7rRQ@mail.gmail.com>

On Mon, Jun 24, 2019 at 1:09 PM Andy Lutomirski <luto@kernel.org> wrote:

> I'm confused.  I understand why we're restricting bpf_probe_read().
> Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
> though?

Hmm. I think the thinking here was around exfiltration mechanisms, but
if the read is blocked then that seems less likely. This seems to
trace back to http://kernsec.org/pipermail/linux-security-module-archive/2017-October/003545.html
- Joey, do you know the reasoning here?

^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Andy Lutomirski @ 2019-06-24 20:08 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Daniel Borkmann, James Morris, LSM List,
	Linux Kernel Mailing List, Linux API, David Howells,
	Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
	bpf
In-Reply-To: <CACdnJuvR2bn3y3fYzg06GWXXgAGjgED2Dfa5g0oAwJ28qCCqBg@mail.gmail.com>

On Mon, Jun 24, 2019 at 12:54 PM Matthew Garrett <mjg59@google.com> wrote:
>
> On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> > > From: David Howells <dhowells@redhat.com>
> > >
> > > There are some bpf functions can be used to read kernel memory:
> >
> > Nit: that
>
> Fixed.
>
> > > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
> >
> > Please explain how bpf_probe_write_user reads kernel memory ... ?!
>
> Ha.
>
> > > private keys in kernel memory (e.g. the hibernation image signing key) to
> > > be read by an eBPF program and kernel memory to be altered without
> >
> > ... and while we're at it, also how they allow "kernel memory to be
> > altered without restriction". I've been pointing this false statement
> > out long ago.
>
> Yup. How's the following description:
>
>     bpf: Restrict bpf when kernel lockdown is in confidentiality mode
>
>     There are some bpf functions that can be used to read kernel memory and
>     exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
>     bpf_trace_printk.  These could be abused to (eg) allow private
> keys in kernel
>     memory to be leaked. Disable them if the kernel has been locked
> down in confidentiality
>     mode.

I'm confused.  I understand why we're restricting bpf_probe_read().
Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
though?

--Andy

^ permalink raw reply

* Re: [PATCH v11 02/13] PKCS#7: Refactor verify_pkcs7_signature()
From: Thiago Jung Bauermann @ 2019-06-24 19:56 UTC (permalink / raw)
  To: David Howells
  Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
	linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
	James Morris, Serge E. Hallyn, David Woodhouse, Jessica Yu,
	Herbert Xu, David S. Miller, Jonathan Corbet, AKASHI, Takahiro,
	linux-integrity
In-Reply-To: <20190611062817.18412-3-bauerman@linux.ibm.com>


Hello David,

AFAIK Mimi is happy with this patch set, but I still need acks from
maintainers of other subsystems that my changes touch before she can
accept it.

Are this patch and the next one ("PKCS#7: Introduce pkcs7_get_digest()")
OK from your PoV?

--
Thiago Jung Bauermann
IBM Linux Technology Center


Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:

> IMA will need to verify a PKCS#7 signature which has already been parsed.
> For this reason, factor out the code which does that from
> verify_pkcs7_signature() into a new function which takes a struct
> pkcs7_message instead of a data buffer.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
>  certs/system_keyring.c       | 61 ++++++++++++++++++++++++++----------
>  include/linux/verification.h | 10 ++++++
>  2 files changed, 55 insertions(+), 16 deletions(-)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index c05c29ae4d5d..4ba82e52e4b4 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -194,33 +194,27 @@ late_initcall(load_system_certificate_list);
>  #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
>
>  /**
> - * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
> + * verify_pkcs7_message_sig - Verify a PKCS#7-based signature on system data.
>   * @data: The data to be verified (NULL if expecting internal data).
>   * @len: Size of @data.
> - * @raw_pkcs7: The PKCS#7 message that is the signature.
> - * @pkcs7_len: The size of @raw_pkcs7.
> + * @pkcs7: The PKCS#7 message that is the signature.
>   * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
>   *					(void *)1UL for all trusted keys).
>   * @usage: The use to which the key is being put.
>   * @view_content: Callback to gain access to content.
>   * @ctx: Context for callback.
>   */
> -int verify_pkcs7_signature(const void *data, size_t len,
> -			   const void *raw_pkcs7, size_t pkcs7_len,
> -			   struct key *trusted_keys,
> -			   enum key_being_used_for usage,
> -			   int (*view_content)(void *ctx,
> -					       const void *data, size_t len,
> -					       size_t asn1hdrlen),
> -			   void *ctx)
> +int verify_pkcs7_message_sig(const void *data, size_t len,
> +			     struct pkcs7_message *pkcs7,
> +			     struct key *trusted_keys,
> +			     enum key_being_used_for usage,
> +			     int (*view_content)(void *ctx,
> +						 const void *data, size_t len,
> +						 size_t asn1hdrlen),
> +			     void *ctx)
>  {
> -	struct pkcs7_message *pkcs7;
>  	int ret;
>
> -	pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
> -	if (IS_ERR(pkcs7))
> -		return PTR_ERR(pkcs7);
> -
>  	/* The data should be detached - so we need to supply it. */
>  	if (data && pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
>  		pr_err("PKCS#7 signature with non-detached data\n");
> @@ -273,6 +267,41 @@ int verify_pkcs7_signature(const void *data, size_t len,
>  	}
>
>  error:
> +	pr_devel("<==%s() = %d\n", __func__, ret);
> +	return ret;
> +}
> +
> +/**
> + * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
> + * @data: The data to be verified (NULL if expecting internal data).
> + * @len: Size of @data.
> + * @raw_pkcs7: The PKCS#7 message that is the signature.
> + * @pkcs7_len: The size of @raw_pkcs7.
> + * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
> + *					(void *)1UL for all trusted keys).
> + * @usage: The use to which the key is being put.
> + * @view_content: Callback to gain access to content.
> + * @ctx: Context for callback.
> + */
> +int verify_pkcs7_signature(const void *data, size_t len,
> +			   const void *raw_pkcs7, size_t pkcs7_len,
> +			   struct key *trusted_keys,
> +			   enum key_being_used_for usage,
> +			   int (*view_content)(void *ctx,
> +					       const void *data, size_t len,
> +					       size_t asn1hdrlen),
> +			   void *ctx)
> +{
> +	struct pkcs7_message *pkcs7;
> +	int ret;
> +
> +	pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
> +	if (IS_ERR(pkcs7))
> +		return PTR_ERR(pkcs7);
> +
> +	ret = verify_pkcs7_message_sig(data, len, pkcs7, trusted_keys, usage,
> +				       view_content, ctx);
> +
>  	pkcs7_free_message(pkcs7);
>  	pr_devel("<==%s() = %d\n", __func__, ret);
>  	return ret;
> diff --git a/include/linux/verification.h b/include/linux/verification.h
> index 018fb5f13d44..5e1d41f2b336 100644
> --- a/include/linux/verification.h
> +++ b/include/linux/verification.h
> @@ -36,6 +36,7 @@ extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
>  #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
>
>  struct key;
> +struct pkcs7_message;
>
>  extern int verify_pkcs7_signature(const void *data, size_t len,
>  				  const void *raw_pkcs7, size_t pkcs7_len,
> @@ -45,6 +46,15 @@ extern int verify_pkcs7_signature(const void *data, size_t len,
>  						      const void *data, size_t len,
>  						      size_t asn1hdrlen),
>  				  void *ctx);
> +extern int verify_pkcs7_message_sig(const void *data, size_t len,
> +				    struct pkcs7_message *pkcs7,
> +				    struct key *trusted_keys,
> +				    enum key_being_used_for usage,
> +				    int (*view_content)(void *ctx,
> +							const void *data,
> +							size_t len,
> +							size_t asn1hdrlen),
> +				    void *ctx);
>
>  #ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
>  extern int verify_pefile_signature(const void *pebuf, unsigned pelen,

^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-24 19:54 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: James Morris, LSM List, Linux Kernel Mailing List, Linux API,
	David Howells, Alexei Starovoitov, Network Development,
	Chun-Yi Lee, Jann Horn, bpf
In-Reply-To: <739e21b5-9559-d588-3542-bf0bc81de1b2@iogearbox.net>

On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> > From: David Howells <dhowells@redhat.com>
> >
> > There are some bpf functions can be used to read kernel memory:
>
> Nit: that

Fixed.

> > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
>
> Please explain how bpf_probe_write_user reads kernel memory ... ?!

Ha.

> > private keys in kernel memory (e.g. the hibernation image signing key) to
> > be read by an eBPF program and kernel memory to be altered without
>
> ... and while we're at it, also how they allow "kernel memory to be
> altered without restriction". I've been pointing this false statement
> out long ago.

Yup. How's the following description:

    bpf: Restrict bpf when kernel lockdown is in confidentiality mode

    There are some bpf functions that can be used to read kernel memory and
    exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
    bpf_trace_printk.  These could be abused to (eg) allow private
keys in kernel
    memory to be leaked. Disable them if the kernel has been locked
down in confidentiality
    mode.

> This whole thing is still buggy as has been pointed out before by
> Jann. For helpers like above and few others below, error conditions
> must clear the buffer ...

Sorry, yes. My fault.

^ permalink raw reply

* Re: [PATCH v11 01/13] MODSIGN: Export module signature definitions
From: Thiago Jung Bauermann @ 2019-06-24 19:52 UTC (permalink / raw)
  To: Jessica Yu
  Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
	linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
	James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
	Herbert Xu, David S. Miller, Jonathan Corbet, AKASHI, Takahiro,
	linux-integrity
In-Reply-To: <20190611062817.18412-2-bauerman@linux.ibm.com>


Hello Jessica,

AFAIK Mimi is happy with this patch set, but I still need acks from
maintainers of other subsystems that my changes touch before she can
accept it.

Is this patch OK from your PoV?

--
Thiago Jung Bauermann
IBM Linux Technology Center


Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:

> IMA will use the module_signature format for append signatures, so export
> the relevant definitions and factor out the code which verifies that the
> appended signature trailer is valid.
>
> Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
> and be able to use mod_check_sig() without having to depend on either
> CONFIG_MODULE_SIG or CONFIG_MODULES.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Jessica Yu <jeyu@kernel.org>
> ---
>  include/linux/module.h           |  3 --
>  include/linux/module_signature.h | 44 +++++++++++++++++++++++++
>  init/Kconfig                     |  6 +++-
>  kernel/Makefile                  |  1 +
>  kernel/module.c                  |  1 +
>  kernel/module_signature.c        | 46 ++++++++++++++++++++++++++
>  kernel/module_signing.c          | 56 +++++---------------------------
>  scripts/Makefile                 |  2 +-
>  8 files changed, 106 insertions(+), 53 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 188998d3dca9..aa56f531cf1e 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -25,9 +25,6 @@
>  #include <linux/percpu.h>
>  #include <asm/module.h>
>
> -/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
> -#define MODULE_SIG_STRING "~Module signature appended~\n"
> -
>  /* Not Yet Implemented */
>  #define MODULE_SUPPORTED_DEVICE(name)
>
> diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
> new file mode 100644
> index 000000000000..523617fc5b6a
> --- /dev/null
> +++ b/include/linux/module_signature.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Module signature handling.
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
> +
> +#ifndef _LINUX_MODULE_SIGNATURE_H
> +#define _LINUX_MODULE_SIGNATURE_H
> +
> +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
> +#define MODULE_SIG_STRING "~Module signature appended~\n"
> +
> +enum pkey_id_type {
> +	PKEY_ID_PGP,		/* OpenPGP generated key ID */
> +	PKEY_ID_X509,		/* X.509 arbitrary subjectKeyIdentifier */
> +	PKEY_ID_PKCS7,		/* Signature in PKCS#7 message */
> +};
> +
> +/*
> + * Module signature information block.
> + *
> + * The constituents of the signature section are, in order:
> + *
> + *	- Signer's name
> + *	- Key identifier
> + *	- Signature data
> + *	- Information block
> + */
> +struct module_signature {
> +	u8	algo;		/* Public-key crypto algorithm [0] */
> +	u8	hash;		/* Digest algorithm [0] */
> +	u8	id_type;	/* Key identifier type [PKEY_ID_PKCS7] */
> +	u8	signer_len;	/* Length of signer's name [0] */
> +	u8	key_id_len;	/* Length of key identifier [0] */
> +	u8	__pad[3];
> +	__be32	sig_len;	/* Length of signature data */
> +};
> +
> +int mod_check_sig(const struct module_signature *ms, size_t file_len,
> +		  const char *name);
> +
> +#endif /* _LINUX_MODULE_SIGNATURE_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index 8b9ffe236e4f..c2286a3c74c5 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1852,6 +1852,10 @@ config BASE_SMALL
>  	default 0 if BASE_FULL
>  	default 1 if !BASE_FULL
>
> +config MODULE_SIG_FORMAT
> +	def_bool n
> +	select SYSTEM_DATA_VERIFICATION
> +
>  menuconfig MODULES
>  	bool "Enable loadable module support"
>  	option modules
> @@ -1929,7 +1933,7 @@ config MODULE_SRCVERSION_ALL
>  config MODULE_SIG
>  	bool "Module signature verification"
>  	depends on MODULES
> -	select SYSTEM_DATA_VERIFICATION
> +	select MODULE_SIG_FORMAT
>  	help
>  	  Check modules for valid signatures upon load: the signature
>  	  is simply appended to the module. For more information see
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 33824f0385b3..f29ae2997a43 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -58,6 +58,7 @@ endif
>  obj-$(CONFIG_UID16) += uid16.o
>  obj-$(CONFIG_MODULES) += module.o
>  obj-$(CONFIG_MODULE_SIG) += module_signing.o
> +obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signature.o
>  obj-$(CONFIG_KALLSYMS) += kallsyms.o
>  obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
>  obj-$(CONFIG_CRASH_CORE) += crash_core.o
> diff --git a/kernel/module.c b/kernel/module.c
> index 6e6712b3aaf5..2712f4d217f5 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -19,6 +19,7 @@
>  #include <linux/export.h>
>  #include <linux/extable.h>
>  #include <linux/moduleloader.h>
> +#include <linux/module_signature.h>
>  #include <linux/trace_events.h>
>  #include <linux/init.h>
>  #include <linux/kallsyms.h>
> diff --git a/kernel/module_signature.c b/kernel/module_signature.c
> new file mode 100644
> index 000000000000..4224a1086b7d
> --- /dev/null
> +++ b/kernel/module_signature.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Module signature checker
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/printk.h>
> +#include <linux/module_signature.h>
> +#include <asm/byteorder.h>
> +
> +/**
> + * mod_check_sig - check that the given signature is sane
> + *
> + * @ms:		Signature to check.
> + * @file_len:	Size of the file to which @ms is appended.
> + * @name:	What is being checked. Used for error messages.
> + */
> +int mod_check_sig(const struct module_signature *ms, size_t file_len,
> +		  const char *name)
> +{
> +	if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
> +		return -EBADMSG;
> +
> +	if (ms->id_type != PKEY_ID_PKCS7) {
> +		pr_err("%s: Module is not signed with expected PKCS#7 message\n",
> +		       name);
> +		return -ENOPKG;
> +	}
> +
> +	if (ms->algo != 0 ||
> +	    ms->hash != 0 ||
> +	    ms->signer_len != 0 ||
> +	    ms->key_id_len != 0 ||
> +	    ms->__pad[0] != 0 ||
> +	    ms->__pad[1] != 0 ||
> +	    ms->__pad[2] != 0) {
> +		pr_err("%s: PKCS#7 signature info has unexpected non-zero params\n",
> +		       name);
> +		return -EBADMSG;
> +	}
> +
> +	return 0;
> +}
> diff --git a/kernel/module_signing.c b/kernel/module_signing.c
> index 6b9a926fd86b..cdd04a6b8074 100644
> --- a/kernel/module_signing.c
> +++ b/kernel/module_signing.c
> @@ -11,37 +11,13 @@
>
>  #include <linux/kernel.h>
>  #include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/module_signature.h>
>  #include <linux/string.h>
>  #include <linux/verification.h>
>  #include <crypto/public_key.h>
>  #include "module-internal.h"
>
> -enum pkey_id_type {
> -	PKEY_ID_PGP,		/* OpenPGP generated key ID */
> -	PKEY_ID_X509,		/* X.509 arbitrary subjectKeyIdentifier */
> -	PKEY_ID_PKCS7,		/* Signature in PKCS#7 message */
> -};
> -
> -/*
> - * Module signature information block.
> - *
> - * The constituents of the signature section are, in order:
> - *
> - *	- Signer's name
> - *	- Key identifier
> - *	- Signature data
> - *	- Information block
> - */
> -struct module_signature {
> -	u8	algo;		/* Public-key crypto algorithm [0] */
> -	u8	hash;		/* Digest algorithm [0] */
> -	u8	id_type;	/* Key identifier type [PKEY_ID_PKCS7] */
> -	u8	signer_len;	/* Length of signer's name [0] */
> -	u8	key_id_len;	/* Length of key identifier [0] */
> -	u8	__pad[3];
> -	__be32	sig_len;	/* Length of signature data */
> -};
> -
>  /*
>   * Verify the signature on a module.
>   */
> @@ -49,6 +25,7 @@ int mod_verify_sig(const void *mod, struct load_info *info)
>  {
>  	struct module_signature ms;
>  	size_t sig_len, modlen = info->len;
> +	int ret;
>
>  	pr_devel("==>%s(,%zu)\n", __func__, modlen);
>
> @@ -56,32 +33,15 @@ int mod_verify_sig(const void *mod, struct load_info *info)
>  		return -EBADMSG;
>
>  	memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
> -	modlen -= sizeof(ms);
> +
> +	ret = mod_check_sig(&ms, modlen, info->name);
> +	if (ret)
> +		return ret;
>
>  	sig_len = be32_to_cpu(ms.sig_len);
> -	if (sig_len >= modlen)
> -		return -EBADMSG;
> -	modlen -= sig_len;
> +	modlen -= sig_len + sizeof(ms);
>  	info->len = modlen;
>
> -	if (ms.id_type != PKEY_ID_PKCS7) {
> -		pr_err("%s: Module is not signed with expected PKCS#7 message\n",
> -		       info->name);
> -		return -ENOPKG;
> -	}
> -
> -	if (ms.algo != 0 ||
> -	    ms.hash != 0 ||
> -	    ms.signer_len != 0 ||
> -	    ms.key_id_len != 0 ||
> -	    ms.__pad[0] != 0 ||
> -	    ms.__pad[1] != 0 ||
> -	    ms.__pad[2] != 0) {
> -		pr_err("%s: PKCS#7 signature info has unexpected non-zero params\n",
> -		       info->name);
> -		return -EBADMSG;
> -	}
> -
>  	return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
>  				      VERIFY_USE_SECONDARY_KEYRING,
>  				      VERIFYING_MODULE_SIGNATURE,
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 9d442ee050bd..52098b080ab7 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -17,7 +17,7 @@ hostprogs-$(CONFIG_VT)           += conmakehash
>  hostprogs-$(BUILD_C_RECORDMCOUNT) += recordmcount
>  hostprogs-$(CONFIG_BUILDTIME_EXTABLE_SORT) += sortextable
>  hostprogs-$(CONFIG_ASN1)	 += asn1_compiler
> -hostprogs-$(CONFIG_MODULE_SIG)	 += sign-file
> +hostprogs-$(CONFIG_MODULE_SIG_FORMAT) += sign-file
>  hostprogs-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += extract-cert
>  hostprogs-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
>


^ permalink raw reply

* [PATCH v3 15/24] LSM: Specify which LSM to display
From: John Johansen @ 2019-06-24 18:48 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-16-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Create a new entry "display" in /proc/.../attr for controlling
> which LSM security information is displayed for a process.
> The name of an active LSM that supplies hooks for human readable
> data may be written to "display" to set the value. The name of
> the LSM currently in use can be read from "display".
> At this point there can only be one LSM capable of display
> active.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>


there is small issue, see below


Also I want to get rid of the list search for the display lsm
but that can come as a separate patch later


> ---
>  fs/proc/base.c      |   1 +
>  security/security.c | 110 +++++++++++++++++++++++++++++++++++---------
>  2 files changed, 90 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ddef482f1334..7bf70e041315 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
>  	ATTR(NULL, "fscreate",		0666),
>  	ATTR(NULL, "keycreate",		0666),
>  	ATTR(NULL, "sockcreate",	0666),
> +	ATTR(NULL, "display",		0666),
>  #ifdef CONFIG_SECURITY_SMACK
>  	DIR("smack",			0555,
>  	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> diff --git a/security/security.c b/security/security.c
> index aa8e1c73a062..92c5aa427b53 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
>  static struct kmem_cache *lsm_inode_cache;
>  
>  char *lsm_names;
> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> +	.lbs_task = sizeof(int),	/* slot number for the "display" LSM */
> +};
>  
>  /* Boot-time LSM user choice */
>  static __initdata const char *chosen_lsm_order;
> @@ -583,6 +585,8 @@ int lsm_inode_alloc(struct inode *inode)
>   */
>  static int lsm_task_alloc(struct task_struct *task)
>  {
> +	int *display;
> +
>  	if (blob_sizes.lbs_task == 0) {
>  		task->security = NULL;
>  		return 0;
> @@ -591,6 +595,15 @@ static int lsm_task_alloc(struct task_struct *task)
>  	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
>  	if (task->security == NULL)
>  		return -ENOMEM;
> +
> +	/*
> +	 * The start of the task blob contains the "display" LSM slot number.
> +	 * Start with it set to the invalid slot number, indicating that the
> +	 * default first registered LSM be displayed.
> +	 */
> +	display = task->security;
> +	*display = LSMBLOB_INVALID;
> +
>  	return 0;
>  }
>  
> @@ -1579,14 +1592,24 @@ int security_file_open(struct file *file)
>  
>  int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
>  {
> +	int *odisplay = current->security;
> +	int *ndisplay;
>  	int rc = lsm_task_alloc(task);
>  
> -	if (rc)
> +	if (unlikely(rc))
>  		return rc;
> +
>  	rc = call_int_hook(task_alloc, 0, task, clone_flags);
> -	if (unlikely(rc))
> +	if (unlikely(rc)) {
>  		security_task_free(task);
> -	return rc;
> +		return rc;
> +	}
> +
> +	ndisplay = task->security;
> +	if (ndisplay && odisplay)
> +		*ndisplay = *odisplay;
> +
> +	return 0;
>  }
>  
>  void security_task_free(struct task_struct *task)
> @@ -1972,10 +1995,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>  				char **value)
>  {
>  	struct security_hook_list *hp;
> +	int *display = current->security;
> +
> +	if (!strcmp(name, "display")) {
> +		hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx,
> +				     list) {
> +			if (*display == LSMBLOB_INVALID ||
> +			    hp->slot == *display) {
> +				*value = kstrdup(hp->lsm, GFP_KERNEL);
> +				if (*value)
> +					return strlen(hp->lsm);
> +				return -ENOMEM;
> +			}
> +		}
> +		return -EINVAL;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsm))
>  			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->slot)
> +			continue;
>  		return hp->hook.getprocattr(p, name, value);
>  	}
>  	return -EINVAL;
> @@ -1985,10 +2026,27 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>  			 size_t size)
>  {
>  	struct security_hook_list *hp;
> +	int *display = current->security;
> +	int len;
> +
> +	if (!strcmp(name, "display")) {
> +		hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx,
> +				     list) {
> +			len = strlen(hp->lsm);
> +			if (size >= len && !strncmp(value, hp->lsm, len)) {

This isn't quite right. It allows a value that contains a prefix of an lsm to match. That
is
  selinux2
etc would match to selinux. While it isn't a real problem atm. Its not something I
want as part of the user space API which would make it so we can't fix it.




> +				*display = hp->slot;
> +				return size;
> +			}
> +		}
> +		return -EINVAL;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsm))
>  			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->slot)
> +			continue;
>  		return hp->hook.setprocattr(name, value, size);
>  	}
>  	return -EINVAL;
> @@ -2008,14 +2066,12 @@ EXPORT_SYMBOL(security_ismaclabel);
>  int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>  {
>  	struct security_hook_list *hp;
> -	int rc;
> +	int *display = current->security;
>  
> -	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
> -		rc = hp->hook.secid_to_secctx(blob->secid[hp->slot],
> -					      secdata, seclen);
> -		if (rc != 0)
> -			return rc;
> -	}
> +	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list)
> +		if (*display == LSMBLOB_INVALID || *display == hp->slot)
> +			return hp->hook.secid_to_secctx(blob->secid[hp->slot],
> +							secdata, seclen);
>  	return 0;
>  }
>  EXPORT_SYMBOL(security_secid_to_secctx);
> @@ -2024,22 +2080,27 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
>  			     struct lsmblob *blob)
>  {
>  	struct security_hook_list *hp;
> -	int rc;
> +	int *display = current->security;
>  
>  	lsmblob_init(blob, 0);
> -	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
> -		rc = hp->hook.secctx_to_secid(secdata, seclen,
> -					      &blob->secid[hp->slot]);
> -		if (rc != 0)
> -			return rc;
> -	}
> +	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list)
> +		if (*display == LSMBLOB_INVALID || *display == hp->slot)
> +			return hp->hook.secctx_to_secid(secdata, seclen,
> +							&blob->secid[hp->slot]);
>  	return 0;
>  }
>  EXPORT_SYMBOL(security_secctx_to_secid);
>  
>  void security_release_secctx(char *secdata, u32 seclen)
>  {
> -	call_void_hook(release_secctx, secdata, seclen);
> +	struct security_hook_list *hp;
> +	int *display = current->security;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> +		if (*display == LSMBLOB_INVALID || *display == hp->slot) {
> +			hp->hook.release_secctx(secdata, seclen);
> +			return;
> +		}
>  }
>  EXPORT_SYMBOL(security_release_secctx);
>  
> @@ -2164,8 +2225,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>  int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>  				      int __user *optlen, unsigned len)
>  {
> -	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> -				optval, optlen, len);
> +	int *display = current->security;
> +	struct security_hook_list *hp;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> +			     list)
> +		if (*display == LSMBLOB_INVALID || *display == hp->slot)
> +			return hp->hook.socket_getpeersec_stream(sock, optval,
> +								 optlen, len);
> +	return -ENOPROTOOPT;
>  }
>  
>  int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> 




^ permalink raw reply

* [PATCH v3 14/24] IMA: Change internal interfaces to use lsmblobs
From: John Johansen @ 2019-06-24 18:40 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-15-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> The IMA interfaces ima_get_action() and ima_match_policy()
> call LSM functions that use lsmblobs. Change the IMA functions
> to pass the lsmblob to be compatible with the LSM functions.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

assuming you have the typo fix Kees already pointed out

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  security/integrity/ima/ima.h          | 10 ++++++----
>  security/integrity/ima/ima_api.c      |  9 +++++----
>  security/integrity/ima/ima_appraise.c |  4 +---
>  security/integrity/ima/ima_main.c     | 27 +++++++++++----------------
>  security/integrity/ima/ima_policy.c   | 12 ++++++------
>  5 files changed, 29 insertions(+), 33 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 5a337239d9e4..73b3b15dec5c 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -192,8 +192,9 @@ enum ima_hooks {
>  };
>  
>  /* LIM API function definitions */
> -int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
> -		   int mask, enum ima_hooks func, int *pcr);
> +int ima_get_action(struct inode *inode, const struct cred *cred,
> +		   struct lsmblob *blob, int mask, enum ima_hooks func,
> +		   int *pcr);
>  int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
>  int ima_collect_measurement(struct integrity_iint_cache *iint,
>  			    struct file *file, void *buf, loff_t size,
> @@ -213,8 +214,9 @@ void ima_free_template_entry(struct ima_template_entry *entry);
>  const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
>  
>  /* IMA policy related functions */
> -int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> -		     enum ima_hooks func, int mask, int flags, int *pcr);
> +int ima_match_policy(struct inode *inode, const struct cred *cred,
> +		     struct lsmblob *blob, enum ima_hooks func, int mask,
> +		     int flags, int *pcr);
>  void ima_init_policy(void);
>  void ima_update_policy(void);
>  void ima_update_policy_flag(void);
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c7505fb122d4..03a2392852de 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -159,7 +159,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
>   * ima_get_action - appraise & measure decision based on policy.
>   * @inode: pointer to inode to measure
>   * @cred: pointer to credentials structure to validate
> - * @secid: secid of the task being validated
> + * @blob: LAM data of the task being validated
>   * @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXEC,
>   *        MAY_APPEND)
>   * @func: caller identifier
> @@ -175,14 +175,15 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
>   * Returns IMA_MEASURE, IMA_APPRAISE mask.
>   *
>   */
> -int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
> -		   int mask, enum ima_hooks func, int *pcr)
> +int ima_get_action(struct inode *inode, const struct cred *cred,
> +		   struct lsmblob *blob, int mask, enum ima_hooks func,
> +		   int *pcr)
>  {
>  	int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
>  
>  	flags &= ima_policy_flag;
>  
> -	return ima_match_policy(inode, cred, secid, func, mask, flags, pcr);
> +	return ima_match_policy(inode, cred, blob, func, mask, flags, pcr);
>  }
>  
>  /*
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 85c7692fc4a3..3ff7aae81829 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -50,15 +50,13 @@ bool is_ima_appraise_enabled(void)
>   */
>  int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
>  {
> -	u32 secid;
>  	struct lsmblob blob;
>  
>  	if (!ima_appraise)
>  		return 0;
>  
>  	security_task_getsecid(current, &blob);
> -	lsmblob_secid(&blob, &secid);
> -	return ima_match_policy(inode, current_cred(), secid, func, mask,
> +	return ima_match_policy(inode, current_cred(), &blob, func, mask,
>  				IMA_APPRAISE | IMA_HASH, NULL);
>  }
>  
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 1afb75a893af..0588dd9a88db 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -169,8 +169,8 @@ void ima_file_free(struct file *file)
>  }
>  
>  static int process_measurement(struct file *file, const struct cred *cred,
> -			       u32 secid, char *buf, loff_t size, int mask,
> -			       enum ima_hooks func)
> +			       struct lsmblob *blob, char *buf, loff_t size,
> +			       int mask, enum ima_hooks func)
>  {
>  	struct inode *inode = file_inode(file);
>  	struct integrity_iint_cache *iint = NULL;
> @@ -192,7 +192,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  	 * bitmask based on the appraise/audit/measurement policy.
>  	 * Included is the appraise submask.
>  	 */
> -	action = ima_get_action(inode, cred, secid, mask, func, &pcr);
> +	action = ima_get_action(inode, cred, blob, mask, func, &pcr);
>  	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
>  			   (ima_policy_flag & IMA_MEASURE));
>  	if (!action && !violation_check)
> @@ -339,8 +339,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
>  
>  	if (file && (prot & PROT_EXEC)) {
>  		security_task_getsecid(current, &blob);
> -		/* scaffolding - until process_measurement changes */
> -		return process_measurement(file, current_cred(), blob.secid[0],
> +		return process_measurement(file, current_cred(), &blob,
>  					   NULL, 0, MAY_EXEC, MMAP_CHECK);
>  	}
>  
> @@ -366,16 +365,14 @@ int ima_bprm_check(struct linux_binprm *bprm)
>  	struct lsmblob blob;
>  
>  	security_task_getsecid(current, &blob);
> -	/* scaffolding until process_measurement changes */
> -	ret = process_measurement(bprm->file, current_cred(), blob.secid[0],
> -				  NULL, 0, MAY_EXEC, BPRM_CHECK);
> +	ret = process_measurement(bprm->file, current_cred(), &blob, NULL, 0,
> +				  MAY_EXEC, BPRM_CHECK);
>  	if (ret)
>  		return ret;
>  
>  	security_cred_getsecid(bprm->cred, &blob);
> -	/* scaffolding until process_measurement changes */
> -	return process_measurement(bprm->file, bprm->cred, blob.secid[0],
> -				   NULL, 0, MAY_EXEC, CREDS_CHECK);
> +	return process_measurement(bprm->file, bprm->cred, &blob, NULL, 0,
> +				   MAY_EXEC, CREDS_CHECK);
>  }
>  
>  /**
> @@ -393,8 +390,7 @@ int ima_file_check(struct file *file, int mask)
>  	struct lsmblob blob;
>  
>  	security_task_getsecid(current, &blob);
> -	/* scaffolding until process_measurement changes */
> -	return process_measurement(file, current_cred(), blob.secid[0], NULL, 0,
> +	return process_measurement(file, current_cred(), &blob, NULL, 0,
>  				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
>  					   MAY_APPEND), FILE_CHECK);
>  }
> @@ -526,9 +522,8 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  
>  	func = read_idmap[read_id] ?: FILE_CHECK;
>  	security_task_getsecid(current, &blob);
> -	/* scaffolding until process_measurement changes */
> -	return process_measurement(file, current_cred(), blob.secid[0], buf,
> -				   size, MAY_READ, func);
> +	return process_measurement(file, current_cred(), &blob, buf, size,
> +				   MAY_READ, func);
>  }
>  
>  /**
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 92ee3d984c73..dbad256aa7b4 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -286,7 +286,7 @@ static void ima_lsm_update_rules(void)
>   * Returns true on rule match, false on failure.
>   */
>  static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> -			    const struct cred *cred, u32 secid,
> +			    const struct cred *cred, struct lsmblob *blob,
>  			    enum ima_hooks func, int mask)
>  {
>  	int i;
> @@ -345,7 +345,6 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  		case LSM_SUBJ_USER:
>  		case LSM_SUBJ_ROLE:
>  		case LSM_SUBJ_TYPE:
> -			lsmblob_init(&blob, secid);
>  			rc = security_filter_rule_match(&blob,
>  							rule->lsm[i].type,
>  							Audit_equal,
> @@ -394,7 +393,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
>   * @inode: pointer to an inode for which the policy decision is being made
>   * @cred: pointer to a credentials structure for which the policy decision is
>   *        being made
> - * @secid: LSM secid of the task to be validated
> + * @blob: LSM data of the task to be validated
>   * @func: IMA hook identifier
>   * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
>   * @pcr: set the pcr to extend
> @@ -406,8 +405,9 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
>   * list when walking it.  Reads are many orders of magnitude more numerous
>   * than writes so ima_match_policy() is classical RCU candidate.
>   */
> -int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> -		     enum ima_hooks func, int mask, int flags, int *pcr)
> +int ima_match_policy(struct inode *inode, const struct cred *cred,
> +		     struct lsmblob *blob, enum ima_hooks func, int mask,
> +		     int flags, int *pcr)
>  {
>  	struct ima_rule_entry *entry;
>  	int action = 0, actmask = flags | (flags << 1);
> @@ -418,7 +418,7 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
>  		if (!(entry->action & actmask))
>  			continue;
>  
> -		if (!ima_match_rules(entry, inode, cred, secid, func, mask))
> +		if (!ima_match_rules(entry, inode, cred, blob, func, mask))
>  			continue;
>  
>  		action |= entry->flags & IMA_ACTION_FLAGS;
> 




^ permalink raw reply

* [PATCH v3 13/24] LSM: Use lsmblob in security_cred_getsecid
From: John Johansen @ 2019-06-24 18:40 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-14-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change the security_cred_getsecid() interface to fill in a
> lsmblob instead of a u32 secid. The associated data elements
> in the audit sub-system are changed from a secid to a lsmblob
> to accomodate multiple possible LSM audit users.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  include/linux/security.h          |  2 +-
>  kernel/audit.c                    | 14 +++++-------
>  kernel/audit.h                    |  5 +++--
>  kernel/auditsc.c                  | 37 +++++++++++--------------------
>  security/integrity/ima/ima_main.c |  8 +++----
>  security/security.c               |  9 +++++---
>  6 files changed, 33 insertions(+), 42 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 6eb64b7f878a..c712fc72b7bd 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -383,7 +383,7 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp);
>  void security_cred_free(struct cred *cred);
>  int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
>  void security_transfer_creds(struct cred *new, const struct cred *old);
> -void security_cred_getsecid(const struct cred *c, u32 *secid);
> +void security_cred_getsecid(const struct cred *c, struct lsmblob *blob);
>  int security_kernel_act_as(struct cred *new, struct lsmblob *blob);
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>  int security_kernel_module_request(char *kmod_name);
> diff --git a/kernel/audit.c b/kernel/audit.c
> index a0205f3c23c7..1b51e907f131 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -137,7 +137,7 @@ static u32	audit_backlog_wait_time = AUDIT_BACKLOG_WAIT_TIME;
>  /* The identity of the user shutting down the audit system. */
>  kuid_t		audit_sig_uid = INVALID_UID;
>  pid_t		audit_sig_pid = -1;
> -u32		audit_sig_sid = 0;
> +struct lsmblob	audit_sig_lsm;
>  
>  /* Records can be lost in several ways:
>     0) [suppressed in audit_alloc]
> @@ -1429,23 +1429,21 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>  	}
>  	case AUDIT_SIGNAL_INFO:
>  		len = 0;
> -		if (audit_sig_sid) {
> -			struct lsmblob blob;
> -
> -			lsmblob_init(&blob, audit_sig_sid);
> -			err = security_secid_to_secctx(&blob, &ctx, &len);
> +		if (lsmblob_is_set(&audit_sig_lsm)) {
> +			err = security_secid_to_secctx(&audit_sig_lsm, &ctx,
> +						       &len);
>  			if (err)
>  				return err;
>  		}
>  		sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
>  		if (!sig_data) {
> -			if (audit_sig_sid)
> +			if (lsmblob_is_set(&audit_sig_lsm))
>  				security_release_secctx(ctx, len);
>  			return -ENOMEM;
>  		}
>  		sig_data->uid = from_kuid(&init_user_ns, audit_sig_uid);
>  		sig_data->pid = audit_sig_pid;
> -		if (audit_sig_sid) {
> +		if (lsmblob_is_set(&audit_sig_lsm)) {
>  			memcpy(sig_data->ctx, ctx, len);
>  			security_release_secctx(ctx, len);
>  		}
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 958d5b8fc1b3..29e29c6f4afb 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -22,6 +22,7 @@
>  #include <linux/fs.h>
>  #include <linux/audit.h>
>  #include <linux/skbuff.h>
> +#include <linux/security.h>
>  #include <uapi/linux/mqueue.h>
>  #include <linux/tty.h>
>  
> @@ -147,7 +148,7 @@ struct audit_context {
>  	kuid_t		    target_auid;
>  	kuid_t		    target_uid;
>  	unsigned int	    target_sessionid;
> -	u32		    target_sid;
> +	struct lsmblob   target_lsm;
>  	char		    target_comm[TASK_COMM_LEN];
>  
>  	struct audit_tree_refs *trees, *first_trees;
> @@ -338,7 +339,7 @@ extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
>  
>  extern pid_t audit_sig_pid;
>  extern kuid_t audit_sig_uid;
> -extern u32 audit_sig_sid;
> +extern struct lsmblob audit_sig_lsm;
>  
>  extern int audit_filter(int msgtype, unsigned int listtype);
>  
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 54797c0fc3b7..c7aa39bda5cc 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -112,7 +112,7 @@ struct audit_aux_data_pids {
>  	kuid_t			target_auid[AUDIT_AUX_PIDS];
>  	kuid_t			target_uid[AUDIT_AUX_PIDS];
>  	unsigned int		target_sessionid[AUDIT_AUX_PIDS];
> -	u32			target_sid[AUDIT_AUX_PIDS];
> +	struct lsmblob	target_lsm[AUDIT_AUX_PIDS];
>  	char 			target_comm[AUDIT_AUX_PIDS][TASK_COMM_LEN];
>  	int			pid_count;
>  };
> @@ -938,14 +938,14 @@ static inline void audit_free_context(struct audit_context *context)
>  }
>  
>  static int audit_log_pid_context(struct audit_context *context, pid_t pid,
> -				 kuid_t auid, kuid_t uid, unsigned int sessionid,
> -				 u32 sid, char *comm)
> +				 kuid_t auid, kuid_t uid,
> +				 unsigned int sessionid,
> +				 struct lsmblob *blob, char *comm)
>  {
>  	struct audit_buffer *ab;
>  	char *ctx = NULL;
>  	u32 len;
>  	int rc = 0;
> -	struct lsmblob blob;
>  
>  	ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
>  	if (!ab)
> @@ -954,9 +954,8 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>  	audit_log_format(ab, "opid=%d oauid=%d ouid=%d oses=%d", pid,
>  			 from_kuid(&init_user_ns, auid),
>  			 from_kuid(&init_user_ns, uid), sessionid);
> -	if (sid) {
> -		lsmblob_init(&blob, sid);
> -		if (security_secid_to_secctx(&blob, &ctx, &len)) {
> +	if (lsmblob_is_set(blob)) {
> +		if (security_secid_to_secctx(blob, &ctx, &len)) {
>  			audit_log_format(ab, " obj=(none)");
>  			rc = 1;
>  		} else {
> @@ -1527,7 +1526,7 @@ static void audit_log_exit(void)
>  						  axs->target_auid[i],
>  						  axs->target_uid[i],
>  						  axs->target_sessionid[i],
> -						  axs->target_sid[i],
> +						  &axs->target_lsm[i],
>  						  axs->target_comm[i]))
>  				call_panic = 1;
>  	}
> @@ -1536,7 +1535,7 @@ static void audit_log_exit(void)
>  	    audit_log_pid_context(context, context->target_pid,
>  				  context->target_auid, context->target_uid,
>  				  context->target_sessionid,
> -				  context->target_sid, context->target_comm))
> +				  &context->target_lsm, context->target_comm))
>  			call_panic = 1;
>  
>  	if (context->pwd.dentry && context->pwd.mnt) {
> @@ -1713,7 +1712,7 @@ void __audit_syscall_exit(int success, long return_code)
>  	context->aux = NULL;
>  	context->aux_pids = NULL;
>  	context->target_pid = 0;
> -	context->target_sid = 0;
> +	lsmblob_init(&context->target_lsm, 0);
>  	context->sockaddr_len = 0;
>  	context->type = 0;
>  	context->fds[0] = -1;
> @@ -2367,15 +2366,12 @@ int __audit_sockaddr(int len, void *a)
>  void __audit_ptrace(struct task_struct *t)
>  {
>  	struct audit_context *context = audit_context();
> -	struct lsmblob blob;
>  
>  	context->target_pid = task_tgid_nr(t);
>  	context->target_auid = audit_get_loginuid(t);
>  	context->target_uid = task_uid(t);
>  	context->target_sessionid = audit_get_sessionid(t);
> -	security_task_getsecid(t, &blob);
> -	/* scaffolding - until target_sid is converted */
> -	context->target_sid = blob.secid[0];
> +	security_task_getsecid(t, &context->target_lsm);
>  	memcpy(context->target_comm, t->comm, TASK_COMM_LEN);
>  }
>  
> @@ -2392,7 +2388,6 @@ int audit_signal_info(int sig, struct task_struct *t)
>  	struct audit_aux_data_pids *axp;
>  	struct audit_context *ctx = audit_context();
>  	kuid_t uid = current_uid(), auid, t_uid = task_uid(t);
> -	struct lsmblob blob;
>  
>  	if (auditd_test_task(t) &&
>  	    (sig == SIGTERM || sig == SIGHUP ||
> @@ -2403,9 +2398,7 @@ int audit_signal_info(int sig, struct task_struct *t)
>  			audit_sig_uid = auid;
>  		else
>  			audit_sig_uid = uid;
> -		security_task_getsecid(current, &blob);
> -		/* scaffolding until audit_sig_sid is converted */
> -		audit_sig_sid = blob.secid[0];
> +		security_task_getsecid(current, &audit_sig_lsm);
>  	}
>  
>  	if (!audit_signals || audit_dummy_context())
> @@ -2418,9 +2411,7 @@ int audit_signal_info(int sig, struct task_struct *t)
>  		ctx->target_auid = audit_get_loginuid(t);
>  		ctx->target_uid = t_uid;
>  		ctx->target_sessionid = audit_get_sessionid(t);
> -		security_task_getsecid(t, &blob);
> -		/* scaffolding until target_sid is converted */
> -		ctx->target_sid = blob.secid[0];
> +		security_task_getsecid(t, &ctx->target_lsm);
>  		memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN);
>  		return 0;
>  	}
> @@ -2441,9 +2432,7 @@ int audit_signal_info(int sig, struct task_struct *t)
>  	axp->target_auid[axp->pid_count] = audit_get_loginuid(t);
>  	axp->target_uid[axp->pid_count] = t_uid;
>  	axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t);
> -	security_task_getsecid(t, &blob);
> -	/* scaffolding until target_sid is converted */
> -	axp->target_sid[axp->pid_count] = blob.secid[0];
> +	security_task_getsecid(t, &axp->target_lsm[axp->pid_count]);
>  	memcpy(axp->target_comm[axp->pid_count], t->comm, TASK_COMM_LEN);
>  	axp->pid_count++;
>  
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index fefa848cf0c7..1afb75a893af 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -363,7 +363,6 @@ int ima_file_mmap(struct file *file, unsigned long prot)
>  int ima_bprm_check(struct linux_binprm *bprm)
>  {
>  	int ret;
> -	u32 secid;
>  	struct lsmblob blob;
>  
>  	security_task_getsecid(current, &blob);
> @@ -373,9 +372,10 @@ int ima_bprm_check(struct linux_binprm *bprm)
>  	if (ret)
>  		return ret;
>  
> -	security_cred_getsecid(bprm->cred, &secid);
> -	return process_measurement(bprm->file, bprm->cred, secid, NULL, 0,
> -				   MAY_EXEC, CREDS_CHECK);
> +	security_cred_getsecid(bprm->cred, &blob);
> +	/* scaffolding until process_measurement changes */
> +	return process_measurement(bprm->file, bprm->cred, blob.secid[0],
> +				   NULL, 0, MAY_EXEC, CREDS_CHECK);
>  }
>  
>  /**
> diff --git a/security/security.c b/security/security.c
> index 6f0635b51180..aa8e1c73a062 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1643,10 +1643,13 @@ void security_transfer_creds(struct cred *new, const struct cred *old)
>  	call_void_hook(cred_transfer, new, old);
>  }
>  
> -void security_cred_getsecid(const struct cred *c, u32 *secid)
> +void security_cred_getsecid(const struct cred *c, struct lsmblob *blob)
>  {
> -	*secid = 0;
> -	call_void_hook(cred_getsecid, c, secid);
> +	struct security_hook_list *hp;
> +
> +	lsmblob_init(blob, 0);
> +	hlist_for_each_entry(hp, &security_hook_heads.cred_getsecid, list)
> +		hp->hook.cred_getsecid(c, &blob->secid[hp->slot]);
>  }
>  EXPORT_SYMBOL(security_cred_getsecid);
>  
> 




^ permalink raw reply

* [PATCH v3 12/24] LSM: Use lsmblob in security_inode_getsecid
From: John Johansen @ 2019-06-24 18:40 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-13-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change the security_inode_getsecid() interface to fill in a
> lsmblob structure instead of a u32 secid. This allows for its
> callers to gather data from all registered LSMs. Data is provided
> for IMA and audit.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  include/linux/security.h            | 7 ++++---
>  kernel/auditsc.c                    | 6 +++++-
>  security/integrity/ima/ima_policy.c | 4 +---
>  security/security.c                 | 8 ++++++--
>  4 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c5fdaaff8806..6eb64b7f878a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -358,7 +358,7 @@ int security_inode_killpriv(struct dentry *dentry);
>  int security_inode_getsecurity(struct inode *inode, const char *name, void **buffer, bool alloc);
>  int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
>  int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
> -void security_inode_getsecid(struct inode *inode, u32 *secid);
> +void security_inode_getsecid(struct inode *inode, struct lsmblob *blob);
>  int security_inode_copy_up(struct dentry *src, struct cred **new);
>  int security_inode_copy_up_xattr(const char *name);
>  int security_file_permission(struct file *file, int mask);
> @@ -854,9 +854,10 @@ static inline int security_inode_listsecurity(struct inode *inode, char *buffer,
>  	return 0;
>  }
>  
> -static inline void security_inode_getsecid(struct inode *inode, u32 *secid)
> +static inline void security_inode_getsecid(struct inode *inode,
> +					   struct lsmblob *blob)
>  {
> -	*secid = 0;
> +	lsmblob_init(blob, 0);
>  }
>  
>  static inline int security_inode_copy_up(struct dentry *src, struct cred **new)
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 7112fe31684d..54797c0fc3b7 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1910,13 +1910,17 @@ static inline int audit_copy_fcaps(struct audit_names *name,
>  void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
>  		      struct inode *inode, unsigned int flags)
>  {
> +	struct lsmblob blob;
> +
>  	name->ino   = inode->i_ino;
>  	name->dev   = inode->i_sb->s_dev;
>  	name->mode  = inode->i_mode;
>  	name->uid   = inode->i_uid;
>  	name->gid   = inode->i_gid;
>  	name->rdev  = inode->i_rdev;
> -	security_inode_getsecid(inode, &name->osid);
> +	security_inode_getsecid(inode, &blob);
> +	/* scaffolding until osid is updated */
> +	name->osid = blob.secid[0];
>  	if (flags & AUDIT_INODE_NOEVAL) {
>  		name->fcap_ver = -1;
>  		return;
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index e7b8ce942950..92ee3d984c73 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -326,7 +326,6 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  		return false;
>  	for (i = 0; i < MAX_LSM_RULES; i++) {
>  		int rc = 0;
> -		u32 osid;
>  		struct lsmblob blob;
>  		int retried = 0;
>  
> @@ -337,8 +336,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  		case LSM_OBJ_USER:
>  		case LSM_OBJ_ROLE:
>  		case LSM_OBJ_TYPE:
> -			security_inode_getsecid(inode, &osid);
> -			lsmblob_init(&blob, osid);
> +			security_inode_getsecid(inode, &blob);
>  			rc = security_filter_rule_match(&blob,
>  							rule->lsm[i].type,
>  							Audit_equal,
> diff --git a/security/security.c b/security/security.c
> index 1184ef092bce..6f0635b51180 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1423,9 +1423,13 @@ int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer
>  }
>  EXPORT_SYMBOL(security_inode_listsecurity);
>  
> -void security_inode_getsecid(struct inode *inode, u32 *secid)
> +void security_inode_getsecid(struct inode *inode, struct lsmblob *blob)
>  {
> -	call_void_hook(inode_getsecid, inode, secid);
> +	struct security_hook_list *hp;
> +
> +	lsmblob_init(blob, 0);
> +	hlist_for_each_entry(hp, &security_hook_heads.inode_getsecid, list)
> +		hp->hook.inode_getsecid(inode, &blob->secid[hp->slot]);
>  }
>  
>  int security_inode_copy_up(struct dentry *src, struct cred **new)
> 




^ permalink raw reply

* [PATCH v3 11/24] LSM: Use lsmblob in security_task_getsecid
From: John Johansen @ 2019-06-24 18:40 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-12-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change the security_task_getsecid() interface to fill in
> a lsmblob structure instead of a u32 secid in support of
> LSM stacking. Audit interfaces will need to collect all
> possible secids for possible reporting.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Review-by: John Johansen <john.johansen@canonical.com>

> ---
>  drivers/android/binder.c              |  4 +---
>  include/linux/security.h              |  7 +++---
>  kernel/audit.c                        |  6 ++---
>  kernel/auditfilter.c                  |  4 +---
>  kernel/auditsc.c                      | 22 ++++++++++++------
>  net/netlabel/netlabel_unlabeled.c     |  5 +++-
>  net/netlabel/netlabel_user.h          |  6 ++++-
>  security/integrity/ima/ima_appraise.c |  4 +++-
>  security/integrity/ima/ima_main.c     | 33 +++++++++++++++------------
>  security/security.c                   |  9 +++++---
>  10 files changed, 60 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 1962f6b8abd0..144ac4f1c24f 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -3119,11 +3119,9 @@ static void binder_transaction(struct binder_proc *proc,
>  	t->priority = task_nice(current);
>  
>  	if (target_node && target_node->txn_security_ctx) {
> -		u32 secid;
>  		struct lsmblob blob;
>  
> -		security_task_getsecid(proc->tsk, &secid);
> -		lsmblob_init(&blob, secid);
> +		security_task_getsecid(proc->tsk, &blob);
>  		ret = security_secid_to_secctx(&blob, &secctx, &secctx_sz);
>  		if (ret) {
>  			return_error = BR_FAILED_REPLY;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 0d5e172341fc..c5fdaaff8806 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -396,7 +396,7 @@ int security_task_fix_setuid(struct cred *new, const struct cred *old,
>  int security_task_setpgid(struct task_struct *p, pid_t pgid);
>  int security_task_getpgid(struct task_struct *p);
>  int security_task_getsid(struct task_struct *p);
> -void security_task_getsecid(struct task_struct *p, u32 *secid);
> +void security_task_getsecid(struct task_struct *p, struct lsmblob *blob);
>  int security_task_setnice(struct task_struct *p, int nice);
>  int security_task_setioprio(struct task_struct *p, int ioprio);
>  int security_task_getioprio(struct task_struct *p);
> @@ -1025,9 +1025,10 @@ static inline int security_task_getsid(struct task_struct *p)
>  	return 0;
>  }
>  
> -static inline void security_task_getsecid(struct task_struct *p, u32 *secid)
> +static inline void security_task_getsecid(struct task_struct *p,
> +					  struct lsmblob *blob)
>  {
> -	*secid = 0;
> +	lsmblob_init(blob, 0);
>  }
>  
>  static inline int security_task_setnice(struct task_struct *p, int nice)
> diff --git a/kernel/audit.c b/kernel/audit.c
> index d0338411d75d..a0205f3c23c7 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2075,14 +2075,12 @@ int audit_log_task_context(struct audit_buffer *ab)
>  	char *ctx = NULL;
>  	unsigned len;
>  	int error;
> -	u32 sid;
>  	struct lsmblob blob;
>  
> -	security_task_getsecid(current, &sid);
> -	if (!sid)
> +	security_task_getsecid(current, &blob);
> +	if (!lsmblob_is_set(&blob))
>  		return 0;
>  
> -	lsmblob_init(&blob, sid);
>  	error = security_secid_to_secctx(&blob, &ctx, &len);
>  	if (error) {
>  		if (error != -EINVAL)
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index da211065160f..76b4c0ca4fd2 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1323,7 +1323,6 @@ int audit_filter(int msgtype, unsigned int listtype)
>  		for (i = 0; i < e->rule.field_count; i++) {
>  			struct audit_field *f = &e->rule.fields[i];
>  			pid_t pid;
> -			u32 sid;
>  			struct lsmblob blob;
>  
>  			switch (f->type) {
> @@ -1354,8 +1353,7 @@ int audit_filter(int msgtype, unsigned int listtype)
>  			case AUDIT_SUBJ_SEN:
>  			case AUDIT_SUBJ_CLR:
>  				if (f->lsm_rule) {
> -					security_task_getsecid(current, &sid);
> -					lsmblob_init(&blob, sid);
> +					security_task_getsecid(current, &blob);
>  					result = security_audit_rule_match(&blob,
>  						   f->type, f->op, f->lsm_rule);
>  				}
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 148733ec3c72..7112fe31684d 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -444,7 +444,6 @@ static int audit_filter_rules(struct task_struct *tsk,
>  {
>  	const struct cred *cred;
>  	int i, need_sid = 1;
> -	u32 sid;
>  	struct lsmblob blob;
>  	unsigned int sessionid;
>  
> @@ -628,10 +627,9 @@ static int audit_filter_rules(struct task_struct *tsk,
>  			   logged upon error */
>  			if (f->lsm_rule) {
>  				if (need_sid) {
> -					security_task_getsecid(tsk, &sid);
> +					security_task_getsecid(tsk, &blob);
>  					need_sid = 0;
>  				}
> -				lsmblob_init(&blob, sid);
>  				result = security_audit_rule_match(&blob,
>  								   f->type,
>  								   f->op,
> @@ -2365,12 +2363,15 @@ int __audit_sockaddr(int len, void *a)
>  void __audit_ptrace(struct task_struct *t)
>  {
>  	struct audit_context *context = audit_context();
> +	struct lsmblob blob;
>  
>  	context->target_pid = task_tgid_nr(t);
>  	context->target_auid = audit_get_loginuid(t);
>  	context->target_uid = task_uid(t);
>  	context->target_sessionid = audit_get_sessionid(t);
> -	security_task_getsecid(t, &context->target_sid);
> +	security_task_getsecid(t, &blob);
> +	/* scaffolding - until target_sid is converted */
> +	context->target_sid = blob.secid[0];
>  	memcpy(context->target_comm, t->comm, TASK_COMM_LEN);
>  }
>  
> @@ -2387,6 +2388,7 @@ int audit_signal_info(int sig, struct task_struct *t)
>  	struct audit_aux_data_pids *axp;
>  	struct audit_context *ctx = audit_context();
>  	kuid_t uid = current_uid(), auid, t_uid = task_uid(t);
> +	struct lsmblob blob;
>  
>  	if (auditd_test_task(t) &&
>  	    (sig == SIGTERM || sig == SIGHUP ||
> @@ -2397,7 +2399,9 @@ int audit_signal_info(int sig, struct task_struct *t)
>  			audit_sig_uid = auid;
>  		else
>  			audit_sig_uid = uid;
> -		security_task_getsecid(current, &audit_sig_sid);
> +		security_task_getsecid(current, &blob);
> +		/* scaffolding until audit_sig_sid is converted */
> +		audit_sig_sid = blob.secid[0];
>  	}
>  
>  	if (!audit_signals || audit_dummy_context())
> @@ -2410,7 +2414,9 @@ int audit_signal_info(int sig, struct task_struct *t)
>  		ctx->target_auid = audit_get_loginuid(t);
>  		ctx->target_uid = t_uid;
>  		ctx->target_sessionid = audit_get_sessionid(t);
> -		security_task_getsecid(t, &ctx->target_sid);
> +		security_task_getsecid(t, &blob);
> +		/* scaffolding until target_sid is converted */
> +		ctx->target_sid = blob.secid[0];
>  		memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN);
>  		return 0;
>  	}
> @@ -2431,7 +2437,9 @@ int audit_signal_info(int sig, struct task_struct *t)
>  	axp->target_auid[axp->pid_count] = audit_get_loginuid(t);
>  	axp->target_uid[axp->pid_count] = t_uid;
>  	axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t);
> -	security_task_getsecid(t, &axp->target_sid[axp->pid_count]);
> +	security_task_getsecid(t, &blob);
> +	/* scaffolding until target_sid is converted */
> +	axp->target_sid[axp->pid_count] = blob.secid[0];
>  	memcpy(axp->target_comm[axp->pid_count], t->comm, TASK_COMM_LEN);
>  	axp->pid_count++;
>  
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 2d8dd5b84457..2294aa9471e6 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1552,11 +1552,14 @@ int __init netlbl_unlabel_defconf(void)
>  	int ret_val;
>  	struct netlbl_dom_map *entry;
>  	struct netlbl_audit audit_info;
> +	struct lsmblob blob;
>  
>  	/* Only the kernel is allowed to call this function and the only time
>  	 * it is called is at bootup before the audit subsystem is reporting
>  	 * messages so don't worry to much about these values. */
> -	security_task_getsecid(current, &audit_info.secid);
> +	security_task_getsecid(current, &blob);
> +	/* scaffolding until audit_info.secid is converted */
> +	audit_info.secid = blob.secid[0];
>  	audit_info.loginuid = GLOBAL_ROOT_UID;
>  	audit_info.sessionid = 0;
>  
> diff --git a/net/netlabel/netlabel_user.h b/net/netlabel/netlabel_user.h
> index 4a397cde1a48..ab88baaaa50d 100644
> --- a/net/netlabel/netlabel_user.h
> +++ b/net/netlabel/netlabel_user.h
> @@ -48,7 +48,11 @@
>  static inline void netlbl_netlink_auditinfo(struct sk_buff *skb,
>  					    struct netlbl_audit *audit_info)
>  {
> -	security_task_getsecid(current, &audit_info->secid);
> +	struct lsmblob blob;
> +
> +	security_task_getsecid(current, &blob);
> +	/* scaffolding until secid is converted */
> +	audit_info->secid = blob.secid[0];
>  	audit_info->loginuid = audit_get_loginuid(current);
>  	audit_info->sessionid = audit_get_sessionid(current);
>  }
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 5fb7127bbe68..85c7692fc4a3 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -51,11 +51,13 @@ bool is_ima_appraise_enabled(void)
>  int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
>  {
>  	u32 secid;
> +	struct lsmblob blob;
>  
>  	if (!ima_appraise)
>  		return 0;
>  
> -	security_task_getsecid(current, &secid);
> +	security_task_getsecid(current, &blob);
> +	lsmblob_secid(&blob, &secid);
>  	return ima_match_policy(inode, current_cred(), secid, func, mask,
>  				IMA_APPRAISE | IMA_HASH, NULL);
>  }
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 357edd140c09..fefa848cf0c7 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -335,12 +335,13 @@ static int process_measurement(struct file *file, const struct cred *cred,
>   */
>  int ima_file_mmap(struct file *file, unsigned long prot)
>  {
> -	u32 secid;
> +	struct lsmblob blob;
>  
>  	if (file && (prot & PROT_EXEC)) {
> -		security_task_getsecid(current, &secid);
> -		return process_measurement(file, current_cred(), secid, NULL,
> -					   0, MAY_EXEC, MMAP_CHECK);
> +		security_task_getsecid(current, &blob);
> +		/* scaffolding - until process_measurement changes */
> +		return process_measurement(file, current_cred(), blob.secid[0],
> +					   NULL, 0, MAY_EXEC, MMAP_CHECK);
>  	}
>  
>  	return 0;
> @@ -363,10 +364,12 @@ int ima_bprm_check(struct linux_binprm *bprm)
>  {
>  	int ret;
>  	u32 secid;
> +	struct lsmblob blob;
>  
> -	security_task_getsecid(current, &secid);
> -	ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0,
> -				  MAY_EXEC, BPRM_CHECK);
> +	security_task_getsecid(current, &blob);
> +	/* scaffolding until process_measurement changes */
> +	ret = process_measurement(bprm->file, current_cred(), blob.secid[0],
> +				  NULL, 0, MAY_EXEC, BPRM_CHECK);
>  	if (ret)
>  		return ret;
>  
> @@ -387,10 +390,11 @@ int ima_bprm_check(struct linux_binprm *bprm)
>   */
>  int ima_file_check(struct file *file, int mask)
>  {
> -	u32 secid;
> +	struct lsmblob blob;
>  
> -	security_task_getsecid(current, &secid);
> -	return process_measurement(file, current_cred(), secid, NULL, 0,
> +	security_task_getsecid(current, &blob);
> +	/* scaffolding until process_measurement changes */
> +	return process_measurement(file, current_cred(), blob.secid[0], NULL, 0,
>  				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
>  					   MAY_APPEND), FILE_CHECK);
>  }
> @@ -499,7 +503,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  		       enum kernel_read_file_id read_id)
>  {
>  	enum ima_hooks func;
> -	u32 secid;
> +	struct lsmblob blob;
>  
>  	if (!file && read_id == READING_FIRMWARE) {
>  		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> @@ -521,9 +525,10 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  	}
>  
>  	func = read_idmap[read_id] ?: FILE_CHECK;
> -	security_task_getsecid(current, &secid);
> -	return process_measurement(file, current_cred(), secid, buf, size,
> -				   MAY_READ, func);
> +	security_task_getsecid(current, &blob);
> +	/* scaffolding until process_measurement changes */
> +	return process_measurement(file, current_cred(), blob.secid[0], buf,
> +				   size, MAY_READ, func);
>  }
>  
>  /**
> diff --git a/security/security.c b/security/security.c
> index d55f01041f05..1184ef092bce 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1729,10 +1729,13 @@ int security_task_getsid(struct task_struct *p)
>  	return call_int_hook(task_getsid, 0, p);
>  }
>  
> -void security_task_getsecid(struct task_struct *p, u32 *secid)
> +void security_task_getsecid(struct task_struct *p, struct lsmblob *blob)
>  {
> -	*secid = 0;
> -	call_void_hook(task_getsecid, p, secid);
> +	struct security_hook_list *hp;
> +
> +	lsmblob_init(blob, 0);
> +	hlist_for_each_entry(hp, &security_hook_heads.task_getsecid, list)
> +		hp->hook.task_getsecid(p, &blob->secid[hp->slot]);
>  }
>  EXPORT_SYMBOL(security_task_getsecid);
>  
> 




^ permalink raw reply

* [PATCH v3 10/24] Use lsmblob in security_ipc_getsecid
From: John Johansen @ 2019-06-24 18:39 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-11-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> There may be more than one LSM that provides IPC data
> for auditing. Change security_ipc_getsecid() to fill in
> a lsmblob structure instead of the u32 secid. The
> audit data structure containing the secid will be updated
> later, so there is a bit of scaffolding here.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  include/linux/security.h | 7 ++++---
>  kernel/auditsc.c         | 5 ++++-
>  security/security.c      | 9 ++++++---
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c6cddeff8a17..0d5e172341fc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -413,7 +413,7 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
>  			unsigned long arg4, unsigned long arg5);
>  void security_task_to_inode(struct task_struct *p, struct inode *inode);
>  int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);
> -void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid);
> +void security_ipc_getsecid(struct kern_ipc_perm *ipcp, struct lsmblob *blob);
>  int security_msg_msg_alloc(struct msg_msg *msg);
>  void security_msg_msg_free(struct msg_msg *msg);
>  int security_msg_queue_alloc(struct kern_ipc_perm *msq);
> @@ -1098,9 +1098,10 @@ static inline int security_ipc_permission(struct kern_ipc_perm *ipcp,
>  	return 0;
>  }
>  
> -static inline void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
> +static inline void security_ipc_getsecid(struct kern_ipc_perm *ipcp,
> +					 struct lsmblob *blob)
>  {
> -	*secid = 0;
> +	lsmblob_init(blob, 0);
>  }
>  
>  static inline int security_msg_msg_alloc(struct msg_msg *msg)
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index d31914088a82..148733ec3c72 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2268,11 +2268,14 @@ void __audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat)
>  void __audit_ipc_obj(struct kern_ipc_perm *ipcp)
>  {
>  	struct audit_context *context = audit_context();
> +	struct lsmblob blob;
>  	context->ipc.uid = ipcp->uid;
>  	context->ipc.gid = ipcp->gid;
>  	context->ipc.mode = ipcp->mode;
>  	context->ipc.has_perm = 0;
> -	security_ipc_getsecid(ipcp, &context->ipc.osid);
> +	security_ipc_getsecid(ipcp, &blob);
> +	/* scaffolding on the [0] - change "osid" to a lsmblob */
> +	context->ipc.osid = blob.secid[0];
>  	context->type = AUDIT_IPC;
>  }
>  
> diff --git a/security/security.c b/security/security.c
> index 5ab07631df75..d55f01041f05 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1812,10 +1812,13 @@ int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
>  	return call_int_hook(ipc_permission, 0, ipcp, flag);
>  }
>  
> -void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
> +void security_ipc_getsecid(struct kern_ipc_perm *ipcp, struct lsmblob *blob)
>  {
> -	*secid = 0;
> -	call_void_hook(ipc_getsecid, ipcp, secid);
> +	struct security_hook_list *hp;
> +
> +	lsmblob_init(blob, 0);
> +	hlist_for_each_entry(hp, &security_hook_heads.ipc_getsecid, list)
> +		hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]);
>  }
>  
>  int security_msg_msg_alloc(struct msg_msg *msg)
> 


^ permalink raw reply

* [PATCH v3 09/24] LSM: Use lsmblob in security_secid_to_secctx
From: John Johansen @ 2019-06-24 18:39 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-10-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change security_secid_to_secctx() to take a lsmblob as input
> instead of a u32 secid. It will then call the LSM hooks
> using the lsmblob element allocated for that module. The
> callers have been updated as well. This allows for the
> possibility that more than one module may called upon
> to translate a secid to a string, as can occur in the
> audit code.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  drivers/android/binder.c                |  4 +++-
>  include/linux/security.h                |  5 +++--
>  include/net/scm.h                       |  5 ++---
>  kernel/audit.c                          |  9 +++++++--
>  kernel/auditsc.c                        | 14 ++++++++++----
>  net/ipv4/ip_sockglue.c                  |  3 +--
>  net/netfilter/nf_conntrack_netlink.c    |  8 ++++++--
>  net/netfilter/nf_conntrack_standalone.c |  4 +++-
>  net/netfilter/nfnetlink_queue.c         |  8 ++++++--
>  net/netlabel/netlabel_unlabeled.c       | 18 ++++++++++++++----
>  net/netlabel/netlabel_user.c            |  6 +++---
>  security/security.c                     | 14 +++++++++++---
>  12 files changed, 69 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 8685882da64c..1962f6b8abd0 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -3120,9 +3120,11 @@ static void binder_transaction(struct binder_proc *proc,
>  
>  	if (target_node && target_node->txn_security_ctx) {
>  		u32 secid;
> +		struct lsmblob blob;
>  
>  		security_task_getsecid(proc->tsk, &secid);
> -		ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);
> +		lsmblob_init(&blob, secid);
> +		ret = security_secid_to_secctx(&blob, &secctx, &secctx_sz);
>  		if (ret) {
>  			return_error = BR_FAILED_REPLY;
>  			return_error_param = ret;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index b0395d224c43..c6cddeff8a17 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -442,7 +442,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>  			 size_t size);
>  int security_netlink_send(struct sock *sk, struct sk_buff *skb);
>  int security_ismaclabel(const char *name);
> -int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
> +int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen);
>  int security_secctx_to_secid(const char *secdata, u32 seclen,
>  			     struct lsmblob *blob);
>  void security_release_secctx(char *secdata, u32 seclen);
> @@ -1220,7 +1220,8 @@ static inline int security_ismaclabel(const char *name)
>  	return 0;
>  }
>  
> -static inline int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
> +static inline int security_secid_to_secctx(struct lsmblob *blob,
> +					   char **secdata, u32 *seclen)
>  {
>  	return -EOPNOTSUPP;
>  }
> diff --git a/include/net/scm.h b/include/net/scm.h
> index e2e71c4bf9d0..31ae605fcc0a 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -97,9 +97,8 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
>  	int err;
>  
>  	if (test_bit(SOCK_PASSSEC, &sock->flags)) {
> -		/* Scaffolding - it has to be element 0 for now */
> -		err = security_secid_to_secctx(scm->lsmblob.secid[0],
> -					       &secdata, &seclen);
> +		err = security_secid_to_secctx(&scm->lsmblob, &secdata,
> +					       &seclen);
>  
>  		if (!err) {
>  			put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, seclen, secdata);
> diff --git a/kernel/audit.c b/kernel/audit.c
> index c89ea48c70a6..d0338411d75d 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1430,7 +1430,10 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>  	case AUDIT_SIGNAL_INFO:
>  		len = 0;
>  		if (audit_sig_sid) {
> -			err = security_secid_to_secctx(audit_sig_sid, &ctx, &len);
> +			struct lsmblob blob;
> +
> +			lsmblob_init(&blob, audit_sig_sid);
> +			err = security_secid_to_secctx(&blob, &ctx, &len);
>  			if (err)
>  				return err;
>  		}
> @@ -2073,12 +2076,14 @@ int audit_log_task_context(struct audit_buffer *ab)
>  	unsigned len;
>  	int error;
>  	u32 sid;
> +	struct lsmblob blob;
>  
>  	security_task_getsecid(current, &sid);
>  	if (!sid)
>  		return 0;
>  
> -	error = security_secid_to_secctx(sid, &ctx, &len);
> +	lsmblob_init(&blob, sid);
> +	error = security_secid_to_secctx(&blob, &ctx, &len);
>  	if (error) {
>  		if (error != -EINVAL)
>  			goto error_path;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 18ee5556c086..d31914088a82 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -947,6 +947,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>  	char *ctx = NULL;
>  	u32 len;
>  	int rc = 0;
> +	struct lsmblob blob;
>  
>  	ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
>  	if (!ab)
> @@ -956,7 +957,8 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>  			 from_kuid(&init_user_ns, auid),
>  			 from_kuid(&init_user_ns, uid), sessionid);
>  	if (sid) {
> -		if (security_secid_to_secctx(sid, &ctx, &len)) {
> +		lsmblob_init(&blob, sid);
> +		if (security_secid_to_secctx(&blob, &ctx, &len)) {
>  			audit_log_format(ab, " obj=(none)");
>  			rc = 1;
>  		} else {
> @@ -1198,7 +1200,10 @@ static void show_special(struct audit_context *context, int *call_panic)
>  		if (osid) {
>  			char *ctx = NULL;
>  			u32 len;
> -			if (security_secid_to_secctx(osid, &ctx, &len)) {
> +			struct lsmblob blob;
> +
> +			lsmblob_init(&blob, osid);
> +			if (security_secid_to_secctx(&blob, &ctx, &len)) {
>  				audit_log_format(ab, " osid=%u", osid);
>  				*call_panic = 1;
>  			} else {
> @@ -1349,9 +1354,10 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
>  	if (n->osid != 0) {
>  		char *ctx = NULL;
>  		u32 len;
> +		struct lsmblob blob;
>  
> -		if (security_secid_to_secctx(
> -			n->osid, &ctx, &len)) {
> +		lsmblob_init(&blob, n->osid);
> +		if (security_secid_to_secctx(&blob, &ctx, &len)) {
>  			audit_log_format(ab, " osid=%u", n->osid);
>  			if (call_panic)
>  				*call_panic = 2;
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index 2a5c868ce135..e05f4ef68bd8 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -139,8 +139,7 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
>  	if (err)
>  		return;
>  
> -	/* Scaffolding - it has to be element 0 */
> -	err = security_secid_to_secctx(lb.secid[0], &secdata, &seclen);
> +	err = security_secid_to_secctx(&lb, &secdata, &seclen);
>  	if (err)
>  		return;
>  
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 66c596d287a5..ca0968f13240 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -330,8 +330,10 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
>  	struct nlattr *nest_secctx;
>  	int len, ret;
>  	char *secctx;
> +	struct lsmblob blob;
>  
> -	ret = security_secid_to_secctx(ct->secmark, &secctx, &len);
> +	lsmblob_init(&blob, ct->secmark);
> +	ret = security_secid_to_secctx(&blob, &secctx, &len);
>  	if (ret)
>  		return 0;
>  
> @@ -615,8 +617,10 @@ static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
>  {
>  #ifdef CONFIG_NF_CONNTRACK_SECMARK
>  	int len, ret;
> +	struct lsmblob blob;
>  
> -	ret = security_secid_to_secctx(ct->secmark, NULL, &len);
> +	lsmblob_init(&blob, ct->secmark);
> +	ret = security_secid_to_secctx(&blob, NULL, &len);
>  	if (ret)
>  		return 0;
>  
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index c2ae14c720b4..c793103f3cd7 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -175,8 +175,10 @@ static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>  	int ret;
>  	u32 len;
>  	char *secctx;
> +	struct lsmblob blob;
>  
> -	ret = security_secid_to_secctx(ct->secmark, &secctx, &len);
> +	lsmblob_init(&blob, ct->secmark);
> +	ret = security_secid_to_secctx(&blob, &secctx, &len);
>  	if (ret)
>  		return;
>  
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index 0dcc3592d053..59211bff90ab 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -309,13 +309,17 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
>  {
>  	u32 seclen = 0;
>  #if IS_ENABLED(CONFIG_NETWORK_SECMARK)
> +	struct lsmblob blob;
> +
>  	if (!skb || !sk_fullsock(skb->sk))
>  		return 0;
>  
>  	read_lock_bh(&skb->sk->sk_callback_lock);
>  
> -	if (skb->secmark)
> -		security_secid_to_secctx(skb->secmark, secdata, &seclen);
> +	if (skb->secmark) {
> +		lsmblob_init(&blob, skb->secmark);
> +		security_secid_to_secctx(&blob, secdata, &seclen);
> +	}
>  
>  	read_unlock_bh(&skb->sk->sk_callback_lock);
>  #endif
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 2976370e41aa..2d8dd5b84457 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -389,6 +389,7 @@ int netlbl_unlhsh_add(struct net *net,
>  	struct audit_buffer *audit_buf = NULL;
>  	char *secctx = NULL;
>  	u32 secctx_len;
> +	struct lsmblob blob;
>  
>  	if (addr_len != sizeof(struct in_addr) &&
>  	    addr_len != sizeof(struct in6_addr))
> @@ -451,7 +452,8 @@ int netlbl_unlhsh_add(struct net *net,
>  unlhsh_add_return:
>  	rcu_read_unlock();
>  	if (audit_buf != NULL) {
> -		if (security_secid_to_secctx(secid,
> +		lsmblob_init(&blob, secid);
> +		if (security_secid_to_secctx(&blob,
>  					     &secctx,
>  					     &secctx_len) == 0) {
>  			audit_log_format(audit_buf, " sec_obj=%s", secctx);
> @@ -488,6 +490,7 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
>  	struct net_device *dev;
>  	char *secctx;
>  	u32 secctx_len;
> +	struct lsmblob blob;
>  
>  	spin_lock(&netlbl_unlhsh_lock);
>  	list_entry = netlbl_af4list_remove(addr->s_addr, mask->s_addr,
> @@ -507,8 +510,10 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
>  					  addr->s_addr, mask->s_addr);
>  		if (dev != NULL)
>  			dev_put(dev);
> +		if (entry != NULL)
> +			lsmblob_init(&blob, entry->secid);
>  		if (entry != NULL &&
> -		    security_secid_to_secctx(entry->secid,
> +		    security_secid_to_secctx(&blob,
>  					     &secctx, &secctx_len) == 0) {
>  			audit_log_format(audit_buf, " sec_obj=%s", secctx);
>  			security_release_secctx(secctx, secctx_len);
> @@ -550,6 +555,7 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
>  	struct net_device *dev;
>  	char *secctx;
>  	u32 secctx_len;
> +	struct lsmblob blob;
>  
>  	spin_lock(&netlbl_unlhsh_lock);
>  	list_entry = netlbl_af6list_remove(addr, mask, &iface->addr6_list);
> @@ -568,8 +574,10 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
>  					  addr, mask);
>  		if (dev != NULL)
>  			dev_put(dev);
> +		if (entry != NULL)
> +			lsmblob_init(&blob, entry->secid);
>  		if (entry != NULL &&
> -		    security_secid_to_secctx(entry->secid,
> +		    security_secid_to_secctx(&blob,
>  					     &secctx, &secctx_len) == 0) {
>  			audit_log_format(audit_buf, " sec_obj=%s", secctx);
>  			security_release_secctx(secctx, secctx_len);
> @@ -1090,6 +1098,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
>  	u32 secid;
>  	char *secctx;
>  	u32 secctx_len;
> +	struct lsmblob blob;
>  
>  	data = genlmsg_put(cb_arg->skb, NETLINK_CB(cb_arg->nl_cb->skb).portid,
>  			   cb_arg->seq, &netlbl_unlabel_gnl_family,
> @@ -1144,7 +1153,8 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
>  		secid = addr6->secid;
>  	}
>  
> -	ret_val = security_secid_to_secctx(secid, &secctx, &secctx_len);
> +	lsmblob_init(&blob, secid);
> +	ret_val = security_secid_to_secctx(&blob, &secctx, &secctx_len);
>  	if (ret_val != 0)
>  		goto list_cb_failure;
>  	ret_val = nla_put(cb_arg->skb,
> diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
> index 4676f5bb16ae..2ccc6567e2a2 100644
> --- a/net/netlabel/netlabel_user.c
> +++ b/net/netlabel/netlabel_user.c
> @@ -100,6 +100,7 @@ struct audit_buffer *netlbl_audit_start_common(int type,
>  	struct audit_buffer *audit_buf;
>  	char *secctx;
>  	u32 secctx_len;
> +	struct lsmblob blob;
>  
>  	if (audit_enabled == AUDIT_OFF)
>  		return NULL;
> @@ -112,10 +113,9 @@ struct audit_buffer *netlbl_audit_start_common(int type,
>  			 from_kuid(&init_user_ns, audit_info->loginuid),
>  			 audit_info->sessionid);
>  
> +	lsmblob_init(&blob, audit_info->secid);
>  	if (audit_info->secid != 0 &&
> -	    security_secid_to_secctx(audit_info->secid,
> -				     &secctx,
> -				     &secctx_len) == 0) {
> +	    security_secid_to_secctx(&blob, &secctx, &secctx_len) == 0) {
>  		audit_log_format(audit_buf, " subj=%s", secctx);
>  		security_release_secctx(secctx, secctx_len);
>  	}
> diff --git a/security/security.c b/security/security.c
> index cb1545bfe8c5..5ab07631df75 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1992,10 +1992,18 @@ int security_ismaclabel(const char *name)
>  }
>  EXPORT_SYMBOL(security_ismaclabel);
>  
> -int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
> +int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>  {
> -	return call_int_hook(secid_to_secctx, -EOPNOTSUPP, secid, secdata,
> -				seclen);
> +	struct security_hook_list *hp;
> +	int rc;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
> +		rc = hp->hook.secid_to_secctx(blob->secid[hp->slot],
> +					      secdata, seclen);
> +		if (rc != 0)
> +			return rc;
> +	}
> +	return 0;
>  }
>  EXPORT_SYMBOL(security_secid_to_secctx);
>  
> 




^ permalink raw reply

* [PATCH v3 07/24] net: Prepare UDS for secuirty module stacking
From: John Johansen @ 2019-06-24 18:39 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-8-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change the data used in UDS SO_PEERSEC processing from a
> secid to a more general struct lsmblob. Update the
> security_socket_getpeersec_dgram() interface to use the
> lsmblob. There is a small amount of scaffolding code
> that will come out when the security_secid_to_secctx()
> code is brought in line with the lsmblob.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  include/linux/security.h |  7 +++++--
>  include/net/af_unix.h    |  2 +-
>  include/net/scm.h        |  8 +++++---
>  net/ipv4/ip_sockglue.c   |  8 +++++---
>  net/unix/af_unix.c       |  6 +++---
>  security/security.c      | 16 +++++++++++++---
>  6 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 4a78516cc74a..905830a90745 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1276,7 +1276,8 @@ int security_socket_shutdown(struct socket *sock, int how);
>  int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb);
>  int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>  				      int __user *optlen, unsigned len);
> -int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid);
> +int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> +				     struct lsmblob *blob);
>  int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
>  void security_sk_free(struct sock *sk);
>  void security_sk_clone(const struct sock *sk, struct sock *newsk);
> @@ -1414,7 +1415,9 @@ static inline int security_socket_getpeersec_stream(struct socket *sock, char __
>  	return -ENOPROTOOPT;
>  }
>  
> -static inline int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
> +static inline int security_socket_getpeersec_dgram(struct socket *sock,
> +						   struct sk_buff *skb,
> +						   struct lsmblob *blob)
>  {
>  	return -ENOPROTOOPT;
>  }
> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
> index 3426d6dacc45..933492c08b8c 100644
> --- a/include/net/af_unix.h
> +++ b/include/net/af_unix.h
> @@ -36,7 +36,7 @@ struct unix_skb_parms {
>  	kgid_t			gid;
>  	struct scm_fp_list	*fp;		/* Passed files		*/
>  #ifdef CONFIG_SECURITY_NETWORK
> -	u32			secid;		/* Security ID		*/
> +	struct lsmblob		lsmblob;	/* Security LSM data	*/
>  #endif
>  	u32			consumed;
>  } __randomize_layout;
> diff --git a/include/net/scm.h b/include/net/scm.h
> index 1ce365f4c256..e2e71c4bf9d0 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -33,7 +33,7 @@ struct scm_cookie {
>  	struct scm_fp_list	*fp;		/* Passed files		*/
>  	struct scm_creds	creds;		/* Skb credentials	*/
>  #ifdef CONFIG_SECURITY_NETWORK
> -	u32			secid;		/* Passed security ID 	*/
> +	struct lsmblob		lsmblob;	/* Passed LSM data	*/
>  #endif
>  };
>  
> @@ -46,7 +46,7 @@ struct scm_fp_list *scm_fp_dup(struct scm_fp_list *fpl);
>  #ifdef CONFIG_SECURITY_NETWORK
>  static __inline__ void unix_get_peersec_dgram(struct socket *sock, struct scm_cookie *scm)
>  {
> -	security_socket_getpeersec_dgram(sock, NULL, &scm->secid);
> +	security_socket_getpeersec_dgram(sock, NULL, &scm->lsmblob);
>  }
>  #else
>  static __inline__ void unix_get_peersec_dgram(struct socket *sock, struct scm_cookie *scm)
> @@ -97,7 +97,9 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
>  	int err;
>  
>  	if (test_bit(SOCK_PASSSEC, &sock->flags)) {
> -		err = security_secid_to_secctx(scm->secid, &secdata, &seclen);
> +		/* Scaffolding - it has to be element 0 for now */
> +		err = security_secid_to_secctx(scm->lsmblob.secid[0],
> +					       &secdata, &seclen);
>  
>  		if (!err) {
>  			put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, seclen, secdata);
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index 82f341e84fae..2a5c868ce135 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -130,15 +130,17 @@ static void ip_cmsg_recv_checksum(struct msghdr *msg, struct sk_buff *skb,
>  
>  static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
>  {
> +	struct lsmblob lb;
>  	char *secdata;
> -	u32 seclen, secid;
> +	u32 seclen;
>  	int err;
>  
> -	err = security_socket_getpeersec_dgram(NULL, skb, &secid);
> +	err = security_socket_getpeersec_dgram(NULL, skb, &lb);
>  	if (err)
>  		return;
>  
> -	err = security_secid_to_secctx(secid, &secdata, &seclen);
> +	/* Scaffolding - it has to be element 0 */
> +	err = security_secid_to_secctx(lb.secid[0], &secdata, &seclen);
>  	if (err)
>  		return;
>  
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index ddb838a1b74c..c50a004a1389 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -143,17 +143,17 @@ static struct hlist_head *unix_sockets_unbound(void *addr)
>  #ifdef CONFIG_SECURITY_NETWORK
>  static void unix_get_secdata(struct scm_cookie *scm, struct sk_buff *skb)
>  {
> -	UNIXCB(skb).secid = scm->secid;
> +	UNIXCB(skb).lsmblob = scm->lsmblob;
>  }
>  
>  static inline void unix_set_secdata(struct scm_cookie *scm, struct sk_buff *skb)
>  {
> -	scm->secid = UNIXCB(skb).secid;
> +	scm->lsmblob = UNIXCB(skb).lsmblob;
>  }
>  
>  static inline bool unix_secdata_eq(struct scm_cookie *scm, struct sk_buff *skb)
>  {
> -	return (scm->secid == UNIXCB(skb).secid);
> +	return lsmblob_equal(&scm->lsmblob, &(UNIXCB(skb).lsmblob));
>  }
>  #else
>  static inline void unix_get_secdata(struct scm_cookie *scm, struct sk_buff *skb)
> diff --git a/security/security.c b/security/security.c
> index 43f8018b9e13..c7b3d1a294ad 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2137,10 +2137,20 @@ int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>  				optval, optlen, len);
>  }
>  
> -int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
> +int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> +				     struct lsmblob *blob)
>  {
> -	return call_int_hook(socket_getpeersec_dgram, -ENOPROTOOPT, sock,
> -			     skb, secid);
> +	struct security_hook_list *hp;
> +	int rc = -ENOPROTOOPT;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_dgram,
> +			     list) {
> +		rc = hp->hook.socket_getpeersec_dgram(sock, skb,
> +						      &blob->secid[hp->slot]);
> +		if (rc != 0)
> +			break;
> +	}
> +	return rc;
>  }
>  EXPORT_SYMBOL(security_socket_getpeersec_dgram);
>  
> 






^ permalink raw reply

* [PATCH v3 05/24] Use lsmblob in security_audit_rule_match
From: John Johansen @ 2019-06-24 18:39 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-6-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change the secid parameter of security_audit_rule_match
> to a lsmblob structure pointer. Pass the entry from the
> lsmblob structure for the approprite slot to the LSM hook.
> 
> Change the users of security_audit_rule_match to use the
> lsmblob instead of a u32. In some cases this requires a
> temporary conversion using lsmblob_init() that will go
> away when other interfaces get converted.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  include/linux/security.h            |  7 ++++---
>  kernel/auditfilter.c                |  4 +++-
>  kernel/auditsc.c                    | 14 ++++++++++----
>  security/integrity/ima/ima.h        |  4 ++--
>  security/integrity/ima/ima_policy.c |  7 +++++--
>  security/security.c                 | 14 ++++++++++++--
>  6 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 0aa9417a5762..52d89c4a9594 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1757,7 +1757,8 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer)
>  #ifdef CONFIG_SECURITY
>  int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule);
>  int security_audit_rule_known(struct audit_krule *krule);
> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule);
> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
> +			      void *lsmrule);
>  void security_audit_rule_free(void *lsmrule);
>  
>  #else
> @@ -1773,8 +1774,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule)
>  	return 0;
>  }
>  
> -static inline int security_audit_rule_match(u32 secid, u32 field, u32 op,
> -					    void *lsmrule)
> +static inline int security_audit_rule_match(struct lsmblob *blob, u32 field,
> +					    u32 op, void *lsmrule)
>  {
>  	return 0;
>  }
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 63f8b3f26fab..da211065160f 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1324,6 +1324,7 @@ int audit_filter(int msgtype, unsigned int listtype)
>  			struct audit_field *f = &e->rule.fields[i];
>  			pid_t pid;
>  			u32 sid;
> +			struct lsmblob blob;
>  
>  			switch (f->type) {
>  			case AUDIT_PID:
> @@ -1354,7 +1355,8 @@ int audit_filter(int msgtype, unsigned int listtype)
>  			case AUDIT_SUBJ_CLR:
>  				if (f->lsm_rule) {
>  					security_task_getsecid(current, &sid);
> -					result = security_audit_rule_match(sid,
> +					lsmblob_init(&blob, sid);
> +					result = security_audit_rule_match(&blob,
>  						   f->type, f->op, f->lsm_rule);
>  				}
>  				break;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index d1eab1d4a930..18ee5556c086 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -445,6 +445,7 @@ static int audit_filter_rules(struct task_struct *tsk,
>  	const struct cred *cred;
>  	int i, need_sid = 1;
>  	u32 sid;
> +	struct lsmblob blob;
>  	unsigned int sessionid;
>  
>  	cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation);
> @@ -630,7 +631,9 @@ static int audit_filter_rules(struct task_struct *tsk,
>  					security_task_getsecid(tsk, &sid);
>  					need_sid = 0;
>  				}
> -				result = security_audit_rule_match(sid, f->type,
> +				lsmblob_init(&blob, sid);
> +				result = security_audit_rule_match(&blob,
> +								   f->type,
>  								   f->op,
>  								   f->lsm_rule);
>  			}
> @@ -645,15 +648,17 @@ static int audit_filter_rules(struct task_struct *tsk,
>  			if (f->lsm_rule) {
>  				/* Find files that match */
>  				if (name) {
> +					lsmblob_init(&blob, name->osid);
>  					result = security_audit_rule_match(
> -								name->osid,
> +								&blob,
>  								f->type,
>  								f->op,
>  								f->lsm_rule);
>  				} else if (ctx) {
>  					list_for_each_entry(n, &ctx->names_list, list) {
> +						lsmblob_init(&blob, n->osid);
>  						if (security_audit_rule_match(
> -								n->osid,
> +								&blob,
>  								f->type,
>  								f->op,
>  								f->lsm_rule)) {
> @@ -665,7 +670,8 @@ static int audit_filter_rules(struct task_struct *tsk,
>  				/* Find ipc objects that match */
>  				if (!ctx || ctx->type != AUDIT_IPC)
>  					break;
> -				if (security_audit_rule_match(ctx->ipc.osid,
> +				lsmblob_init(&blob, ctx->ipc.osid);
> +				if (security_audit_rule_match(&blob,
>  							      f->type, f->op,
>  							      f->lsm_rule))
>  					++result;
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d213e835c498..5a337239d9e4 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -307,8 +307,8 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr,
>  	return -EINVAL;
>  }
>  
> -static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
> -					     void *lsmrule)
> +static inline int security_filter_rule_match(struct lsmblob *blob, u32 field,
> +					     u32 op, void *lsmrule)
>  {
>  	return -EINVAL;
>  }
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index e0cc323f948f..e7b8ce942950 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -327,6 +327,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  	for (i = 0; i < MAX_LSM_RULES; i++) {
>  		int rc = 0;
>  		u32 osid;
> +		struct lsmblob blob;
>  		int retried = 0;
>  
>  		if (!rule->lsm[i].rule)
> @@ -337,7 +338,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  		case LSM_OBJ_ROLE:
>  		case LSM_OBJ_TYPE:
>  			security_inode_getsecid(inode, &osid);
> -			rc = security_filter_rule_match(osid,
> +			lsmblob_init(&blob, osid);
> +			rc = security_filter_rule_match(&blob,
>  							rule->lsm[i].type,
>  							Audit_equal,
>  							rule->lsm[i].rule);
> @@ -345,7 +347,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  		case LSM_SUBJ_USER:
>  		case LSM_SUBJ_ROLE:
>  		case LSM_SUBJ_TYPE:
> -			rc = security_filter_rule_match(secid,
> +			lsmblob_init(&blob, secid);
> +			rc = security_filter_rule_match(&blob,
>  							rule->lsm[i].type,
>  							Audit_equal,
>  							rule->lsm[i].rule);
> diff --git a/security/security.c b/security/security.c
> index 7618c761060d..4692f44718c6 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2452,9 +2452,19 @@ void security_audit_rule_free(void *lsmrule)
>  	call_void_hook(audit_rule_free, lsmrule);
>  }
>  
> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule)
> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
> +			      void *lsmrule)
>  {
> -	return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule);
> +	struct security_hook_list *hp;
> +	int rc;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) {
> +		rc = hp->hook.audit_rule_match(blob->secid[hp->slot], field,
> +					       op, lsmrule);
> +		if (rc != 0)
> +			return rc;
> +	}
> +	return 0;
>  }
>  #endif /* CONFIG_AUDIT */
>  
> 






^ permalink raw reply

* [PATCH v3 08/24] LSM: Use lsmblob in security_secctx_to_secid
From: John Johansen @ 2019-06-24 18:38 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-9-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change security_secctx_to_secid() to fill in a lsmblob instead
> of a u32 secid. Multiple LSMs may be able to interpret the
> string, and this allows for setting whichever secid is
> appropriate. In some cases there is scaffolding where other
> interfaces have yet to be converted.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  include/linux/security.h          |  5 +++--
>  kernel/cred.c                     |  4 +---
>  net/netfilter/nft_meta.c          | 13 ++++++-------
>  net/netfilter/xt_SECMARK.c        |  5 ++++-
>  net/netlabel/netlabel_unlabeled.c | 14 ++++++++------
>  security/security.c               | 16 +++++++++++++---
>  6 files changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 905830a90745..b0395d224c43 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -443,7 +443,8 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>  int security_netlink_send(struct sock *sk, struct sk_buff *skb);
>  int security_ismaclabel(const char *name);
>  int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
> -int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
> +int security_secctx_to_secid(const char *secdata, u32 seclen,
> +			     struct lsmblob *blob);
>  void security_release_secctx(char *secdata, u32 seclen);
>  
>  void security_inode_invalidate_secctx(struct inode *inode);
> @@ -1226,7 +1227,7 @@ static inline int security_secid_to_secctx(u32 secid, char **secdata, u32 *secle
>  
>  static inline int security_secctx_to_secid(const char *secdata,
>  					   u32 seclen,
> -					   u32 *secid)
> +					   struct lsmblob *blob)
>  {
>  	return -EOPNOTSUPP;
>  }
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 71c14dda107e..d70a2c02ced4 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -725,14 +725,12 @@ EXPORT_SYMBOL(set_security_override);
>  int set_security_override_from_ctx(struct cred *new, const char *secctx)
>  {
>  	struct lsmblob blob;
> -	u32 secid;
>  	int ret;
>  
> -	ret = security_secctx_to_secid(secctx, strlen(secctx), &secid);
> +	ret = security_secctx_to_secid(secctx, strlen(secctx), &blob);
>  	if (ret < 0)
>  		return ret;
>  
> -	lsmblob_init(&blob, secid);
>  	return set_security_override(new, &blob);
>  }
>  EXPORT_SYMBOL(set_security_override_from_ctx);
> diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
> index 987d2d6ce624..91973d3a5f6a 100644
> --- a/net/netfilter/nft_meta.c
> +++ b/net/netfilter/nft_meta.c
> @@ -576,21 +576,20 @@ static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
>  
>  static int nft_secmark_compute_secid(struct nft_secmark *priv)
>  {
> -	u32 tmp_secid = 0;
> +	struct lsmblob blob;
>  	int err;
>  
> -	err = security_secctx_to_secid(priv->ctx, strlen(priv->ctx), &tmp_secid);
> +	err = security_secctx_to_secid(priv->ctx, strlen(priv->ctx), &blob);
>  	if (err)
>  		return err;
>  
> -	if (!tmp_secid)
> -		return -ENOENT;
> -
> -	err = security_secmark_relabel_packet(tmp_secid);
> +	/* Using le[0] is scaffolding */
> +	err = security_secmark_relabel_packet(blob.secid[0]);
>  	if (err)
>  		return err;
>  
> -	priv->secid = tmp_secid;
> +	/* Using le[1] is scaffolding */
> +	priv->secid = blob.secid[0];
>  	return 0;
>  }
>  
> diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
> index f16202d26c20..8081fadc30e9 100644
> --- a/net/netfilter/xt_SECMARK.c
> +++ b/net/netfilter/xt_SECMARK.c
> @@ -49,13 +49,14 @@ secmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
>  
>  static int checkentry_lsm(struct xt_secmark_target_info *info)
>  {
> +	struct lsmblob blob;
>  	int err;
>  
>  	info->secctx[SECMARK_SECCTX_MAX - 1] = '\0';
>  	info->secid = 0;
>  
>  	err = security_secctx_to_secid(info->secctx, strlen(info->secctx),
> -				       &info->secid);
> +				       &blob);
>  	if (err) {
>  		if (err == -EINVAL)
>  			pr_info_ratelimited("invalid security context \'%s\'\n",
> @@ -63,6 +64,8 @@ static int checkentry_lsm(struct xt_secmark_target_info *info)
>  		return err;
>  	}
>  
> +	/* scaffolding during the transition */
> +	info->secid = blob.secid[0];
>  	if (!info->secid) {
>  		pr_info_ratelimited("unable to map security context \'%s\'\n",
>  				    info->secctx);
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index c92894c3e40a..2976370e41aa 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -895,7 +895,7 @@ static int netlbl_unlabel_staticadd(struct sk_buff *skb,
>  	void *addr;
>  	void *mask;
>  	u32 addr_len;
> -	u32 secid;
> +	struct lsmblob blob;
>  	struct netlbl_audit audit_info;
>  
>  	/* Don't allow users to add both IPv4 and IPv6 addresses for a
> @@ -919,12 +919,13 @@ static int netlbl_unlabel_staticadd(struct sk_buff *skb,
>  	ret_val = security_secctx_to_secid(
>  		                  nla_data(info->attrs[NLBL_UNLABEL_A_SECCTX]),
>  				  nla_len(info->attrs[NLBL_UNLABEL_A_SECCTX]),
> -				  &secid);
> +				  &blob);
>  	if (ret_val != 0)
>  		return ret_val;
>  
> +	/* scaffolding with the [0] */
>  	return netlbl_unlhsh_add(&init_net,
> -				 dev_name, addr, mask, addr_len, secid,
> +				 dev_name, addr, mask, addr_len, blob.secid[0],
>  				 &audit_info);
>  }
>  
> @@ -946,7 +947,7 @@ static int netlbl_unlabel_staticadddef(struct sk_buff *skb,
>  	void *addr;
>  	void *mask;
>  	u32 addr_len;
> -	u32 secid;
> +	struct lsmblob blob;
>  	struct netlbl_audit audit_info;
>  
>  	/* Don't allow users to add both IPv4 and IPv6 addresses for a
> @@ -968,12 +969,13 @@ static int netlbl_unlabel_staticadddef(struct sk_buff *skb,
>  	ret_val = security_secctx_to_secid(
>  		                  nla_data(info->attrs[NLBL_UNLABEL_A_SECCTX]),
>  				  nla_len(info->attrs[NLBL_UNLABEL_A_SECCTX]),
> -				  &secid);
> +				  &blob);
>  	if (ret_val != 0)
>  		return ret_val;
>  
> +	/* scaffolding with the [0] */
>  	return netlbl_unlhsh_add(&init_net,
> -				 NULL, addr, mask, addr_len, secid,
> +				 NULL, addr, mask, addr_len, blob.secid[0],
>  				 &audit_info);
>  }
>  
> diff --git a/security/security.c b/security/security.c
> index c7b3d1a294ad..cb1545bfe8c5 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1999,10 +1999,20 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
>  }
>  EXPORT_SYMBOL(security_secid_to_secctx);
>  
> -int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> +int security_secctx_to_secid(const char *secdata, u32 seclen,
> +			     struct lsmblob *blob)
>  {
> -	*secid = 0;
> -	return call_int_hook(secctx_to_secid, 0, secdata, seclen, secid);
> +	struct security_hook_list *hp;
> +	int rc;
> +
> +	lsmblob_init(blob, 0);
> +	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
> +		rc = hp->hook.secctx_to_secid(secdata, seclen,
> +					      &blob->secid[hp->slot]);
> +		if (rc != 0)
> +			return rc;
> +	}
> +	return 0;
>  }
>  EXPORT_SYMBOL(security_secctx_to_secid);
>  
> 




^ permalink raw reply

* [PATCH v3 06/24] LSM: Use lsmblob in security_kernel_act_as
From: John Johansen @ 2019-06-24 18:38 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
  Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190621185233.6766-7-casey@schaufler-ca.com>

On 6/21/19 11:52 AM, Casey Schaufler wrote:
> Change the security_kernel_act_as interface to use a lsmblob
> structure in place of the single u32 secid in support of
> module stacking. Change it's only caller, set_security_override,
> to do the same. Change that one's only caller,
> set_security_override_from_ctx, to call it with the new
> parameter type.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>


> ---
>  include/linux/cred.h     |  3 ++-
>  include/linux/security.h |  5 +++--
>  kernel/cred.c            | 10 ++++++----
>  security/security.c      | 12 ++++++++++--
>  4 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index efb6edf32de7..9a21c376ed97 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -22,6 +22,7 @@
>  
>  struct cred;
>  struct inode;
> +struct lsmblob;
>  
>  /*
>   * COW Supplementary groups list
> @@ -165,7 +166,7 @@ extern const struct cred *override_creds(const struct cred *);
>  extern void revert_creds(const struct cred *);
>  extern struct cred *prepare_kernel_cred(struct task_struct *);
>  extern int change_create_files_as(struct cred *, struct inode *);
> -extern int set_security_override(struct cred *, u32);
> +extern int set_security_override(struct cred *, struct lsmblob *);
>  extern int set_security_override_from_ctx(struct cred *, const char *);
>  extern int set_create_files_as(struct cred *, struct inode *);
>  extern int cred_fscmp(const struct cred *, const struct cred *);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 52d89c4a9594..4a78516cc74a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -384,7 +384,7 @@ void security_cred_free(struct cred *cred);
>  int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
>  void security_transfer_creds(struct cred *new, const struct cred *old);
>  void security_cred_getsecid(const struct cred *c, u32 *secid);
> -int security_kernel_act_as(struct cred *new, u32 secid);
> +int security_kernel_act_as(struct cred *new, struct lsmblob *blob);
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>  int security_kernel_module_request(char *kmod_name);
>  int security_kernel_load_data(enum kernel_load_data_id id);
> @@ -967,7 +967,8 @@ static inline void security_transfer_creds(struct cred *new,
>  {
>  }
>  
> -static inline int security_kernel_act_as(struct cred *cred, u32 secid)
> +static inline int security_kernel_act_as(struct cred *cred,
> +					 struct lsmblob *blob)
>  {
>  	return 0;
>  }
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 45d77284aed0..71c14dda107e 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -701,14 +701,14 @@ EXPORT_SYMBOL(prepare_kernel_cred);
>  /**
>   * set_security_override - Set the security ID in a set of credentials
>   * @new: The credentials to alter
> - * @secid: The LSM security ID to set
> + * @blob: The LSM security information to set
>   *
>   * Set the LSM security ID in a set of credentials so that the subjective
>   * security is overridden when an alternative set of credentials is used.
>   */
> -int set_security_override(struct cred *new, u32 secid)
> +int set_security_override(struct cred *new, struct lsmblob *blob)
>  {
> -	return security_kernel_act_as(new, secid);
> +	return security_kernel_act_as(new, blob);
>  }
>  EXPORT_SYMBOL(set_security_override);
>  
> @@ -724,6 +724,7 @@ EXPORT_SYMBOL(set_security_override);
>   */
>  int set_security_override_from_ctx(struct cred *new, const char *secctx)
>  {
> +	struct lsmblob blob;
>  	u32 secid;
>  	int ret;
>  
> @@ -731,7 +732,8 @@ int set_security_override_from_ctx(struct cred *new, const char *secctx)
>  	if (ret < 0)
>  		return ret;
>  
> -	return set_security_override(new, secid);
> +	lsmblob_init(&blob, secid);
> +	return set_security_override(new, &blob);
>  }
>  EXPORT_SYMBOL(set_security_override_from_ctx);
>  
> diff --git a/security/security.c b/security/security.c
> index 4692f44718c6..43f8018b9e13 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1646,9 +1646,17 @@ void security_cred_getsecid(const struct cred *c, u32 *secid)
>  }
>  EXPORT_SYMBOL(security_cred_getsecid);
>  
> -int security_kernel_act_as(struct cred *new, u32 secid)
> +int security_kernel_act_as(struct cred *new, struct lsmblob *blob)
>  {
> -	return call_int_hook(kernel_act_as, 0, new, secid);
> +	struct security_hook_list *hp;
> +	int rc;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.kernel_act_as, list) {
> +		rc = hp->hook.kernel_act_as(new, blob->secid[hp->slot]);
> +		if (rc != 0)
> +			return rc;
> +	}
> +	return 0;
>  }
>  
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode)
> 




^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox