[PATCH] XFRM: SPD auditing fix to include the netmask/prefix-length

[PATCH] XFRM: SPD auditing fix to include the netmask/prefix-length

[Paul Moore]

Currently the netmask/prefix-length of an IPsec SPD entry is not included in
any of the SPD related audit messages.  This can cause a problem when the
audit log is examined as the netmask/prefix-length is vital in determining
what network traffic is affected by a particular SPD entry.  This patch fixes
this problem by adding two additional fields, "src_prefixlen" and
"dst_prefixlen", to the SPD audit messages to indicate the source and
destination netmasks.  These new fields are only included in the audit message
when the netmask/prefix-length is less than the address length, i.e. the SPD
entry applies to a network address and not a host address.

Example audit message:

 type=UNKNOWN[1415] msg=audit(1196105849.752:25): auid=0 \
   subj=root:system_r:unconfined_t:s0-s0:c0.c1023 op=SPD-add res=1 \
   src=192.168.0.0 src_prefixlen=24 dst=192.168.1.0 dst_prefixlen=24

In addition, this patch also fixes a few other things in the
xfrm_audit_common_policyinfo() function.  The IPv4 string formatting was
converted to use the standard NIPQUAD_FMT constant, the memcpy() was removed
from the IPv6 code path and replaced with a typecast (the memcpy() was acting
as a slow, implicit typecast anyway), and two local variables were created to
make referencing the XFRM security context and selector information cleaner.

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

 net/xfrm/xfrm_policy.c |   44 ++++++++++++++++++++++++++------------------
 1 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index b702bd8..bd70d79 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2123,29 +2123,37 @@ void __init xfrm_init(void)
 static inline void xfrm_audit_common_policyinfo(struct xfrm_policy *xp,
 						struct audit_buffer *audit_buf)
 {
-	if (xp->security)
+	struct xfrm_sec_ctx *ctx = xp->security;
+	struct xfrm_selector *sel = &xp->selector;
+
+	if (ctx)
 		audit_log_format(audit_buf, " sec_alg=%u sec_doi=%u sec_obj=%s",
-				 xp->security->ctx_alg, xp->security->ctx_doi,
-				 xp->security->ctx_str);
+				 ctx->ctx_alg, ctx->ctx_doi, ctx->ctx_str);
 
-	switch(xp->selector.family) {
+	switch(sel->family) {
 	case AF_INET:
-		audit_log_format(audit_buf, " src=%u.%u.%u.%u dst=%u.%u.%u.%u",
-				 NIPQUAD(xp->selector.saddr.a4),
-				 NIPQUAD(xp->selector.daddr.a4));
+		audit_log_format(audit_buf, " src=" NIPQUAD_FMT,
+				 NIPQUAD(sel->saddr.a4));
+		if (sel->prefixlen_s != 32)
+			audit_log_format(audit_buf, " src_prefixlen=%d",
+					 sel->prefixlen_s);
+		audit_log_format(audit_buf, " dst=" NIPQUAD_FMT,
+				 NIPQUAD(sel->daddr.a4));
+		if (sel->prefixlen_d != 32)
+			audit_log_format(audit_buf, " dst_prefixlen=%d",
+					 sel->prefixlen_d);
 		break;
 	case AF_INET6:
-		{
-			struct in6_addr saddr6, daddr6;
-
-			memcpy(&saddr6, xp->selector.saddr.a6,
-				sizeof(struct in6_addr));
-			memcpy(&daddr6, xp->selector.daddr.a6,
-				sizeof(struct in6_addr));
-			audit_log_format(audit_buf,
-				" src=" NIP6_FMT " dst=" NIP6_FMT,
-				NIP6(saddr6), NIP6(daddr6));
-		}
+		audit_log_format(audit_buf, " src=" NIP6_FMT,
+				 NIP6(*(struct in6_addr *)sel->saddr.a6));
+		if (sel->prefixlen_s != 128)
+			audit_log_format(audit_buf, " src_prefixlen=%d",
+					 sel->prefixlen_s);
+		audit_log_format(audit_buf, " dst=" NIP6_FMT,
+				 NIP6(*(struct in6_addr *)sel->daddr.a6));
+		if (sel->prefixlen_d != 128)
+			audit_log_format(audit_buf, " dst_prefixlen=%d",
+					 sel->prefixlen_d);
 		break;
 	}
 }

Re: [PATCH] XFRM: SPD auditing fix to include the netmask/prefix-length

[Herbert Xu]

On Mon, Nov 26, 2007 at 07:55:12PM +0000, Paul Moore wrote:
> Currently the netmask/prefix-length of an IPsec SPD entry is not included in
> any of the SPD related audit messages.  This can cause a problem when the
> audit log is examined as the netmask/prefix-length is vital in determining
> what network traffic is affected by a particular SPD entry.  This patch fixes
> this problem by adding two additional fields, "src_prefixlen" and
> "dst_prefixlen", to the SPD audit messages to indicate the source and
> destination netmasks.  These new fields are only included in the audit message
> when the netmask/prefix-length is less than the address length, i.e. the SPD
> entry applies to a network address and not a host address.

Any reason why we don't just always include them?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] XFRM: SPD auditing fix to include the netmask/prefix-length

[Paul Moore]

