linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Smack: Use the netlbl incoming cache
       [not found] <20200812003943.3036-1-casey.ref@schaufler-ca.com>
@ 2020-08-12  0:39 ` Casey Schaufler
  2020-08-12  0:39   ` [PATCH 1/3] Smack: Consolidate uses of secmark into a function Casey Schaufler
                     ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Casey Schaufler @ 2020-08-12  0:39 UTC (permalink / raw)
  To: SMACK-discuss, casey.schaufler, linux-security-module; +Cc: paul, casey

Update the Smack security module to use the Netlabel cache
mechanism to speed the processing of incoming labeled packets.
There is some refactoring of the existing code that makes it
simpler, and reduces duplication. The outbound packet labeling
is also optimized to track the labeling state of the socket.
Prior to this the socket label was redundantly set on each
packet send.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack.h        |  19 ++--
 security/smack/smack_access.c |  55 ++++++----
 security/smack/smack_lsm.c    | 245 ++++++++++++++++++++++++------------------
 security/smack/smackfs.c      |  23 ++--
 4 files changed, 193 insertions(+), 149 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH 1/3] Smack: Consolidate uses of secmark into a function
  2020-08-12  0:39 ` [PATCH 0/3] Smack: Use the netlbl incoming cache Casey Schaufler
@ 2020-08-12  0:39   ` Casey Schaufler
  2020-08-12  0:39   ` [PATCH 2/3] Smack: Set socket labels only once Casey Schaufler
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Casey Schaufler @ 2020-08-12  0:39 UTC (permalink / raw)
  To: SMACK-discuss, casey.schaufler, linux-security-module; +Cc: paul, casey

Add a function smack_from_skb() that returns the Smack label
identified by a network secmark. Replace the explicit uses of
the secmark with this function.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_lsm.c | 61 +++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 8ffbf951b7ed..3402ac4aa28e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3810,6 +3810,20 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
 }
 #endif /* CONFIG_IPV6 */
 
+/**
+ * smack_from_skb - Smack data from the secmark in an skb
+ * @skb: packet
+ *
+ * Returns smack_known of the secmark or NULL if that won't work.
+ */
+static struct smack_known *smack_from_skb(struct sk_buff *skb)
+{
+	if (skb == NULL || skb->secmark == 0)
+		return NULL;
+
+	return smack_from_secid(skb->secmark);
+}
+
 /**
  * smack_socket_sock_rcv_skb - Smack packet delivery access check
  * @sk: socket
@@ -3838,17 +3852,14 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 
 	switch (family) {
 	case PF_INET:
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 		/*
 		 * If there is a secmark use it rather than the CIPSO label.
 		 * If there is no secmark fall back to CIPSO.
 		 * The secmark is assumed to reflect policy better.
 		 */
-		if (skb && skb->secmark != 0) {
-			skp = smack_from_secid(skb->secmark);
+		skp = smack_from_skb(skb);
+		if (skp)
 			goto access_check;
-		}
-#endif /* CONFIG_SECURITY_SMACK_NETFILTER */
 		/*
 		 * Translate what netlabel gave us.
 		 */
@@ -3862,9 +3873,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 
 		netlbl_secattr_destroy(&secattr);
 
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 access_check:
-#endif
+
 #ifdef CONFIG_AUDIT
 		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
 		ad.a.u.net->family = family;
@@ -3890,16 +3900,14 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		    proto != IPPROTO_TCP && proto != IPPROTO_DCCP)
 			break;
 #ifdef SMACK_IPV6_SECMARK_LABELING
-		if (skb && skb->secmark != 0)
-			skp = smack_from_secid(skb->secmark);
-		else if (smk_ipv6_localhost(&sadd))
-			break;
-		else
+		skp = smack_from_skb(skb);
+		if (skp == NULL) {
+			if (smk_ipv6_localhost(&sadd))
+				break;
 			skp = smack_ipv6host_label(&sadd);
-		if (skp == NULL)
-			skp = smack_net_ambient;
-		if (skb == NULL)
-			break;
+			if (skp == NULL)
+				skp = smack_net_ambient;
+		}
 #ifdef CONFIG_AUDIT
 		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
 		ad.a.u.net->family = family;
@@ -3995,11 +4003,11 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		s = ssp->smk_out->smk_secid;
 		break;
 	case PF_INET:
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
-		s = skb->secmark;
-		if (s != 0)
+		skp = smack_from_skb(skb);
+		if (skp) {
+			s = skp->smk_secid;
 			break;
-#endif
+		}
 		/*
 		 * Translate what netlabel gave us.
 		 */
@@ -4015,7 +4023,9 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		break;
 	case PF_INET6:
 #ifdef SMACK_IPV6_SECMARK_LABELING
-		s = skb->secmark;
+		skp = smack_from_skb(skb);
+		if (skp)
+			s = skp->smk_secid;
 #endif
 		break;
 	}
@@ -4087,17 +4097,14 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 	}
 #endif /* CONFIG_IPV6 */
 
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 	/*
 	 * If there is a secmark use it rather than the CIPSO label.
 	 * If there is no secmark fall back to CIPSO.
 	 * The secmark is assumed to reflect policy better.
 	 */
-	if (skb && skb->secmark != 0) {
-		skp = smack_from_secid(skb->secmark);
+	skp = smack_from_skb(skb);
+	if (skp)
 		goto access_check;
-	}
-#endif /* CONFIG_SECURITY_SMACK_NETFILTER */
 
 	netlbl_secattr_init(&secattr);
 	rc = netlbl_skbuff_getattr(skb, family, &secattr);
@@ -4107,9 +4114,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 		skp = &smack_known_huh;
 	netlbl_secattr_destroy(&secattr);
 
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 access_check:
-#endif
 
 #ifdef CONFIG_AUDIT
 	smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 2/3] Smack: Set socket labels only once
  2020-08-12  0:39 ` [PATCH 0/3] Smack: Use the netlbl incoming cache Casey Schaufler
  2020-08-12  0:39   ` [PATCH 1/3] Smack: Consolidate uses of secmark into a function Casey Schaufler
@ 2020-08-12  0:39   ` Casey Schaufler
  2020-08-12  0:39   ` [PATCH 3/3] Smack: Use the netlabel cache Casey Schaufler
  2020-08-12  2:10   ` [PATCH 0/3] Smack: Use the netlbl incoming cache Paul Moore
  3 siblings, 0 replies; 8+ messages in thread
