xfrm: Move the test on replay window size into the replay check functions

PATCH In review Steffen Klassert 3 participants 6 messages Mar 29, 2011

Subsystems (3)

Networking Network drivers Networking IPsec

lore

Share & tools

lore

Steffen Klassert

author maintainer

2011-03-29 05:45 UTC #0 lore raw

PATCH 1/4 xfrm: Move the test on replay window size into the replay check functions

As it is, the replay check is just performed if the replay window of the legacy implementation is nonzero. So we move the test on a nonzero replay window inside the replay check functions to be sure we are testing for the right implementation.

Signed-off-by Steffen Klassert <[email protected]>

---
 net/xfrm/xfrm_input.c  |    2 +-
 net/xfrm/xfrm_replay.c |   17 +++++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

net/xfrm/xfrm_input.c +1 -1

index 872065c..e063638 100644
@@ -173,7 +173,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 			goto drop_unlock;
 		}
 
-		if (x->props.replay_window && x->repl->check(x, skb, seq)) {
+		if (x->repl->check(x, skb, seq)) {
 			XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATESEQERROR);
 			goto drop_unlock;
 		}

net/xfrm/xfrm_replay.c +15 -2

index 2f5be5b..f218385 100644
@@ -118,6 +118,9 @@ static int xfrm_replay_check(struct xfrm_state *x,
 	u32 diff;
 	u32 seq = ntohl(net_seq);
 
+	if (!x->props.replay_window)
+		return 0;
+
 	if (unlikely(seq == 0))
 		goto err;

@@ -193,9 +196,14 @@ static int xfrm_replay_check_bmp(struct xfrm_state *x,
 {
 	unsigned int bitnr, nr;
 	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
+	u32 pos;
 	u32 seq = ntohl(net_seq);
 	u32 diff =  replay_esn->seq - seq;
-	u32 pos = (replay_esn->seq - 1) % replay_esn->replay_window;
+
+	if (!replay_esn->replay_window)
+		return 0;
+
+	pos = (replay_esn->seq - 1) % replay_esn->replay_window;
 
 	if (unlikely(seq == 0))
 		goto err;

@@ -373,12 +381,17 @@ static int xfrm_replay_check_esn(struct xfrm_state *x,
 	unsigned int bitnr, nr;
 	u32 diff;
 	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
+	u32 pos;
 	u32 seq = ntohl(net_seq);
-	u32 pos = (replay_esn->seq - 1) % replay_esn->replay_window;
 	u32 wsize = replay_esn->replay_window;
 	u32 top = replay_esn->seq;
 	u32 bottom = top - wsize + 1;
 
+	if (!wsize)
+		return 0;
+
+	pos = (replay_esn->seq - 1) % replay_esn->replay_window;
+
 	if (unlikely(seq == 0 && replay_esn->seq_hi == 0 &&
 		     (replay_esn->seq < replay_esn->replay_window - 1)))
 		goto err;

Signature · 1 line

1.7.0.4

Steffen Klassert

author maintainer

2011-03-29 05:46 UTC #1 lore raw

PATCH 2/4 xfrm: Assign esn pointers when cloning a state

When we clone a xfrm state we have to assign the replay_esn and the preplay_esn pointers to the state if we use the new replay detection method. To this end, we add a xfrm_replay_clone() function that allocates memory for the replay detection and takes over the necessary values from the original state.

Signed-off-by Steffen Klassert <[email protected]>

---
 include/net/xfrm.h    |   22 ++++++++++++++++++++++
 net/xfrm/xfrm_state.c |    6 ++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

include/net/xfrm.h +22 -0

index cffa5dc..6ae4bc5 100644
@@ -1601,6 +1601,28 @@ static inline int xfrm_replay_state_esn_len(struct xfrm_replay_state_esn *replay
 }
 
 #ifdef CONFIG_XFRM_MIGRATE
+static inline int xfrm_replay_clone(struct xfrm_state *x,
+				     struct xfrm_state *orig)
+{
+	x->replay_esn = kzalloc(xfrm_replay_state_esn_len(orig->replay_esn),
+				GFP_KERNEL);
+	if (!x->replay_esn)
+		return -ENOMEM;
+
+	x->replay_esn->bmp_len = orig->replay_esn->bmp_len;
+	x->replay_esn->replay_window = orig->replay_esn->replay_window;
+
+	x->preplay_esn = kmemdup(x->replay_esn,
+				 xfrm_replay_state_esn_len(x->replay_esn),
+				 GFP_KERNEL);
+	if (!x->preplay_esn) {
+		kfree(x->replay_esn);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
 static inline struct xfrm_algo *xfrm_algo_clone(struct xfrm_algo *orig)
 {
 	return kmemdup(orig, xfrm_alg_len(orig), GFP_KERNEL);

net/xfrm/xfrm_state.c +6 -0

index f83a3d1..dd78536 100644
@@ -1181,6 +1181,12 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig, int *errp)
 			goto error;
 	}
 
+	if (orig->replay_esn) {
+		err = xfrm_replay_clone(x, orig);
+		if (err)
+			goto error;
+	}
+
 	memcpy(&x->mark, &orig->mark, sizeof(x->mark));
 
 	err = xfrm_init_state(x);

Signature · 1 line

1.7.0.4

Steffen Klassert

author maintainer

2011-03-29 05:47 UTC #2 lore raw

PATCH 3/4 xfrm: Check for esn buffer len in xfrm_new_ae

In reply to Steffen Klassert

In xfrm_new_ae() we may overwrite the allocated esn replay state buffer with a wrong size. So check that the new size matches the original allocated size and return an error if this is not the case.

Signed-off-by Steffen Klassert <[email protected]>

---
 net/xfrm/xfrm_user.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

net/xfrm/xfrm_user.c +21 -0

index fc152d2..ccc4c0c 100644
@@ -360,6 +360,23 @@ static int attach_aead(struct xfrm_algo_aead **algpp, u8 *props,
 	return 0;
 }
 
+static inline int xfrm_replay_verify_len(struct xfrm_replay_state_esn *replay_esn,
+					 struct nlattr *rp)
+{
+	struct xfrm_replay_state_esn *up;
+
+	if (!replay_esn || !rp)
+		return 0;
+
+	up = nla_data(rp);
+
+	if (xfrm_replay_state_esn_len(replay_esn) !=
+			xfrm_replay_state_esn_len(up))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int xfrm_alloc_replay_state_esn(struct xfrm_replay_state_esn **replay_esn,
 				       struct xfrm_replay_state_esn **preplay_esn,
 				       struct nlattr *rta)

@@ -1766,6 +1783,10 @@ static int xfrm_new_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (x->km.state != XFRM_STATE_VALID)
 		goto out;
 
+	err = xfrm_replay_verify_len(x->replay_esn, rp);
+	if (err)
+		goto out;
+
 	spin_lock_bh(&x->lock);
 	xfrm_update_ae_params(x, attrs);
 	spin_unlock_bh(&x->lock);

Signature · 1 line

1.7.0.4

Steffen Klassert

author maintainer

2011-03-29 05:48 UTC #3 lore raw

PATCH 4/4 xfrm: Restrict extended sequence numbers to esp

In reply to Steffen Klassert

The IPsec extended sequence numbers are fully implemented just for esp. So restrict the usage to esp until other protocols have support too.

Signed-off-by Steffen Klassert <[email protected]>

---
 net/xfrm/xfrm_user.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

net/xfrm/xfrm_user.c +3 -0

index ccc4c0c..3d15d3e 100644
@@ -127,6 +127,9 @@ static inline int verify_replay(struct xfrm_usersa_info *p,
 	if (!rt)
 		return 0;
 
+	if (p->id.proto != IPPROTO_ESP)
+		return -EINVAL;
+
 	if (p->replay_window != 0)
 		return -EINVAL;

Signature · 1 line

1.7.0.4

Herbert Xu

maintainer Acked-by on v1 1/4

2011-03-29 06:20 UTC #4 lore raw

In reply to Steffen Klassert

Quoted 6 lines · Steffen Klassert

As it is, the replay check is just performed if the replay window of the legacy implementation is nonzero. So we move the test on a nonzero replay window inside the replay check functions to be sure we are testing for the right implementation.
Signed-off-by Steffen Klassert <[email protected]>

Acked-by Herbert Xu <[email protected]>

for all 4 patches.

Thanks,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

David Miller

maintainer

2011-03-29 06:49 UTC #5 lore raw

From: Herbert Xu <[email protected]>
Date: Tue, 29 Mar 2011 14:20:06 +0800

Quoted 11 lines · Steffen Klassert

Quoted 6 lines
As it is, the replay check is just performed if the replay window of the legacy implementation is nonzero. So we move the test on a nonzero replay window inside the replay check functions to be sure we are testing for the right implementation.
Signed-off-by Steffen Klassert <[email protected]>
Acked-by Herbert Xu <[email protected]>
for all 4 patches.

All applied, thanks Steffen.