[PATCH] Add xfrm policy change auditing to pfkey_spdget

[PATCH] Add xfrm policy change auditing to pfkey_spdget

[Eric Paris]

pfkey_spdget neither had an LSM security hook nor auditing for the
removal of xfrm_policy structs.  The security hook was added when it was
moved into xfrm_policy_byid instead of the callers to that function by
my earlier patch and this patch adds the auditing hooks as well.

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

 net/key/af_key.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 3542435..7cbf0a2 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2537,7 +2537,7 @@ static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
 static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
 	unsigned int dir;
-	int err;
+	int err = 0, delete;
 	struct sadb_x_policy *pol;
 	struct xfrm_policy *xp;
 	struct km_event c;
@@ -2549,16 +2549,20 @@ static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, struct sadb_msg *h
 	if (dir >= XFRM_POLICY_MAX)
 		return -EINVAL;
 
+	delete = (hdr->sadb_msg_type == SADB_X_SPDDELETE2);
 	xp = xfrm_policy_byid(XFRM_POLICY_TYPE_MAIN, dir, pol->sadb_x_policy_id,
-			      hdr->sadb_msg_type == SADB_X_SPDDELETE2, &err);
+			      delete, &err);
 	if (xp == NULL)
 		return -ENOENT;
 
-	err = 0;
+	if (delete)
+		xfrm_audit_log(audit_get_loginuid(current->audit_context), 0,
+			       AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL);
 
-	c.seq = hdr->sadb_msg_seq;
-	c.pid = hdr->sadb_msg_pid;
-	if (hdr->sadb_msg_type == SADB_X_SPDDELETE2) {
+		if (err)
+			goto out;
+		c.seq = hdr->sadb_msg_seq;
+		c.pid = hdr->sadb_msg_pid;
 		c.data.byid = 1;
 		c.event = XFRM_MSG_DELPOLICY;
 		km_policy_notify(xp, dir, &c);
@@ -2566,6 +2570,7 @@ static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, struct sadb_msg *h
 		err = key_pol_get_resp(sk, xp, hdr, dir);
 	}
 
+out:
 	xfrm_pol_put(xp);
 	return err;
 }

RE: [PATCH] Add xfrm policy change auditing to pfkey_spdget

[Venkat Yekkirala]

> pfkey_spdget neither had an LSM security hook nor auditing for the
> removal of xfrm_policy structs.  The security hook was added 
> when it was
> moved into xfrm_policy_byid instead of the callers to that function by
> my earlier patch and this patch adds the auditing hooks as well.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
Acked-by: Venkat Yekkirala <vyekkirala@trustedcs.com>  

Re: [PATCH] Add xfrm policy change auditing to pfkey_spdget

[James Morris]

On Fri, 2 Mar 2007, Eric Paris wrote:

> pfkey_spdget neither had an LSM security hook nor auditing for the
> removal of xfrm_policy structs.  The security hook was added when it was
> moved into xfrm_policy_byid instead of the callers to that function by
> my earlier patch and this patch adds the auditing hooks as well.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>

Acked-by: James Morris <jmorris@namei.org>


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH] Add xfrm policy change auditing to pfkey_spdget

[David Miller]

From: Eric Paris <eparis@parisplace.org>
Date: Fri, 02 Mar 2007 13:51:24 -0500

> pfkey_spdget neither had an LSM security hook nor auditing for the
> removal of xfrm_policy structs.  The security hook was added when it was
> moved into xfrm_policy_byid instead of the callers to that function by
> my earlier patch and this patch adds the auditing hooks as well.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>

Applied.

Re: [PATCH] Add xfrm policy change auditing to pfkey_spdget

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Wed, 07 Mar 2007 15:43:16 -0800 (PST)

> From: Eric Paris <eparis@parisplace.org>
> Date: Fri, 02 Mar 2007 13:51:24 -0500
> 
> > pfkey_spdget neither had an LSM security hook nor auditing for the
> > removal of xfrm_policy structs.  The security hook was added when it was
> > moved into xfrm_policy_byid instead of the callers to that function by
> > my earlier patch and this patch adds the auditing hooks as well.
> > 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> 
> Applied.