From: Casey Schaufler @ 2020-08-12  0:39 UTC (permalink / raw)
  To: SMACK-discuss, casey.schaufler, linux-security-module; +Cc: paul, casey

Refactor the IP send checks so that the netlabel value
is set only when necessary, not on every send. Some functions
get renamed as the changes made the old name misleading.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack.h     |  18 ++--
 security/smack/smack_lsm.c | 169 ++++++++++++++++++++-----------------
 2 files changed, 98 insertions(+), 89 deletions(-)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index e9e817d09785..c5d745a3ada8 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -100,7 +100,12 @@ struct socket_smack {
 	struct smack_known	*smk_out;	/* outbound label */
 	struct smack_known	*smk_in;	/* inbound label */
 	struct smack_known	*smk_packet;	/* TCP peer label */
+	int			smk_state;	/* netlabel socket states */
 };
+#define	SMK_NETLBL_UNSET	0
+#define	SMK_NETLBL_UNLABELED	1
+#define	SMK_NETLBL_LABELED	2
+#define	SMK_NETLBL_REQSKB	3
 
 /*
  * Inode smack data
@@ -196,19 +201,6 @@ enum {
 #define SMACK_DELETE_OPTION	"-DELETE"
 #define SMACK_CIPSO_OPTION 	"-CIPSO"
 
-/*
- * How communications on this socket are treated.
- * Usually it's determined by the underlying netlabel code
- * but there are certain cases, including single label hosts
- * and potentially single label interfaces for which the
- * treatment can not be known in advance.
- *
- * The possibility of additional labeling schemes being
- * introduced in the future exists as well.
- */
-#define SMACK_UNLABELED_SOCKET	0
-#define SMACK_CIPSO_SOCKET	1
-
 /*
  * CIPSO defaults.
  */
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 3402ac4aa28e..7a79ddb39e94 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2383,38 +2383,31 @@ static struct smack_known *smack_ipv6host_label(struct sockaddr_in6 *sip)
 }
 
 /**
- * smack_netlabel - Set the secattr on a socket
+ * smack_netlbl_add - Set the secattr on a socket
  * @sk: the socket
- * @labeled: socket label scheme
  *
- * Convert the outbound smack value (smk_out) to a
- * secattr and attach it to the socket.
+ * Attach the outbound smack value (smk_out) to the socket.
  *
  * Returns 0 on success or an error code
  */
