Labeled networking patches for 3.11

PATCH In review Paul Moore 3 participants 12 messages Jun 25, 2013 – Jun 26, 2013

Subsystems (2)

SELinux security module Security (LSM)

lore

Share & tools

lore

Paul Moore

author

2013-06-25 21:18 UTC #0 lore raw

cover Labeled networking patches for 3.11

Nothing new here, all of these patches have been posted before. I'm posting these patches again for two reasons:

1. Remind Eric he still hasn't merged them into the SELinux tree
2. Send notice that I've pushed the patches to my -next tree so they should be in the next spin of linux-next

I was hoping that there patches would have hit linux-next by now via the SELinux tree but that hasn't happened so I'm going to do it via my labeled networking tree (all the patches are labeled networking related anyway).

For those who like their patches via git, here is the lblnet-next tree:

* git://git.infradead.org/users/pcmoore/lblnet-2.6_next

Enjoy,
-Paul

---

Paul Moore (9):
selinux: fix problems in netnode when BUG() is compiled out
lsm: split the xfrm_state_alloc_security() hook implementation
selinux: cleanup and consolidate the XFRM alloc/clone/delete/free code
selinux: cleanup selinux_xfrm_policy_lookup() and selinux_xfrm_state_pol_flow_match()
selinux: cleanup selinux_xfrm_sock_rcv_skb() and selinux_xfrm_postroute_last()
selinux: cleanup some comment and whitespace issues in the XFRM code
selinux: cleanup selinux_xfrm_decode_session()
selinux: cleanup the XFRM header
selinux: remove the BUG_ON() from selinux_skb_xfrm_sid()

 include/linux/security.h        |   26 ++
 security/capability.c           |   15 +
 security/security.c             |   13 -
 security/selinux/hooks.c        |   11 +
 security/selinux/include/xfrm.h |   45 ++--
 security/selinux/netnode.c      |    2 
 security/selinux/xfrm.c         |  453 +++++++++++++++++----------------------
 7 files changed, 264 insertions(+), 301 deletions(-)

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Paul Moore

author

2013-06-25 21:18 UTC #1 lore raw

PATCH 1/9 selinux: fix problems in netnode when BUG() is compiled out

When the BUG() macro is disabled at compile time it can cause some problems in the SELinux netnode code: invalid return codes and uninitialized variables. This patch fixes this by making sure we take some corrective action after the BUG() macro.

Reported-by Geert Uytterhoeven <[email protected]>

Signed-off-by Paul Moore <[email protected]>

---

In the process of cleaning up a local git tree I realized I had this patch still sitting in my tree that for some reason hadn't been merged upstream. Originally sent back on December 5, 2012.

---
 security/selinux/netnode.c |    2 ++
 1 file changed, 2 insertions(+)

security/selinux/netnode.c +2 -0

index c5454c0..03a72c3 100644
@@ -166,6 +166,7 @@ static void sel_netnode_insert(struct sel_netnode *node)
 		break;
 	default:
 		BUG();
