[PATCH] XFRM: assorted IPsec fixups

[PATCH] XFRM: assorted IPsec fixups

[Paul Moore]

This patch fixes a number of small but potentially troublesome things in the
XFRM/IPsec code:

 * Use the 'audit_enabled' variable already in include/linux/audit.h
   Removed the need for extern declarations local to each XFRM audit fuction

 * Convert 'sid' to 'secid'
   The 'sid' name is specific to SELinux, 'secid' is the common naming
   convention used by the kernel when refering to tokenized LSM labels

 * Convert address display to use standard NIP* macros
   Similar to what was recently done with the SPD audit code, this also
   includes the removal of some unnecessary memcpy() calls

 * Move common code to xfrm_audit_common_stateinfo()
   Code consolidation from the "less is more" book on software development

 * Convert the SPI in audit records to host byte order
   The current SPI values in the audit record are being displayed in
   network byte order, probably not what was intended

 * Proper spacing around commas in function arguments
   Minor style tweak since I was already touching the code

Signed-off-by: Paul Moore <paul.moore@hp.com>
---

 include/linux/xfrm.h    |    2 +
 include/net/xfrm.h      |   18 ++++++------
 net/xfrm/xfrm_policy.c  |   15 +++++-----
 net/xfrm/xfrm_state.c   |   69 +++++++++++++++++++++--------------------------
 security/selinux/xfrm.c |   20 +++++++-------
 5 files changed, 58 insertions(+), 66 deletions(-)

diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index b58adc5..f75a337 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -31,7 +31,7 @@ struct xfrm_sec_ctx {
 	__u8	ctx_doi;
 	__u8	ctx_alg;
 	__u16	ctx_len;
-	__u32	ctx_sid;
+	__u32	ctx_secid;
 	char	ctx_str[0];
 };
 
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 58dfa82..c02e230 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -462,7 +462,7 @@ struct xfrm_audit
 };
 
 #ifdef CONFIG_AUDITSYSCALL
-static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
+static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 secid)
 {
 	struct audit_buffer *audit_buf = NULL;
 	char *secctx;
@@ -475,8 +475,8 @@ static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
 
 	audit_log_format(audit_buf, "auid=%u", auid);
 
-	if (sid != 0 &&
-	    security_secid_to_secctx(sid, &secctx, &secctx_len) == 0) {
+	if (secid != 0 &&
+	    security_secid_to_secctx(secid, &secctx, &secctx_len) == 0) {
 		audit_log_format(audit_buf, " subj=%s", secctx);
 		security_release_secctx(secctx, secctx_len);
 	} else
@@ -485,13 +485,13 @@ static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
 }
 
 extern void xfrm_audit_policy_add(struct xfrm_policy *xp, int result,
-				  u32 auid, u32 sid);
+				  u32 auid, u32 secid);
 extern void xfrm_audit_policy_delete(struct xfrm_policy *xp, int result,
-				  u32 auid, u32 sid);
+				  u32 auid, u32 secid);
 extern void xfrm_audit_state_add(struct xfrm_state *x, int result,
-				 u32 auid, u32 sid);
+				 u32 auid, u32 secid);
 extern void xfrm_audit_state_delete(struct xfrm_state *x, int result,
-				    u32 auid, u32 sid);
+				    u32 auid, u32 secid);
 #else
 #define xfrm_audit_policy_add(x, r, a, s)	do { ; } while (0)
 #define xfrm_audit_policy_delete(x, r, a, s)	do { ; } while (0)
