diff mbox series

xfrm: Forbid state updates from changing encap type

Message ID 20171226063444.GA26819@gondor.apana.org.au
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show
Series xfrm: Forbid state updates from changing encap type | expand

Commit Message

Herbert Xu Dec. 26, 2017, 6:34 a.m. UTC
Currently we allow state updates to competely replace the contents
of x->encap.  This is bad because on the user side ESP only sets up
header lengths depending on encap_type once when the state is first
created.  This could result in the header lengths getting out of
sync with the actual state configuration.

In practice key managers will never do a state update to change the
encapsulation type.  Only the port numbers need to be changed as the
peer NAT entry is updated.

Therefore this patch adds a check in xfrm_state_update to forbid
any changes to the encap_type.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Comments

Steffen Klassert Dec. 31, 2017, 7:49 a.m. UTC | #1
On Tue, Dec 26, 2017 at 05:34:44PM +1100, Herbert Xu wrote:
> Currently we allow state updates to competely replace the contents
> of x->encap.  This is bad because on the user side ESP only sets up
> header lengths depending on encap_type once when the state is first
> created.  This could result in the header lengths getting out of
> sync with the actual state configuration.
> 
> In practice key managers will never do a state update to change the
> encapsulation type.  Only the port numbers need to be changed as the
> peer NAT entry is updated.
> 
> Therefore this patch adds a check in xfrm_state_update to forbid
> any changes to the encap_type.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied to the ipsec tree, thanks Herbert!
diff mbox series

Patch

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 065d896..dc1cdde 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1533,8 +1533,12 @@  int xfrm_state_update(struct xfrm_state *x)
 	err = -EINVAL;
 	spin_lock_bh(&x1->lock);
 	if (likely(x1->km.state == XFRM_STATE_VALID)) {
-		if (x->encap && x1->encap)
+		if (x->encap && x1->encap &&
+		    x->encap->encap_type == x1->encap->encap_type)
 			memcpy(x1->encap, x->encap, sizeof(*x1->encap));
+		else if (x->encap || x1->encap)
+			goto fail;
+
 		if (x->coaddr && x1->coaddr) {
 			memcpy(x1->coaddr, x->coaddr, sizeof(*x1->coaddr));
 		}
@@ -1551,6 +1555,8 @@  int xfrm_state_update(struct xfrm_state *x)
 		x->km.state = XFRM_STATE_DEAD;
 		__xfrm_state_put(x);
 	}
+
+fail:
 	spin_unlock_bh(&x1->lock);
 
 	xfrm_state_put(x1);