+		return;
 	}
 
 	/* we need to impose a limit on the growth of the hash table so check

@@ -225,6 +226,7 @@ static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
 		break;
 	default:
 		BUG();
+		ret = -EINVAL;
 	}
 	if (ret != 0)
 		goto out;

Paul Moore

author

2013-06-25 21:18 UTC #2 lore raw

PATCH 2/9 lsm: split the xfrm_state_alloc_security() hook implementation

In reply to Paul Moore

The xfrm_state_alloc_security() LSM hook implementation is really a multiplexed hook with two different behaviors depending on the arguments passed to it by the caller. This patch splits the LSM hook implementation into two new hook implementations, which match the LSM hooks in the rest of the kernel:

* xfrm_state_alloc
* xfrm_state_alloc_acquire

Also included in this patch are the necessary changes to the SELinux code; no other LSMs are affected.

Signed-off-by Paul Moore <[email protected]>

---
 include/linux/security.h        |   26 ++++-
 security/capability.c           |   15 ++-
 security/security.c             |   13 +--
 security/selinux/hooks.c        |    3 -
 security/selinux/include/xfrm.h |    4 +
 security/selinux/xfrm.c         |  199 ++++++++++++++++++---------------------
 6 files changed, 128 insertions(+), 132 deletions(-)

include/linux/security.h +18 -8

index 4686491..e5a5e8a 100644
@@ -1039,17 +1039,25 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  * @xfrm_policy_delete_security:
  *	@ctx contains the xfrm_sec_ctx.
  *	Authorize deletion of xp->security.
- * @xfrm_state_alloc_security:
+ * @xfrm_state_alloc:
  *	@x contains the xfrm_state being added to the Security Association
  *	Database by the XFRM system.
  *	@sec_ctx contains the security context information being provided by
  *	the user-level SA generation program (e.g., setkey or racoon).
- *	@secid contains the secid from which to take the mls portion of the context.
  *	Allocate a security structure to the x->security field; the security
  *	field is initialized to NULL when the xfrm_state is allocated. Set the
- *	context to correspond to either sec_ctx or polsec, with the mls portion
- *	taken from secid in the latter case.
- *	Return 0 if operation was successful (memory to allocate, legal context).
+ *	context to correspond to sec_ctx. Return 0 if operation was successful
+ *	(memory to allocate, legal context).
+ * @xfrm_state_alloc_acquire:
+ *	@x contains the xfrm_state being added to the Security Association
+ *	Database by the XFRM system.
+ *	@polsec contains the policy's security context.
+ *	@secid contains the secid from which to take the mls portion of the
+ *	context.
+ *	Allocate a security structure to the x->security field; the security
+ *	field is initialized to NULL when the xfrm_state is allocated. Set the
+ *	context to correspond to secid. Return 0 if operation was successful
+ *	(memory to allocate, legal context).
  * @xfrm_state_free_security:
  *	@x contains the xfrm_state.
  *	Deallocate x->security.

@@ -1651,9 +1659,11 @@ struct security_operations {
 	int (*xfrm_policy_clone_security) (struct xfrm_sec_ctx *old_ctx, struct xfrm_sec_ctx **new_ctx);
 	void (*xfrm_policy_free_security) (struct xfrm_sec_ctx *ctx);
 	int (*xfrm_policy_delete_security) (struct xfrm_sec_ctx *ctx);
-	int (*xfrm_state_alloc_security) (struct xfrm_state *x,
-		struct xfrm_user_sec_ctx *sec_ctx,
-		u32 secid);
+	int (*xfrm_state_alloc) (struct xfrm_state *x,
+				 struct xfrm_user_sec_ctx *sec_ctx);
+	int (*xfrm_state_alloc_acquire) (struct xfrm_state *x,
+					 struct xfrm_sec_ctx *polsec,
+					 u32 secid);
 	void (*xfrm_state_free_security) (struct xfrm_state *x);
 	int (*xfrm_state_delete_security) (struct xfrm_state *x);
 	int (*xfrm_policy_lookup) (struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir);

security/capability.c +11 -4

index 1728d4e..67afc67 100644
@@ -767,9 +767,15 @@ static int cap_xfrm_policy_delete_security(struct xfrm_sec_ctx *ctx)
 	return 0;
 }
 
-static int cap_xfrm_state_alloc_security(struct xfrm_state *x,
-					 struct xfrm_user_sec_ctx *sec_ctx,
-					 u32 secid)
+static int cap_xfrm_state_alloc(struct xfrm_state *x,
+				struct xfrm_user_sec_ctx *sec_ctx)
+{
+	return 0;
+}
+
+static int cap_xfrm_state_alloc_acquire(struct xfrm_state *x,
+					struct xfrm_sec_ctx *polsec,
+					u32 secid)
 {
 	return 0;
 }

@@ -1084,7 +1090,8 @@ void __init security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, xfrm_policy_clone_security);
 	set_to_cap_if_null(ops, xfrm_policy_free_security);
 	set_to_cap_if_null(ops, xfrm_policy_delete_security);
-	set_to_cap_if_null(ops, xfrm_state_alloc_security);
+	set_to_cap_if_null(ops, xfrm_state_alloc);
+	set_to_cap_if_null(ops, xfrm_state_alloc_acquire);
 	set_to_cap_if_null(ops, xfrm_state_free_security);
 	set_to_cap_if_null(ops, xfrm_state_delete_security);
 	set_to_cap_if_null(ops, xfrm_policy_lookup);

security/security.c +4 -9

index a3dce87..57e25c9 100644
@@ -1322,22 +1322,17 @@ int security_xfrm_policy_delete(struct xfrm_sec_ctx *ctx)
 	return security_ops->xfrm_policy_delete_security(ctx);
 }
 
-int security_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx)
+int security_xfrm_state_alloc(struct xfrm_state *x,
+			      struct xfrm_user_sec_ctx *sec_ctx)
 {
-	return security_ops->xfrm_state_alloc_security(x, sec_ctx, 0);
+	return security_ops->xfrm_state_alloc(x, sec_ctx);
 }
 EXPORT_SYMBOL(security_xfrm_state_alloc);
 
 int security_xfrm_state_alloc_acquire(struct xfrm_state *x,
 				      struct xfrm_sec_ctx *polsec, u32 secid)
 {
-	if (!polsec)
-		return 0;
-	/*
-	 * We want the context to be taken from secid which is usually
-	 * from the sock.
-	 */
-	return security_ops->xfrm_state_alloc_security(x, NULL, secid);
+	return security_ops->xfrm_state_alloc_acquire(x, polsec, secid);
 }
 
 int security_xfrm_state_delete(struct xfrm_state *x)