@@ -621,13 +621,13 @@ extern int xfrm_selector_match(struct xfrm_selector *sel, struct flowi *fl,
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 /*	If neither has a context --> match
- * 	Otherwise, both must have a context and the sids, doi, alg must match
+ * 	Otherwise, both must have a context and the secids, doi, alg must match
  */
 static inline int xfrm_sec_ctx_match(struct xfrm_sec_ctx *s1, struct xfrm_sec_ctx *s2)
 {
 	return ((!s1 && !s2) ||
 		(s1 && s2 &&
-		 (s1->ctx_sid == s2->ctx_sid) &&
+		 (s1->ctx_secid == s2->ctx_secid) &&
 		 (s1->ctx_doi == s2->ctx_doi) &&
 		 (s1->ctx_alg == s2->ctx_alg)));
 }
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index b702bd8..75f25c4 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -23,6 +23,7 @@
 #include <linux/netfilter.h>
 #include <linux/module.h>
 #include <linux/cache.h>
+#include <linux/audit.h>
 #include <net/xfrm.h>
 #include <net/ip.h>
 
@@ -2150,15 +2151,14 @@ static inline void xfrm_audit_common_policyinfo(struct xfrm_policy *xp,
 	}
 }
 
-void
-xfrm_audit_policy_add(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
+void xfrm_audit_policy_add(struct xfrm_policy *xp, int result,
+			   u32 auid, u32 secid)
 {
 	struct audit_buffer *audit_buf;
-	extern int audit_enabled;
 
 	if (audit_enabled == 0)
 		return;
-	audit_buf = xfrm_audit_start(sid, auid);
+	audit_buf = xfrm_audit_start(secid, auid);
 	if (audit_buf == NULL)
 		return;
 	audit_log_format(audit_buf, " op=SPD-add res=%u", result);
@@ -2167,15 +2167,14 @@ xfrm_audit_policy_add(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
 }
 EXPORT_SYMBOL_GPL(xfrm_audit_policy_add);
 
-void
-xfrm_audit_policy_delete(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
+void xfrm_audit_policy_delete(struct xfrm_policy *xp, int result,
+			      u32 auid, u32 secid)
 {
 	struct audit_buffer *audit_buf;
-	extern int audit_enabled;
 
 	if (audit_enabled == 0)
 		return;
-	audit_buf = xfrm_audit_start(sid, auid);
+	audit_buf = xfrm_audit_start(secid, auid);
 	if (audit_buf == NULL)
 		return;
 	audit_log_format(audit_buf, " op=SPD-delete res=%u", result);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index cf43c49..b291a82 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -19,6 +19,7 @@
 #include <linux/ipsec.h>
 #include <linux/module.h>
 #include <linux/cache.h>
+#include <linux/audit.h>
 #include <asm/uaccess.h>
 
 #include "xfrm_hash.h"
@@ -1997,67 +1998,59 @@ void __init xfrm_state_init(void)
 static inline void xfrm_audit_common_stateinfo(struct xfrm_state *x,
 					       struct audit_buffer *audit_buf)
 {
-	if (x->security)
-		audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
-				 x->security->ctx_alg, x->security->ctx_doi,
-				 x->security->ctx_str);
+	struct xfrm_sec_ctx *ctx = x->security;
+	u32 spi = ntohl(x->id.spi);
 
-	switch(x->props.family) {
-	case AF_INET:
-		audit_log_format(audit_buf, " src=%u.%u.%u.%u dst=%u.%u.%u.%u",
-				 NIPQUAD(x->props.saddr.a4),
-				 NIPQUAD(x->id.daddr.a4));
-		break;
-	case AF_INET6:
-		{
-			struct in6_addr saddr6, daddr6;
-
-			memcpy(&saddr6, x->props.saddr.a6,
-				sizeof(struct in6_addr));
-			memcpy(&daddr6, x->id.daddr.a6,
-				sizeof(struct in6_addr));
-			audit_log_format(audit_buf,
-					 " src=" NIP6_FMT " dst=" NIP6_FMT,
-					 NIP6(saddr6), NIP6(daddr6));
-		}
-		break;
-	}
+        if (ctx)
+                audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
+                                 ctx->ctx_alg, ctx->ctx_doi, ctx->ctx_str);
+
+        switch(x->props.family) {
+        case AF_INET:
+                audit_log_format(audit_buf,
+				 " src=" NIPQUAD_FMT " dst=" NIPQUAD_FMT,
+                                 NIPQUAD(x->props.saddr.a4),
+                                 NIPQUAD(x->id.daddr.a4));
+                break;
+        case AF_INET6:
+		audit_log_format(audit_buf,
+				 " src=" NIP6_FMT " dst=" NIP6_FMT,
+				 NIP6(*(struct in6_addr *)x->props.saddr.a6),
+				 NIP6(*(struct in6_addr *)x->id.daddr.a6));
+                break;
+        }
+
+	audit_log_format(audit_buf, " spi=%u(0x%x)", spi, spi);
 }
 
-void
-xfrm_audit_state_add(struct xfrm_state *x, int result, u32 auid, u32 sid)
+void xfrm_audit_state_add(struct xfrm_state *x, int result,
+			  u32 auid, u32 secid)
 {
 	struct audit_buffer *audit_buf;
-	extern int audit_enabled;
 
 	if (audit_enabled == 0)
 		return;
-	audit_buf = xfrm_audit_start(sid, auid);
+	audit_buf = xfrm_audit_start(secid, auid);
 	if (audit_buf == NULL)
 		return;
-	audit_log_format(audit_buf, " op=SAD-add res=%u",result);
+	audit_log_format(audit_buf, " op=SAD-add res=%u", result);
 	xfrm_audit_common_stateinfo(x, audit_buf);
-	audit_log_format(audit_buf, " spi=%lu(0x%lx)",
-			 (unsigned long)x->id.spi, (unsigned long)x->id.spi);
 	audit_log_end(audit_buf);
 }
 EXPORT_SYMBOL_GPL(xfrm_audit_state_add);
 
-void
-xfrm_audit_state_delete(struct xfrm_state *x, int result, u32 auid, u32 sid)
+void xfrm_audit_state_delete(struct xfrm_state *x, int result,
+			     u32 auid, u32 secid)
 {
 	struct audit_buffer *audit_buf;
-	extern int audit_enabled;
 
 	if (audit_enabled == 0)
 		return;
-	audit_buf = xfrm_audit_start(sid, auid);
+	audit_buf = xfrm_audit_start(secid, auid);
 	if (audit_buf == NULL)
 		return;
-	audit_log_format(audit_buf, " op=SAD-delete res=%u",result);
+	audit_log_format(audit_buf, " op=SAD-delete res=%u", result);
 	xfrm_audit_common_stateinfo(x, audit_buf);
-	audit_log_format(audit_buf, " spi=%lu(0x%lx)",
-			 (unsigned long)x->id.spi, (unsigned long)x->id.spi);
 	audit_log_end(audit_buf);
 }
 EXPORT_SYMBOL_GPL(xfrm_audit_state_delete);
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index e076039..c925880 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -85,7 +85,7 @@ int selinux_xfrm_policy_lookup(struct xfrm_policy *xp, u32 fl_secid, u8 dir)
 		if (!selinux_authorizable_ctx(ctx))
 			return -EINVAL;
 
-		sel_sid = ctx->ctx_sid;
+		sel_sid = ctx->ctx_secid;
 	}
 	else
 		/*
@@ -132,7 +132,7 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *
 				/* Not a SELinux-labeled SA */
 				return 0;
 
-	state_sid = x->security->ctx_sid;
+	state_sid = x->security->ctx_secid;
 
 	if (fl->secid != state_sid)
 		return 0;
@@ -175,13 +175,13 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
 				struct xfrm_sec_ctx *ctx = x->security;
 
 				if (!sid_set) {
-					*sid = ctx->ctx_sid;
+					*sid = ctx->ctx_secid;
 					sid_set = 1;
 
 					if (!ckall)
 						break;
 				}
-				else if (*sid != ctx->ctx_sid)
+				else if (*sid != ctx->ctx_secid)
 					return -EINVAL;
 			}
 		}
