BLAKE2s is faster and more secure. SHA-1 has been broken for a long time
now. This also removes some code complexity, and lets us potentially
remove sha1 from lib, which would further reduce vmlinux size.
Cc: Geert Uytterhoeven <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Jean-Philippe Aumasson <[email protected]>
Cc: [email protected]
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
net/ipv6/addrconf.c | 31 +++++++++----------------------
1 file changed, 9 insertions(+), 22 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3445f8017430..f5cb534aa261 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -61,7 +61,7 @@
#include <linux/delay.h>
#include <linux/notifier.h>
#include <linux/string.h>
-#include <linux/hash.h>
+#include <crypto/blake2s.h>
#include <net/net_namespace.h>
#include <net/sock.h>
@@ -3225,25 +3225,16 @@ static int ipv6_generate_stable_address(struct in6_addr *address,
const struct inet6_dev *idev)
{
static DEFINE_SPINLOCK(lock);
- static __u32 digest[SHA1_DIGEST_WORDS];
- static __u32 workspace[SHA1_WORKSPACE_WORDS];
-
- static union {
- char __data[SHA1_BLOCK_SIZE];
- struct {
- struct in6_addr secret;
- __be32 prefix[2];
- unsigned char hwaddr[MAX_ADDR_LEN];
- u8 dad_count;
- } __packed;
- } data;
-
+ struct {
+ struct in6_addr secret;
+ __be32 prefix[2];
+ unsigned char hwaddr[MAX_ADDR_LEN];
+ u8 dad_count;
+ } __packed data;
struct in6_addr secret;
struct in6_addr temp;
struct net *net = dev_net(idev->dev);
- BUILD_BUG_ON(sizeof(data.__data) != sizeof(data));
-
if (idev->cnf.stable_secret.initialized)
secret = idev->cnf.stable_secret.secret;
else if (net->ipv6.devconf_dflt->stable_secret.initialized)
@@ -3254,20 +3245,16 @@ static int ipv6_generate_stable_address(struct in6_addr *address,
retry:
spin_lock_bh(&lock);
- sha1_init(digest);
memset(&data, 0, sizeof(data));
- memset(workspace, 0, sizeof(workspace));
memcpy(data.hwaddr, idev->dev->perm_addr, idev->dev->addr_len);
data.prefix[0] = address->s6_addr32[0];
data.prefix[1] = address->s6_addr32[1];
data.secret = secret;
data.dad_count = dad_count;
- sha1_transform(digest, data.__data, workspace);
-
temp = *address;
- temp.s6_addr32[2] = (__force __be32)digest[0];
- temp.s6_addr32[3] = (__force __be32)digest[1];
+ blake2s((u8 *)&temp.s6_addr32[2], (u8 *)&data, NULL,
+ sizeof(temp.s6_addr32[2]) * 2, sizeof(data), 0);
spin_unlock_bh(&lock);
--
2.34.1
For the record, I've been able to simplify this even more in my
remove-sha1 branch: https://git.zx2c4.com/linux-dev/log/?h=remove-sha1
. We no longer need the packed struct and we handle that secret a bit
better too. If this patchset moves onto a non-RFC v2, that'll be part
of it.
"Jason A. Donenfeld" <[email protected]> writes:
> BLAKE2s is faster and more secure. SHA-1 has been broken for a long time
> now. This also removes some code complexity, and lets us potentially
> remove sha1 from lib, which would further reduce vmlinux size.
So this one is a bit less obvious than the BPF case: the "stable address
generation" is supposed to result in generating addresses that are,
well, stable. The documentation for the stable_secret sysctl implies
that this should be for the lifetime of the system:
It is recommended to generate this secret during installation
of a system and keep it stable after that.
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)?
Adding some of the people involved in the original work on stable
address generation in the hope that they can shed some light on the
real-world uses for this feature.
-Toke
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Jean-Philippe Aumasson <[email protected]>
> Cc: [email protected]
> Signed-off-by: Jason A. Donenfeld <[email protected]>
> ---
> net/ipv6/addrconf.c | 31 +++++++++----------------------
> 1 file changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 3445f8017430..f5cb534aa261 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -61,7 +61,7 @@
> #include <linux/delay.h>
> #include <linux/notifier.h>
> #include <linux/string.h>
> -#include <linux/hash.h>
> +#include <crypto/blake2s.h>
>
> #include <net/net_namespace.h>
> #include <net/sock.h>
> @@ -3225,25 +3225,16 @@ static int ipv6_generate_stable_address(struct in6_addr *address,
> const struct inet6_dev *idev)
> {
> static DEFINE_SPINLOCK(lock);
> - static __u32 digest[SHA1_DIGEST_WORDS];
> - static __u32 workspace[SHA1_WORKSPACE_WORDS];
> -
> - static union {
> - char __data[SHA1_BLOCK_SIZE];
> - struct {
> - struct in6_addr secret;
> - __be32 prefix[2];
> - unsigned char hwaddr[MAX_ADDR_LEN];
> - u8 dad_count;
> - } __packed;
> - } data;
> -
> + struct {
> + struct in6_addr secret;
> + __be32 prefix[2];
> + unsigned char hwaddr[MAX_ADDR_LEN];
> + u8 dad_count;
> + } __packed data;
> struct in6_addr secret;
> struct in6_addr temp;
> struct net *net = dev_net(idev->dev);
>
> - BUILD_BUG_ON(sizeof(data.__data) != sizeof(data));
> -
> if (idev->cnf.stable_secret.initialized)
> secret = idev->cnf.stable_secret.secret;
> else if (net->ipv6.devconf_dflt->stable_secret.initialized)
> @@ -3254,20 +3245,16 @@ static int ipv6_generate_stable_address(struct in6_addr *address,
> retry:
> spin_lock_bh(&lock);
>
> - sha1_init(digest);
> memset(&data, 0, sizeof(data));
> - memset(workspace, 0, sizeof(workspace));
> memcpy(data.hwaddr, idev->dev->perm_addr, idev->dev->addr_len);
> data.prefix[0] = address->s6_addr32[0];
> data.prefix[1] = address->s6_addr32[1];
> data.secret = secret;
> data.dad_count = dad_count;
>
> - sha1_transform(digest, data.__data, workspace);
> -
> temp = *address;
> - temp.s6_addr32[2] = (__force __be32)digest[0];
> - temp.s6_addr32[3] = (__force __be32)digest[1];
> + blake2s((u8 *)&temp.s6_addr32[2], (u8 *)&data, NULL,
> + sizeof(temp.s6_addr32[2]) * 2, sizeof(data), 0);
>
> spin_unlock_bh(&lock);
>
> --
> 2.34.1
Hi Toke,
On 1/13/22, Toke Høiland-Jørgensen <[email protected]> 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
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.
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
Jason
Hello,
On 13.01.22 00:31, Jason A. Donenfeld wrote:
> On 1/13/22, Toke Høiland-Jørgensen <[email protected]> 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
On Thu, 13 Jan 2022 at 12:15, Hannes Frederic Sowa
<[email protected]> wrote:
>
> Hello,
>
> On 13.01.22 00:31, Jason A. Donenfeld wrote:
> > On 1/13/22, Toke Høiland-Jørgensen <[email protected]> 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?
>
The question is not whether but when we can/will change this.
SHA-1 is broken and should be removed at *some* point, so unless the
feature itself is going to be obsolete, its implementation will need
to switch to a PRF that fulfils the requirements in RFC7217 once SHA-1
ceases to do so.
And I should also point out that the current implementation does not
even use SHA-1 correctly, as it omits the finalization step. This may
or may not matter in practice, but it deviates from crypto best
practices, as well as from RFC7217
I already pointed out to Jason (in private) that the PRF does not need
to be based on a cryptographic hash, so as far as I can tell, siphash
would be a suitable candidate here as well, and I already switched the
TCP fastopen code to that in the past. But SHA-1 definitely has to go.
On 1/13/22, Ard Biesheuvel <[email protected]> wrote:
>
> The question is not whether but when we can/will change this.
>
> SHA-1 is broken and should be removed at *some* point, so unless the
> feature itself is going to be obsolete, its implementation will need
> to switch to a PRF that fulfils the requirements in RFC7217 once SHA-1
> ceases to do so.
>
> And I should also point out that the current implementation does not
> even use SHA-1 correctly, as it omits the finalization step. This may
> or may not matter in practice, but it deviates from crypto best
> practices, as well as from RFC7217
>
> I already pointed out to Jason (in private) that the PRF does not need
> to be based on a cryptographic hash, so as far as I can tell, siphash
> would be a suitable candidate here as well, and I already switched the
> TCP fastopen code to that in the past. But SHA-1 definitely has to go.
>
Correction: this should be a cryptographically secure. That's part of
the point of moving away from SHA-1 of course. But fortunately,
siphash *is*
considered to be cryptographically secure. Whether you want blake2s's
keyed mode or siphash doesn't really matter to me. I thought the
former's API mapped a bit neater here.
On Thu, 13 Jan 2022 at 13:22, Jason A. Donenfeld <[email protected]> wrote:
>
> On 1/13/22, Ard Biesheuvel <[email protected]> wrote:
> >
> > The question is not whether but when we can/will change this.
> >
> > SHA-1 is broken and should be removed at *some* point, so unless the
> > feature itself is going to be obsolete, its implementation will need
> > to switch to a PRF that fulfils the requirements in RFC7217 once SHA-1
> > ceases to do so.
> >
> > And I should also point out that the current implementation does not
> > even use SHA-1 correctly, as it omits the finalization step. This may
> > or may not matter in practice, but it deviates from crypto best
> > practices, as well as from RFC7217
> >
> > I already pointed out to Jason (in private) that the PRF does not need
> > to be based on a cryptographic hash, so as far as I can tell, siphash
> > would be a suitable candidate here as well, and I already switched the
> > TCP fastopen code to that in the past. But SHA-1 definitely has to go.
> >
>
> Correction: this should be a cryptographically secure.
Of course. I said it does not need to be based on a cryptographic *hash*.
> That's part of
> the point of moving away from SHA-1 of course. But fortunately,
> siphash *is*
> considered to be cryptographically secure. Whether you want blake2s's
> keyed mode or siphash doesn't really matter to me. I thought the
> former's API mapped a bit neater here.
Fair enough. This is not on a hot path anyway, so it doesn't really
matter performance wise.
Ard Biesheuvel <[email protected]> writes:
> On Thu, 13 Jan 2022 at 12:15, Hannes Frederic Sowa
> <[email protected]> wrote:
>>
>> Hello,
>>
>> On 13.01.22 00:31, Jason A. Donenfeld wrote:
>> > On 1/13/22, Toke Høiland-Jørgensen <[email protected]> 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?
>>
>
> The question is not whether but when we can/will change this.
>
> SHA-1 is broken and should be removed at *some* point, so unless the
> feature itself is going to be obsolete, its implementation will need
> to switch to a PRF that fulfils the requirements in RFC7217 once SHA-1
> ceases to do so.
>
> And I should also point out that the current implementation does not
> even use SHA-1 correctly, as it omits the finalization step. This may
> or may not matter in practice, but it deviates from crypto best
> practices, as well as from RFC7217
Right, but that implies we need to work on a transition mechanism. For
newly deployed systems changing the hash is obviously fine, it's the
"reboot and you have a new address" problem.
We could introduce new values to the addr_gen_mode? I.e. values of 4 and
5 would be equivalent to 2 and 3 (respectively), but with the new
hashing algorithm? And then document that 2 and 3 are considered
deprecated to be removed at some point in the future...
-Toke
On Thu, 13 Jan 2022 at 14:30, Toke Høiland-Jørgensen <[email protected]> wrote:
>
> Ard Biesheuvel <[email protected]> writes:
>
> > On Thu, 13 Jan 2022 at 12:15, Hannes Frederic Sowa
> > <[email protected]> wrote:
> >>
> >> Hello,
> >>
> >> On 13.01.22 00:31, Jason A. Donenfeld wrote:
> >> > On 1/13/22, Toke Høiland-Jørgensen <[email protected]> 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?
> >>
> >
> > The question is not whether but when we can/will change this.
> >
> > SHA-1 is broken and should be removed at *some* point, so unless the
> > feature itself is going to be obsolete, its implementation will need
> > to switch to a PRF that fulfils the requirements in RFC7217 once SHA-1
> > ceases to do so.
> >
> > And I should also point out that the current implementation does not
> > even use SHA-1 correctly, as it omits the finalization step. This may
> > or may not matter in practice, but it deviates from crypto best
> > practices, as well as from RFC7217
>
> Right, but that implies we need to work on a transition mechanism. For
> newly deployed systems changing the hash is obviously fine, it's the
> "reboot and you have a new address" problem.
>
> We could introduce new values to the addr_gen_mode? I.e. values of 4 and
> 5 would be equivalent to 2 and 3 (respectively), but with the new
> hashing algorithm? And then document that 2 and 3 are considered
> deprecated to be removed at some point in the future...
>
I guess that for the time being, we could use assignments of
stable_secret by user space as a hint that we should switch to the old
scheme. We'd also need a knob to opt into the new scheme in that case,
and maybe print a warning otherwise? That would at least give us a
path forward where we can rip it out /some/ point in the future.
Hi Toke,
On Thu, Jan 13, 2022 at 2:30 PM Toke Høiland-Jørgensen <[email protected]> wrote:
> Right, but that implies we need to work on a transition mechanism. For
> newly deployed systems changing the hash is obviously fine, it's the
> "reboot and you have a new address" problem.
>
> We could introduce new values to the addr_gen_mode? I.e. values of 4 and
> 5 would be equivalent to 2 and 3 (respectively), but with the new
> hashing algorithm? And then document that 2 and 3 are considered
> deprecated to be removed at some point in the future...
Right, so this is exactly the flow of conversation I anticipated.
"Let's change it!" "No, we can't." "Okay, let's add a knob."
The knob I was thinking about, though, was actually a compile-time one
CONFIG_NET_OBSOLETE_INSECURE_ADDRCONF_HASH, which itself is a `depends
on CONFIG_OLD_N_CRUSTY` or something. This way we could gate the
inclusion of sha1.c/sha1.o on that at compile time, and shave down
vmlinux a bit, which would make Geert happy.
Then, at some point down the road, we can talk about removing
CONFIG_NET_OBSOLETE_INSECURE_ADDRCONF_HASH too.
Jason
On Thu, 13 Jan 2022 at 14:46, Jason A. Donenfeld <[email protected]> wrote:
>
> Hi Toke,
>
> On Thu, Jan 13, 2022 at 2:30 PM Toke Høiland-Jørgensen <[email protected]> wrote:
> > Right, but that implies we need to work on a transition mechanism. For
> > newly deployed systems changing the hash is obviously fine, it's the
> > "reboot and you have a new address" problem.
> >
> > We could introduce new values to the addr_gen_mode? I.e. values of 4 and
> > 5 would be equivalent to 2 and 3 (respectively), but with the new
> > hashing algorithm? And then document that 2 and 3 are considered
> > deprecated to be removed at some point in the future...
>
> Right, so this is exactly the flow of conversation I anticipated.
> "Let's change it!" "No, we can't." "Okay, let's add a knob."
>
> The knob I was thinking about, though, was actually a compile-time one
> CONFIG_NET_OBSOLETE_INSECURE_ADDRCONF_HASH, which itself is a `depends
> on CONFIG_OLD_N_CRUSTY` or something. This way we could gate the
> inclusion of sha1.c/sha1.o on that at compile time, and shave down
> vmlinux a bit, which would make Geert happy.
>
> Then, at some point down the road, we can talk about removing
> CONFIG_NET_OBSOLETE_INSECURE_ADDRCONF_HASH too.
>
What is the point of having CONFIG_OLD_N_CRUSTY if all distros are
going to enable it indefinitely?
On Thu, Jan 13, 2022 at 2:50 PM Ard Biesheuvel <[email protected]> wrote:
> > Then, at some point down the road, we can talk about removing
> > CONFIG_NET_OBSOLETE_INSECURE_ADDRCONF_HASH too.
> >
>
> What is the point of having CONFIG_OLD_N_CRUSTY if all distros are
> going to enable it indefinitely?
I think there's probably some combination of
CONFIG_NET_OBSOLETE_INSECURE_ADDRCONF_HASH and CONFIG_OLD_N_CRUSTY and
maybe even a CONFIG_GOD_MURDERS_KITTENS that might be sufficiently
disincentivizing? Or this ties into other general ideas on a gradual
obsolescence->removal flow for things.
"Jason A. Donenfeld" <[email protected]> writes:
> On Thu, Jan 13, 2022 at 2:50 PM Ard Biesheuvel <[email protected]> wrote:
>> > Then, at some point down the road, we can talk about removing
>> > CONFIG_NET_OBSOLETE_INSECURE_ADDRCONF_HASH too.
>> >
>>
>> What is the point of having CONFIG_OLD_N_CRUSTY if all distros are
>> going to enable it indefinitely?
>
> I think there's probably some combination of
> CONFIG_NET_OBSOLETE_INSECURE_ADDRCONF_HASH and CONFIG_OLD_N_CRUSTY and
> maybe even a CONFIG_GOD_MURDERS_KITTENS that might be sufficiently
> disincentivizing? Or this ties into other general ideas on a gradual
> obsolescence->removal flow for things.
Making it a compile-time switch doesn't really solve anything, though.
It'll need to be a runtime switch for people to be able to opt-in to the
new behaviour; otherwise there would still be a flag day when
distributions switch on the new config option.
I don't think there's any reason to offload this decision on
distributions either: there's clearly a "best option" here, absent any
backwards compatibility concerns. So it's on us to design a proper
transition mechanism. Defaulting to SHA1 when stable_secret is set, as
Ard suggested, sounds like a reasonable default; then we only need a
single new value for addr_gen_mode to opt-in to using blake2s even when
setting the stable_secret.
-Toke
Hi Hannes,
On Thu, Jan 13, 2022 at 12:15 PM Hannes Frederic Sowa
<[email protected]> wrote:
> > 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
> > guarantee 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.
I don't mean to belabor a point where I'm likely wrong anyway, but
this DAD business has kept me thinking...
Attacker is hanging out on the network sending DAD responses, forcing
those counters to increment, and thus making SHA1(stuff || counter)
result in a different IPv6 address than usual. Outcomes:
1) The administrator cannot handle this, did not understand the
semantics of this address generation feature, and will now have a
broken network;
2) The administrator knows what he's doing, and will be able to handle
a different IPv6 address coming up.
Do we really care about case (1)? That sounds like emacs spacebar
heating https://xkcd.com/1172/. And case (2) seems like something that
would tolerate us changing the hash function.
> Afterwards bringing the interface down and
> up again should revert the interface to its initial (dad_counter == 0)
> address.
Except the attacker is still on the network, and the administrator
can't figure it out because the mac addresses keep changing and it's
arriving from seemingly random switches! Plot twist: the attack is
being conducted from an implant in the switch firmware. There are a
lot of creative different takes on the same basic scenario. The point
is - the administrator really _can't_ rely on the address always being
the same, because it's simply out of his control.
Given that the admin already *must* be prepared for the address to
change, doesn't that give us some leeway to change the algorithm used
between kernels?
Or to put it differently, are there _actually_ braindead deployments
out there that truly rely on the address never ever changing, and
should we be going out of our way to support what is arguably a
misreading and misdeployment of the feature?
(Feel free to smack this line of argumentation down if you disagree. I
just thought it should be a bit more thoroughly explored.)
Jason
"Jason A. Donenfeld" <[email protected]> writes:
> Hi Hannes,
>
> On Thu, Jan 13, 2022 at 12:15 PM Hannes Frederic Sowa
> <[email protected]> wrote:
>> > 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
>> > guarantee 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.
>
> I don't mean to belabor a point where I'm likely wrong anyway, but
> this DAD business has kept me thinking...
>
> Attacker is hanging out on the network sending DAD responses, forcing
> those counters to increment, and thus making SHA1(stuff || counter)
> result in a different IPv6 address than usual. Outcomes:
> 1) The administrator cannot handle this, did not understand the
> semantics of this address generation feature, and will now have a
> broken network;
> 2) The administrator knows what he's doing, and will be able to handle
> a different IPv6 address coming up.
>
> Do we really care about case (1)? That sounds like emacs spacebar
> heating https://xkcd.com/1172/. And case (2) seems like something that
> would tolerate us changing the hash function.
Privacy addresses mostly address identification outside of the local
network (because on the local network you can see the MAC address), so I
don't think it's unreasonable for someone to enable this and not have a
procedure in place to deal with DAD causing the address to change. For
instance, they could manage their network in a way that they won't
happen (or just turn off DAD entirely on the affected boxes).
>> Afterwards bringing the interface down and
>> up again should revert the interface to its initial (dad_counter == 0)
>> address.
>
> Except the attacker is still on the network, and the administrator
> can't figure it out because the mac addresses keep changing and it's
> arriving from seemingly random switches! Plot twist: the attack is
> being conducted from an implant in the switch firmware. There are a
> lot of creative different takes on the same basic scenario. The point
> is - the administrator really _can't_ rely on the address always being
> the same, because it's simply out of his control.
>
> Given that the admin already *must* be prepared for the address to
> change, doesn't that give us some leeway to change the algorithm used
> between kernels?
>
> Or to put it differently, are there _actually_ braindead deployments
> out there that truly rely on the address never ever changing, and
> should we be going out of our way to support what is arguably a
> misreading and misdeployment of the feature?
>
> (Feel free to smack this line of argumentation down if you disagree. I
> just thought it should be a bit more thoroughly explored.)
I kinda get where you're coming from, but most systems are not actively
under attack, and those will still "break" if this is just changed.
Which is one of those "a kernel upgrade broke my system" type of events
that we want to avoid because it makes people vary of upgrading, so
they'll keep running old kernels way past their expiry dates.
-Toke
Hello,
On Fri, Jan 14, 2022, at 17:07, Jason A. Donenfeld wrote:
> On Thu, Jan 13, 2022 at 12:15 PM Hannes Frederic Sowa
> <[email protected]> wrote:
>> > 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
>> > guarantee 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.
>
> I don't mean to belabor a point where I'm likely wrong anyway, but
> this DAD business has kept me thinking...
>
> Attacker is hanging out on the network sending DAD responses, forcing
> those counters to increment, and thus making SHA1(stuff || counter)
> result in a different IPv6 address than usual. Outcomes:
> 1) The administrator cannot handle this, did not understand the
> semantics of this address generation feature, and will now have a
> broken network;
> 2) The administrator knows what he's doing, and will be able to handle
> a different IPv6 address coming up.
>
> Do we really care about case (1)? That sounds like emacs spacebar
> heating https://xkcd.com/1172/. And case (2) seems like something that
> would tolerate us changing the hash function.
Taking a step back, there is the base case where we don't have duplicate
addresses on the network nor an attack is on-going. We would break those
setups with that patch. And those are the ones that matter most. In
particular those stable-random addresses are being used in router
advertisements for announcing the next-hop/default gateway on the
broadcast domain. During my time in IPv6 land I have seen lots of setups
where those automatic advertisements got converted into static
configuration for the sake of getting hands on a cool looking IPv6
address on another host (I did that as well ;) ). In particular, in the
last example, you might not only have one administrator at hand to
handle the issue, but probably multiple roles are involved (host admin
and network admin maybe from different organizations - how annoying -
but that's a worst case scenario).
Furthermore most L2 attacks nowadays are stopped by smarter switches or
wifi access points(?) anyway with per-port MAC learning and other
hardening features. Obviously this only happens in more managed
environments but probably already also at smaller home networks
nowadays. Datacenters probably already limit access to the Layer 2 raw
network in such a way that this attack is probably not possible either.
Same for IoT stuff where you probably have a point-to-point IPv6
connection anyway.
The worst case scenario is someone upgrading their kernel during a
trip away from home, rebooting, and losing access to their system. If we
experience just one of those cases we have violated Linux strict uAPI
rules (in my opinion). Thus, yes, we care about both, (1) and (2) cases.
I don't think we can argue our way out of this by stating that there are
no guarantees anyway, as much as I would like to change the hash
function as well.
As much as I know about the problems with SHA1 and would like to see it
removed from the kernel as well, I fear that in this case it seems hard
to do. I would propose putting sha1 into a compilation unit and
overwrite the compiler flags to optimize the function optimized for size
and maybe add another mode or knob to switch the hashing algorithm if
necessary.
>> Afterwards bringing the interface down and
>> up again should revert the interface to its initial (dad_counter == 0)
>> address.
>
> Except the attacker is still on the network, and the administrator
> can't figure it out because the mac addresses keep changing and it's
> arriving from seemingly random switches! Plot twist: the attack is
> being conducted from an implant in the switch firmware. There are a
> lot of creative different takes on the same basic scenario. The point
> is - the administrator really _can't_ rely on the address always being
> the same, because it's simply out of his control.
This is a very pessimistic scenario bordering a nightmare. I hope the
new hashing algorithm will protect them. ;)
> Given that the admin already *must* be prepared for the address to
> change, doesn't that give us some leeway to change the algorithm used
> between kernels?
>
> Or to put it differently, are there _actually_ braindead deployments
> out there that truly rely on the address never ever changing, and
> should we be going out of our way to support what is arguably a
> misreading and misdeployment of the feature?
Given the example above, users might hardcode this generated IP address
as a default gateway in their configs on other hosts. This is actually a
very common thing to do.
> (Feel free to smack this line of argumentation down if you disagree. I
> just thought it should be a bit more thoroughly explored.)
I haven't investigated recent research into breakage of SHA1, I mostly
remember the chosen-image and collision attacks against it. Given the
particular usage of SHA1 in this case, do you think switching the
hashing function increases security? I am asking because of the desire
to decrease the instruction size of the kernel, but adding a switch
will actually increase the size in the foreseeable future (and I agree
with Toke that offloading this decision to distributions is probably
not fair).
Maybe at some point the networking subsystem will adapt a generic knob
like LD_ASSUME_KERNEL? ;)
Bye,
Hannes
Hi Hannes,
On Fri, Jan 14, 2022 at 6:44 PM Hannes Frederic Sowa
<[email protected]> wrote:
> I don't think we can argue our way out of this by stating that there are
> no guarantees anyway, as much as I would like to change the hash
> function as well.
Shucks. Alright then.
> As much as I know about the problems with SHA1 and would like to see it
> removed from the kernel as well, I fear that in this case it seems hard
> to do. I would propose putting sha1 into a compilation unit and
> overwrite the compiler flags to optimize the function optimized for size
> and maybe add another mode or knob to switch the hashing algorithm if
> necessary.
Already on it! :)
https://lore.kernel.org/linux-crypto/[email protected]/
> I haven't investigated recent research into breakage of SHA1, I mostly
> remember the chosen-image and collision attacks against it. Given the
> particular usage of SHA1 in this case, do you think switching the
> hashing function increases security?
Considering we're only using 64-bits of SHA-1 output, I don't think
the SHA-1 collision attacks give you that much here. And it seems like
there are other network-level security concerns with the whole scheme
anyway. So it might not be the largest of matters. However...
> I am asking because of the desire
> to decrease the instruction size of the kernel
Indeed this is what I was hoping for.
Jason