-static int smack_netlabel(struct sock *sk, int labeled)
+static int smack_netlbl_add(struct sock *sk)
 {
-	struct smack_known *skp;
 	struct socket_smack *ssp = sk->sk_security;
-	int rc = 0;
+	struct smack_known *skp = ssp->smk_out;
+	int rc;
 
-	/*
-	 * Usually the netlabel code will handle changing the
-	 * packet labeling based on the label.
-	 * The case of a single label host is different, because
-	 * a single label host should never get a labeled packet
-	 * even though the label is usually associated with a packet
-	 * label.
-	 */
 	local_bh_disable();
 	bh_lock_sock_nested(sk);
 
-	if (ssp->smk_out == smack_net_ambient ||
-	    labeled == SMACK_UNLABELED_SOCKET)
-		netlbl_sock_delattr(sk);
-	else {
-		skp = ssp->smk_out;
-		rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel);
+	rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel);
+	switch (rc) {
+	case 0:
+		ssp->smk_state = SMK_NETLBL_LABELED;
+		break;
+	case -EDESTADDRREQ:
+		ssp->smk_state = SMK_NETLBL_REQSKB;
+		rc = 0;
+		break;
 	}
 
 	bh_unlock_sock(sk);
@@ -2424,7 +2417,31 @@ static int smack_netlabel(struct sock *sk, int labeled)
 }
 
 /**
- * smack_netlbel_send - Set the secattr on a socket and perform access checks
+ * smack_netlbl_delete - Remove the secattr from a socket
+ * @sk: the socket
+ *
+ * Remove the outbound smack value from a socket
+ */
+static void smack_netlbl_delete(struct sock *sk)
+{
+	struct socket_smack *ssp = sk->sk_security;
+
+	/*
+	 * Take the label off the socket if one is set.
+	 */
+	if (ssp->smk_state != SMK_NETLBL_LABELED)
+		return;
+
+	local_bh_disable();
+	bh_lock_sock_nested(sk);
+	netlbl_sock_delattr(sk);
+	bh_unlock_sock(sk);
+	local_bh_enable();
+	ssp->smk_state = SMK_NETLBL_UNLABELED;
+}
+
+/**
+ * smk_ipv4_check - Perform IPv4 host access checks
  * @sk: the socket
  * @sap: the destination address
  *
@@ -2434,11 +2451,10 @@ static int smack_netlabel(struct sock *sk, int labeled)
  * Returns 0 on success or an error code.
  *
  */
-static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap)
+static int smk_ipv4_check(struct sock *sk, struct sockaddr_in *sap)
 {
 	struct smack_known *skp;
-	int rc;
-	int sk_lbl;
+	int rc = 0;
 	struct smack_known *hkp;
 	struct socket_smack *ssp = sk->sk_security;
 	struct smk_audit_info ad;
@@ -2454,19 +2470,18 @@ static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap)
 		ad.a.u.net->dport = sap->sin_port;
 		ad.a.u.net->v4info.daddr = sap->sin_addr.s_addr;
 #endif
-		sk_lbl = SMACK_UNLABELED_SOCKET;
 		skp = ssp->smk_out;
 		rc = smk_access(skp, hkp, MAY_WRITE, &ad);
 		rc = smk_bu_note("IPv4 host check", skp, hkp, MAY_WRITE, rc);
-	} else {
-		sk_lbl = SMACK_CIPSO_SOCKET;
-		rc = 0;
+		/*
+		 * Clear the socket netlabel if it's set.
+		 */
+		if (!rc)
+			smack_netlbl_delete(sk);
 	}
 	rcu_read_unlock();
-	if (rc != 0)
-		return rc;
 
-	return smack_netlabel(sk, sk_lbl);
+	return rc;
 }
 
 /**
@@ -2703,7 +2718,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) {
 		ssp->smk_out = skp;
 		if (sock->sk->sk_family == PF_INET) {
-			rc = smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET);
+			rc = smack_netlbl_add(sock->sk);
 			if (rc != 0)
 				printk(KERN_WARNING
 					"Smack: \"%s\" netlbl error %d.\n",
@@ -2754,7 +2769,7 @@ static int smack_socket_post_create(struct socket *sock, int family,
 	/*
 	 * Set the outbound netlbl.
 	 */
