Ulrich Weber schrieb:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi all,

IPSec uses sequence numbers to protect against replay attacks.
So far there is no way to get or set these sequence numbers in the kernel.
The attached patches will remedy these issues. Any comments are
welcome! :)

I would be pleased if at least 2.6.13-rc3_ipsec_pfkey_seqnr.diff and
2.6.13-rc3_ipsec_xfrm_seqnr.diff find there way in the vanilla kernel.

Some comments:

------------------------------------------------------------------------

+#define XFRM_REPLAY_BOUND_MASK 3

This is unnecessary.


@@ -312,6 +320,7 @@
        struct xfrm_policy      *(*compile_policy)(u16 family, int opt, u8 
*data, int len, int *dir);
        int                     (*new_mapping)(struct xfrm_state *x, 
xfrm_address_t *ipaddr, u16 sport);
        int                     (*notify_policy)(struct xfrm_policy *x, int 
dir, struct km_event *c);
+       int                     (*notify_seq)(struct xfrm_state *x, u32, u32);

Please do this consistently. Either add the argument names, or don't.

+void km_replay_notify(struct xfrm_state *);

extern

+#ifdef CONFIG_XFRM
+extern u32 sysctl_xfrm_seqdiff_in;
+extern u32 sysctl_xfrm_seqdiff_out;
+#endif /* CONFIG_XFRM */

Sysctls are a bad idea for this. The replay window is a per-state
parameter, so the diff should be too.

+void xfrm_state_replay_update(struct xfrm_state *x, struct xfrm_replay_state 
*replay)
+{
+       spin_lock(&x->lock);

You need spin_lock_bh here.

+       memcpy(&x->replay, replay, sizeof(*replay));
+       memcpy(&x->preplay, replay, sizeof(*replay));
+       spin_unlock(&x->lock);
+}
+EXPORT_SYMBOL(xfrm_state_replay_update);

There's no external user. Why is this exported?

+       if(pid == 0)
+       {
+               NETLINK_CB(skb).dst_groups = XFRMGRP_REPLAY;
+               return netlink_broadcast(xfrm_nl, skb, 0, XFRMGRP_REPLAY, 
GFP_ATOMIC);
+       }
+       else

Missing whitespace and incorrect placed braces

+static int xfrm_update_seq(struct sk_buff *skb, struct nlmsghdr *nlh, void 
**xfrma)
+{
+       struct xfrm_state *x;
+       struct xfrm_usersa_id *p = NLMSG_DATA(nlh);
+       struct xfrm_replay_state *replay;
+       struct rtattr *rt = xfrma[XFRMA_REPLAY-1];
+ + x = xfrm_state_lookup(&p->daddr, p->spi, p->proto, p->family);
+       if (x == NULL) {
+               printk(KERN_INFO "Found no xfrm state for sa seq update\n");

Please remove this debugging stuff

+               return -ESRCH;
+       }
+
+       if( xfrma[XFRMA_REPLAY-1] != NULL && (rt->rta_len == (sizeof(struct 
xfrm_replay_state) + sizeof(struct rtattr)))) {

Whitespace broken, too long line

+               replay = RTA_DATA(xfrma[XFRMA_REPLAY - 1]);
+               xfrm_state_replay_update(x, replay);
+               xfrm_state_put(x);
+       }
+       else {
+               xfrm_send_replay_notify(x, nlh->nlmsg_pid, nlh->nlmsg_seq);

xfrm_state leak

Regards
Patrick
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to