selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: casey.schaufler@intel.com, jmorris@namei.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org
Cc: casey@schaufler-ca.com, keescook@chromium.org,
	john.johansen@canonical.com, penguin-kernel@i-love.sakura.ne.jp,
	paul@paul-moore.com, sds@tycho.nsa.gov
Subject: [PATCH v7 10/16] LSM: Change error detection for UDP peer security
Date: Wed,  7 Aug 2019 15:42:39 -0700	[thread overview]
Message-ID: <20190807224245.10798-12-casey@schaufler-ca.com> (raw)
In-Reply-To: <20190807224245.10798-1-casey@schaufler-ca.com>

security_socket_getpeercred_dgram() supplies secids for use
by security_secid_to_secctx(). Sometimes a secid will be invalid.
Move the check for an invalid secid from the LSM specific
socket_getpeercred_dgram hooks into the secid_to_secctx hooks.
This allows for the case where one LSM (Smack) will provide a
secid and another (SELinux) to have an error for the same call.
Regardless of which LSM the caller wants to see the peer security
attributes for the correct result will be provided.

As there is no longer any reason for security_secid_to_secctx()
to return a value make all the secid_to_secctx functions void
instead of int. Add checking for a invalid secid to the Smack
and SELinux secid_to_secctx hooks.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hooks.h  |  3 +--
 include/linux/security.h   | 11 +++++------
 net/ipv4/ip_sockglue.c     |  4 +---
 security/security.c        | 12 ++++--------
 security/selinux/hooks.c   | 10 ++++++----
 security/smack/smack_lsm.c | 15 +++++++++------
 6 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a54a2f4788af..67797c67093b 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -881,7 +881,6 @@
  *	@sock contains the peer socket. May be NULL.
  *	@skb is the sk_buff for the packet being queried. May be NULL.
  *	@secid pointer to store the secid of the packet.
- *	Return 0 on success, error on failure.
  * @sk_alloc_security:
  *	Allocate and attach a security structure to the sk->sk_security field,
  *	which is used to copy security attributes between local stream sockets.
@@ -1699,7 +1698,7 @@ union security_list_options {
 	int (*socket_sock_rcv_skb)(struct sock *sk, struct sk_buff *skb);
 	int (*socket_getpeersec_stream)(struct socket *sock, char **optval,
 					int *optlen, unsigned len);
-	int (*socket_getpeersec_dgram)(struct socket *sock,
+	void (*socket_getpeersec_dgram)(struct socket *sock,
 					struct sk_buff *skb, u32 *secid);
 	int (*sk_alloc_security)(struct sock *sk, int family, gfp_t priority);
 	void (*sk_free_security)(struct sock *sk);
diff --git a/include/linux/security.h b/include/linux/security.h
index 2f442746dede..0e699d4ed13a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1329,8 +1329,8 @@ 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 display);
-int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
-				     struct lsmblob *blob);
+void 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);
@@ -1470,11 +1470,10 @@ static inline int security_socket_getpeersec_stream(struct socket *sock,
 	return -ENOPROTOOPT;
 }
 
-static inline int security_socket_getpeersec_dgram(struct socket *sock,
-						   struct sk_buff *skb,
-						   struct lsmblob *blob)
+static inline void security_socket_getpeersec_dgram(struct socket *sock,
+						    struct sk_buff *skb,
+						    struct lsmblob *blob)
 {
-	return -ENOPROTOOPT;
 }
 
 static inline int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 447fe60af0cd..c28cbb15cee2 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -134,9 +134,7 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
 	struct lsmblob lb;
 	int err;
 
-	err = security_socket_getpeersec_dgram(NULL, skb, &lb);
-	if (err)
-		return;
+	security_socket_getpeersec_dgram(NULL, skb, &lb);
 
 	err = security_secid_to_secctx(&lb, &context, LSMBLOB_DISPLAY);
 	if (err)
diff --git a/security/security.c b/security/security.c
index 325e745ac8f5..e726fc7c6712 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2612,22 +2612,18 @@ int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
 	return rc;
 }
 
