xfrm: Return error on unknown switch in init_state

xfrm: Return error on unknown switch in init_state

[Herbert Xu]

Currently esp will happily create an xfrm state with an unknown
encap type for IPv4 or an unknown mode for IPv6, without setting
the necessary state parameters.  This patch fixes it by returning
-EINVAL.

Fixes: 38320c70d282 ("[IPSEC]: Use crypto_aead and authenc in ESP")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index d57aa64..61fe6e4 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -981,6 +981,7 @@ static int esp_init_state(struct xfrm_state *x)
 
 		switch (encap->encap_type) {
 		default:
+			err = -EINVAL;
 			goto error;
 		case UDP_ENCAP_ESPINUDP:
 			x->props.header_len += sizeof(struct udphdr);
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index a902ff8..f2130ff 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -896,6 +896,7 @@ static int esp6_init_state(struct xfrm_state *x)
 		x->props.header_len += sizeof(struct ipv6hdr);
 		break;
 	default:
+		err = -EINVAL;
 		goto error;
 	}
 
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: xfrm: Return error on unknown switch in init_state

[Steffen Klassert]

On Thu, Jan 04, 2018 at 10:21:04PM +1100, Herbert Xu wrote:
> Currently esp will happily create an xfrm state with an unknown
> encap type for IPv4 or an unknown mode for IPv6, without setting
> the necessary state parameters.  This patch fixes it by returning
> -EINVAL.

Looks like we catch the unknown mode in __xfrm_init_state().
But in any case, if we want to return -EINVAL on unknown mode,
we should do it for IPv6 and for IPv4.

[PATCH v2] xfrm: Return error on unknown encap_type in init_state

[Herbert Xu]

On Fri, Jan 05, 2018 at 09:32:47AM +0100, Steffen Klassert wrote:
>
> Looks like we catch the unknown mode in __xfrm_init_state().
> But in any case, if we want to return -EINVAL on unknown mode,
> we should do it for IPv6 and for IPv4.

OK, how about this one then:

---8<---
Currently esp will happily create an xfrm state with an unknown
encap type for IPv4, without setting the necessary state parameters.
This patch fixes it by returning -EINVAL.

There is a similar problem in IPv6 where if the mode is unknown
we will skip initialisation while returning zero.  However, this
is harmless as the mode has already been checked further up the
stack.  This patch removes this anomaly by aligning the IPv6
behaviour with IPv4 and treating unknown modes (which cannot
actually happen) as transport mode.

Fixes: 38320c70d282 ("[IPSEC]: Use crypto_aead and authenc in ESP")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index d57aa64..61fe6e4 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -981,6 +981,7 @@ static int esp_init_state(struct xfrm_state *x)
 
 		switch (encap->encap_type) {
 		default:
+			err = -EINVAL;
 			goto error;
 		case UDP_ENCAP_ESPINUDP:
 			x->props.header_len += sizeof(struct udphdr);
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index a902ff8..1a7f00c 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -890,13 +890,12 @@ static int esp6_init_state(struct xfrm_state *x)
 			x->props.header_len += IPV4_BEET_PHMAXLEN +
 					       (sizeof(struct ipv6hdr) - sizeof(struct iphdr));
 		break;
+	default:
 	case XFRM_MODE_TRANSPORT:
 		break;
 	case XFRM_MODE_TUNNEL:
 		x->props.header_len += sizeof(struct ipv6hdr);
 		break;
-	default:
-		goto error;
 	}
 
 	align = ALIGN(crypto_aead_blocksize(aead), 4);
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v2] xfrm: Return error on unknown encap_type in init_state

[Steffen Klassert]

On Fri, Jan 05, 2018 at 10:12:32PM +1100, Herbert Xu wrote:
> On Fri, Jan 05, 2018 at 09:32:47AM +0100, Steffen Klassert wrote:
> >
> > Looks like we catch the unknown mode in __xfrm_init_state().
> > But in any case, if we want to return -EINVAL on unknown mode,
> > we should do it for IPv6 and for IPv4.
> 
> OK, how about this one then:
> 
> ---8<---
> Currently esp will happily create an xfrm state with an unknown
> encap type for IPv4, without setting the necessary state parameters.
> This patch fixes it by returning -EINVAL.
> 
> There is a similar problem in IPv6 where if the mode is unknown
> we will skip initialisation while returning zero.  However, this
> is harmless as the mode has already been checked further up the
> stack.  This patch removes this anomaly by aligning the IPv6
> behaviour with IPv4 and treating unknown modes (which cannot
> actually happen) as transport mode.
> 
> Fixes: 38320c70d282 ("[IPSEC]: Use crypto_aead and authenc in ESP")
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Patch applied, thanks Herbert!