netdev - Re: [PATCH net] net/ipv6: allow token to be set when accept

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f216cb8ed643979e9e71da0f0af361340fd444fd.camel@redhat.com>
Date:   Fri, 10 Apr 2020 10:11:04 +0200
From:   Thomas Haller <thaller@...hat.com>
To:     David Miller <davem@...emloft.net>, liuhangbin@...il.com
Cc:     netdev@...r.kernel.org, daniel@...earbox.net,
        yoshfuji@...ux-ipv6.org
Subject: Re: [PATCH net] net/ipv6: allow token to be set when accept_ra
 disabled

On Thu, 2020-04-09 at 10:13 -0700, David Miller wrote:
> From: Hangbin Liu <liuhangbin@...il.com>
> Date: Thu,  9 Apr 2020 14:56:04 +0800
> 
> > The token setting should not depend on whether accept_ra is enabled
> or
> > disabled. The user could set the token at any time. Enable or
> disable
> > accept_ra only affects when the token address take effective.
> > 
> > On the other hand, we didn't remove the token setting when disable
> > accept_ra. So let's just remove the accept_ra checking when user
> want
> > to set token address.
> > 
> > Fixes: f53adae4eae5 ("net: ipv6: add tokenized interface identifier
> support")
> > Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
> 

Hi,

I don't agree.

> It is dangerous to change this, because now people can write bootup
> and configuration scripts that will work with newer kernels yet fail
> unexpectedly in older kernels.

This concern is a very strict interpretation of "forward
compatibility".

The patch relaxes a check. Every change that enables something that
wasn't possible before has this danger. It seems acceptable that people
cannot use newer kernel features on kernel that don't support it. 

> I think requiring that RA be enabled in order to set the token is
> an absolutely reasonable requirement.

That seems to be the real problem: "Why would you posibly want to do
this?"

1)

NetworkManager sets accept_ra=0, because it does autoconf in user
space. It supports tokens, which are entirely handled in user space.

However, when using tokens, NetworkManager likes to configure the token
also in kernel. Yes, it's not overly useful, but it's pretty nice that
you see the token in `ip token` too.

This wasn't an issue until recently, because NetworkManager didn't
actually set accept_ra=0.

2)

If you want to set

  a) token ::1 dev eth0
  b) echo 1 >  /proc/sys/net/ipv6/conf/eth0/accept_ra
  c) ip link set
eth0 up

then you can do the 3 steps in several different orders, but not in the
most(?) sensible one: a,b,c).

Yes, this makes the earlier concern about the danger of people doing
the sensible thing on newer kernels bigger.

3)

There is the oddity that

  # echo 1 >  /proc/sys/net/ipv6/conf/w/accept_ra
  # ip token set ::123 dev w
  # ip token

shows the token. Then, 

  # echo 0 >  /proc/sys/net/ipv6/conf/w/accept_ra
  # ip token

still shows the token. The EINVAL indicates you that having a token
with accept_ra=0 is wrong. But still, it shows a token set, and you
have no way of clearing it (except toggling accept_ra).

I don't care so much about 1) either. If this is really how kernel
wants to do it, fine. NetworkManager won't set the token. It just
doesn't seem sensible to me.

best,
Thomas

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)