Skip to contentSkip to conversation

xfrm audit hook misplaced in pfkey_delete and xfrm_del_sa

PATCH In review Eric Paris 4 participants 4 messages Mar 2, 2007 – Mar 7, 2007

Subsystems (3)
lore
Share & tools
lore

Eric Paris

author
#0 lore raw

PATCH 1/1 xfrm audit hook misplaced in pfkey_delete and xfrm_del_sa

Inside pfkey_delete and xfrm_del_sa the audit hooks were not called if there was any permission/security failures in attempting to do the del operation (such as permission denied from security_xfrm_state_delete). This patch moves the audit hook to the exit path such that all failures (and successes) will actually get audited.

 net/key/af_key.c     |    5 ++---
 net/xfrm/xfrm_user.c |    5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)
net/key/af_key.c +2 -3
index 1c58204..d8fc88c 100644
@@ -1467,9 +1467,6 @@ static int pfkey_delete(struct sock *sk, struct sk_buff *skb, struct sadb_msg *h
 
 	err = xfrm_state_delete(x);
 
-	xfrm_audit_log(audit_get_loginuid(current->audit_context), 0,
-		       AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x);
-
 	if (err < 0)
 		goto out;
 
@@ -1478,6 +1475,8 @@ static int pfkey_delete(struct sock *sk, struct sk_buff *skb, struct sadb_msg *h
 	c.event = XFRM_MSG_DELSA;
 	km_state_notify(x, &c);
 out:
+	xfrm_audit_log(audit_get_loginuid(current->audit_context), 0,
+		       AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x);
 	xfrm_state_put(x);
 
 	return err;
net/xfrm/xfrm_user.c +2 -3
index 956cfe0..4264473 100644
@@ -530,9 +530,6 @@ static int xfrm_del_sa(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	err = xfrm_state_delete(x);
 
-	xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid,
-		       AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x);
-
 	if (err < 0)
 		goto out;
 
@@ -542,6 +539,8 @@ static int xfrm_del_sa(struct sk_buff *skb, struct nlmsghdr *nlh,
 	km_state_notify(x, &c);
 
 out:
+	xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid,
+		       AUDIT_MAC_IPSEC_DELSA, err ? 0 : 1, NULL, x);
 	xfrm_state_put(x);
 	return err;
 }
Acked-by on v1 1/1
#1 lore raw
Quoted 6 lines

Inside pfkey_delete and xfrm_del_sa the audit hooks were not called if there was any permission/security failures in attempting to do the del operation (such as permission denied from security_xfrm_state_delete). This patch moves the audit hook to the exit path such that all failures
(and successes) will actually get audited.

Not sure ALL failures are being audited this way elsewhere, but I guess they would catchup in course of time.

Quoted 2 lines
Acked-by on v1 1/1
#2 lore raw
In reply to Eric Paris
Quoted 7 lines · Eric Paris

Inside pfkey_delete and xfrm_del_sa the audit hooks were not called if there was any permission/security failures in attempting to do the del operation (such as permission denied from security_xfrm_state_delete). This patch moves the audit hook to the exit path such that all failures (and successes) will actually get audited.

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

From: Eric Paris <[email protected]>
Date: Fri, 02 Mar 2007 15:15:36 -0500

Quoted 7 lines

Inside pfkey_delete and xfrm_del_sa the audit hooks were not called if there was any permission/security failures in attempting to do the del operation (such as permission denied from security_xfrm_state_delete). This patch moves the audit hook to the exit path such that all failures (and successes) will actually get audited.

Also applied, thanks a lot Eric.