@@ -232,7 +232,7 @@ static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
 	ctx->ctx_str[str_len] = 0;
 	rc = security_context_to_sid(ctx->ctx_str,
 				     str_len,
-				     &ctx->ctx_sid);
+				     &ctx->ctx_secid);
 
 	if (rc)
 		goto out;
@@ -240,7 +240,7 @@ static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
 	/*
 	 * Does the subject have permission to set security context?
 	 */
-	rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
+	rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
 			  SECCLASS_ASSOCIATION,
 			  ASSOCIATION__SETCONTEXT, NULL);
 	if (rc)
@@ -264,7 +264,7 @@ not_from_user:
 
 	ctx->ctx_doi = XFRM_SC_DOI_LSM;
 	ctx->ctx_alg = XFRM_SC_ALG_SELINUX;
-	ctx->ctx_sid = sid;
+	ctx->ctx_secid = sid;
 	ctx->ctx_len = str_len;
 	memcpy(ctx->ctx_str,
 	       ctx_str,
@@ -341,7 +341,7 @@ int selinux_xfrm_policy_delete(struct xfrm_policy *xp)
 	int rc = 0;
 
 	if (ctx)
-		rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
+		rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
 				  SECCLASS_ASSOCIATION,
 				  ASSOCIATION__SETCONTEXT, NULL);
 