-	return smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET);
+	return smack_netlbl_add(sock->sk);
 }
 
 /**
@@ -2845,7 +2860,7 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap,
 	}
 	if (sap->sa_family != AF_INET || addrlen < sizeof(struct sockaddr_in))
 		return 0;
-	rc = smack_netlabel_send(sock->sk, (struct sockaddr_in *)sap);
+	rc = smk_ipv4_check(sock->sk, (struct sockaddr_in *)sap);
 	return rc;
 }
 
@@ -3663,7 +3678,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg,
 		if (msg->msg_namelen < sizeof(struct sockaddr_in) ||
 		    sip->sin_family != AF_INET)
 			return -EINVAL;
-		rc = smack_netlabel_send(sock->sk, sip);
+		rc = smk_ipv4_check(sock->sk, sip);
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
@@ -3824,6 +3839,33 @@ static struct smack_known *smack_from_skb(struct sk_buff *skb)
 	return smack_from_secid(skb->secmark);
 }
 
+/**
+ * smack_from_netlbl - Smack data from the IP options in an skb
+ * @sk: socket data came in on
+ * @family: address family
+ * @skb: packet
+ *
+ * Returns smack_known of the IP options or NULL if that won't work.
+ */
+static struct smack_known *smack_from_netlbl(struct sock *sk, u16 family,
+					     struct sk_buff *skb)
+{
+	struct netlbl_lsm_secattr secattr;
+	struct socket_smack *ssp = NULL;
+	struct smack_known *skp = NULL;
+
+	netlbl_secattr_init(&secattr);
+
+	if (sk)
+		ssp = sk->sk_security;
+	if (netlbl_skbuff_getattr(skb, family, &secattr) == 0)
+		skp = smack_from_secattr(&secattr, ssp);
+
+	netlbl_secattr_destroy(&secattr);
+
+	return skp;
+}
+
 /**
  * smack_socket_sock_rcv_skb - Smack packet delivery access check
  * @sk: socket
@@ -3833,7 +3875,6 @@ static struct smack_known *smack_from_skb(struct sk_buff *skb)
  */
 static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
-	struct netlbl_lsm_secattr secattr;
 	struct socket_smack *ssp = sk->sk_security;
 	struct smack_known *skp = NULL;
 	int rc = 0;
@@ -3858,22 +3899,11 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		 * The secmark is assumed to reflect policy better.
 		 */
 		skp = smack_from_skb(skb);
-		if (skp)
-			goto access_check;
-		/*
-		 * Translate what netlabel gave us.
-		 */
-		netlbl_secattr_init(&secattr);
-
-		rc = netlbl_skbuff_getattr(skb, family, &secattr);
-		if (rc == 0)
-			skp = smack_from_secattr(&secattr, ssp);
-		else
-			skp = smack_net_ambient;
-
-		netlbl_secattr_destroy(&secattr);
-
-access_check:
+		if (skp == NULL) {
+			skp = smack_from_netlbl(sk, family, skb);
+			if (skp == NULL)
+				skp = smack_net_ambient;
+		}
 
 #ifdef CONFIG_AUDIT
 		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
