On Thursday 26 October 2006 20:23, Joy Latten wrote:
> Please let me know if this patch is acceptable.
Sorry about the delay...but I have some feedback. The patch got wrapped by
email client, so I can't apply it. But I can see a few issues.
> diff -urpN linux-2.6.18.ppc64/include/linux/audit.h
> linux-2.6.18.ppc64.patch/include/linux/audit.h
> --- linux-2.6.18.ppc64/include/linux/audit.h 2006-10-26
> 03:10:10.000000000 -0500
> +++ linux-2.6.18.ppc64.patch/include/linux/audit.h 2006-10-26
> 07:04:08.000000000 -0500
> @@ -100,6 +100,10 @@
> #define AUDIT_MAC_CIPSOV4_DEL 1408 /* NetLabel: del CIPSOv4 DOI
> entry
> */
> #define AUDIT_MAC_MAP_ADD 1409 /* NetLabel: add LSM domain mapping */
> #define AUDIT_MAC_MAP_DEL 1410 /* NetLabel: del LSM domain mapping */
> +#define AUDIT_MAC_IPSEC_ADDSA 1411 /* Add a XFRM state */
> +#define AUDIT_MAC_IPSEC_DELSA 1412 /* Delete a XFRM state */
> +#define AUDIT_MAC_IPSEC_ADDSPD 1413 /* Add a XFRM policy */
> +#define AUDIT_MAC_IPSEC_DELSPD 1414 /* Delete a XFRM policy */
Looks good.
> diff -urpN linux-2.6.18.ppc64/net/xfrm/xfrm_policy.c
> linux-2.6.18.ppc64.patch/net/xfrm/xfrm_policy.c
> --- linux-2.6.18.ppc64/net/xfrm/xfrm_policy.c 2006-10-26
> 03:10:11.000000000 -0500
> +++ linux-2.6.18.ppc64.patch/net/xfrm/xfrm_policy.c 2006-10-26
> 07:04:08.000000000 -0500
> @@ -374,13 +374,21 @@ static void xfrm_policy_gc_task(void *da
> * entry dead. The rule must be unlinked from lists to the moment.
> */
>
> -static void xfrm_policy_kill(struct xfrm_policy *policy)
> +static void xfrm_policy_kill(struct xfrm_policy *policy, uid_t auid)
> {
> int dead;
>
> write_lock_bh(&policy->lock);
> dead = policy->dead;
> policy->dead = 1;
> +
> + if (policy->security)
If this is NULL we get no audit message?
> + audit_log(current->audit_context, GFP_ATOMIC,
> + AUDIT_MAC_IPSEC_DELSPD,
> + "spd delete: auid=%u
subj= should be after auid field. This means that you need to collect the
secid out of the netlink packets and the audit context and send it as well.
> ctx_alg=%d ctx_doi=%d
I'd drop the ctx in favor of sp.
> ctx=%s",
> + auid, policy->security->ctx_alg,
> + policy->security->ctx_doi, policy->security->ctx_str);
> +
Also, the last field should be res=%u. res is the results, 1 meaning success
and 0 failure. This means we want this function called on failure, too.
> write_unlock_bh(&policy->lock);
>
> if (unlikely(dead)) {
> @@ -417,7 +425,7 @@ static u32 xfrm_gen_index(int dir)
> }
> }
>
> -int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
> +int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl,
> uid_t auid)
> {
> struct xfrm_policy *pol, **p;
> struct xfrm_policy *delpol = NULL;
> @@ -460,10 +468,18 @@ int xfrm_policy_insert(int dir, struct x
> write_unlock_bh(&xfrm_policy_lock);
>
> if (delpol)
> - xfrm_policy_kill(delpol);
> + xfrm_policy_kill(delpol, auid);
>
> read_lock_bh(&xfrm_policy_lock);
> gc_list = NULL;
> +
> + if (policy->security)
> + audit_log(current->audit_context, GFP_ATOMIC,
> + AUDIT_MAC_IPSEC_ADDSPD,
> + "spd add: auid=%u ctx_alg=%d ctx_doi=%d ctx=%s",
> + auid, policy->security->ctx_alg,
> + policy->security->ctx_doi, policy->security->ctx_str);
> +
Same comments apply here.
> diff -urpN linux-2.6.18.ppc64/net/xfrm/xfrm_state.c
> linux-2.6.18.ppc64.patch/net/xfrm/xfrm_state.c
> --- linux-2.6.18.ppc64/net/xfrm/xfrm_state.c 2006-10-26
> 03:10:09.000000000 -0500
> +++ linux-2.6.18.ppc64.patch/net/xfrm/xfrm_state.c 2006-10-26
> 07:04:08.000000000 -0500
> @@ -244,6 +245,13 @@ int __xfrm_state_delete(struct xfrm_stat
> list_del(&x->byspi);
> __xfrm_state_put(x);
> }
> + if (x->security)
> + audit_log(current->audit_context, GFP_ATOMIC,
> + AUDIT_MAC_IPSEC_ADDSA,
> + "SAD delete: auid=%u
need subj=
> ctx_alg=%d ctx_doi=%d ctx=%s",
I'd drop ctx in favor of sa.
> + auid, x->security->ctx_alg,
> + x->security->ctx_doi, x->security->ctx_str);
res=%u.
> @@ -441,7 +449,7 @@ out:
> return x;
> }
>
> -static void __xfrm_state_insert(struct xfrm_state *x)
> +static void __xfrm_state_insert(struct xfrm_state *x, uid_t auid)
> {
> unsigned h = xfrm_dst_hash(&x->id.daddr, x->props.family);
>
> @@ -461,12 +469,20 @@ static void __xfrm_state_insert(struct x
> xfrm_state_hold(x);
>
> wake_up(&km_waitq);
> +
> + if (x->security)
> + audit_log(current->audit_context, GFP_ATOMIC,
> + AUDIT_MAC_IPSEC_ADDSA,
> + "SAD add: auid=%u ctx_alg=%d ctx_doi=%d, ctx=%s",
> + auid, x->security->ctx_alg, x->security->ctx_doi,
> + x->security->ctx_str);
Same comments as the previous.
> @@ -1125,11 +1141,15 @@ static void xfrm_state_put_afinfo(struct
> /* Temporarily located here until net/xfrm/xfrm_tunnel.c is created */
> void xfrm_state_delete_tunnel(struct xfrm_state *x)
> {
> + uid_t auid;
> +
> + auid = audit_get_loginuid(current->audit_context);
> +
> if (x->tunnel) {
> struct xfrm_state *t = x->tunnel;
>
> if (atomic_read(&t->tunnel_users) == 2)
> - xfrm_state_delete(t);
> + xfrm_state_delete(t, auid);
auid appears to only get used here, you can probably move get_loginuid inside
the if statement so the other path is not penalized.
> diff -urpN linux-2.6.18.ppc64/net/xfrm/xfrm_user.c
> linux-2.6.18.ppc64.patch/net/xfrm/xfrm_user.c
> --- linux-2.6.18.ppc64/net/xfrm/xfrm_user.c 2006-10-26
> 03:10:11.000000000 -0500
> +++ linux-2.6.18.ppc64.patch/net/xfrm/xfrm_user.c 2006-10-26
> 07:04:08.000000000 -0500
> @@ -27,6 +27,7 @@
> #include <net/xfrm.h>
> #include <net/netlink.h>
> #include <asm/uaccess.h>
> +#include <linux/audit.h>
>
> static int verify_one_alg(struct rtattr **xfrma, enum xfrm_attr_type_t
> type)
> {
> @@ -396,9 +397,9 @@ static int xfrm_add_sa(struct sk_buff *s
>
> xfrm_state_hold(x);
> if (nlh->nlmsg_type == XFRM_MSG_NEWSA)
> - err = xfrm_state_add(x);
> + err = xfrm_state_add(x, NETLINK_CB(skb).loginuid);
Also need to collect NETLINK_CB(skb).sid in this and other places below.
> else
> - err = xfrm_state_update(x);
> + err = xfrm_state_update(x, NETLINK_CB(skb).loginuid);
>
> if (err < 0) {
> x->km.state = XFRM_STATE_DEAD;
> @@ -435,7 +436,7 @@ static int xfrm_del_sa(struct sk_buff *s
> goto out;
> }
>
> - err = xfrm_state_delete(x);
> + err = xfrm_state_delete(x, NETLINK_CB(skb).loginuid);
> if (err < 0)
> goto out;
>
> @@ -859,7 +860,7 @@ static int xfrm_add_policy(struct sk_buf
> * in netlink excl is a flag and you wouldnt need
> * a type XFRM_MSG_UPDPOLICY - JHS */
> excl = nlh->nlmsg_type == XFRM_MSG_NEWPOLICY;
> - err = xfrm_policy_insert(p->dir, xp, excl);
> + err = xfrm_policy_insert(p->dir, xp, excl, NETLINK_CB(skb).loginuid);
> if (err) {
> security_xfrm_policy_free(xp);
> kfree(xp);
> @@ -1026,6 +1027,7 @@ static int xfrm_get_policy(struct sk_buf
> int err;
> struct km_event c;
> int delete;
> + uid_t auid;
>
> p = NLMSG_DATA(nlh);
> delete = nlh->nlmsg_type == XFRM_MSG_DELPOLICY;
> @@ -1034,8 +1036,9 @@ static int xfrm_get_policy(struct sk_buf
> if (err)
> return err;
>
> + auid = NETLINK_CB(skb).loginuid;
> if (p->index)
> - xp = xfrm_policy_byid(p->dir, p->index, delete);
> + xp = xfrm_policy_byid(p->dir, p->index, delete, auid);
> else {
> struct rtattr **rtattrs = (struct rtattr **)xfrma;
> struct rtattr *rt = rtattrs[XFRMA_SEC_CTX-1];
> @@ -1052,7 +1055,7 @@ static int xfrm_get_policy(struct sk_buf
> if ((err = security_xfrm_policy_alloc(&tmp, uctx)))
> return err;
> }
> - xp = xfrm_policy_bysel_ctx(p->dir, &p->sel, tmp.security,
> delete);
> + xp = xfrm_policy_bysel_ctx(p->dir, &p->sel, tmp.security,
> delete,
> auid);
> security_xfrm_policy_free(&tmp);
> }
> if (xp == NULL)
> @@ -1090,7 +1093,7 @@ static int xfrm_flush_sa(struct sk_buff
> struct km_event c;
> struct xfrm_usersa_flush *p = NLMSG_DATA(nlh);
>
> - xfrm_state_flush(p->proto);
> + xfrm_state_flush(p->proto, NETLINK_CB(skb).loginuid);
> c.data.proto = p->proto;
> c.event = nlh->nlmsg_type;
> c.seq = nlh->nlmsg_seq;
> @@ -1237,7 +1240,7 @@ static int xfrm_flush_policy(struct sk_b
> {
> struct km_event c;
>
> - xfrm_policy_flush();
> + xfrm_policy_flush(NETLINK_CB(skb).loginuid);
> c.event = nlh->nlmsg_type;
> c.seq = nlh->nlmsg_seq;
> c.pid = nlh->nlmsg_pid;
> @@ -1251,9 +1254,12 @@ static int xfrm_add_pol_expire(struct sk
> struct xfrm_user_polexpire *up = NLMSG_DATA(nlh);
> struct xfrm_userpolicy_info *p = &up->pol;
> int err = -ENOENT;
> + uid_t auid;
> +
> + auid = NETLINK_CB(skb).loginuid;
>
> if (p->index)
> - xp = xfrm_policy_byid(p->dir, p->index, 0);
> + xp = xfrm_policy_byid(p->dir, p->index, 0, auid);
> else {
> struct rtattr **rtattrs = (struct rtattr **)xfrma;
> struct rtattr *rt = rtattrs[XFRMA_SEC_CTX-1];
> @@ -1270,7 +1276,7 @@ static int xfrm_add_pol_expire(struct sk
> if ((err = security_xfrm_policy_alloc(&tmp, uctx)))
> return err;
> }
> - xp = xfrm_policy_bysel_ctx(p->dir, &p->sel, tmp.security, 0);
> + xp = xfrm_policy_bysel_ctx(p->dir, &p->sel, tmp.security, 0,
> auid);
> security_xfrm_policy_free(&tmp);
> }
>
> @@ -1285,7 +1291,7 @@ static int xfrm_add_pol_expire(struct sk
> read_unlock(&xp->lock);
> err = 0;
> if (up->hard) {
> - xfrm_policy_delete(xp, p->dir);
> + xfrm_policy_delete(xp, p->dir, auid);
> } else {
> // reset the timers here?
> printk("Dont know what to do with soft policy expire\n");
> @@ -1318,7 +1324,7 @@ static int xfrm_add_sa_expire(struct sk_
> km_state_expired(x, ue->hard, current->pid);
>
> if (ue->hard)
> - __xfrm_state_delete(x);
> + __xfrm_state_delete(x, NETLINK_CB(skb).loginuid);
> out:
> spin_unlock_bh(&x->lock);
> xfrm_state_put(x);
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