security/selinux/hooks.c +2 -1

index 5c6f2cd..d3555bd 100644
@@ -5708,7 +5708,8 @@ static struct security_operations selinux_ops = {
 	.xfrm_policy_clone_security =	selinux_xfrm_policy_clone,
 	.xfrm_policy_free_security =	selinux_xfrm_policy_free,
 	.xfrm_policy_delete_security =	selinux_xfrm_policy_delete,
-	.xfrm_state_alloc_security =	selinux_xfrm_state_alloc,
+	.xfrm_state_alloc =		selinux_xfrm_state_alloc,
+	.xfrm_state_alloc_acquire =	selinux_xfrm_state_alloc_acquire,
 	.xfrm_state_free_security =	selinux_xfrm_state_free,
 	.xfrm_state_delete_security =	selinux_xfrm_state_delete,
 	.xfrm_policy_lookup =		selinux_xfrm_policy_lookup,

security/selinux/include/xfrm.h +3 -1

index 65f67cb..f2a2314 100644
@@ -16,7 +16,9 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx,
 void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx);
 int selinux_xfrm_policy_delete(struct xfrm_sec_ctx *ctx);
 int selinux_xfrm_state_alloc(struct xfrm_state *x,
-	struct xfrm_user_sec_ctx *sec_ctx, u32 secid);
+			     struct xfrm_user_sec_ctx *uctx);
+int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x,
+				     struct xfrm_sec_ctx *polsec, u32 secid);
 void selinux_xfrm_state_free(struct xfrm_state *x);
 int selinux_xfrm_state_delete(struct xfrm_state *x);
 int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir);

security/selinux/xfrm.c +90 ~~-109~~

227 lines not on this page · raw message

Paul Moore

author

2013-06-25 21:18 UTC #3 lore raw

PATCH 3/9 selinux: cleanup and consolidate the XFRM alloc/clone/delete/free code

In reply to Paul Moore

The SELinux labeled IPsec code state management functions have been long neglected and could use some cleanup and consolidation.

Signed-off-by Paul Moore <[email protected]>

---
 security/selinux/xfrm.c |   71 ++++++++++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 31 deletions(-)

security/selinux/xfrm.c +40 ~~-31~~

index 07ae0c0..f8d7126 100644
@@ -122,6 +122,33 @@ err:
 }
 
 /*
+ * Free the xfrm_sec_ctx structure.
+ */
+static void selinux_xfrm_free(struct xfrm_sec_ctx *ctx)
+{
+	if (!ctx)
+		return;
+
+	atomic_dec(&selinux_xfrm_refcount);
+	kfree(ctx);
+}
+
+/*
+ * Authorize the deletion of a labeled SA or policy rule.
+ */
+static int selinux_xfrm_delete(struct xfrm_sec_ctx *ctx)
+{
+	const struct task_security_struct *tsec = current_security();
+
+	if (!ctx)
+		return 0;
+
+	return avc_has_perm(tsec->sid, ctx->ctx_sid,
+			    SECCLASS_ASSOCIATION, ASSOCIATION__SETCONTEXT,
+			    NULL);
+}
+
+/*
  * LSM hook implementation that authorizes that a flow can use
  * a xfrm policy rule.
  */