@@ -3979,12 +4009,11 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 					 struct sk_buff *skb, u32 *secid)
 
 {
-	struct netlbl_lsm_secattr secattr;
 	struct socket_smack *ssp = NULL;
 	struct smack_known *skp;
+	struct sock *sk = NULL;
 	int family = PF_UNSPEC;
 	u32 s = 0;	/* 0 is the invalid secid */
-	int rc;
 
 	if (skb != NULL) {
 		if (skb->protocol == htons(ETH_P_IP))
@@ -4011,15 +4040,11 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		/*
 		 * Translate what netlabel gave us.
 		 */
-		if (sock != NULL && sock->sk != NULL)
-			ssp = sock->sk->sk_security;
-		netlbl_secattr_init(&secattr);
-		rc = netlbl_skbuff_getattr(skb, family, &secattr);
-		if (rc == 0) {
-			skp = smack_from_secattr(&secattr, ssp);
+		if (sock != NULL)
+			sk = sock->sk;
+		skp = smack_from_netlbl(sk, family, skb);
+		if (skp != NULL)
 			s = skp->smk_secid;
-		}
-		netlbl_secattr_destroy(&secattr);
 		break;
 	case PF_INET6:
 #ifdef SMACK_IPV6_SECMARK_LABELING
@@ -4073,7 +4098,6 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 	u16 family = sk->sk_family;
 	struct smack_known *skp;
 	struct socket_smack *ssp = sk->sk_security;
-	struct netlbl_lsm_secattr secattr;
 	struct sockaddr_in addr;
 	struct iphdr *hdr;
 	struct smack_known *hskp;
@@ -4103,18 +4127,11 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 	 * The secmark is assumed to reflect policy better.
 	 */
 	skp = smack_from_skb(skb);
-	if (skp)
-		goto access_check;
-
-	netlbl_secattr_init(&secattr);
-	rc = netlbl_skbuff_getattr(skb, family, &secattr);
-	if (rc == 0)
-		skp = smack_from_secattr(&secattr, ssp);
-	else
-		skp = &smack_known_huh;
-	netlbl_secattr_destroy(&secattr);
-
-access_check:
+	if (skp == NULL) {
+		skp = smack_from_netlbl(sk, family, skb);
+		if (skp == NULL)
+			skp = &smack_known_huh;
+	}
 
 #ifdef CONFIG_AUDIT
 	smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 3/3] Smack: Use the netlabel cache
  2020-08-12  0:39 ` [PATCH 0/3] Smack: Use the netlbl incoming cache Casey Schaufler
  2020-08-12  0:39   ` [PATCH 1/3] Smack: Consolidate uses of secmark into a function Casey Schaufler
  2020-08-12  0:39   ` [PATCH 2/3] Smack: Set socket labels only once Casey Schaufler
@ 2020-08-12  0:39   ` Casey Schaufler
  2020-08-12  2:10   ` [PATCH 0/3] Smack: Use the netlbl incoming cache Paul Moore
  3 siblings, 0 replies; 8+ messages in thread
From: Casey Schaufler @ 2020-08-12  0:39 UTC (permalink / raw)
  To: SMACK-discuss, casey.schaufler, linux-security-module; +Cc: paul, casey

Utilize the Netlabel cache mechanism for incoming packet matching.
Refactor the initialization of secattr structures, as it was being
done in two places.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack.h        |  1 +
 security/smack/smack_access.c | 55 +++++++++++++++++++++++------------
 security/smack/smack_lsm.c    | 27 +++++++++++++----
 security/smack/smackfs.c      | 23 ++++++---------
 4 files changed, 68 insertions(+), 38 deletions(-)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index c5d745a3ada8..a9768b12716b 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -297,6 +297,7 @@ struct smack_known *smk_find_entry(const char *);
 bool smack_privileged(int cap);
 bool smack_privileged_cred(int cap, const struct cred *cred);
 void smk_destroy_label_list(struct list_head *list);
+int smack_populate_secattr(struct smack_known *skp);
 
 /*
  * Shared data.
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 38ac3da4e791..efe2406a3960 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -510,6 +510,42 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap,
 	return 0;
 }
 
+/**
+ * smack_populate_secattr - fill in the smack_known netlabel information
+ * @skp: pointer to the structure to fill
+ *
+ * Populate the netlabel secattr structure for a Smack label.
+ *
+ * Returns 0 unless creating the category mapping fails
+ */
+int smack_populate_secattr(struct smack_known *skp)
+{
+	int slen;
+
+	skp->smk_netlabel.attr.secid = skp->smk_secid;
+	skp->smk_netlabel.domain = skp->smk_known;
+	skp->smk_netlabel.cache = netlbl_secattr_cache_alloc(GFP_ATOMIC);
+	if (skp->smk_netlabel.cache != NULL) {
+		skp->smk_netlabel.flags |= NETLBL_SECATTR_CACHE;
+		skp->smk_netlabel.cache->free = NULL;
+		skp->smk_netlabel.cache->data = skp;
+	}
+	skp->smk_netlabel.flags |= NETLBL_SECATTR_SECID |
+				   NETLBL_SECATTR_MLS_LVL |
+				   NETLBL_SECATTR_DOMAIN;
+	/*
+	 * If direct labeling works use it.
+	 * Otherwise use mapped labeling.
+	 */
+	slen = strlen(skp->smk_known);
+	if (slen < SMK_CIPSOLEN)
+		return smk_netlbl_mls(smack_cipso_direct, skp->smk_known,
+				      &skp->smk_netlabel, slen);
+
+	return smk_netlbl_mls(smack_cipso_mapped, (char *)&skp->smk_secid,
+			      &skp->smk_netlabel, sizeof(skp->smk_secid));
+}
+
 /**
  * smk_import_entry - import a label, return the list entry
  * @string: a text string that might be a Smack label
@@ -523,7 +559,6 @@ struct smack_known *smk_import_entry(const char *string, int len)
 {
 	struct smack_known *skp;
 	char *smack;
-	int slen;
 	int rc;
 
 	smack = smk_parse_smack(string, len);
@@ -544,21 +579,8 @@ struct smack_known *smk_import_entry(const char *string, int len)
 
 	skp->smk_known = smack;
 	skp->smk_secid = smack_next_secid++;
-	skp->smk_netlabel.domain = skp->smk_known;
-	skp->smk_netlabel.flags =
-		NETLBL_SECATTR_DOMAIN | NETLBL_SECATTR_MLS_LVL;
-	/*
-	 * If direct labeling works use it.
-	 * Otherwise use mapped labeling.
-	 */
-	slen = strlen(smack);
-	if (slen < SMK_CIPSOLEN)
-		rc = smk_netlbl_mls(smack_cipso_direct, skp->smk_known,
-			       &skp->smk_netlabel, slen);
-	else
-		rc = smk_netlbl_mls(smack_cipso_mapped, (char *)&skp->smk_secid,
-			       &skp->smk_netlabel, sizeof(skp->smk_secid));
 
+	rc = smack_populate_secattr(skp);
 	if (rc >= 0) {
 		INIT_LIST_HEAD(&skp->smk_rules);
 		mutex_init(&skp->smk_rules_lock);
@@ -569,9 +591,6 @@ struct smack_known *smk_import_entry(const char *string, int len)
 		smk_insert_entry(skp);
 		goto unlockout;
 	}
-	/*
-	 * smk_netlbl_mls failed.
-	 */
 	kfree(skp);
 	skp = ERR_PTR(rc);
 freeout:
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 7a79ddb39e94..86db667ce319 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3715,6 +3715,18 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap,
 	int acat;
 	int kcat;
 
+	/*
+	 * Netlabel found it in the cache.
+	 */
+	if ((sap->flags & NETLBL_SECATTR_CACHE) != 0)
+		return (struct smack_known *)sap->cache->data;
+
+	if ((sap->flags & NETLBL_SECATTR_SECID) != 0)
+		/*
+		 * Looks like a fallback, which gives us a secid.
+		 */
+		return smack_from_secid(sap->attr.secid);
+
 	if ((sap->flags & NETLBL_SECATTR_MLS_LVL) != 0) {
 		/*
 		 * Looks like a CIPSO packet.
@@ -3762,11 +3774,6 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap,
 			return &smack_known_web;
 		return &smack_known_star;
 	}
-	if ((sap->flags & NETLBL_SECATTR_SECID) != 0)
-		/*
-		 * Looks like a fallback, which gives us a secid.
-		 */
-		return smack_from_secid(sap->attr.secid);
 	/*
 	 * Without guidance regarding the smack value
 	 * for the packet fall back on the network
@@ -3845,6 +3852,9 @@ static struct smack_known *smack_from_skb(struct sk_buff *skb)
  * @family: address family
  * @skb: packet
  *
+ * Find the Smack label in the IP options. If it hasn't been
+ * added to the netlabel cache, add it here.
+ *
  * Returns smack_known of the IP options or NULL if that won't work.
  */
 static struct smack_known *smack_from_netlbl(struct sock *sk, u16 family,
@@ -3853,13 +3863,18 @@ static struct smack_known *smack_from_netlbl(struct sock *sk, u16 family,
 	struct netlbl_lsm_secattr secattr;
 	struct socket_smack *ssp = NULL;
 	struct smack_known *skp = NULL;
+	int rc = 0;
 
 	netlbl_secattr_init(&secattr);
 
 	if (sk)
 		ssp = sk->sk_security;
-	if (netlbl_skbuff_getattr(skb, family, &secattr) == 0)
+
+	if (netlbl_skbuff_getattr(skb, family, &secattr) == 0) {
 		skp = smack_from_secattr(&secattr, ssp);
+		if (secattr.flags & NETLBL_SECATTR_CACHEABLE)
+			rc = netlbl_cache_add(skb, family, &skp->smk_netlabel);
+	}
 
 	netlbl_secattr_destroy(&secattr);
 
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 9c4308077574..e567b4baf3a0 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -922,6 +922,10 @@ static ssize_t smk_set_cipso(struct file *file, const char __user *buf,
 		skp->smk_netlabel.attr.mls.cat = ncats.attr.mls.cat;
 		skp->smk_netlabel.attr.mls.lvl = ncats.attr.mls.lvl;
 		rc = count;
+		/*
+		 * This mapping may have been cached, so clear the cache.
+		 */
+		netlbl_cache_invalidate();
 	}
 
 out:
@@ -2950,15 +2954,6 @@ static struct file_system_type smk_fs_type = {
 
 static struct vfsmount *smackfs_mount;
 
-static int __init smk_preset_netlabel(struct smack_known *skp)
-{
-	skp->smk_netlabel.domain = skp->smk_known;
-	skp->smk_netlabel.flags =
-		NETLBL_SECATTR_DOMAIN | NETLBL_SECATTR_MLS_LVL;
-	return smk_netlbl_mls(smack_cipso_direct, skp->smk_known,
-				&skp->smk_netlabel, strlen(skp->smk_known));
-}
-
 /**
  * init_smk_fs - get the smackfs superblock
  *
@@ -2997,19 +2992,19 @@ static int __init init_smk_fs(void)
 	smk_cipso_doi();
 	smk_unlbl_ambient(NULL);
 
-	rc = smk_preset_netlabel(&smack_known_floor);
+	rc = smack_populate_secattr(&smack_known_floor);
 	if (err == 0 && rc < 0)
 		err = rc;
-	rc = smk_preset_netlabel(&smack_known_hat);
+	rc = smack_populate_secattr(&smack_known_hat);
 	if (err == 0 && rc < 0)
 		err = rc;
-	rc = smk_preset_netlabel(&smack_known_huh);
+	rc = smack_populate_secattr(&smack_known_huh);
 	if (err == 0 && rc < 0)
 		err = rc;
-	rc = smk_preset_netlabel(&smack_known_star);
+	rc = smack_populate_secattr(&smack_known_star);
 	if (err == 0 && rc < 0)
 		err = rc;
-	rc = smk_preset_netlabel(&smack_known_web);
+	rc = smack_populate_secattr(&smack_known_web);
 	if (err == 0 && rc < 0)
 		err = rc;
 
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] Smack: Use the netlbl incoming cache
  2020-08-12  0:39 ` [PATCH 0/3] Smack: Use the netlbl incoming cache Casey Schaufler
                     ` (2 preceding siblings ...)
  2020-08-12  0:39   ` [PATCH 3/3] Smack: Use the netlabel cache Casey Schaufler
@ 2020-08-12  2:10   ` Paul Moore
  2020-08-13 16:36     ` Casey Schaufler
  3 siblings, 1 reply; 8+ messages in thread
From: Paul Moore @ 2020-08-12  2:10 UTC (permalink / raw)
  To: Casey Schaufler; +Cc: SMACK-discuss, casey.schaufler, linux-security-module

On Tue, Aug 11, 2020 at 8:39 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Update the Smack security module to use the Netlabel cache
> mechanism to speed the processing of incoming labeled packets.
> There is some refactoring of the existing code that makes it
> simpler, and reduces duplication. The outbound packet labeling
> is also optimized to track the labeling state of the socket.
> Prior to this the socket label was redundantly set on each
> packet send.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  security/smack/smack.h        |  19 ++--
>  security/smack/smack_access.c |  55 ++++++----
>  security/smack/smack_lsm.c    | 245 ++++++++++++++++++++++++------------------
>  security/smack/smackfs.c      |  23 ++--
>  4 files changed, 193 insertions(+), 149 deletions(-)

FWIW, I gave this a cursory look just now and the NetLabel usage
seemed reasonable.  Out of curiosity, have you done any before/after
performance tests?  It was quite significant when we adopted it in
SELinux, but that was some time ago, it would be nice to know that it
is still working well and hasn't been invalidated by some other,
unrelated change.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] Smack: Use the netlbl incoming cache
  2020-08-12  2:10   ` [PATCH 0/3] Smack: Use the netlbl incoming cache Paul Moore
@ 2020-08-13 16:36     ` Casey Schaufler
  2020-08-14  0:32       ` Casey Schaufler
  0 siblings, 1 reply; 8+ messages in thread
From: Casey Schaufler @ 2020-08-13 16:36 UTC (permalink / raw)
  To: Paul Moore; +Cc: SMACK-discuss, Casey Schaufler, linux-security-module

On 8/11/2020 7:10 PM, Paul Moore wrote:
> On Tue, Aug 11, 2020 at 8:39 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Update the Smack security module to use the Netlabel cache
>> mechanism to speed the processing of incoming labeled packets.
>> There is some refactoring of the existing code that makes it
>> simpler, and reduces duplication. The outbound packet labeling
>> is also optimized to track the labeling state of the socket.
>> Prior to this the socket label was redundantly set on each
>> packet send.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  security/smack/smack.h        |  19 ++--
>>  security/smack/smack_access.c |  55 ++++++----
>>  security/smack/smack_lsm.c    | 245 ++++++++++++++++++++++++------------------
>>  security/smack/smackfs.c      |  23 ++--
>>  4 files changed, 193 insertions(+), 149 deletions(-)
> FWIW, I gave this a cursory look just now and the NetLabel usage
> seemed reasonable.  Out of curiosity, have you done any before/after
> performance tests?

It's early in the benchmark process, but TCP looks to be about 6% better.
UDP numbers should be better. I'm not expecting the level of improvement
SELinux saw because the label mapping from CIPSO isn't as sophisticated
for Smack as it is for SELinux.

>   It was quite significant when we adopted it in
> SELinux, but that was some time ago, it would be nice to know that it
> is still working well and hasn't been invalidated by some other,
> unrelated change.
>

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] Smack: Use the netlbl incoming cache
  2020-08-13 16:36     ` Casey Schaufler
