[PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[Hagen Paul Pfeifer]

Reduce the attack vector and stop generating IPv6 Fragment Header for
paths with an MTU smaller than the minimum required IPv6 MTU
size (1280 byte) - called atomic fragments.

See IETF I-D "Deprecating the Generation of IPv6 Atomic Fragments" [1]
for more information and how this "feature" can be misused.

[1] https://tools.ietf.org/html/draft-ietf-6man-deprecate-atomfrag-generation-00

Cc: stable@vger.kernel.org
Signed-off-by: Fernando Gont <fgont@si6networks.com>
Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
---
 net/ipv6/route.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 34dcbb5..d4603fb 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1160,12 +1160,9 @@ static void ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
 		struct net *net = dev_net(dst->dev);
 
 		rt6->rt6i_flags |= RTF_MODIFIED;
-		if (mtu < IPV6_MIN_MTU) {
-			u32 features = dst_metric(dst, RTAX_FEATURES);
+		if (mtu < IPV6_MIN_MTU)
 			mtu = IPV6_MIN_MTU;
-			features |= RTAX_FEATURE_ALLFRAG;
-			dst_metric_set(dst, RTAX_FEATURES, features);
-		}
+
 		dst_metric_set(dst, RTAX_MTU, mtu);
 		rt6_update_expires(rt6, net->ipv6.sysctl.ip6_rt_mtu_expires);
 	}
-- 
2.1.4

Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[Hannes Frederic Sowa]

On Do, 2015-01-15 at 22:34 +0100, Hagen Paul Pfeifer wrote:
> Reduce the attack vector and stop generating IPv6 Fragment Header for
> paths with an MTU smaller than the minimum required IPv6 MTU
> size (1280 byte) - called atomic fragments.
> 
> See IETF I-D "Deprecating the Generation of IPv6 Atomic Fragments" [1]
> for more information and how this "feature" can be misused.
> 
> [1] https://tools.ietf.org/html/draft-ietf-6man-deprecate-atomfrag-generation-00
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Fernando Gont <fgont@si6networks.com>
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

I think this is the correct way forward on how to deal with atomic
fragments.

Hagen, do you submit patches to remove dst_allfrag/RTAX_FEATURE_ALLFRAG,
IPCORK_ALLFRAG, etc. for net-next, too?

Thanks,
Hannes

Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[Hagen Paul Pfeifer]

On 19 January 2015 at 14:55, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> I think this is the correct way forward on how to deal with atomic
> fragments.
>
> Hagen, do you submit patches to remove dst_allfrag/RTAX_FEATURE_ALLFRAG,
> IPCORK_ALLFRAG, etc. for net-next, too?

Yes, patch sits already in the pipe. I wanted to wait for davem's pull.

Hagen

Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[David Miller]

From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Mon, 19 Jan 2015 15:00:21 +0100

> On 19 January 2015 at 14:55, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>>
>> I think this is the correct way forward on how to deal with atomic
>> fragments.
>>
>> Hagen, do you submit patches to remove dst_allfrag/RTAX_FEATURE_ALLFRAG,
>> IPCORK_ALLFRAG, etc. for net-next, too?
> 
> Yes, patch sits already in the pipe. I wanted to wait for davem's pull.

It's one thing to change policy about how we might or might not
automatically set this bit in the kernel, but at a minimum you cannot
just remove RTAX_FEATURE_ALLFRAG, it's in a userspace header and
you'll break application builds.

Second of all, there is absolutely no reason to prevent the user from
setting this bit.  If someone wants to set RTAX_FEATURE_ALLFRAG on a
route on their own system, that is their business.

Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[Hannes Frederic Sowa]

On Mo, 2015-01-19 at 14:50 -0500, David Miller wrote:
> From: Hagen Paul Pfeifer <hagen@jauu.net>
> Date: Mon, 19 Jan 2015 15:00:21 +0100
> 
> > On 19 January 2015 at 14:55, Hannes Frederic Sowa
> > <hannes@stressinduktion.org> wrote:
> >> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> >>
> >> I think this is the correct way forward on how to deal with atomic
> >> fragments.
> >>
> >> Hagen, do you submit patches to remove dst_allfrag/RTAX_FEATURE_ALLFRAG,
> >> IPCORK_ALLFRAG, etc. for net-next, too?
> > 
> > Yes, patch sits already in the pipe. I wanted to wait for davem's pull.
> 
> It's one thing to change policy about how we might or might not
> automatically set this bit in the kernel, but at a minimum you cannot
> just remove RTAX_FEATURE_ALLFRAG, it's in a userspace header and
> you'll break application builds.

