Hi,
There are currently two remaining users of SHA-1 left in the kernel: bpf
tag generation, and ipv6 address calculation. In an effort to reduce
code size and rid ourselves of insecure primitives, this RFC patchset
moves to using the more secure BLAKE2s function. I wanted to get your
feedback on how feasible this patchset is, and if there is some
remaining attachment to SHA-1, why exactly, and what could be done to
mitigate it. Rather than sending a mailing list post just asking, "what
do you think?" I figured it'd be easier to send this as an RFC patchset,
so you see specifically what I mean.
Thoughts? Comments?
Thanks,
Jason
Cc: Geert Uytterhoeven <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Jean-Philippe Aumasson <[email protected]>
Cc: [email protected]
Jason A. Donenfeld (3):
bpf: move from sha1 to blake2s in tag calculation
ipv6: move from sha1 to blake2s in address calculation
crypto: sha1_generic - import lib/sha1.c locally
crypto/sha1_generic.c | 114 +++++++++++++++++++++++++++++++++++
include/crypto/sha1.h | 10 ---
kernel/bpf/core.c | 39 ++----------
lib/Makefile | 2 +-
lib/sha1.c | 137 ------------------------------------------
net/ipv6/addrconf.c | 31 +++-------
6 files changed, 128 insertions(+), 205 deletions(-)
delete mode 100644 lib/sha1.c
--
2.34.1
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
With no non-crypto API users of this function, we can move it into the
generic crypto/ code where it belongs.
Cc: Geert Uytterhoeven <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: [email protected]
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
crypto/sha1_generic.c | 114 +++++++++++++++++++++++++++++++++++
include/crypto/sha1.h | 10 ---
lib/Makefile | 2 +-
lib/sha1.c | 137 ------------------------------------------
4 files changed, 115 insertions(+), 148 deletions(-)
delete mode 100644 lib/sha1.c
diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c
index 325b57fe28dc..a2b019803561 100644
--- a/crypto/sha1_generic.c
+++ b/crypto/sha1_generic.c
@@ -16,9 +16,123 @@
#include <linux/module.h>
#include <linux/mm.h>
#include <linux/types.h>
+#include <linux/bitops.h>
+#include <linux/string.h>
#include <crypto/sha1.h>
#include <crypto/sha1_base.h>
#include <asm/byteorder.h>
+#include <asm/unaligned.h>
+
+#define SHA1_DIGEST_WORDS (SHA1_DIGEST_SIZE / 4)
+#define SHA1_WORKSPACE_WORDS 16
+
+/*
+ * If you have 32 registers or more, the compiler can (and should)
+ * try to change the array[] accesses into registers. However, on
+ * machines with less than ~25 registers, that won't really work,
+ * and at least gcc will make an unholy mess of it.
+ *
+ * So to avoid that mess which just slows things down, we force
+ * the stores to memory to actually happen (we might be better off
+ * with a 'W(t)=(val);asm("":"+m" (W(t))' there instead, as
+ * suggested by Artur Skawina - that will also make gcc unable to
+ * try to do the silly "optimize away loads" part because it won't
+ * see what the value will be).
+ *
+ * Ben Herrenschmidt reports that on PPC, the C version comes close
+ * to the optimized asm with this (ie on PPC you don't want that
+ * 'volatile', since there are lots of registers).
+ *
+ * On ARM we get the best code generation by forcing a full memory barrier
+ * between each SHA_ROUND, otherwise gcc happily get wild with spilling and
+ * the stack frame size simply explode and performance goes down the drain.
+ */
+
+#ifdef CONFIG_X86
+ #define setW(x, val) (*(volatile __u32 *)&W(x) = (val))
+#elif defined(CONFIG_ARM)
+ #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
+#else
+ #define setW(x, val) (W(x) = (val))
+#endif
+
+/* This "rolls" over the 512-bit array */
+#define W(x) (array[(x)&15])
+
+/*
+ * Where do we get the source from? The first 16 iterations get it from
+ * the input data, the next mix it from the 512-bit array.
+ */
+#define SHA_SRC(t) get_unaligned_be32((__u32 *)data + t)
+#define SHA_MIX(t) rol32(W(t+13) ^ W(t+8) ^ W(t+2) ^ W(t), 1)
+
+#define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \
+ __u32 TEMP = input(t); setW(t, TEMP); \
+ E += TEMP + rol32(A,5) + (fn) + (constant); \
+ B = ror32(B, 2); \
+ TEMP = E; E = D; D = C; C = B; B = A; A = TEMP; } while (0)
+
+#define T_0_15(t, A, B, C, D, E) SHA_ROUND(t, SHA_SRC, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
+#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
+#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0x6ed9eba1, A, B, C, D, E )
+#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E )
+#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0xca62c1d6, A, B, C, D, E )
+
+/**
+ * sha1_transform - single block SHA1 transform (deprecated)
+ *
+ * @digest: 160 bit digest to update
+ * @data: 512 bits of data to hash
+ * @array: 16 words of workspace (see note)
+ *
+ * This function executes SHA-1's internal compression function. It updates the
+ * 160-bit internal state (@digest) with a single 512-bit data block (@data).
+ *
+ * Don't use this function. SHA-1 is no longer considered secure. And even if
+ * you do have to use SHA-1, this isn't the correct way to hash something with
+ * SHA-1 as this doesn't handle padding and finalization.
+ *
+ * Note: If the hash is security sensitive, the caller should be sure
+ * to clear the workspace. This is left to the caller to avoid
+ * unnecessary clears between chained hashing operations.
+ */
+static void sha1_transform(__u32 *digest, const char *data, __u32 *array)
+{
+ __u32 A, B, C, D, E;
+ unsigned int i = 0;
+
+ A = digest[0];
+ B = digest[1];
+ C = digest[2];
+ D = digest[3];
+ E = digest[4];
+
+ /* Round 1 - iterations 0-16 take their input from 'data' */
+ for (; i < 16; ++i)
+ T_0_15(i, A, B, C, D, E);
+
+ /* Round 1 - tail. Input from 512-bit mixing array */
+ for (; i < 20; ++i)
+ T_16_19(i, A, B, C, D, E);
+
+ /* Round 2 */
+ for (; i < 40; ++i)
+ T_20_39(i, A, B, C, D, E);
+
+ /* Round 3 */
+ for (; i < 60; ++i)
+ T_40_59(i, A, B, C, D, E);
+
+ /* Round 4 */
+ for (; i < 80; ++i)
+ T_60_79(i, A, B, C, D, E);
+
+ digest[0] += A;
+ digest[1] += B;
+ digest[2] += C;
+ digest[3] += D;
+ digest[4] += E;
+}
const u8 sha1_zero_message_hash[SHA1_DIGEST_SIZE] = {
0xda, 0x39, 0xa3, 0xee, 0x5e, 0x6b, 0x4b, 0x0d,
diff --git a/include/crypto/sha1.h b/include/crypto/sha1.h
index 044ecea60ac8..118a3cad5eb3 100644
--- a/include/crypto/sha1.h
+++ b/include/crypto/sha1.h
@@ -33,14 +33,4 @@ extern int crypto_sha1_update(struct shash_desc *desc, const u8 *data,
extern int crypto_sha1_finup(struct shash_desc *desc, const u8 *data,
unsigned int len, u8 *hash);
-/*
- * An implementation of SHA-1's compression function. Don't use in new code!
- * You shouldn't be using SHA-1, and even if you *have* to use SHA-1, this isn't
- * the correct way to hash something with SHA-1 (use crypto_shash instead).
- */
-#define SHA1_DIGEST_WORDS (SHA1_DIGEST_SIZE / 4)
-#define SHA1_WORKSPACE_WORDS 16
-void sha1_init(__u32 *buf);
-void sha1_transform(__u32 *digest, const char *data, __u32 *W);
-
#endif /* _CRYPTO_SHA1_H */
diff --git a/lib/Makefile b/lib/Makefile
index 364c23f15578..83ac3f0c1fbe 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -29,7 +29,7 @@ endif
lib-y := ctype.o string.o vsprintf.o cmdline.o \
rbtree.o radix-tree.o timerqueue.o xarray.o \
- idr.o extable.o sha1.o irq_regs.o argv_split.o \
+ idr.o extable.o irq_regs.o argv_split.o \
flex_proportions.o ratelimit.o show_mem.o \
is_single_threaded.o plist.o decompress.o kobject_uevent.o \
earlycpio.o seq_buf.o siphash.o dec_and_lock.o \
diff --git a/lib/sha1.c b/lib/sha1.c
deleted file mode 100644
index 0494766fc574..000000000000
--- a/lib/sha1.c
+++ /dev/null
@@ -1,137 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * SHA1 routine optimized to do word accesses rather than byte accesses,
- * and to avoid unnecessary copies into the context array.
- *
- * This was based on the git SHA1 implementation.
- */
-
-#include <linux/kernel.h>
-#include <linux/export.h>
-#include <linux/bitops.h>
-#include <linux/string.h>
-#include <crypto/sha1.h>
-#include <asm/unaligned.h>
-
-/*
- * If you have 32 registers or more, the compiler can (and should)
- * try to change the array[] accesses into registers. However, on
- * machines with less than ~25 registers, that won't really work,
- * and at least gcc will make an unholy mess of it.
- *
- * So to avoid that mess which just slows things down, we force
- * the stores to memory to actually happen (we might be better off
- * with a 'W(t)=(val);asm("":"+m" (W(t))' there instead, as
- * suggested by Artur Skawina - that will also make gcc unable to
- * try to do the silly "optimize away loads" part because it won't
- * see what the value will be).
- *
- * Ben Herrenschmidt reports that on PPC, the C version comes close
- * to the optimized asm with this (ie on PPC you don't want that
- * 'volatile', since there are lots of registers).
- *
- * On ARM we get the best code generation by forcing a full memory barrier
- * between each SHA_ROUND, otherwise gcc happily get wild with spilling and
- * the stack frame size simply explode and performance goes down the drain.
- */
-
-#ifdef CONFIG_X86
- #define setW(x, val) (*(volatile __u32 *)&W(x) = (val))
-#elif defined(CONFIG_ARM)
- #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
-#else
- #define setW(x, val) (W(x) = (val))
-#endif
-
-/* This "rolls" over the 512-bit array */
-#define W(x) (array[(x)&15])
-
-/*
- * Where do we get the source from? The first 16 iterations get it from
- * the input data, the next mix it from the 512-bit array.
- */
-#define SHA_SRC(t) get_unaligned_be32((__u32 *)data + t)
-#define SHA_MIX(t) rol32(W(t+13) ^ W(t+8) ^ W(t+2) ^ W(t), 1)
-
-#define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \
- __u32 TEMP = input(t); setW(t, TEMP); \
- E += TEMP + rol32(A,5) + (fn) + (constant); \
- B = ror32(B, 2); \
- TEMP = E; E = D; D = C; C = B; B = A; A = TEMP; } while (0)
-
-#define T_0_15(t, A, B, C, D, E) SHA_ROUND(t, SHA_SRC, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
-#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
-#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0x6ed9eba1, A, B, C, D, E )
-#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E )
-#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0xca62c1d6, A, B, C, D, E )
-
-/**
- * sha1_transform - single block SHA1 transform (deprecated)
- *
- * @digest: 160 bit digest to update
- * @data: 512 bits of data to hash
- * @array: 16 words of workspace (see note)
- *
- * This function executes SHA-1's internal compression function. It updates the
- * 160-bit internal state (@digest) with a single 512-bit data block (@data).
- *
- * Don't use this function. SHA-1 is no longer considered secure. And even if
- * you do have to use SHA-1, this isn't the correct way to hash something with
- * SHA-1 as this doesn't handle padding and finalization.
- *
- * Note: If the hash is security sensitive, the caller should be sure
- * to clear the workspace. This is left to the caller to avoid
- * unnecessary clears between chained hashing operations.
- */
-void sha1_transform(__u32 *digest, const char *data, __u32 *array)
-{
- __u32 A, B, C, D, E;
- unsigned int i = 0;
-
- A = digest[0];
- B = digest[1];
- C = digest[2];
- D = digest[3];
- E = digest[4];
-
- /* Round 1 - iterations 0-16 take their input from 'data' */
- for (; i < 16; ++i)
- T_0_15(i, A, B, C, D, E);
-
- /* Round 1 - tail. Input from 512-bit mixing array */
- for (; i < 20; ++i)
- T_16_19(i, A, B, C, D, E);
-
- /* Round 2 */
- for (; i < 40; ++i)
- T_20_39(i, A, B, C, D, E);
-
- /* Round 3 */
- for (; i < 60; ++i)
- T_40_59(i, A, B, C, D, E);
-
- /* Round 4 */
- for (; i < 80; ++i)
- T_60_79(i, A, B, C, D, E);
-
- digest[0] += A;
- digest[1] += B;
- digest[2] += C;
- digest[3] += D;
- digest[4] += E;
-}
-EXPORT_SYMBOL(sha1_transform);
-
-/**
- * sha1_init - initialize the vectors for a SHA1 digest
- * @buf: vector to initialize
- */
-void sha1_init(__u32 *buf)
-{
- buf[0] = 0x67452301;
- buf[1] = 0xefcdab89;
- buf[2] = 0x98badcfe;
- buf[3] = 0x10325476;
- buf[4] = 0xc3d2e1f0;
-}
-EXPORT_SYMBOL(sha1_init);
--
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.
On Wed, Jan 12, 2022 at 02:12:01PM +0100, Jason A. Donenfeld wrote:
> Hi,
>
> There are currently two remaining users of SHA-1 left in the kernel: bpf
> tag generation, and ipv6 address calculation. In an effort to reduce
> code size and rid ourselves of insecure primitives, this RFC patchset
> moves to using the more secure BLAKE2s function.
What's the rationale to use 2s and not 2b? Everywhere I can find the 2s
version is said to be for 8bit up to 32bit machines and it's worse than
2b in benchmarks (reading https://bench.cr.yp.to/results-hash.html).
I'd understand you go with 2s because you also chose it for wireguard
but I'd like know why 2s again even if it's not made for 64bit
architectures that are preferred nowadays.
On Wed, Jan 12, 2022 at 7:50 PM David Sterba <[email protected]> wrote:
>
> On Wed, Jan 12, 2022 at 02:12:01PM +0100, Jason A. Donenfeld wrote:
> > Hi,
> >
> > There are currently two remaining users of SHA-1 left in the kernel: bpf
> > tag generation, and ipv6 address calculation. In an effort to reduce
> > code size and rid ourselves of insecure primitives, this RFC patchset
> > moves to using the more secure BLAKE2s function.
>
> What's the rationale to use 2s and not 2b? Everywhere I can find the 2s
> version is said to be for 8bit up to 32bit machines and it's worse than
> 2b in benchmarks (reading https://bench.cr.yp.to/results-hash.html).
>
> I'd understand you go with 2s because you also chose it for wireguard
> but I'd like know why 2s again even if it's not made for 64bit
> architectures that are preferred nowadays.
Fast for small inputs on all architectures, small code size. And it
performs well on Intel - there are avx512 and ssse3 implementations.
Even blake3 went with the 32-bit choice and abandoned 2b's thing.
Plus, this makes it even more similar to the well trusted chacha
permutation. As far as a general purpose high security library (keyed)
hash function for internal kernel usages, it seems pretty ideal.
Your choice for btrfs though is fine; don't let this patchset change
your thinking on that.
Anyway, I hope that's interesting to you, but I'm not so much
interested in bikeshedding about blake variants as I am in learning
from the net people on the feasibility of getting rid of sha1 in those
two places. So I'd appreciate it if we can keep the discussion focused
on that and not let this veer off into a tangential thread on blakes.
"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
Jason A. Donenfeld <[email protected]> wrote:
> There are currently two remaining users of SHA-1 left in the kernel: bpf
> tag generation, and ipv6 address calculation.
I think there are three, since drivers/char/random.c also uses it.
Moreover, there's some inefficiency there (or was last time I
looked) since it produces a 160-bit hash then folds it in half
to give an 80-bit output.
A possible fix would be to use a more modern 512-bit hash.
SHA3 would be the obvious one, but Blake2 would work,
Blake3 might be faster & there are several other possibilities.
Hash context size would then match ChaCha so you could
update the whole CC context at once, maybe even use the
same context for both.
That approach has difficulties, Extracting 512 bits every
time might drain the input pool too quickly & it is overkill
for ChaCha which should be secure with smaller rekeyings.
If you look at IPsec, SSL & other such protocols, many
have now mostly replaced the hash-based HMAC
constructions used in previous generations with things
like Galois field calculations (e.g. AES-GCM) or other
strange math (e,g. poly 1305). These have most of the
desirable properties of hashes & are much faster. As
far as I know, they all give 128-bit outputs.
I think we should replace SHA-1 with GCM. Give
ChaCha 128 bits somewhat more often than current
code gives it 256.
On Thu, 13 Jan 2022 at 04:24, Sandy Harris <[email protected]> wrote:
>
> Jason A. Donenfeld <[email protected]> wrote:
>
> > There are currently two remaining users of SHA-1 left in the kernel: bpf
> > tag generation, and ipv6 address calculation.
>
> I think there are three, since drivers/char/random.c also uses it.
> Moreover, there's some inefficiency there (or was last time I
> looked) since it produces a 160-bit hash then folds it in half
> to give an 80-bit output.
>
That code was removed, hence the two /remaining/ users.
> A possible fix would be to use a more modern 512-bit hash.
> SHA3 would be the obvious one, but Blake2 would work,
> Blake3 might be faster & there are several other possibilities.
> Hash context size would then match ChaCha so you could
> update the whole CC context at once, maybe even use the
> same context for both.
>
> That approach has difficulties, Extracting 512 bits every
> time might drain the input pool too quickly & it is overkill
> for ChaCha which should be secure with smaller rekeyings.
>
> If you look at IPsec, SSL & other such protocols, many
> have now mostly replaced the hash-based HMAC
> constructions used in previous generations with things
> like Galois field calculations (e.g. AES-GCM) or other
> strange math (e,g. poly 1305). These have most of the
> desirable properties of hashes & are much faster. As
> far as I know, they all give 128-bit outputs.
>
> I think we should replace SHA-1 with GCM. Give
> ChaCha 128 bits somewhat more often than current
> code gives it 256.
You are conflating MACs with hashes. A MAC does is not suitable for
backtrack resistance, and GHASH in particular is really only suited to
be used in the context of GCM.
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
On Thu, Jan 13, 2022 at 11:24:10AM +0800, Sandy Harris wrote:
> Jason A. Donenfeld <[email protected]> wrote:
>
> > There are currently two remaining users of SHA-1 left in the kernel: bpf
> > tag generation, and ipv6 address calculation.
>
> I think there are three, since drivers/char/random.c also uses it.
This was changed as of commit 9f9eff85a008 ("random: use BLAKE2s
instead of SHA1 in extraction"), which just landed in Linus's tree.
> Moreover, there's some inefficiency there (or was last time I
> looked) since it produces a 160-bit hash then folds it in half
> to give an 80-bit output.
This dates back to very early days of the /dev/random driver, back
when all that was known about SHA-1 was that it was designed by the
NSA using classified design principles, and it had not yet been as
well studied outside of the halls of the NSA. So folding the SHA-1
hash in half was done deliberately, since at the time, performance was
*not* the primary goal; security was.
(This was also back in the days when encryption algorithms would run
you into export control difficulties, since this is around the times
when the source code of PGP was being published in an OCR font with a
barcode containing the checksum of the content of every single page
was being published by the MIT press, and we were publishing Kerberos
with all of the *calls* to the crypto stripped out and calling it
"Bones" since there were assertions that code that *called*
cryptographic algoriothms might be subject to export control, even if
it didn't have any crypto algorithms in the program themselves. This
is also why HMAC-based constructions were so popular. People seem to
forget how much things have changed since the late 1980's....)
- Ted