@@ -258,17 +285,16 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx,
 {
 	struct xfrm_sec_ctx *new_ctx;
 
-	if (old_ctx) {
-		new_ctx = kmalloc(sizeof(*old_ctx) + old_ctx->ctx_len,
-				  GFP_ATOMIC);
-		if (!new_ctx)
-			return -ENOMEM;
+	if (!old_ctx)
+		return 0;
+
+	new_ctx = kmalloc(sizeof(*old_ctx) + old_ctx->ctx_len, GFP_ATOMIC);
+	if (!new_ctx)
+		return -ENOMEM;
+	memcpy(new_ctx, old_ctx, sizeof(*old_ctx) + old_ctx->ctx_len);
+	atomic_inc(&selinux_xfrm_refcount);
+	*new_ctxp = new_ctx;
 
-		memcpy(new_ctx, old_ctx, sizeof(*new_ctx));
-		memcpy(new_ctx->ctx_str, old_ctx->ctx_str, new_ctx->ctx_len);
-		atomic_inc(&selinux_xfrm_refcount);
-		*new_ctxp = new_ctx;
-	}
 	return 0;
 }

@@ -277,8 +303,7 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx,
  */
 void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx)
 {
-	atomic_dec(&selinux_xfrm_refcount);
-	kfree(ctx);
+	selinux_xfrm_free(ctx);
 }
 
 /*

@@ -286,14 +311,7 @@ void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx)
  */
 int selinux_xfrm_policy_delete(struct xfrm_sec_ctx *ctx)
 {
-	const struct task_security_struct *tsec = current_security();
-
-	if (!ctx)
-		return 0;
-
-	return avc_has_perm(tsec->sid, ctx->ctx_sid,
-			    SECCLASS_ASSOCIATION, ASSOCIATION__SETCONTEXT,
-			    NULL);
+	return selinux_xfrm_delete(ctx);
 }
 
 /*

@@ -349,8 +367,7 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x,
  */
 void selinux_xfrm_state_free(struct xfrm_state *x)
 {
-	atomic_dec(&selinux_xfrm_refcount);
-	kfree(x->security);
+	selinux_xfrm_free(x->security);
 }
 
  /*

@@ -358,15 +375,7 @@ void selinux_xfrm_state_free(struct xfrm_state *x)
   */
 int selinux_xfrm_state_delete(struct xfrm_state *x)
 {
-	const struct task_security_struct *tsec = current_security();
-	struct xfrm_sec_ctx *ctx = x->security;
-
-	if (!ctx)
-		return 0;
-
-	return avc_has_perm(tsec->sid, ctx->ctx_sid,
-			    SECCLASS_ASSOCIATION, ASSOCIATION__SETCONTEXT,
-			    NULL);
+	return selinux_xfrm_delete(x->security);
 }
 
 /*

Paul Moore

author

2013-06-25 21:18 UTC #4 lore raw

PATCH 4/9 selinux: cleanup selinux_xfrm_policy_lookup() and selinux_xfrm_state_pol_flow_match()

In reply to Paul Moore

Do some basic simplification and comment reformatting.

Signed-off-by Paul Moore <[email protected]>

---
 security/selinux/xfrm.c |   54 ++++++++++++++++-------------------------------
 1 file changed, 18 insertions(+), 36 deletions(-)

security/selinux/xfrm.c +18 ~~-36~~

index f8d7126..4a7ba4a 100644
@@ -155,42 +155,30 @@ static int selinux_xfrm_delete(struct xfrm_sec_ctx *ctx)
 int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir)
 {
 	int rc;
-	u32 sel_sid;
 
-	/* Context sid is either set to label or ANY_ASSOC */
-	if (ctx) {
-		if (!selinux_authorizable_ctx(ctx))
-			return -EINVAL;
-
-		sel_sid = ctx->ctx_sid;
-	} else
-		/*
-		 * All flows should be treated as polmatch'ing an
-		 * otherwise applicable "non-labeled" policy. This
-		 * would prevent inadvertent "leaks".
-		 */
+	/* All flows should be treated as polmatch'ing an otherwise applicable
+	 * "non-labeled" policy. This would prevent inadvertent "leaks". */
+	if (!ctx)
 		return 0;
 
