Skip to contentSkip to conversation

Add xfrm policy change auditing to pfkey_spdget

PATCH Applied Eric Paris 5 participants 8 messages Mar 2, 2007 – Mar 8, 2007

Subsystems (3)
lore
Share & tools
lore

Eric Paris

author
#0 lore raw

PATCH 1/1 Add xfrm policy change auditing to pfkey_spdget

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.

 net/key/af_key.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)
net/key/af_key.c +11 -6
index 3542435..7cbf0a2 100644
@@ -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;
 }
Acked-by on v1 1/1
#2 lore raw
In reply to Eric Paris
Quoted 6 lines · 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.

Signature · 2 lines
James Morris
<[email protected]>
maintainer Applied
#3 lore raw
In reply to Eric Paris

From: Eric Paris <[email protected]>
Date: Fri, 02 Mar 2007 13:51:24 -0500

Quoted 6 lines

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.

Applied.

maintainer
#4 lore raw

From: David Miller <[email protected]>
Date: Wed, 07 Mar 2007 15:43:16 -0800 (PST)

Quoted 11 lines

From: Eric Paris <[email protected]>
Date: Fri, 02 Mar 2007 13:51:24 -0500

Quoted 6 lines

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.

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

author
#5 lore raw
Quoted 15 lines · David Miller

From: David Miller <[email protected]>
Date: Wed, 07 Mar 2007 15:43:16 -0800 (PST)

Quoted 11 lines

From: Eric Paris <[email protected]>
Date: Fri, 02 Mar 2007 13:51:24 -0500

Quoted 6 lines

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.

Applied.

Quoted 7 lines

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

#6 lore raw
Quoted 4 lines · Eric Paris

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

Signature · 2 lines
James Morris
<[email protected]>
#7 lore raw
In reply to Eric Paris
Quoted 3 lines · Eric Paris

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

Signature · 2 lines
paul moore
linux security @ hp