-int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
-				     struct lsmblob *blob)
+void security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
+				      struct lsmblob *blob)
 {
 	struct security_hook_list *hp;
-	int rc = -ENOPROTOOPT;
 
 	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_dgram,
 			     list) {
 		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
 			continue;
-		rc = hp->hook.socket_getpeersec_dgram(sock, skb,
-						&blob->secid[hp->lsmid->slot]);
-		if (rc != 0)
-			break;
+		hp->hook.socket_getpeersec_dgram(sock, skb,
+						 &blob->secid[hp->lsmid->slot]);
 	}
-	return rc;
 }
 EXPORT_SYMBOL(security_socket_getpeersec_dgram);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 65bd62dca9e9..91ef2ae77abb 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4954,7 +4954,8 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char **optval,
 	return err;
 }
 
-static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
+static void selinux_socket_getpeersec_dgram(struct socket *sock,
+					    struct sk_buff *skb, u32 *secid)
 {
 	u32 peer_secid = SECSID_NULL;
 	u16 family;
@@ -4977,9 +4978,7 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *
 
 out:
 	*secid = peer_secid;
-	if (peer_secid == SECSID_NULL)
-		return -EINVAL;
-	return 0;
+	return;
 }
 
 static int selinux_sk_alloc_security(struct sock *sk, int family, gfp_t priority)
@@ -6321,6 +6320,9 @@ static int selinux_ismaclabel(const char *name)
 
 static int selinux_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
 {
+	if (secid == SECSID_NULL)
+		return -EINVAL;
+
 	return security_sid_to_context(&selinux_state, secid,
 				       secdata, seclen);
 }
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a9fb5f53a248..2d88983868e8 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3970,8 +3970,8 @@ static int smack_socket_getpeersec_stream(struct socket *sock, char **optval,
  *
  * Sets the netlabel socket state on sk from parent
  */
-static int smack_socket_getpeersec_dgram(struct socket *sock,
-					 struct sk_buff *skb, u32 *secid)
+static void smack_socket_getpeersec_dgram(struct socket *sock,
+					  struct sk_buff *skb, u32 *secid)
 
 {
 	struct netlbl_lsm_secattr secattr;
@@ -4025,9 +4025,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		break;
 	}
 	*secid = s;
-	if (s == 0)
-		return -EINVAL;
-	return 0;
+	return;
 }
 
 /**
@@ -4426,7 +4424,12 @@ static int smack_ismaclabel(const char *name)
  */
 static int smack_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
 {
-	struct smack_known *skp = smack_from_secid(secid);
+	struct smack_known *skp;
+
+	if (secid == 0)
+		return -EINVAL;
+
+	skp = smack_from_secid(secid);
 
 	if (secdata)
 		*secdata = skp->smk_known;
-- 
2.20.1


  parent reply	other threads:[~2019-08-07 22:43 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
2019-08-07 22:42 ` Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 01/16] LSM: Single hook called in secmark refcounting Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 02/16] Smack: Detect if secmarks can be safely used Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 03/16] LSM: Support multiple LSMs using inode_init_security Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 04/16] LSM: List multiple security attributes in security_inode_listsecurity Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 05/16] LSM: Multiple modules using security_ismaclabel Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 06/16] LSM: Make multiple MAC modules safe in nfs and kernfs Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 07/16] LSM: Correct handling of ENOSYS in inode_setxattr Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 08/16] LSM: Infrastructure security blobs for mount options Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 09/16] LSM: Fix for security_init_inode_security Casey Schaufler
2019-08-07 22:42 ` Casey Schaufler [this message]
2019-08-07 22:42 ` [PATCH v7 11/16] Netlabel: Add a secattr comparison API function Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 12/16] Netlabel: Provide labeling type to security modules Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 13/16] LSM: Remember the NLTYPE of netlabel sockets Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 14/16] LSM: Hook for netlabel reconciliation Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 15/16] LSM: Avoid network conflicts in SELinux and Smack Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 16/16] Smack: Remove the exclusive flag Casey Schaufler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190807224245.10798-12-casey@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=casey.schaufler@intel.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=keescook@chromium.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).