This patch was missing an openning brace on the "if (delete)" line.
Eric you don't post patches without at least compile testing
them now do you? :-)

I fixed this up, but I will just kick it back to you next time,
and I will likely growl very loudly in your general direction
too. ;)

Re: [PATCH] Add xfrm policy change auditing to pfkey_spdget

[Eric Paris]

On Wed, 2007-03-07 at 16:07 -0800, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 07 Mar 2007 15:43:16 -0800 (PST)
> 
> > From: Eric Paris <eparis@parisplace.org>
> > Date: Fri, 02 Mar 2007 13:51:24 -0500
> > 
> > > pfkey_spdget neither had an LSM security hook nor auditing for the
> > > removal of xfrm_policy structs.  The security hook was added when it was
> > > moved into xfrm_policy_byid instead of the callers to that function by
> > > my earlier patch and this patch adds the auditing hooks as well.
> > > 
> > > Signed-off-by: Eric Paris <eparis@redhat.com>
> > 
> > Applied.
> 
> This patch was missing an openning brace on the "if (delete)" line.
> Eric you don't post patches without at least compile testing
> them now do you? :-)
> 
> I fixed this up, but I will just kick it back to you next time,
> and I will likely growl very loudly in your general direction
> too. ;)

I lose at using git.  Sorry.  I'll be more careful to check that all of
my changes on the current branch are committed before I run my git diff.
Or maybe someone will convince me to use git in an all new better way.
I created a branch that has your tree and then created a new branch off
of that for my changes.  I checked out my branch made my patch and
commited.  I then tried to compile failed and fixed it up.  I then
compiled, booted, and tested.  When I thought it was working I did a

git diff miller..my-branch-with-pfkey_spdget

which didn't have my fix up because i didn't commit it to my local
branch.  Is there a better way to get a diff between my miller tree and
'everything in the branch I have checked out even if it is not
committed'?

Sorry, even if there are no ideas I'll be more careful.

-Eric

Re: [PATCH] Add xfrm policy change auditing to pfkey_spdget

[James Morris]

On Thu, 8 Mar 2007, Eric Paris wrote:

> which didn't have my fix up because i didn't commit it to my local
> branch.  Is there a better way to get a diff between my miller tree and
> 'everything in the branch I have checked out even if it is not
> committed'?

I'd suggest you commit all your changes in local branches, then export 
them as emails via git-format-patch for posting.

Something like:

 davem-upstream         [1]
  +- for-davem          [2]
     +- for-davem-prep  [3]

Do all of your work in [3], so you can manage the queue of patches there 
before merging/applying them into [2] as a final patch series.

So, if one of the commits in [3] needs fixing, you can, for example, 
export commits up to that with git-format-patch, git-reset --hard to the 
broken commit, fix, compile, test then reapply the exported commits.  
Then, once it's all ready, merge into [2] (or export & apply to avoid 
merge commits).

This is just one possible workflow.  There are probably several better.


- James
-- 
James Morris
<jmorris@namei.org>

Re: [PATCH] Add xfrm policy change auditing to pfkey_spdget

[Paul Moore]

On Thursday, March 8 2007 11:20:15 am Eric Paris wrote:
> I lose at using git.  Sorry.  I'll be more careful to check that all of
> my changes on the current branch are committed before I run my git diff.
> Or maybe someone will convince me to use git in an all new better way.

[NOTE: I stripped a lot of addresses from this mail as I suspect it to be 
somewhat off-topic]

Perhaps not quite what you were asking for, but I've found quilt[1] to be an 
*extremely* wonderful tool for creating and manipulating patches to send 
upstream.  It doesn't replace git, you still need it to fetch/update the 
source tree, but it plays well with git (there is the "stacked git" project 
which aims to integrate the two bits of software but I have never tried it).

[1] http://savannah.nongnu.org/projects/quilt

-- 
paul moore
linux security @ hp