@ 2020-08-14  0:32       ` Casey Schaufler
  2020-08-14  2:03         ` Paul Moore
  0 siblings, 1 reply; 8+ messages in thread
From: Casey Schaufler @ 2020-08-14  0:32 UTC (permalink / raw)
  To: Paul Moore; +Cc: SMACK-discuss, linux-security-module, Casey Schaufler

On 8/13/2020 9:36 AM, Casey Schaufler wrote:
> On 8/11/2020 7:10 PM, Paul Moore wrote:
>> On Tue, Aug 11, 2020 at 8:39 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> Update the Smack security module to use the Netlabel cache
>>> mechanism to speed the processing of incoming labeled packets.
>>> There is some refactoring of the existing code that makes it
>>> simpler, and reduces duplication. The outbound packet labeling
>>> is also optimized to track the labeling state of the socket.
>>> Prior to this the socket label was redundantly set on each
>>> packet send.
>>>
>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>> ---
>>>  security/smack/smack.h        |  19 ++--
>>>  security/smack/smack_access.c |  55 ++++++----
>>>  security/smack/smack_lsm.c    | 245 ++++++++++++++++++++++++------------------
>>>  security/smack/smackfs.c      |  23 ++--
>>>  4 files changed, 193 insertions(+), 149 deletions(-)
>> FWIW, I gave this a cursory look just now and the NetLabel usage
>> seemed reasonable.  Out of curiosity, have you done any before/after
>> performance tests?
> It's early in the benchmark process, but TCP looks to be about 6% better.
> UDP numbers should be better. I'm not expecting the level of improvement
> SELinux saw because the label mapping from CIPSO isn't as sophisticated
> for Smack as it is for SELinux.

