netdev - Re: [PATCH] ipv6: Add IN6_ADDR_GEN_MODE_STABLE_PRIVACY

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 May 2020 15:16:53 +0200
From:   Bram Bonné <brambonne@...gle.com>
To:     Lorenzo Colitti <lorenzo@...gle.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Linux NetDev <netdev@...r.kernel.org>,
        Jeffrey Vander Stoep <jeffv@...gle.com>,
        Maciej Żenczykowski <maze@...gle.com>
Subject: Re: [PATCH] ipv6: Add IN6_ADDR_GEN_MODE_STABLE_PRIVACY_SOFTMAC mode

Thanks for your comments Lorenzo!

On Tue, May 19, 2020 at 6:11 PM Lorenzo Colitti <lorenzo@...gle.com> wrote:
>
> On Tue, May 19, 2020 at 9:08 PM Bram Bonné <brambonne@...gle.com> wrote:
> > @@ -381,7 +382,8 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
> >         timer_setup(&ndev->rs_timer, addrconf_rs_timer, 0);
> >         memcpy(&ndev->cnf, dev_net(dev)->ipv6.devconf_dflt, sizeof(ndev->cnf));
> >
> > -       if (ndev->cnf.stable_secret.initialized)
> > +       if (ndev->cnf.stable_secret.initialized &&
> > +           !ipv6_addr_gen_use_softmac(ndev))
> >                 ndev->cnf.addr_gen_mode = IN6_ADDR_GEN_MODE_STABLE_PRIVACY;
>
> Looks like if stable_secret is set, then when the interface is brought
> up it defaults to stable privacy addresses. But if
> ipv6_addr_gen_use_softmac(), then this remains unset (which means...
> EUI-64?) Any reason you don't set it to
> IN6_ADDR_GEN_MODE_STABLE_PRIVACY_SOFTMAC in this case?

ipv6_addr_gen_use_softmac() means that addr_gen_mode is set to
IN6_ADDR_GEN_MODE_STABLE_PRIVACY_SOFTMAC. I was debating making that
explicit here, instead of using the inline function. It sounds like
that might be a better idea to make the implementation clearer?

The intention here (and below) was to keep the existing default
behavior of using IN6_ADDR_GEN_MODE_STABLE_PRIVACY if stable_secret is
set, while allowing to override this behavior by setting add_gen_mode
to IN6_ADDR_GEN_MODE_STABLE_PRIVACY_SOFTMAC. I tested this locally,
and it seems to work as expected, but please let me know if you think
the above approach does not make sense (or if you'd prefer
IN6_ADDR_GEN_MODE_STABLE_PRIVACY_SOFTMAC to be the default instead).

> Since you haven't changed the netlink code, I assume that this address
> is going to appear to userspace as IFA_F_STABLE_PRIVACY. I assume
> that's what we want here? It's not really "stable", it's only as
> stable as the MAC address. Does the text of the RFC support this
> definition of "stable"?

The RFC considers the generated identifiers as: "stable for each
network interface within each subnet, but [changing] as a host moves
from one network to another". If the MAC address stays stable for a
specific subnet, this implementation fits that definition.
That being said, if you think it makes sense to define a separate
IFA_F_STABLE_PRIVACY_SOFTMAC flag (or some better name), I'm happy to
add that in.

> Can it happen that the MAC address when the device is up and an IPv6
> address already exists? If so, what happens to the address? Will the
> system create a second stable privacy address when the next RA
> arrives? That seems bad. But perhaps this cannot happen.

Trying to change the MAC when the device is up errors with EBUSY on my
machines, so I was under the assumption that the device needs to be
down to change the MAC. I could very well be wrong though.