-	rc = avc_has_perm(fl_secid, sel_sid, SECCLASS_ASSOCIATION,
-			  ASSOCIATION__POLMATCH,
-			  NULL);
-
-	if (rc == -EACCES)
-		return -ESRCH;
+	/* Context sid is either set to label or ANY_ASSOC */
+	if (!selinux_authorizable_ctx(ctx))
+		return -EINVAL;
 
-	return rc;
+	rc = avc_has_perm(fl_secid, ctx->ctx_sid,
+			  SECCLASS_ASSOCIATION, ASSOCIATION__POLMATCH, NULL);
+	return (rc == -EACCES ? -ESRCH : rc);
 }
 
 /*
  * LSM hook implementation that authorizes that a state matches
  * the given policy, flow combo.
  */
-
-int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *xp,
-			const struct flowi *fl)
+int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x,
+				      struct xfrm_policy *xp,
+				      const struct flowi *fl)
 {
 	u32 state_sid;
-	int rc;
 
 	if (!xp->security)
 		if (x->security)

@@ -213,18 +201,12 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *
 	if (fl->flowi_secid != state_sid)
 		return 0;
 
-	rc = avc_has_perm(fl->flowi_secid, state_sid, SECCLASS_ASSOCIATION,
-			  ASSOCIATION__SENDTO,
-			  NULL)? 0:1;
-
-	/*
-	 * We don't need a separate SA Vs. policy polmatch check
-	 * since the SA is now of the same label as the flow and
-	 * a flow Vs. policy polmatch check had already happened
-	 * in selinux_xfrm_policy_lookup() above.
-	 */
-
-	return rc;
+	/* We don't need a separate SA Vs. policy polmatch check since the SA
+	 * is now of the same label as the flow and a flow Vs. policy polmatch
+	 * check had already happened in selinux_xfrm_policy_lookup() above. */
+	return (avc_has_perm(fl->flowi_secid, state_sid,
+			    SECCLASS_ASSOCIATION, ASSOCIATION__SENDTO,
+			    NULL) ? 0 : 1);
 }
 
 /*

Paul Moore

author

2013-06-25 21:18 UTC #5 lore raw

PATCH 5/9 selinux: cleanup selinux_xfrm_sock_rcv_skb() and selinux_xfrm_postroute_last()

In reply to Paul Moore

Some basic simplification and comment reformatting.

Signed-off-by Paul Moore <[email protected]>

---
 security/selinux/include/xfrm.h |   17 ++++----
 security/selinux/xfrm.c         |   85 +++++++++++++++------------------------
 2 files changed, 42 insertions(+), 60 deletions(-)

security/selinux/include/xfrm.h +9 -8

index f2a2314..b463f210 100644
@@ -44,10 +44,10 @@ static inline int selinux_xfrm_enabled(void)
 	return (atomic_read(&selinux_xfrm_refcount) > 0);
 }
 
-int selinux_xfrm_sock_rcv_skb(u32 sid, struct sk_buff *skb,
-			struct common_audit_data *ad);
-int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb,
-			struct common_audit_data *ad, u8 proto);
+int selinux_xfrm_sock_rcv_skb(u32 sk_sid, struct sk_buff *skb,
+			      struct common_audit_data *ad);
+int selinux_xfrm_postroute_last(u32 sk_sid, struct sk_buff *skb,
+				struct common_audit_data *ad, u8 proto);
 int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall);
 
 static inline void selinux_xfrm_notify_policyload(void)

@@ -61,14 +61,15 @@ static inline int selinux_xfrm_enabled(void)
 	return 0;
 }
 
-static inline int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb,
-			struct common_audit_data *ad)
+static inline int selinux_xfrm_sock_rcv_skb(u32 sk_sid, struct sk_buff *skb,
+					    struct common_audit_data *ad)
 {
 	return 0;
 }
 
-static inline int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb,
-			struct common_audit_data *ad, u8 proto)
+static inline int selinux_xfrm_postroute_last(u32 sk_sid, struct sk_buff *skb,
+					      struct common_audit_data *ad,
+					      u8 proto)
 {
 	return 0;
 }

security/selinux/xfrm.c +33 ~~-52~~

119 lines not on this page · raw message

Paul Moore

author

2013-06-25 21:18 UTC #6 lore raw

PATCH 6/9 selinux: cleanup some comment and whitespace issues in the XFRM code

In reply to Paul Moore

Signed-off-by Paul Moore <[email protected]>

---
 security/selinux/xfrm.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

security/selinux/xfrm.c +10 ~~-13~~

index 1f6c6e6..00801ce 100644
@@ -56,7 +56,7 @@
 atomic_t selinux_xfrm_refcount = ATOMIC_INIT(0);
 
 /*
- * Returns true if an LSM/SELinux context
+ * Returns true if the context is an LSM/SELinux context.
  */
 static inline int selinux_authorizable_ctx(struct xfrm_sec_ctx *ctx)
 {

@@ -66,7 +66,7 @@ static inline int selinux_authorizable_ctx(struct xfrm_sec_ctx *ctx)
 }
 
 /*
- * Returns true if the xfrm contains a security blob for SELinux
+ * Returns true if the xfrm contains a security blob for SELinux.
  */
 static inline int selinux_authorizable_xfrm(struct xfrm_state *x)
 {

@@ -149,8 +149,8 @@ static int selinux_xfrm_delete(struct xfrm_sec_ctx *ctx)
 }
 
 /*
- * LSM hook implementation that authorizes that a flow can use
- * a xfrm policy rule.
+ * LSM hook implementation that authorizes that a flow can use a xfrm policy
+ * rule.
  */
 int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir)
 {

@@ -213,7 +213,6 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x,
  * LSM hook implementation that checks and/or returns the xfrm sid for the
  * incoming packet.
  */
-
 int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
 {
 	struct sec_path *sp;

@@ -248,8 +247,7 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
 }
 
 /*
- * LSM hook implementation that allocs and transfers uctx spec to
- * xfrm_policy.
+ * LSM hook implementation that allocs and transfers uctx spec to xfrm_policy.
  */
 int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp,
 			      struct xfrm_user_sec_ctx *uctx)