@@ -383,7 +383,7 @@ int selinux_xfrm_state_delete(struct xfrm_state *x)
 	int rc = 0;
 
 	if (ctx)
-		rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
+		rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
 				  SECCLASS_ASSOCIATION,
 				  ASSOCIATION__SETCONTEXT, NULL);
 
@@ -412,7 +412,7 @@ int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb,
 
 			if (x && selinux_authorizable_xfrm(x)) {
 				struct xfrm_sec_ctx *ctx = x->security;
-				sel_sid = ctx->ctx_sid;
+				sel_sid = ctx->ctx_secid;
 				break;
 			}
 		}

Re: [PATCH] XFRM: assorted IPsec fixups

[Eric Paris]

On Fri, 2007-12-07 at 12:11 -0500, Paul Moore wrote:
> This patch fixes a number of small but potentially troublesome things in the
> XFRM/IPsec code:
> 
>  * Use the 'audit_enabled' variable already in include/linux/audit.h
>    Removed the need for extern declarations local to each XFRM audit fuction
> 
>  * Convert 'sid' to 'secid'
>    The 'sid' name is specific to SELinux, 'secid' is the common naming
>    convention used by the kernel when refering to tokenized LSM labels
> 
>  * Convert address display to use standard NIP* macros
>    Similar to what was recently done with the SPD audit code, this also
>    includes the removal of some unnecessary memcpy() calls
> 
>  * Move common code to xfrm_audit_common_stateinfo()
>    Code consolidation from the "less is more" book on software development
> 
>  * Convert the SPI in audit records to host byte order
>    The current SPI values in the audit record are being displayed in
>    network byte order, probably not what was intended
> 
>  * Proper spacing around commas in function arguments
>    Minor style tweak since I was already touching the code
> 
> Signed-off-by: Paul Moore <paul.moore@hp.com>

Acked-by: Eric Paris <eparis@redhat.com>

although it does make me wonder why audit_log_start doesn't just check
audit_enabled itself....   Anyway, this patch looks good.

