[PATCH] net/ipv6: Skip policy check to improve compliance

[PATCH] net/ipv6: Skip policy check to improve compliance

[Andrew Boyer]

From: Farrell Woods <farrell_woods@dell.com>

The patch fixes an IPv6 conformance test failure (v6LC_1_2_03a in the
UNH INTACT suite) that occurs specifically when IPsec is in use.  The
test iterates through the set of unassigned protocol numbers (currently,
143 through 252) and inserts these into the next header field of a
Destination Options header.  The expected test result is that an
ICMPv6 Parameter Problem is sent back.  But if there's a policy in
place that requires an active SA between the Test Node and the
Device Under Test (and none exists), the inbound packet is quietly
dropped.

Signed-off-by: Farrell Woods <farrell_woods@dell.com>
---
 net/ipv6/ip6_input.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index c7ed2b6..26259b3 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -409,12 +409,10 @@ void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
 		}
 	} else {
 		if (!raw) {
-			if (xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
-				__IP6_INC_STATS(net, idev,
-						IPSTATS_MIB_INUNKNOWNPROTOS);
-				icmpv6_send(skb, ICMPV6_PARAMPROB,
-					    ICMPV6_UNK_NEXTHDR, nhoff);
-			}
+			__IP6_INC_STATS(net, idev,
+					IPSTATS_MIB_INUNKNOWNPROTOS);
+			icmpv6_send(skb, ICMPV6_PARAMPROB,
+				    ICMPV6_UNK_NEXTHDR, nhoff);
 			kfree_skb(skb);
 		} else {
 			__IP6_INC_STATS(net, idev, IPSTATS_MIB_INDELIVERS);

-- 
2.7.4

Re: [PATCH] net/ipv6: Skip policy check to improve compliance

[David Miller]

Why did you post this twice?

Re: [PATCH] net/ipv6: Skip policy check to improve compliance

[David Miller]

From: Andrew Boyer <andrew.boyer@dell.com>
Date: Fri,  8 Mar 2019 14:01:11 -0500

> From: Farrell Woods <farrell_woods@dell.com>
> 
> The patch fixes an IPv6 conformance test failure (v6LC_1_2_03a in the
> UNH INTACT suite) that occurs specifically when IPsec is in use.  The
> test iterates through the set of unassigned protocol numbers (currently,
> 143 through 252) and inserts these into the next header field of a
> Destination Options header.  The expected test result is that an
> ICMPv6 Parameter Problem is sent back.  But if there's a policy in
> place that requires an active SA between the Test Node and the
> Device Under Test (and none exists), the inbound packet is quietly
> dropped.
> 
> Signed-off-by: Farrell Woods <farrell_woods@dell.com>

First of all, please CC: the IPSEC maintainers on all IPSEC changes.

Second of all, is the conformance test setting up these IPSEC rules?

Re: [PATCH] net/ipv6: Skip policy check to improve compliance

[Herbert Xu]

On Sun, Mar 10, 2019 at 11:47:47AM -0700, David Miller wrote:
> From: Andrew Boyer <andrew.boyer@dell.com>
> Date: Fri,  8 Mar 2019 14:01:11 -0500
> 
> > From: Farrell Woods <farrell_woods@dell.com>
> > 
> > The patch fixes an IPv6 conformance test failure (v6LC_1_2_03a in the
> > UNH INTACT suite) that occurs specifically when IPsec is in use.  The
> > test iterates through the set of unassigned protocol numbers (currently,
> > 143 through 252) and inserts these into the next header field of a
> > Destination Options header.  The expected test result is that an
> > ICMPv6 Parameter Problem is sent back.  But if there's a policy in
> > place that requires an active SA between the Test Node and the
> > Device Under Test (and none exists), the inbound packet is quietly
> > dropped.
> > 
> > Signed-off-by: Farrell Woods <farrell_woods@dell.com>
> 
> First of all, please CC: the IPSEC maintainers on all IPSEC changes.
> 
> Second of all, is the conformance test setting up these IPSEC rules?

On the face of it I don't see why we shouldn't be dropping the
packets when there is a relevant IPsec policy in place as to do
otherwise makes us vulnerable to DoS attacks.

Please provide a rationale why such packets should *not* be dropped
based on a relevant RFC document.

Thanks,
-- 
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] net/ipv6: Skip policy check to improve compliance

[Boyer, Andrew]

On 3/12/19, 1:09 AM, "Herbert Xu" <herbert@gondor.apana.org.au> wrote:    
    On Sun, Mar 10, 2019 at 11:47:47AM -0700, David Miller wrote:
    > From: Andrew Boyer <andrew.boyer@dell.com>
    > Date: Fri,  8 Mar 2019 14:01:11 -0500
    > 
    > > From: Farrell Woods <farrell_woods@dell.com>
    > > 
    > > The patch fixes an IPv6 conformance test failure (v6LC_1_2_03a in the
    > > UNH INTACT suite) that occurs specifically when IPsec is in use.  The
    > > test iterates through the set of unassigned protocol numbers (currently,
    > > 143 through 252) and inserts these into the next header field of a
    > > Destination Options header.  The expected test result is that an
    > > ICMPv6 Parameter Problem is sent back.  But if there's a policy in
    > > place that requires an active SA between the Test Node and the
    > > Device Under Test (and none exists), the inbound packet is quietly
    > > dropped.
    > > 
    > > Signed-off-by: Farrell Woods <farrell_woods@dell.com>
    > 
    > First of all, please CC: the IPSEC maintainers on all IPSEC changes.
    > 
    > Second of all, is the conformance test setting up these IPSEC rules?
    
    On the face of it I don't see why we shouldn't be dropping the
    packets when there is a relevant IPsec policy in place as to do
    otherwise makes us vulnerable to DoS attacks.
    
    Please provide a rationale why such packets should *not* be dropped
    based on a relevant RFC document.
    
    Thanks,

Hello Herbert,
Our product was configured with IPSEC security policies before sending it through the UNH suite. Farrell listed the test that failed in the commit message.

I have no more info to share, since he is no longer available and I was just helping with the formatting, If you are not interested in taking this change, it's fine with us. You can just drop the patch.

Thank you,
Andrew

[PATCH] net/ipv6: Skip policy check to improve compliance

[Andrew Boyer]

From: Farrell Woods <farrell_woods@dell.com>

The patch fixes an IPv6 conformance test failure (v6LC_1_2_03a in the
UNH INTACT suite) that occurs specifically when IPsec is in use.  The
test iterates through the set of unassigned protocol numbers (currently,
143 through 252) and inserts these into the next header field of a
Destination Options header.  The expected test result is that an
ICMPv6 Parameter Problem is sent back.  But if there's a policy in
place that requires an active SA between the Test Node and the
Device Under Test (and none exists), the inbound packet is quietly
dropped.

Signed-off-by: Farrell Woods <farrell_woods@dell.com>
---
 net/ipv6/ip6_input.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index c7ed2b6..26259b3 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -409,12 +409,10 @@ void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
 		}
 	} else {
 		if (!raw) {
-			if (xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
-				__IP6_INC_STATS(net, idev,
-						IPSTATS_MIB_INUNKNOWNPROTOS);
-				icmpv6_send(skb, ICMPV6_PARAMPROB,
-					    ICMPV6_UNK_NEXTHDR, nhoff);
-			}
+			__IP6_INC_STATS(net, idev,
+					IPSTATS_MIB_INUNKNOWNPROTOS);
+			icmpv6_send(skb, ICMPV6_PARAMPROB,
+				    ICMPV6_UNK_NEXTHDR, nhoff);
 			kfree_skb(skb);
 		} else {
 			__IP6_INC_STATS(net, idev, IPSTATS_MIB_INDELIVERS);

-- 
2.7.4