@@ -257,10 +255,9 @@ int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp,
 	return selinux_xfrm_alloc_user(ctxp, uctx);
 }
 
-
 /*
- * LSM hook implementation that copies security data structure from old to
- * new for policy cloning.
+ * LSM hook implementation that copies security data structure from old to new
+ * for policy cloning.
  */
 int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx,
 			      struct xfrm_sec_ctx **new_ctxp)

@@ -352,9 +349,9 @@ void selinux_xfrm_state_free(struct xfrm_state *x)
 	selinux_xfrm_free(x->security);
 }
 
- /*
-  * LSM hook implementation that authorizes deletion of labeled SAs.
-  */
+/*
+ * LSM hook implementation that authorizes deletion of labeled SAs.
+ */
 int selinux_xfrm_state_delete(struct xfrm_state *x)
 {
 	return selinux_xfrm_delete(x->security);

Paul Moore

author

2013-06-25 21:19 UTC #7 lore raw

PATCH 7/9 selinux: cleanup selinux_xfrm_decode_session()

In reply to Paul Moore

Some basic simplification.

Signed-off-by Paul Moore <[email protected]>

---
 security/selinux/xfrm.c |   23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

security/selinux/xfrm.c +12 ~~-11~~

index 00801ce..425b9f9 100644
@@ -215,34 +215,35 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x,
  */
 int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
 {
+	u32 sid_session = SECSID_NULL;
 	struct sec_path *sp;
 
-	*sid = SECSID_NULL;
-
 	if (skb == NULL)
-		return 0;
+		goto out;
 
 	sp = skb->sp;
 	if (sp) {
-		int i, sid_set = 0;
+		int i;
 
-		for (i = sp->len-1; i >= 0; i--) {
+		for (i = sp->len - 1; i >= 0; i--) {
 			struct xfrm_state *x = sp->xvec[i];
 			if (selinux_authorizable_xfrm(x)) {
 				struct xfrm_sec_ctx *ctx = x->security;
 
-				if (!sid_set) {
-					*sid = ctx->ctx_sid;
-					sid_set = 1;
-
+				if (sid_session == SECSID_NULL) {
+					sid_session = ctx->ctx_sid;
 					if (!ckall)
-						break;
-				} else if (*sid != ctx->ctx_sid)
+						goto out;
+				} else if (sid_session != ctx->ctx_sid) {
+					*sid = SECSID_NULL;
 					return -EINVAL;
+				}
 			}
 		}
 	}
 
+out:
+	*sid = sid_session;
 	return 0;
 }

Paul Moore

author

2013-06-25 21:19 UTC #8 lore raw

PATCH 8/9 selinux: cleanup the XFRM header

In reply to Paul Moore

Remove the unused get_sock_isec() function and do some formatting fixes.

Signed-off-by Paul Moore <[email protected]>

---
 security/selinux/include/xfrm.h |   19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

security/selinux/include/xfrm.h +5 ~~-14~~

index b463f210..60774bc 100644
@@ -10,7 +10,7 @@
 #include <net/flow.h>
 
 int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp,
-			      struct xfrm_user_sec_ctx *sec_ctx);
+			      struct xfrm_user_sec_ctx *uctx);
 int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx,
 			      struct xfrm_sec_ctx **new_ctxp);
 void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx);