> ---
> 
>  include/linux/xfrm.h    |    2 +
>  include/net/xfrm.h      |   18 ++++++------
>  net/xfrm/xfrm_policy.c  |   15 +++++-----
>  net/xfrm/xfrm_state.c   |   69 +++++++++++++++++++++--------------------------
>  security/selinux/xfrm.c |   20 +++++++-------
>  5 files changed, 58 insertions(+), 66 deletions(-)
> 
> diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
> index b58adc5..f75a337 100644
> --- a/include/linux/xfrm.h
> +++ b/include/linux/xfrm.h
> @@ -31,7 +31,7 @@ struct xfrm_sec_ctx {
>  	__u8	ctx_doi;
>  	__u8	ctx_alg;
>  	__u16	ctx_len;
> -	__u32	ctx_sid;
> +	__u32	ctx_secid;
>  	char	ctx_str[0];
>  };
>  
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index 58dfa82..c02e230 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -462,7 +462,7 @@ struct xfrm_audit
>  };
>  
>  #ifdef CONFIG_AUDITSYSCALL
> -static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
> +static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 secid)
>  {
>  	struct audit_buffer *audit_buf = NULL;
>  	char *secctx;
> @@ -475,8 +475,8 @@ static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
>  
>  	audit_log_format(audit_buf, "auid=%u", auid);
>  
> -	if (sid != 0 &&
> -	    security_secid_to_secctx(sid, &secctx, &secctx_len) == 0) {
> +	if (secid != 0 &&
> +	    security_secid_to_secctx(secid, &secctx, &secctx_len) == 0) {
>  		audit_log_format(audit_buf, " subj=%s", secctx);
>  		security_release_secctx(secctx, secctx_len);
>  	} else
> @@ -485,13 +485,13 @@ static inline struct audit_buffer *xfrm_audit_start(u32 auid, u32 sid)
>  }
>  
>  extern void xfrm_audit_policy_add(struct xfrm_policy *xp, int result,
> -				  u32 auid, u32 sid);
> +				  u32 auid, u32 secid);
>  extern void xfrm_audit_policy_delete(struct xfrm_policy *xp, int result,
> -				  u32 auid, u32 sid);
> +				  u32 auid, u32 secid);
>  extern void xfrm_audit_state_add(struct xfrm_state *x, int result,
> -				 u32 auid, u32 sid);
> +				 u32 auid, u32 secid);
>  extern void xfrm_audit_state_delete(struct xfrm_state *x, int result,
> -				    u32 auid, u32 sid);
> +				    u32 auid, u32 secid);
>  #else
>  #define xfrm_audit_policy_add(x, r, a, s)	do { ; } while (0)
>  #define xfrm_audit_policy_delete(x, r, a, s)	do { ; } while (0)
> @@ -621,13 +621,13 @@ extern int xfrm_selector_match(struct xfrm_selector *sel, struct flowi *fl,
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>  /*	If neither has a context --> match
> - * 	Otherwise, both must have a context and the sids, doi, alg must match
> + * 	Otherwise, both must have a context and the secids, doi, alg must match
>   */
>  static inline int xfrm_sec_ctx_match(struct xfrm_sec_ctx *s1, struct xfrm_sec_ctx *s2)
>  {
>  	return ((!s1 && !s2) ||
>  		(s1 && s2 &&
> -		 (s1->ctx_sid == s2->ctx_sid) &&
> +		 (s1->ctx_secid == s2->ctx_secid) &&
>  		 (s1->ctx_doi == s2->ctx_doi) &&
>  		 (s1->ctx_alg == s2->ctx_alg)));
>  }
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index b702bd8..75f25c4 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -23,6 +23,7 @@
>  #include <linux/netfilter.h>
>  #include <linux/module.h>
>  #include <linux/cache.h>
> +#include <linux/audit.h>
>  #include <net/xfrm.h>
>  #include <net/ip.h>
>  
> @@ -2150,15 +2151,14 @@ static inline void xfrm_audit_common_policyinfo(struct xfrm_policy *xp,
>  	}
>  }
>  
> -void
> -xfrm_audit_policy_add(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
> +void xfrm_audit_policy_add(struct xfrm_policy *xp, int result,
> +			   u32 auid, u32 secid)
>  {
>  	struct audit_buffer *audit_buf;
> -	extern int audit_enabled;
>  
>  	if (audit_enabled == 0)
>  		return;
> -	audit_buf = xfrm_audit_start(sid, auid);
> +	audit_buf = xfrm_audit_start(secid, auid);
>  	if (audit_buf == NULL)
>  		return;
>  	audit_log_format(audit_buf, " op=SPD-add res=%u", result);
> @@ -2167,15 +2167,14 @@ xfrm_audit_policy_add(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
>  }
>  EXPORT_SYMBOL_GPL(xfrm_audit_policy_add);
>  
> -void
> -xfrm_audit_policy_delete(struct xfrm_policy *xp, int result, u32 auid, u32 sid)
> +void xfrm_audit_policy_delete(struct xfrm_policy *xp, int result,
> +			      u32 auid, u32 secid)
>  {
>  	struct audit_buffer *audit_buf;
> -	extern int audit_enabled;
>  
>  	if (audit_enabled == 0)
>  		return;
> -	audit_buf = xfrm_audit_start(sid, auid);
> +	audit_buf = xfrm_audit_start(secid, auid);
>  	if (audit_buf == NULL)
>  		return;
>  	audit_log_format(audit_buf, " op=SPD-delete res=%u", result);
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index cf43c49..b291a82 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -19,6 +19,7 @@
>  #include <linux/ipsec.h>
>  #include <linux/module.h>
>  #include <linux/cache.h>
> +#include <linux/audit.h>
>  #include <asm/uaccess.h>
>  
>  #include "xfrm_hash.h"
> @@ -1997,67 +1998,59 @@ void __init xfrm_state_init(void)
>  static inline void xfrm_audit_common_stateinfo(struct xfrm_state *x,
>  					       struct audit_buffer *audit_buf)
>  {
> -	if (x->security)
> -		audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
> -				 x->security->ctx_alg, x->security->ctx_doi,
> -				 x->security->ctx_str);
> +	struct xfrm_sec_ctx *ctx = x->security;
> +	u32 spi = ntohl(x->id.spi);
>  
> -	switch(x->props.family) {
> -	case AF_INET:
> -		audit_log_format(audit_buf, " src=%u.%u.%u.%u dst=%u.%u.%u.%u",
> -				 NIPQUAD(x->props.saddr.a4),
> -				 NIPQUAD(x->id.daddr.a4));
> -		break;
> -	case AF_INET6:
> -		{
> -			struct in6_addr saddr6, daddr6;
> -
> -			memcpy(&saddr6, x->props.saddr.a6,
> -				sizeof(struct in6_addr));
> -			memcpy(&daddr6, x->id.daddr.a6,
> -				sizeof(struct in6_addr));
> -			audit_log_format(audit_buf,
> -					 " src=" NIP6_FMT " dst=" NIP6_FMT,
> -					 NIP6(saddr6), NIP6(daddr6));
> -		}
> -		break;
> -	}
> +        if (ctx)
> +                audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
> +                                 ctx->ctx_alg, ctx->ctx_doi, ctx->ctx_str);
> +
> +        switch(x->props.family) {
> +        case AF_INET:
> +                audit_log_format(audit_buf,
> +				 " src=" NIPQUAD_FMT " dst=" NIPQUAD_FMT,
> +                                 NIPQUAD(x->props.saddr.a4),
> +                                 NIPQUAD(x->id.daddr.a4));
> +                break;
> +        case AF_INET6:
> +		audit_log_format(audit_buf,
> +				 " src=" NIP6_FMT " dst=" NIP6_FMT,
> +				 NIP6(*(struct in6_addr *)x->props.saddr.a6),
> +				 NIP6(*(struct in6_addr *)x->id.daddr.a6));
> +                break;
> +        }
> +
> +	audit_log_format(audit_buf, " spi=%u(0x%x)", spi, spi);
>  }
>  
> -void
> -xfrm_audit_state_add(struct xfrm_state *x, int result, u32 auid, u32 sid)
> +void xfrm_audit_state_add(struct xfrm_state *x, int result,
> +			  u32 auid, u32 secid)
>  {
>  	struct audit_buffer *audit_buf;
> -	extern int audit_enabled;
>  
>  	if (audit_enabled == 0)
>  		return;
> -	audit_buf = xfrm_audit_start(sid, auid);
> +	audit_buf = xfrm_audit_start(secid, auid);
>  	if (audit_buf == NULL)
>  		return;
> -	audit_log_format(audit_buf, " op=SAD-add res=%u",result);
> +	audit_log_format(audit_buf, " op=SAD-add res=%u", result);
>  	xfrm_audit_common_stateinfo(x, audit_buf);
> -	audit_log_format(audit_buf, " spi=%lu(0x%lx)",
> -			 (unsigned long)x->id.spi, (unsigned long)x->id.spi);
>  	audit_log_end(audit_buf);
>  }
>  EXPORT_SYMBOL_GPL(xfrm_audit_state_add);
>  
> -void
> -xfrm_audit_state_delete(struct xfrm_state *x, int result, u32 auid, u32 sid)
> +void xfrm_audit_state_delete(struct xfrm_state *x, int result,
> +			     u32 auid, u32 secid)
>  {
>  	struct audit_buffer *audit_buf;
> -	extern int audit_enabled;
>  
>  	if (audit_enabled == 0)
>  		return;
> -	audit_buf = xfrm_audit_start(sid, auid);
> +	audit_buf = xfrm_audit_start(secid, auid);
>  	if (audit_buf == NULL)
>  		return;
> -	audit_log_format(audit_buf, " op=SAD-delete res=%u",result);
> +	audit_log_format(audit_buf, " op=SAD-delete res=%u", result);
>  	xfrm_audit_common_stateinfo(x, audit_buf);
> -	audit_log_format(audit_buf, " spi=%lu(0x%lx)",
> -			 (unsigned long)x->id.spi, (unsigned long)x->id.spi);
>  	audit_log_end(audit_buf);
>  }
>  EXPORT_SYMBOL_GPL(xfrm_audit_state_delete);
> diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
> index e076039..c925880 100644
> --- a/security/selinux/xfrm.c
> +++ b/security/selinux/xfrm.c
> @@ -85,7 +85,7 @@ int selinux_xfrm_policy_lookup(struct xfrm_policy *xp, u32 fl_secid, u8 dir)
>  		if (!selinux_authorizable_ctx(ctx))
>  			return -EINVAL;
>  
> -		sel_sid = ctx->ctx_sid;
> +		sel_sid = ctx->ctx_secid;
>  	}
>  	else
>  		/*
> @@ -132,7 +132,7 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *
>  				/* Not a SELinux-labeled SA */
>  				return 0;
>  
> -	state_sid = x->security->ctx_sid;
> +	state_sid = x->security->ctx_secid;
>  
>  	if (fl->secid != state_sid)
>  		return 0;
> @@ -175,13 +175,13 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
>  				struct xfrm_sec_ctx *ctx = x->security;
>  
>  				if (!sid_set) {
> -					*sid = ctx->ctx_sid;
> +					*sid = ctx->ctx_secid;
>  					sid_set = 1;
>  
>  					if (!ckall)
>  						break;
>  				}
> -				else if (*sid != ctx->ctx_sid)
> +				else if (*sid != ctx->ctx_secid)
>  					return -EINVAL;
>  			}
>  		}
> @@ -232,7 +232,7 @@ static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
>  	ctx->ctx_str[str_len] = 0;
>  	rc = security_context_to_sid(ctx->ctx_str,
>  				     str_len,
> -				     &ctx->ctx_sid);
> +				     &ctx->ctx_secid);
>  
>  	if (rc)
>  		goto out;
> @@ -240,7 +240,7 @@ static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
>  	/*
>  	 * Does the subject have permission to set security context?
>  	 */
> -	rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
> +	rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
>  			  SECCLASS_ASSOCIATION,
>  			  ASSOCIATION__SETCONTEXT, NULL);
>  	if (rc)
> @@ -264,7 +264,7 @@ not_from_user:
>  
>  	ctx->ctx_doi = XFRM_SC_DOI_LSM;
>  	ctx->ctx_alg = XFRM_SC_ALG_SELINUX;
> -	ctx->ctx_sid = sid;
> +	ctx->ctx_secid = sid;
>  	ctx->ctx_len = str_len;
>  	memcpy(ctx->ctx_str,
>  	       ctx_str,
> @@ -341,7 +341,7 @@ int selinux_xfrm_policy_delete(struct xfrm_policy *xp)
>  	int rc = 0;
>  
>  	if (ctx)
> -		rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
> +		rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
>  				  SECCLASS_ASSOCIATION,
>  				  ASSOCIATION__SETCONTEXT, NULL);
>  
> @@ -383,7 +383,7 @@ int selinux_xfrm_state_delete(struct xfrm_state *x)
>  	int rc = 0;
>  
>  	if (ctx)
> -		rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
> +		rc = avc_has_perm(tsec->sid, ctx->ctx_secid,
>  				  SECCLASS_ASSOCIATION,
>  				  ASSOCIATION__SETCONTEXT, NULL);
>  
> @@ -412,7 +412,7 @@ int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb,
>  
>  			if (x && selinux_authorizable_xfrm(x)) {
>  				struct xfrm_sec_ctx *ctx = x->security;
> -				sel_sid = ctx->ctx_sid;
> +				sel_sid = ctx->ctx_secid;
>  				break;
>  			}
>  		}
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH] XFRM: assorted IPsec fixups

