Message-ID: <55d185a8-31ea-51d0-d9be-debd490cd204@stressinduktion.org>
Date:   Thu, 13 Jan 2022 12:15:36 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Content-Language: en-US
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>,
        =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@redhat.com>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Ard Biesheuvel <ardb@kernel.org>,
        Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>,
        linux-crypto@vger.kernel.org, Erik Kline <ek@google.com>,
        Fernando Gont <fgont@si6networks.com>,
        Lorenzo Colitti <lorenzo@google.com>,
        hideaki.yoshifuji@miraclelinux.com
References: <20220112131204.800307-1-Jason@zx2c4.com>
 <20220112131204.800307-3-Jason@zx2c4.com> <87r19cftbr.fsf@toke.dk>
 <CAHmME9pieaBBhKc1uKABjTmeKAL_t-CZa_WjCVnUr_Y1_D7A0g@mail.gmail.com>
From:   Hannes Frederic Sowa <hannes@stressinduktion.org>
Subject: Re: [PATCH RFC v1 2/3] ipv6: move from sha1 to blake2s in address
 calculation
In-Reply-To: <CAHmME9pieaBBhKc1uKABjTmeKAL_t-CZa_WjCVnUr_Y1_D7A0g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hello,

On 13.01.22 00:31, Jason A. Donenfeld wrote:
> On 1/13/22, Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> However, if we make this change, systems setting a stable_secret and
>> using addr_gen_mode 2 or 3 will come up with a completely different
>> address after a kernel upgrade. Which would be bad for any operator
>> expecting to be able to find their machine again after a reboot,
>> especially if it is accessed remotely.
>>
>> I haven't ever used this feature myself, though, or seen it in use. So I
>> don't know if this is purely a theoretical concern, or if the
>> stable_address feature is actually used in this way in practice. If it
>> is, I guess the switch would have to be opt-in, which kinda defeats the
>> purpose, no (i.e., we'd have to keep the SHA1 code around

Yes, it is hard to tell if such a change would have real world impact 
due to not knowing its actual usage in the field - but I would avoid 
such a change. The reason for this standard is to have stable addresses 
across reboots. The standard is widely used but most servers or desktops 
might get their stable privacy addresses being generated by user space 
network management systems (NetworkManager/networkd) nowadays. I would 
guess it could be used in embedded installations.

The impact of this change could be annoying though: users could suddenly 
lose connectivity due to e.g. changes to the default gateway after an 
upgrade.

> I'm not even so sure that's true. That was my worry at first, but
> actually, looking at this more closely, DAD means that the address can
> be changed anyway - a byte counter is hashed in - so there's no
> gurantee there.

The duplicate address detection counter is a way to merely provide basic 
network connectivity in case of duplicate addresses on the network 
(maybe some kind misconfiguration or L2 attack). Such detected addresses 
would show up in the kernel log and an administrator should investigate 
and clean up the situation. Afterwards bringing the interface down and 
up again should revert the interface to its initial (dad_counter == 0) 
address.

> There's also the other aspect that open coding sha1_transform like
> this and prepending it with the secret (rather than a better
> construction) isn't so great... Take a look at the latest version of
> this in my branch to see a really nice simplification and security
> improvement:
> 
> https://git.zx2c4.com/linux-dev/log/?h=remove-sha1

All in all, I consider the hash produced here as being part of uAPI 
unfortunately and thus cannot be changed. It is unfortunate that it 
can't easily be improved (I assume a separate mode for this is not 
reasonable). The patches definitely look like a nice cleanup.

Would this be the only user of sha_transform left?

Bye,
Hannes