On Thursday 29 November 2007 5:34:59 am Herbert Xu wrote:
> On Mon, Nov 26, 2007 at 07:55:12PM +0000, Paul Moore wrote:
> > Currently the netmask/prefix-length of an IPsec SPD entry is not included
> > in any of the SPD related audit messages.  This can cause a problem when
> > the audit log is examined as the netmask/prefix-length is vital in
> > determining what network traffic is affected by a particular SPD entry. 
> > This patch fixes this problem by adding two additional fields,
> > "src_prefixlen" and "dst_prefixlen", to the SPD audit messages to
> > indicate the source and destination netmasks.  These new fields are only
> > included in the audit message when the netmask/prefix-length is less than
> > the address length, i.e. the SPD entry applies to a network address and
> > not a host address.
>
> Any reason why we don't just always include them?

The audit folks seem to be very sensitive to the size/length of the audit 
messages, they prefer they be as small as possible.  I thought that one way 
to save space would be to only print the prefix length information when the 
address referred to a network and not a single host.

Would you prefer it if the prefix length information was always included in 
the audit message?  Joy?  Audit folks?

-- 
paul moore
linux security @ hp

Re: [PATCH] XFRM: SPD auditing fix to include the netmask/prefix-length

[Paul Moore]

On Thursday 29 November 2007 8:45:46 am Paul Moore wrote:
> On Thursday 29 November 2007 5:34:59 am Herbert Xu wrote:
> > On Mon, Nov 26, 2007 at 07:55:12PM +0000, Paul Moore wrote:
> > > Currently the netmask/prefix-length of an IPsec SPD entry is not
> > > included in any of the SPD related audit messages.  This can cause a
> > > problem when the audit log is examined as the netmask/prefix-length is
> > > vital in determining what network traffic is affected by a particular
> > > SPD entry. This patch fixes this problem by adding two additional
> > > fields, "src_prefixlen" and "dst_prefixlen", to the SPD audit messages
> > > to indicate the source and destination netmasks.  These new fields are
> > > only included in the audit message when the netmask/prefix-length is
> > > less than the address length, i.e. the SPD entry applies to a network
> > > address and not a host address.
> >
> > Any reason why we don't just always include them?
>
> The audit folks seem to be very sensitive to the size/length of the audit
> messages, they prefer they be as small as possible.  I thought that one way
> to save space would be to only print the prefix length information when the
> address referred to a network and not a single host.
>
> Would you prefer it if the prefix length information was always included in
> the audit message?  Joy?  Audit folks?

Steve and/or Joy, could we get a verdict on this issue?  The lack of a netmask 
in the SPD audit messages is pretty serious so I'd like to see this fixed as 
soon as possible.

-- 
paul moore
linux security @ hp

Re: [PATCH] XFRM: SPD auditing fix to include the netmask/prefix-length

[Joy Latten]

On Fri, 2007-11-30 at 09:51 -0500, Paul Moore wrote:
> On Thursday 29 November 2007 8:45:46 am Paul Moore wrote:
> > On Thursday 29 November 2007 5:34:59 am Herbert Xu wrote:
> > > On Mon, Nov 26, 2007 at 07:55:12PM +0000, Paul Moore wrote:
> > > > Currently the netmask/prefix-length of an IPsec SPD entry is not
> > > > included in any of the SPD related audit messages.  This can cause a
> > > > problem when the audit log is examined as the netmask/prefix-length is
> > > > vital in determining what network traffic is affected by a particular
> > > > SPD entry. This patch fixes this problem by adding two additional
> > > > fields, "src_prefixlen" and "dst_prefixlen", to the SPD audit messages
> > > > to indicate the source and destination netmasks.  These new fields are
> > > > only included in the audit message when the netmask/prefix-length is
> > > > less than the address length, i.e. the SPD entry applies to a network
> > > > address and not a host address.
> > >
> > > Any reason why we don't just always include them?
> >
> > The audit folks seem to be very sensitive to the size/length of the audit
> > messages, they prefer they be as small as possible.  I thought that one way
> > to save space would be to only print the prefix length information when the
> > address referred to a network and not a single host.
> >
> > Would you prefer it if the prefix length information was always included in
> > the audit message?  Joy?  Audit folks?
> 
> Steve and/or Joy, could we get a verdict on this issue?  The lack of a netmask 
> in the SPD audit messages is pretty serious so I'd like to see this fixed as 
> soon as possible.
> 
I think Steve may be able to answer this much better than I can in 
regards to audit. In my opinion having the netmask is good.

regards,
Joy

Re: [PATCH] XFRM: SPD auditing fix to include the netmask/prefix-length

[Herbert Xu]

On Fri, Nov 30, 2007 at 09:51:48AM -0500, Paul Moore wrote:
>
> Steve and/or Joy, could we get a verdict on this issue?  The lack of a netmask 
> in the SPD audit messages is pretty serious so I'd like to see this fixed as 
> soon as possible.

I'll take the resounding silence as an indication of approval :)

Patch applied to net-2.6.25.  Thanks Paul.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] XFRM: SPD auditing fix to include the netmask/prefix-length

[Paul Moore]

On Saturday 01 December 2007 7:28:34 am Herbert Xu wrote:
> On Fri, Nov 30, 2007 at 09:51:48AM -0500, Paul Moore wrote:
> > Steve and/or Joy, could we get a verdict on this issue?  The lack of a
> > netmask in the SPD audit messages is pretty serious so I'd like to see
> > this fixed as soon as possible.
>
> I'll take the resounding silence as an indication of approval :)
>
> Patch applied to net-2.6.25.  Thanks Paul.

Thanks Herbert.  If I hear any grumblings from the audit folks I'll send out 
another patch to make the prefix length a permanent fixture.

-- 
paul moore
linux security @ hp