[Paul Moore]

On Friday 07 December 2007 3:36:08 pm Eric Paris wrote:
> On Fri, 2007-12-07 at 12:11 -0500, Paul Moore wrote:
> > This patch fixes a number of small but potentially troublesome things in
> > the XFRM/IPsec code:
> >
> >  * Use the 'audit_enabled' variable already in include/linux/audit.h
> >    Removed the need for extern declarations local to each XFRM audit
> > fuction

{snip}

> although it does make me wonder why audit_log_start doesn't just check
> audit_enabled itself....

/me shrugs ... I have no idea, I've just always followed the lead of what was 
already written, but now that you mention it - it doesn't make much sense.  I 
suppose at some point we can go through and change all the 'audit_enabled' 
users, but I wonder if there is some point (?performance?) to having the 
callers check?

-- 
paul moore
linux security @ hp

Re: [PATCH] XFRM: assorted IPsec fixups

[David Miller]

From: Eric Paris <eparis@redhat.com>
Date: Fri, 07 Dec 2007 15:36:08 -0500

> 
> On Fri, 2007-12-07 at 12:11 -0500, Paul Moore wrote:
> > This patch fixes a number of small but potentially troublesome things in the
> > XFRM/IPsec code:
...
> > Signed-off-by: Paul Moore <paul.moore@hp.com>
> 
> Acked-by: Eric Paris <eparis@redhat.com>
> 
> although it does make me wonder why audit_log_start doesn't just check
> audit_enabled itself....   Anyway, this patch looks good.

I want to apply this but it doesn't apply cleanly to
net-2.6.25 right now, could you respin this Paul?

Thanks.

Re: [PATCH] XFRM: assorted IPsec fixups

[Paul Moore]

On Tuesday 11 December 2007 5:22:02 am David Miller wrote:
> From: Eric Paris <eparis@redhat.com>
> Date: Fri, 07 Dec 2007 15:36:08 -0500
>
> > On Fri, 2007-12-07 at 12:11 -0500, Paul Moore wrote:
> > > This patch fixes a number of small but potentially troublesome things
> > > in the XFRM/IPsec code:
>
> ...
>
> > > Signed-off-by: Paul Moore <paul.moore@hp.com>
> >
> > Acked-by: Eric Paris <eparis@redhat.com>
> >
> > although it does make me wonder why audit_log_start doesn't just check
> > audit_enabled itself....   Anyway, this patch looks good.
>
> I want to apply this but it doesn't apply cleanly to
> net-2.6.25 right now, could you respin this Paul?

Sure, sorry about that.  I wasn't certain who was going to pick this up so I 
just backed again Linus' tree.  I'll get a respun version out later today.

Thanks.

-- 
paul moore
linux security @ hp