Sure, the define would need to be left alone.

> Second of all, there is absolutely no reason to prevent the user from
> setting this bit.  If someone wants to set RTAX_FEATURE_ALLFRAG on a
> route on their own system, that is their business.

Oh yes, although we never exposed an ip route knob for that, it is still
possible users did set manually, so we cannot get rid of that, agreed.

Bye,
Hannes

Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[Hagen Paul Pfeifer]

On 19 January 2015 at 21:05, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:

> Oh yes, although we never exposed an ip route knob for that, it is still
> possible users did set manually, so we cannot get rid of that, agreed.

I thought about that, sure but come to the conclusion that the code
cleanup outweigh an potential user somewhere in space (well, the use
case is broken and should be fixed). Additionally, I left the
RTAX_FEATURE_ALLFRAG (as mentioned) and removed all related
functionality - no visible API change (except no-op behavior).

Davem, I will sent the patch anyway. Feel free to accept/ignore.

Hagen

Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[Loganaden Velvindron]

On Mon, Jan 19, 2015 at 10:05 PM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> On 19 January 2015 at 21:05, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>
>> Oh yes, although we never exposed an ip route knob for that, it is still
>> possible users did set manually, so we cannot get rid of that, agreed.
>
> I thought about that, sure but come to the conclusion that the code
> cleanup outweigh an potential user somewhere in space (well, the use
> case is broken and should be fixed). Additionally, I left the
> RTAX_FEATURE_ALLFRAG (as mentioned) and removed all related
> functionality - no visible API change (except no-op behavior).
>
> Davem, I will sent the patch anyway. Feel free to accept/ignore.
>
> Hagen
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi.

Last time I was inquiring about depracated atomic fragments, people
were concerned that there wasn't enough practical data to decide
whether to go forward or not.

Would a sysctl with it turned on by default be a good option, until we
are 100% sure ?

Kind regards,
//Logan
C-x-C-c


-- 
This message is strictly personal and the opinions expressed do not
represent those of my employers, either past or present.

Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[Fernando Gont]

Hi, Loganaden,

On 01/20/2015 01:02 AM, Loganaden Velvindron wrote:
> 
> Last time I was inquiring about depracated atomic fragments, people
> were concerned that there wasn't enough practical data to decide
> whether to go forward or not.

What kind of practical data?

FWIW,
<https://tools.ietf.org/id/draft-ietf-6man-deprecate-atomfrag-generation-00.txt>
seems to be good enough when it comes to reasons for deprecating them.


Besides, please check Section 5.2 of
<http://www.ietf.org/id/draft-gont-v6ops-ipv6-ehs-in-real-world-01.txt>
-- my "connection" to kernel.org was vulnerable to such attack.



> Would a sysctl with it turned on by default be a good option, until we
> are 100% sure ?

<https://tools.ietf.org/id/draft-ietf-6man-deprecate-atomfrag-generation-00.txt>
 was adopted by the 6man wg last November. While the I-D is not ready an
RFC, and there might be minor modifications, it seems that there's
agreement in not generating atomic fragments.

If you do want to have a sysctl for this, please make it default to "off".

Thanks!

Best regards,
-- 
Fernando Gont
SI6 Networks
e-mail: fgont@si6networks.com
PGP Fingerprint: 6666 31C6 D484 63B2 8FB1 E3C4 AE25 0D55 1D4E 7492

Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280

[David Miller]

From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Thu, 15 Jan 2015 22:34:25 +0100

> Reduce the attack vector and stop generating IPv6 Fragment Header for
> paths with an MTU smaller than the minimum required IPv6 MTU
> size (1280 byte) - called atomic fragments.
> 
> See IETF I-D "Deprecating the Generation of IPv6 Atomic Fragments" [1]
> for more information and how this "feature" can be misused.
> 
> [1] https://tools.ietf.org/html/draft-ietf-6man-deprecate-atomfrag-generation-00
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Fernando Gont <fgont@si6networks.com>
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>

Applied, thanks.

But any follow-ups to get rid of RTAX_FEATURE_ALLFRAG altogether are
not to be seriously considered in my opinion.