@@ -23,18 +23,8 @@ void selinux_xfrm_state_free(struct xfrm_state *x);
 int selinux_xfrm_state_delete(struct xfrm_state *x);
 int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir);
 int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x,
-			struct xfrm_policy *xp, const struct flowi *fl);
-
-/*
- * Extract the security blob from the sock (it's actually on the socket)
- */
-static inline struct inode_security_struct *get_sock_isec(struct sock *sk)
-{
-	if (!sk->sk_socket)
-		return NULL;
-
-	return SOCK_INODE(sk->sk_socket)->i_security;
-}
+				      struct xfrm_policy *xp,
+				      const struct flowi *fl);
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 extern atomic_t selinux_xfrm_refcount;

@@ -74,7 +64,8 @@ static inline int selinux_xfrm_postroute_last(u32 sk_sid, struct sk_buff *skb,
 	return 0;
 }
 
-static inline int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
+static inline int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid,
+					      int ckall)
 {
 	*sid = SECSID_NULL;
 	return 0;

Paul Moore

author

2013-06-25 21:19 UTC #9 lore raw

PATCH 9/9 selinux: remove the BUG_ON() from selinux_skb_xfrm_sid()

In reply to Paul Moore

Remove the BUG_ON() from selinux_skb_xfrm_sid() and propogate the error code up to the caller. Also check the return values in the only caller function, selinux_skb_peerlbl_sid().

Signed-off-by Paul Moore <[email protected]>

---
 security/selinux/hooks.c        |    8 ++++++--
 security/selinux/include/xfrm.h |    5 ++---
 2 files changed, 8 insertions(+), 5 deletions(-)

security/selinux/hooks.c +6 -2

index d3555bd..4bc068b 100644
@@ -3722,8 +3722,12 @@ static int selinux_skb_peerlbl_sid(struct sk_buff *skb, u16 family, u32 *sid)
 	u32 nlbl_sid;
 	u32 nlbl_type;
 
-	selinux_skb_xfrm_sid(skb, &xfrm_sid);
-	selinux_netlbl_skbuff_getsid(skb, family, &nlbl_type, &nlbl_sid);
+	err = selinux_skb_xfrm_sid(skb, &xfrm_sid);
+	if (unlikely(err))
+		return -EACCES;
+	err = selinux_netlbl_skbuff_getsid(skb, family, &nlbl_type, &nlbl_sid);
+	if (unlikely(err))
+		return -EACCES;
 
 	err = security_net_peersid_resolve(nlbl_sid, nlbl_type, xfrm_sid, sid);
 	if (unlikely(err)) {

security/selinux/include/xfrm.h +2 -3

index 60774bc..7605251 100644
@@ -76,10 +76,9 @@ static inline void selinux_xfrm_notify_policyload(void)
 }
 #endif
 
-static inline void selinux_skb_xfrm_sid(struct sk_buff *skb, u32 *sid)
+static inline int selinux_skb_xfrm_sid(struct sk_buff *skb, u32 *sid)
 {
-	int err = selinux_xfrm_decode_session(skb, sid, 0);
-	BUG_ON(err);
+	return selinux_xfrm_decode_session(skb, sid, 0);
 }
 
 #endif /* _SELINUX_XFRM_H_ */

Casey Schaufler

2013-06-25 23:54 UTC #10 lore raw

In reply to Paul Moore

Quoted 11 lines · Paul Moore

Nothing new here, all of these patches have been posted before. I'm posting these patches again for two reasons:
1. Remind Eric he still hasn't merged them into the SELinux tree
2. Send notice that I've pushed the patches to my -next tree so they should be in the next spin of linux-next
I was hoping that there patches would have hit linux-next by now via the SELinux tree but that hasn't happened so I'm going to do it via my labeled networking tree (all the patches are labeled networking related anyway).

No objection from this end, but I'm curious about the motivation for the changes as they affect the LSM interface.

Quoted 31 lines

For those who like their patches via git, here is the lblnet-next tree:
* git://git.infradead.org/users/pcmoore/lblnet-2.6_next
Enjoy,
-Paul
---
Paul Moore (9):
selinux: fix problems in netnode when BUG() is compiled out
lsm: split the xfrm_state_alloc_security() hook implementation
selinux: cleanup and consolidate the XFRM alloc/clone/delete/free code
selinux: cleanup selinux_xfrm_policy_lookup() and selinux_xfrm_state_pol_flow_match()
selinux: cleanup selinux_xfrm_sock_rcv_skb() and selinux_xfrm_postroute_last()
selinux: cleanup some comment and whitespace issues in the XFRM code
selinux: cleanup selinux_xfrm_decode_session()
selinux: cleanup the XFRM header
selinux: remove the BUG_ON() from selinux_skb_xfrm_sid()
 include/linux/security.h        |   26 ++
 security/capability.c           |   15 +
 security/security.c             |   13 -
 security/selinux/hooks.c        |   11 +
 security/selinux/include/xfrm.h |   45 ++--
 security/selinux/netnode.c      |    2 
 security/selinux/xfrm.c         |  453 +++++++++++++++++----------------------
 7 files changed, 264 insertions(+), 301 deletions(-)

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Paul Moore

maintainer

2013-06-26 13:52 UTC #11 lore raw

Quoted 8 lines · Casey Schaufler

Quoted 7 lines · Paul Moore
Nothing new here, all of these patches have been posted before. I'm
posting these patches again for two reasons:
1. Remind Eric he still hasn't merged them into the SELinux tree
2. Send notice that I've pushed the patches to my -next tree so they should be in the next spin of linux-next

Quoted 7 lines

Quoted 4 lines
I was hoping that there patches would have hit linux-next by now via the SELinux tree but that hasn't happened so I'm going to do it via my labeled networking tree (all the patches are labeled networking related anyway).
No objection from this end, but I'm curious about the motivation for the changes as they affect the LSM interface.

I assume you are talking about patch 2/9?

I guess first things first, the changes don't affect how the rest of the kernel sees the LSM, only how an individual LSM is implemented. If you look at the pre-patch LSM hook implementation for security_xfrm_state_alloc() and security_xfrm_state_alloc_acquire() you notice that they share a common LSM- specific implementation function, xfrm_state_alloc_security(), which takes different arguments depending on the LSM hook. If you look at how SELinux implements this function (SELinux is the only example available that uses this hook) you will notice that there the behavior varies quite a bit depending on the LSM hook caller; in reality, the function is much cleaner and simpler if we split it so that we have one hook implementation for each LSM hook - like pretty much everything else in the LSM.

Signature · 7 lines

paul moore
security and virtualization @ redhat

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.