UDP looks like a 12% improvement, which I had expected.
On the whole, worth the effort.


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] Smack: Use the netlbl incoming cache
  2020-08-14  0:32       ` Casey Schaufler
@ 2020-08-14  2:03         ` Paul Moore
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Moore @ 2020-08-14  2:03 UTC (permalink / raw)
  To: Casey Schaufler; +Cc: SMACK-discuss, linux-security-module

On August 13, 2020 8:32:16 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 8/13/2020 9:36 AM, Casey Schaufler wrote:
>> On 8/11/2020 7:10 PM, Paul Moore wrote:
>>> On Tue, Aug 11, 2020 at 8:39 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>> Update the Smack security module to use the Netlabel cache
>>>> mechanism to speed the processing of incoming labeled packets.
>>>> There is some refactoring of the existing code that makes it
>>>> simpler, and reduces duplication. The outbound packet labeling
>>>> is also optimized to track the labeling state of the socket.
>>>> Prior to this the socket label was redundantly set on each
>>>> packet send.
>>>>
>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>> ---
>>>> security/smack/smack.h        |  19 ++--
>>>> security/smack/smack_access.c |  55 ++++++----
>>>> security/smack/smack_lsm.c    | 245 ++++++++++++++++++++++++------------------
>>>> security/smack/smackfs.c      |  23 ++--
>>>> 4 files changed, 193 insertions(+), 149 deletions(-)
>>> FWIW, I gave this a cursory look just now and the NetLabel usage
>>> seemed reasonable.  Out of curiosity, have you done any before/after
>>> performance tests?
>> It's early in the benchmark process, but TCP looks to be about 6% better.
>> UDP numbers should be better. I'm not expecting the level of improvement
>> SELinux saw because the label mapping from CIPSO isn't as sophisticated
>> for Smack as it is for SELinux.
>
> UDP looks like a 12% improvement, which I had expected.
> On the whole, worth the effort.

Great, thanks for the follow-up.

--
paul moore
www.paul-moore.com




^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-08-14  2:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200812003943.3036-1-casey.ref@schaufler-ca.com>
2020-08-12  0:39 ` [PATCH 0/3] Smack: Use the netlbl incoming cache Casey Schaufler
2020-08-12  0:39   ` [PATCH 1/3] Smack: Consolidate uses of secmark into a function Casey Schaufler
2020-08-12  0:39   ` [PATCH 2/3] Smack: Set socket labels only once Casey Schaufler
2020-08-12  0:39   ` [PATCH 3/3] Smack: Use the netlabel cache Casey Schaufler
2020-08-12  2:10   ` [PATCH 0/3] Smack: Use the netlbl incoming cache Paul Moore
2020-08-13 16:36     ` Casey Schaufler
2020-08-14  0:32       ` Casey Schaufler
2020-08-14  2:03         ` Paul Moore

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