* [PATCH 0/2] crypto: remove MD4 generic shash @ 2021-08-18 14:46 Ard Biesheuvel 2021-08-18 14:46 ` [PATCH 1/2] fs/cifs: Incorporate obsolete MD4 crypto code Ard Biesheuvel ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Ard Biesheuvel @ 2021-08-18 14:46 UTC (permalink / raw) To: linux-crypto Cc: herbert, Ard Biesheuvel, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings As discussed on the list [0], MD4 is still being relied upon by the CIFS driver, even though successful attacks on MD4 are as old as Linux itself. So let's move the code into the CIFS driver, and remove it from the crypto API so that it is no longer exposed to other subsystems or to user space via AF_ALG. Note: this leaves the code in crypto/asymmetric_keys that is able to parse RSA+MD4 keys if an "md4" shash is available. Given that its Kconfig symbol does not select CRYPTO_MD4, it only has a runtime dependency on md4 and so we can either decide remove it later, or just let it fail on the missing MD4 shash as it would today if the module is not enabled. [0] https://lore.kernel.org/linux-cifs/YRXlwDBfQql36wJx@sol.localdomain/ Cc: Eric Biggers <ebiggers@kernel.org> Cc: ronnie sahlberg <ronniesahlberg@gmail.com> Cc: linux-cifs <linux-cifs@vger.kernel.org> Cc: Steve French <sfrench@samba.org> Cc: David Howells <dhowells@redhat.com> Cc: keyrings@vger.kernel.org Ard Biesheuvel (2): fs/cifs: Incorporate obsolete MD4 crypto code crypto: md4 - Remove obsolete algorithm crypto/Kconfig | 6 - crypto/Makefile | 1 - crypto/md4.c | 241 -------------------- crypto/tcrypt.c | 14 +- crypto/testmgr.c | 6 - crypto/testmgr.h | 42 ---- fs/cifs/Kconfig | 1 - fs/cifs/cifsfs.c | 1 - fs/cifs/smbencrypt.c | 200 ++++++++++++++-- 9 files changed, 178 insertions(+), 334 deletions(-) delete mode 100644 crypto/md4.c -- 2.20.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/2] fs/cifs: Incorporate obsolete MD4 crypto code 2021-08-18 14:46 [PATCH 0/2] crypto: remove MD4 generic shash Ard Biesheuvel @ 2021-08-18 14:46 ` Ard Biesheuvel 2021-08-18 14:46 ` [PATCH 2/2] crypto: md4 - Remove obsolete algorithm Ard Biesheuvel 2021-08-18 14:51 ` [PATCH 0/2] crypto: remove MD4 generic shash Denis Kenzior 2 siblings, 0 replies; 19+ messages in thread From: Ard Biesheuvel @ 2021-08-18 14:46 UTC (permalink / raw) To: linux-crypto; +Cc: herbert, Ard Biesheuvel MD4 belongs in a museum, not in a modern OS kernel, but sadly, the CIFS code still relies on it, and so we have had to keep it around. So let's move the MD4 implementation back into the CIFS code (where it used to reside up until 2003), so that we can drop it from the crypto API in a subsequent patch. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- fs/cifs/Kconfig | 1 - fs/cifs/cifsfs.c | 1 - fs/cifs/smbencrypt.c | 200 +++++++++++++++++--- 3 files changed, 177 insertions(+), 25 deletions(-) diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig index 7364950a9ef4..748f4dd3466c 100644 --- a/fs/cifs/Kconfig +++ b/fs/cifs/Kconfig @@ -4,7 +4,6 @@ config CIFS depends on INET select NLS select CRYPTO - select CRYPTO_MD4 select CRYPTO_MD5 select CRYPTO_SHA256 select CRYPTO_SHA512 diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 64b71c4e2a9d..06ce13d274f8 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -1755,7 +1755,6 @@ MODULE_DESCRIPTION MODULE_VERSION(CIFS_VERSION); MODULE_SOFTDEP("ecb"); MODULE_SOFTDEP("hmac"); -MODULE_SOFTDEP("md4"); MODULE_SOFTDEP("md5"); MODULE_SOFTDEP("nls"); MODULE_SOFTDEP("aes"); diff --git a/fs/cifs/smbencrypt.c b/fs/cifs/smbencrypt.c index 39a938443e3e..77f720dca86f 100644 --- a/fs/cifs/smbencrypt.c +++ b/fs/cifs/smbencrypt.c @@ -38,6 +38,177 @@ #define SSVALX(buf,pos,val) (CVAL(buf,pos)=(val)&0xFF,CVAL(buf,pos+1)=(val)>>8) #define SSVAL(buf,pos,val) SSVALX((buf),(pos),((__u16)(val))) +#define MD4_DIGEST_SIZE 16 +#define MD4_HMAC_BLOCK_SIZE 64 +#define MD4_BLOCK_WORDS 16 +#define MD4_HASH_WORDS 4 + +struct md4_ctx { + u32 hash[MD4_HASH_WORDS]; + u32 block[MD4_BLOCK_WORDS]; + u64 byte_count; +}; + +static inline u32 lshift(u32 x, unsigned int s) +{ + x &= 0xFFFFFFFF; + return ((x << s) & 0xFFFFFFFF) | (x >> (32 - s)); +} + +static inline u32 F(u32 x, u32 y, u32 z) +{ + return (x & y) | ((~x) & z); +} + +static inline u32 G(u32 x, u32 y, u32 z) +{ + return (x & y) | (x & z) | (y & z); +} + +static inline u32 H(u32 x, u32 y, u32 z) +{ + return x ^ y ^ z; +} + +#define ROUND1(a,b,c,d,k,s) (a = lshift(a + F(b,c,d) + k, s)) +#define ROUND2(a,b,c,d,k,s) (a = lshift(a + G(b,c,d) + k + (u32)0x5A827999,s)) +#define ROUND3(a,b,c,d,k,s) (a = lshift(a + H(b,c,d) + k + (u32)0x6ED9EBA1,s)) + +static void md4_transform(u32 *hash, u32 const *in) +{ + u32 a, b, c, d; + + a = hash[0]; + b = hash[1]; + c = hash[2]; + d = hash[3]; + + ROUND1(a, b, c, d, in[0], 3); + ROUND1(d, a, b, c, in[1], 7); + ROUND1(c, d, a, b, in[2], 11); + ROUND1(b, c, d, a, in[3], 19); + ROUND1(a, b, c, d, in[4], 3); + ROUND1(d, a, b, c, in[5], 7); + ROUND1(c, d, a, b, in[6], 11); + ROUND1(b, c, d, a, in[7], 19); + ROUND1(a, b, c, d, in[8], 3); + ROUND1(d, a, b, c, in[9], 7); + ROUND1(c, d, a, b, in[10], 11); + ROUND1(b, c, d, a, in[11], 19); + ROUND1(a, b, c, d, in[12], 3); + ROUND1(d, a, b, c, in[13], 7); + ROUND1(c, d, a, b, in[14], 11); + ROUND1(b, c, d, a, in[15], 19); + + ROUND2(a, b, c, d,in[ 0], 3); + ROUND2(d, a, b, c, in[4], 5); + ROUND2(c, d, a, b, in[8], 9); + ROUND2(b, c, d, a, in[12], 13); + ROUND2(a, b, c, d, in[1], 3); + ROUND2(d, a, b, c, in[5], 5); + ROUND2(c, d, a, b, in[9], 9); + ROUND2(b, c, d, a, in[13], 13); + ROUND2(a, b, c, d, in[2], 3); + ROUND2(d, a, b, c, in[6], 5); + ROUND2(c, d, a, b, in[10], 9); + ROUND2(b, c, d, a, in[14], 13); + ROUND2(a, b, c, d, in[3], 3); + ROUND2(d, a, b, c, in[7], 5); + ROUND2(c, d, a, b, in[11], 9); + ROUND2(b, c, d, a, in[15], 13); + + ROUND3(a, b, c, d,in[ 0], 3); + ROUND3(d, a, b, c, in[8], 9); + ROUND3(c, d, a, b, in[4], 11); + ROUND3(b, c, d, a, in[12], 15); + ROUND3(a, b, c, d, in[2], 3); + ROUND3(d, a, b, c, in[10], 9); + ROUND3(c, d, a, b, in[6], 11); + ROUND3(b, c, d, a, in[14], 15); + ROUND3(a, b, c, d, in[1], 3); + ROUND3(d, a, b, c, in[9], 9); + ROUND3(c, d, a, b, in[5], 11); + ROUND3(b, c, d, a, in[13], 15); + ROUND3(a, b, c, d, in[3], 3); + ROUND3(d, a, b, c, in[11], 9); + ROUND3(c, d, a, b, in[7], 11); + ROUND3(b, c, d, a, in[15], 15); + + hash[0] += a; + hash[1] += b; + hash[2] += c; + hash[3] += d; +} + +static inline void md4_transform_helper(struct md4_ctx *ctx) +{ + le32_to_cpu_array(ctx->block, ARRAY_SIZE(ctx->block)); + md4_transform(ctx->hash, ctx->block); +} + +static void md4_init(struct md4_ctx *mctx) +{ + mctx->hash[0] = 0x67452301; + mctx->hash[1] = 0xefcdab89; + mctx->hash[2] = 0x98badcfe; + mctx->hash[3] = 0x10325476; + mctx->byte_count = 0; +} + +static void md4_update(struct md4_ctx *mctx, const u8 *data, unsigned int len) +{ + const u32 avail = sizeof(mctx->block) - (mctx->byte_count & 0x3f); + + mctx->byte_count += len; + + if (avail > len) { + memcpy((char *)mctx->block + (sizeof(mctx->block) - avail), + data, len); + return; + } + + memcpy((char *)mctx->block + (sizeof(mctx->block) - avail), + data, avail); + + md4_transform_helper(mctx); + data += avail; + len -= avail; + + while (len >= sizeof(mctx->block)) { + memcpy(mctx->block, data, sizeof(mctx->block)); + md4_transform_helper(mctx); + data += sizeof(mctx->block); + len -= sizeof(mctx->block); + } + + memcpy(mctx->block, data, len); +} + +static void md4_final(struct md4_ctx *mctx, u8 *out) +{ + const unsigned int offset = mctx->byte_count & 0x3f; + char *p = (char *)mctx->block + offset; + int padding = 56 - (offset + 1); + + *p++ = 0x80; + if (padding < 0) { + memset(p, 0x00, padding + sizeof (u64)); + md4_transform_helper(mctx); + p = (char *)mctx->block; + padding = 56; + } + + memset(p, 0, padding); + mctx->block[14] = mctx->byte_count << 3; + mctx->block[15] = mctx->byte_count >> 29; + le32_to_cpu_array(mctx->block, (sizeof(mctx->block) - + sizeof(u64)) / sizeof(u32)); + md4_transform(mctx->hash, mctx->block); + cpu_to_le32_array(mctx->hash, ARRAY_SIZE(mctx->hash)); + memcpy(out, mctx->hash, sizeof(mctx->hash)); + memset(mctx, 0, sizeof(*mctx)); +} + static void str_to_key(unsigned char *str, unsigned char *key) { @@ -108,31 +279,14 @@ E_P24(unsigned char *p21, const unsigned char *c8, unsigned char *p24) int mdfour(unsigned char *md4_hash, unsigned char *link_str, int link_len) { - int rc; - struct crypto_shash *md4 = NULL; - struct sdesc *sdescmd4 = NULL; + struct md4_ctx md4; - rc = cifs_alloc_hash("md4", &md4, &sdescmd4); - if (rc) - goto mdfour_err; - - rc = crypto_shash_init(&sdescmd4->shash); - if (rc) { - cifs_dbg(VFS, "%s: Could not init md4 shash\n", __func__); - goto mdfour_err; - } - rc = crypto_shash_update(&sdescmd4->shash, link_str, link_len); - if (rc) { - cifs_dbg(VFS, "%s: Could not update with link_str\n", __func__); - goto mdfour_err; - } - rc = crypto_shash_final(&sdescmd4->shash, md4_hash); - if (rc) - cifs_dbg(VFS, "%s: Could not generate md4 hash\n", __func__); + md4_init(&md4); + md4_update(&md4, link_str, link_len); + md4_final(&md4, md4_hash); -mdfour_err: - cifs_free_hash(&md4, &sdescmd4); - return rc; + memzero_explicit(&md4, sizeof(md4)); + return 0; } /* -- 2.20.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/2] crypto: md4 - Remove obsolete algorithm 2021-08-18 14:46 [PATCH 0/2] crypto: remove MD4 generic shash Ard Biesheuvel 2021-08-18 14:46 ` [PATCH 1/2] fs/cifs: Incorporate obsolete MD4 crypto code Ard Biesheuvel @ 2021-08-18 14:46 ` Ard Biesheuvel 2021-08-18 14:51 ` [PATCH 0/2] crypto: remove MD4 generic shash Denis Kenzior 2 siblings, 0 replies; 19+ messages in thread From: Ard Biesheuvel @ 2021-08-18 14:46 UTC (permalink / raw) To: linux-crypto; +Cc: herbert, Ard Biesheuvel MD4 is terminally broken, and has been known to broken since 1991. For this reason, it was requalified as 'historic' by RFC 6150 back in 2011. To celebrate the 10th birthday of this RFC, let's finally get rid of the generic shash implementation of MD4. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- crypto/Kconfig | 6 - crypto/Makefile | 1 - crypto/md4.c | 241 -------------------- crypto/tcrypt.c | 14 +- crypto/testmgr.c | 6 - crypto/testmgr.h | 42 ---- 6 files changed, 1 insertion(+), 309 deletions(-) diff --git a/crypto/Kconfig b/crypto/Kconfig index 64b772c5d1c9..5826f3e0b1eb 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -780,12 +780,6 @@ config CRYPTO_POLY1305_MIPS depends on MIPS select CRYPTO_ARCH_HAVE_LIB_POLY1305 -config CRYPTO_MD4 - tristate "MD4 digest algorithm" - select CRYPTO_HASH - help - MD4 message digest algorithm (RFC1320). - config CRYPTO_MD5 tristate "MD5 digest algorithm" select CRYPTO_HASH diff --git a/crypto/Makefile b/crypto/Makefile index 10526d4559b8..51be241df46f 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -71,7 +71,6 @@ obj-$(CONFIG_CRYPTO_HMAC) += hmac.o obj-$(CONFIG_CRYPTO_VMAC) += vmac.o obj-$(CONFIG_CRYPTO_XCBC) += xcbc.o obj-$(CONFIG_CRYPTO_NULL2) += crypto_null.o -obj-$(CONFIG_CRYPTO_MD4) += md4.o obj-$(CONFIG_CRYPTO_MD5) += md5.o obj-$(CONFIG_CRYPTO_RMD160) += rmd160.o obj-$(CONFIG_CRYPTO_RMD320) += rmd320.o diff --git a/crypto/md4.c b/crypto/md4.c deleted file mode 100644 index 2e7f2f319f95..000000000000 --- a/crypto/md4.c +++ /dev/null @@ -1,241 +0,0 @@ -/* - * Cryptographic API. - * - * MD4 Message Digest Algorithm (RFC1320). - * - * Implementation derived from Andrew Tridgell and Steve French's - * CIFS MD4 implementation, and the cryptoapi implementation - * originally based on the public domain implementation written - * by Colin Plumb in 1993. - * - * Copyright (c) Andrew Tridgell 1997-1998. - * Modified by Steve French (sfrench@us.ibm.com) 2002 - * Copyright (c) Cryptoapi developers. - * Copyright (c) 2002 David S. Miller (davem@redhat.com) - * Copyright (c) 2002 James Morris <jmorris@intercode.com.au> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - */ -#include <crypto/internal/hash.h> -#include <linux/init.h> -#include <linux/kernel.h> -#include <linux/module.h> -#include <linux/string.h> -#include <linux/types.h> -#include <asm/byteorder.h> - -#define MD4_DIGEST_SIZE 16 -#define MD4_HMAC_BLOCK_SIZE 64 -#define MD4_BLOCK_WORDS 16 -#define MD4_HASH_WORDS 4 - -struct md4_ctx { - u32 hash[MD4_HASH_WORDS]; - u32 block[MD4_BLOCK_WORDS]; - u64 byte_count; -}; - -static inline u32 lshift(u32 x, unsigned int s) -{ - x &= 0xFFFFFFFF; - return ((x << s) & 0xFFFFFFFF) | (x >> (32 - s)); -} - -static inline u32 F(u32 x, u32 y, u32 z) -{ - return (x & y) | ((~x) & z); -} - -static inline u32 G(u32 x, u32 y, u32 z) -{ - return (x & y) | (x & z) | (y & z); -} - -static inline u32 H(u32 x, u32 y, u32 z) -{ - return x ^ y ^ z; -} - -#define ROUND1(a,b,c,d,k,s) (a = lshift(a + F(b,c,d) + k, s)) -#define ROUND2(a,b,c,d,k,s) (a = lshift(a + G(b,c,d) + k + (u32)0x5A827999,s)) -#define ROUND3(a,b,c,d,k,s) (a = lshift(a + H(b,c,d) + k + (u32)0x6ED9EBA1,s)) - -static void md4_transform(u32 *hash, u32 const *in) -{ - u32 a, b, c, d; - - a = hash[0]; - b = hash[1]; - c = hash[2]; - d = hash[3]; - - ROUND1(a, b, c, d, in[0], 3); - ROUND1(d, a, b, c, in[1], 7); - ROUND1(c, d, a, b, in[2], 11); - ROUND1(b, c, d, a, in[3], 19); - ROUND1(a, b, c, d, in[4], 3); - ROUND1(d, a, b, c, in[5], 7); - ROUND1(c, d, a, b, in[6], 11); - ROUND1(b, c, d, a, in[7], 19); - ROUND1(a, b, c, d, in[8], 3); - ROUND1(d, a, b, c, in[9], 7); - ROUND1(c, d, a, b, in[10], 11); - ROUND1(b, c, d, a, in[11], 19); - ROUND1(a, b, c, d, in[12], 3); - ROUND1(d, a, b, c, in[13], 7); - ROUND1(c, d, a, b, in[14], 11); - ROUND1(b, c, d, a, in[15], 19); - - ROUND2(a, b, c, d,in[ 0], 3); - ROUND2(d, a, b, c, in[4], 5); - ROUND2(c, d, a, b, in[8], 9); - ROUND2(b, c, d, a, in[12], 13); - ROUND2(a, b, c, d, in[1], 3); - ROUND2(d, a, b, c, in[5], 5); - ROUND2(c, d, a, b, in[9], 9); - ROUND2(b, c, d, a, in[13], 13); - ROUND2(a, b, c, d, in[2], 3); - ROUND2(d, a, b, c, in[6], 5); - ROUND2(c, d, a, b, in[10], 9); - ROUND2(b, c, d, a, in[14], 13); - ROUND2(a, b, c, d, in[3], 3); - ROUND2(d, a, b, c, in[7], 5); - ROUND2(c, d, a, b, in[11], 9); - ROUND2(b, c, d, a, in[15], 13); - - ROUND3(a, b, c, d,in[ 0], 3); - ROUND3(d, a, b, c, in[8], 9); - ROUND3(c, d, a, b, in[4], 11); - ROUND3(b, c, d, a, in[12], 15); - ROUND3(a, b, c, d, in[2], 3); - ROUND3(d, a, b, c, in[10], 9); - ROUND3(c, d, a, b, in[6], 11); - ROUND3(b, c, d, a, in[14], 15); - ROUND3(a, b, c, d, in[1], 3); - ROUND3(d, a, b, c, in[9], 9); - ROUND3(c, d, a, b, in[5], 11); - ROUND3(b, c, d, a, in[13], 15); - ROUND3(a, b, c, d, in[3], 3); - ROUND3(d, a, b, c, in[11], 9); - ROUND3(c, d, a, b, in[7], 11); - ROUND3(b, c, d, a, in[15], 15); - - hash[0] += a; - hash[1] += b; - hash[2] += c; - hash[3] += d; -} - -static inline void md4_transform_helper(struct md4_ctx *ctx) -{ - le32_to_cpu_array(ctx->block, ARRAY_SIZE(ctx->block)); - md4_transform(ctx->hash, ctx->block); -} - -static int md4_init(struct shash_desc *desc) -{ - struct md4_ctx *mctx = shash_desc_ctx(desc); - - mctx->hash[0] = 0x67452301; - mctx->hash[1] = 0xefcdab89; - mctx->hash[2] = 0x98badcfe; - mctx->hash[3] = 0x10325476; - mctx->byte_count = 0; - - return 0; -} - -static int md4_update(struct shash_desc *desc, const u8 *data, unsigned int len) -{ - struct md4_ctx *mctx = shash_desc_ctx(desc); - const u32 avail = sizeof(mctx->block) - (mctx->byte_count & 0x3f); - - mctx->byte_count += len; - - if (avail > len) { - memcpy((char *)mctx->block + (sizeof(mctx->block) - avail), - data, len); - return 0; - } - - memcpy((char *)mctx->block + (sizeof(mctx->block) - avail), - data, avail); - - md4_transform_helper(mctx); - data += avail; - len -= avail; - - while (len >= sizeof(mctx->block)) { - memcpy(mctx->block, data, sizeof(mctx->block)); - md4_transform_helper(mctx); - data += sizeof(mctx->block); - len -= sizeof(mctx->block); - } - - memcpy(mctx->block, data, len); - - return 0; -} - -static int md4_final(struct shash_desc *desc, u8 *out) -{ - struct md4_ctx *mctx = shash_desc_ctx(desc); - const unsigned int offset = mctx->byte_count & 0x3f; - char *p = (char *)mctx->block + offset; - int padding = 56 - (offset + 1); - - *p++ = 0x80; - if (padding < 0) { - memset(p, 0x00, padding + sizeof (u64)); - md4_transform_helper(mctx); - p = (char *)mctx->block; - padding = 56; - } - - memset(p, 0, padding); - mctx->block[14] = mctx->byte_count << 3; - mctx->block[15] = mctx->byte_count >> 29; - le32_to_cpu_array(mctx->block, (sizeof(mctx->block) - - sizeof(u64)) / sizeof(u32)); - md4_transform(mctx->hash, mctx->block); - cpu_to_le32_array(mctx->hash, ARRAY_SIZE(mctx->hash)); - memcpy(out, mctx->hash, sizeof(mctx->hash)); - memset(mctx, 0, sizeof(*mctx)); - - return 0; -} - -static struct shash_alg alg = { - .digestsize = MD4_DIGEST_SIZE, - .init = md4_init, - .update = md4_update, - .final = md4_final, - .descsize = sizeof(struct md4_ctx), - .base = { - .cra_name = "md4", - .cra_driver_name = "md4-generic", - .cra_blocksize = MD4_HMAC_BLOCK_SIZE, - .cra_module = THIS_MODULE, - } -}; - -static int __init md4_mod_init(void) -{ - return crypto_register_shash(&alg); -} - -static void __exit md4_mod_fini(void) -{ - crypto_unregister_shash(&alg); -} - -subsys_initcall(md4_mod_init); -module_exit(md4_mod_fini); - -MODULE_LICENSE("GPL"); -MODULE_DESCRIPTION("MD4 Message Digest Algorithm"); -MODULE_ALIAS_CRYPTO("md4"); diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c index f8d06da78e4f..dcb42a9e8cc6 100644 --- a/crypto/tcrypt.c +++ b/crypto/tcrypt.c @@ -68,7 +68,7 @@ static char *tvmem[TVMEMSIZE]; static const char *check[] = { "des", "md5", "des3_ede", "rot13", "sha1", "sha224", "sha256", "sm3", - "blowfish", "twofish", "serpent", "sha384", "sha512", "md4", "aes", + "blowfish", "twofish", "serpent", "sha384", "sha512", "aes", "cast6", "arc4", "michael_mic", "deflate", "crc32c", "tea", "xtea", "khazad", "wp512", "wp384", "wp256", "xeta", "fcrypt", "camellia", "seed", "rmd160", @@ -1703,10 +1703,6 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb) ret += tcrypt_test("ctr(des3_ede)"); break; - case 5: - ret += tcrypt_test("md4"); - break; - case 6: ret += tcrypt_test("sha256"); break; @@ -2328,10 +2324,6 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb) break; } fallthrough; - case 301: - test_hash_speed("md4", sec, generic_hash_speed_template); - if (mode > 300 && mode < 400) break; - fallthrough; case 302: test_hash_speed("md5", sec, generic_hash_speed_template); if (mode > 300 && mode < 400) break; @@ -2440,10 +2432,6 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb) break; } fallthrough; - case 401: - test_ahash_speed("md4", sec, generic_hash_speed_template); - if (mode > 400 && mode < 500) break; - fallthrough; case 402: test_ahash_speed("md5", sec, generic_hash_speed_template); if (mode > 400 && mode < 500) break; diff --git a/crypto/testmgr.c b/crypto/testmgr.c index c978e41f11a1..3e9378130150 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -5153,12 +5153,6 @@ static const struct alg_test_desc alg_test_descs[] = { .decomp = __VECS(lzorle_decomp_tv_template) } } - }, { - .alg = "md4", - .test = alg_test_hash, - .suite = { - .hash = __VECS(md4_tv_template) - } }, { .alg = "md5", .test = alg_test_hash, diff --git a/crypto/testmgr.h b/crypto/testmgr.h index 3ed6ab34ab51..04e58adee80d 100644 --- a/crypto/testmgr.h +++ b/crypto/testmgr.h @@ -2872,48 +2872,6 @@ static const struct kpp_testvec ecdh_p384_tv_template[] = { } }; -/* - * MD4 test vectors from RFC1320 - */ -static const struct hash_testvec md4_tv_template[] = { - { - .plaintext = "", - .digest = "\x31\xd6\xcf\xe0\xd1\x6a\xe9\x31" - "\xb7\x3c\x59\xd7\xe0\xc0\x89\xc0", - }, { - .plaintext = "a", - .psize = 1, - .digest = "\xbd\xe5\x2c\xb3\x1d\xe3\x3e\x46" - "\x24\x5e\x05\xfb\xdb\xd6\xfb\x24", - }, { - .plaintext = "abc", - .psize = 3, - .digest = "\xa4\x48\x01\x7a\xaf\x21\xd8\x52" - "\x5f\xc1\x0a\xe8\x7a\xa6\x72\x9d", - }, { - .plaintext = "message digest", - .psize = 14, - .digest = "\xd9\x13\x0a\x81\x64\x54\x9f\xe8" - "\x18\x87\x48\x06\xe1\xc7\x01\x4b", - }, { - .plaintext = "abcdefghijklmnopqrstuvwxyz", - .psize = 26, - .digest = "\xd7\x9e\x1c\x30\x8a\xa5\xbb\xcd" - "\xee\xa8\xed\x63\xdf\x41\x2d\xa9", - }, { - .plaintext = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789", - .psize = 62, - .digest = "\x04\x3f\x85\x82\xf2\x41\xdb\x35" - "\x1c\xe6\x27\xe1\x53\xe7\xf0\xe4", - }, { - .plaintext = "123456789012345678901234567890123456789012345678901234567890123" - "45678901234567890", - .psize = 80, - .digest = "\xe3\x3b\x4d\xdc\x9c\x38\xf2\x19" - "\x9c\x3e\x7b\x16\x4f\xcc\x05\x36", - }, -}; - static const struct hash_testvec sha3_224_tv_template[] = { { .plaintext = "", -- 2.20.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 14:46 [PATCH 0/2] crypto: remove MD4 generic shash Ard Biesheuvel 2021-08-18 14:46 ` [PATCH 1/2] fs/cifs: Incorporate obsolete MD4 crypto code Ard Biesheuvel 2021-08-18 14:46 ` [PATCH 2/2] crypto: md4 - Remove obsolete algorithm Ard Biesheuvel @ 2021-08-18 14:51 ` Denis Kenzior 2021-08-18 16:10 ` Ard Biesheuvel 2 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2021-08-18 14:51 UTC (permalink / raw) To: Ard Biesheuvel, linux-crypto Cc: herbert, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings Hi Ard, On 8/18/21 9:46 AM, Ard Biesheuvel wrote: > As discussed on the list [0], MD4 is still being relied upon by the CIFS > driver, even though successful attacks on MD4 are as old as Linux > itself. > > So let's move the code into the CIFS driver, and remove it from the > crypto API so that it is no longer exposed to other subsystems or to > user space via AF_ALG. > Can we please stop removing algorithms from AF_ALG? The previous ARC4 removal already caused some headaches [0]. Please note that iwd does use MD4 for MSCHAP and MSCHAPv2 based 802.1X authentication. Regards, -Denis [0] https://bugs.launchpad.net/ubuntu/+source/iwd/+bug/1938650 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 14:51 ` [PATCH 0/2] crypto: remove MD4 generic shash Denis Kenzior @ 2021-08-18 16:10 ` Ard Biesheuvel 2021-08-18 16:23 ` Denis Kenzior 2021-08-19 10:42 ` Jarkko Sakkinen 0 siblings, 2 replies; 19+ messages in thread From: Ard Biesheuvel @ 2021-08-18 16:10 UTC (permalink / raw) To: Denis Kenzior Cc: Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings On Wed, 18 Aug 2021 at 16:51, Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Ard, > > On 8/18/21 9:46 AM, Ard Biesheuvel wrote: > > As discussed on the list [0], MD4 is still being relied upon by the CIFS > > driver, even though successful attacks on MD4 are as old as Linux > > itself. > > > > So let's move the code into the CIFS driver, and remove it from the > > crypto API so that it is no longer exposed to other subsystems or to > > user space via AF_ALG. > > > > Can we please stop removing algorithms from AF_ALG? I don't think we can, to be honest. We need to have a deprecation path for obsolete and insecure algorithms: the alternative is to keep supporting a long tail of broken crypto indefinitely. > The previous ARC4 removal > already caused some headaches [0]. This is the first time this has been reported on an upstream kernel list. As you know, I went out of my way to ensure that this removal would happen as smoothly as possible, which is why I contributed code to both iwd and libell beforehand, and worked with distros to ensure that the updated versions would land before the removal of ARC4 from the kernel. It is unfortunate that one of the distros failed to take that into account for the backport of a newer kernel to an older distro release, but I don't think it is fair to blame that on the process. > Please note that iwd does use MD4 for MSCHAP > and MSCHAPv2 based 802.1X authentication. > Thanks for reporting that. So what is your timeline for retaining MD4 support in iwd? You are aware that it has been broken since 1991, right? Please, consider having a deprecation path, so we can at least agree on *some* point in time (in 6 months, in 6 years, etc) where we can start culling this junk. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 16:10 ` Ard Biesheuvel @ 2021-08-18 16:23 ` Denis Kenzior 2021-08-18 16:47 ` Steve French ` (2 more replies) 2021-08-19 10:42 ` Jarkko Sakkinen 1 sibling, 3 replies; 19+ messages in thread From: Denis Kenzior @ 2021-08-18 16:23 UTC (permalink / raw) To: Ard Biesheuvel Cc: Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings Hi Ard, >> The previous ARC4 removal >> already caused some headaches [0]. > > This is the first time this has been reported on an upstream kernel list. > > As you know, I went out of my way to ensure that this removal would > happen as smoothly as possible, which is why I contributed code to > both iwd and libell beforehand, and worked with distros to ensure that > the updated versions would land before the removal of ARC4 from the > kernel. > > It is unfortunate that one of the distros failed to take that into > account for the backport of a newer kernel to an older distro release, > but I don't think it is fair to blame that on the process. Please don't misunderstand, I don't blame you at all. I was in favor of ARC4 removal since the kernel AF_ALG implementation was broken and the ell implementation had to work around that. And you went the extra mile to make sure the migration was smooth. The reported bug is still a fairly minor inconvenience in the grand scheme of things. But, I'm not in favor of doing the same for MD4... > >> Please note that iwd does use MD4 for MSCHAP >> and MSCHAPv2 based 802.1X authentication. >> > > Thanks for reporting that. > > So what is your timeline for retaining MD4 support in iwd? You are > aware that it has been broken since 1991, right? Please, consider > having a deprecation path, so we can at least agree on *some* point in > time (in 6 months, in 6 years, etc) where we can start culling this > junk. > That is not something that iwd has any control over though? We have to support it for as long as there are organizations using TTLS + MD5 or PEAPv0. There are still surprisingly many today. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 16:23 ` Denis Kenzior @ 2021-08-18 16:47 ` Steve French 2021-08-18 22:08 ` Jeremy Allison 2021-08-18 21:11 ` ronnie sahlberg 2021-08-18 22:10 ` Ard Biesheuvel 2 siblings, 1 reply; 19+ messages in thread From: Steve French @ 2021-08-18 16:47 UTC (permalink / raw) To: Denis Kenzior Cc: Ard Biesheuvel, Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings, samba-technical I don't object to moving ARC4 and/or MD4 into cifs.ko, but we do have to be careful that we don't make the defaults "less secure" as an unintentional side effect. The use of ARC4 and MD4 is the NTLMSSP authentication workflow is relatively small (narrow use case), and NTLMSSP is the default for many servers and clients - and some of the exploits do not apply in the case of SMB2.1 and later (which is usually required on mount in any case). There is little argument that kerberos ("sec=krb5") is more secure and does not rely on these algorithms ... but there are millions of devices (probably over 100 million) that can support SMB3.1.1 (or at least SMB3) mounts but couldn't realistically join a domain and use "sec=krb5" so would be forced to use "guest" mounts (or in the case of removing RC4 use less secure version of NTLMSSP). In the longer term where I would like this to go is: - make it easier to "require Kerberos" (in module load parameters for cifs.ko) - make sure cifs.ko builds even if these algorithms are removed from the kernel, and that mount by default isn't broken if the user chooses to build without support for NTLMSSP, the default auth mechanism (ie NTLMSSP being disabled because required crypto algorithms aren't available) - add support in Linux for a "peer to peer" auth mechanism (even if it requires an upcall), perhaps one that is certificate based and one that is not (and thus much easier to use) that we can plumb into SPNEGO (RFC2478). By comparison, it sounds like it is much easier in Windows to add in additional authentication mechanisms (beyond Kerberos, PKU2U and NTLMSSP) - see https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/jj852232(v=ws.11) so perhaps we just need to do something similar for the kernel client and samba and ksmbd where they call out to a library which is configured to provide the SPNEGO blobs for the new stronger peer-to-peer authentication mechanism the distro chooses to enable (for cases where using Kerberos for authentication is not practical) On Wed, Aug 18, 2021 at 11:24 AM Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Ard, > > >> The previous ARC4 removal > >> already caused some headaches [0]. > > > > This is the first time this has been reported on an upstream kernel list. > > > > As you know, I went out of my way to ensure that this removal would > > happen as smoothly as possible, which is why I contributed code to > > both iwd and libell beforehand, and worked with distros to ensure that > > the updated versions would land before the removal of ARC4 from the > > kernel. > > > > It is unfortunate that one of the distros failed to take that into > > account for the backport of a newer kernel to an older distro release, > > but I don't think it is fair to blame that on the process. > > Please don't misunderstand, I don't blame you at all. I was in favor of ARC4 > removal since the kernel AF_ALG implementation was broken and the ell > implementation had to work around that. And you went the extra mile to make > sure the migration was smooth. The reported bug is still a fairly minor > inconvenience in the grand scheme of things. > > But, I'm not in favor of doing the same for MD4... > > > > >> Please note that iwd does use MD4 for MSCHAP > >> and MSCHAPv2 based 802.1X authentication. > >> > > > > Thanks for reporting that. > > > > So what is your timeline for retaining MD4 support in iwd? You are > > aware that it has been broken since 1991, right? Please, consider > > having a deprecation path, so we can at least agree on *some* point in > > time (in 6 months, in 6 years, etc) where we can start culling this > > junk. > > > > That is not something that iwd has any control over though? We have to support > it for as long as there are organizations using TTLS + MD5 or PEAPv0. There > are still surprisingly many today. > > Regards, > -Denis -- Thanks, Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 16:47 ` Steve French @ 2021-08-18 22:08 ` Jeremy Allison 2021-08-19 3:49 ` Andrew Bartlett 0 siblings, 1 reply; 19+ messages in thread From: Jeremy Allison @ 2021-08-18 22:08 UTC (permalink / raw) To: Steve French Cc: Denis Kenzior, linux-cifs, David Howells, Herbert Xu, samba-technical, Eric Biggers, Steve French, keyrings, Linux Crypto Mailing List, Ard Biesheuvel On Wed, Aug 18, 2021 at 11:47:53AM -0500, Steve French via samba-technical wrote: >I don't object to moving ARC4 and/or MD4 into cifs.ko, but we do have >to be careful that we don't make the defaults "less secure" as an >unintentional side effect. > >The use of ARC4 and MD4 is the NTLMSSP authentication workflow is >relatively small (narrow use case), and NTLMSSP is the default for >many servers and clients - and some of the exploits do not apply in >the case of SMB2.1 and later (which is usually required on mount in >any case). > >There is little argument that kerberos ("sec=krb5") is more secure and >does not rely on these algorithms ... but there are millions of >devices (probably over 100 million) that can support SMB3.1.1 (or at >least SMB3) mounts but couldn't realistically join a domain and use >"sec=krb5" so would be forced to use "guest" mounts (or in the case of >removing RC4 use less secure version of NTLMSSP). > >In the longer term where I would like this to go is: >- make it easier to "require Kerberos" (in module load parameters for cifs.ko) >- make sure cifs.ko builds even if these algorithms are removed from >the kernel, and that mount by default isn't broken if the user chooses >to build without support for NTLMSSP, the default auth mechanism (ie >NTLMSSP being disabled because required crypto algorithms aren't >available) >- add support in Linux for a "peer to peer" auth mechanism (even if it >requires an upcall), perhaps one that is certificate based and one >that is not (and thus much easier to use) that we can plumb into >SPNEGO (RFC2478). By comparison, it sounds like it is much easier >in Windows to add in additional authentication mechanisms (beyond >Kerberos, PKU2U and NTLMSSP) - see >https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/jj852232(v=ws.11) >so perhaps we just need to do something similar for the kernel client >and samba and ksmbd where they call out to a library which is >configured to provide the SPNEGO blobs for the new stronger >peer-to-peer authentication mechanism the distro chooses to enable >(for cases where using Kerberos for authentication is not practical) My 2 cents. Preventing NTLM authentication/signing from working would be a negative for the Linux kernel client. I don't mind if that code has to be isolated inside cifs.ko, but it really needs to keep working, at least until we have a pluggable client auth in cifs.ko and Samba that allows the single-server (non AD-Domain) case to keep working easily. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 22:08 ` Jeremy Allison @ 2021-08-19 3:49 ` Andrew Bartlett 2021-08-19 5:18 ` Eric Biggers 0 siblings, 1 reply; 19+ messages in thread From: Andrew Bartlett @ 2021-08-19 3:49 UTC (permalink / raw) To: Jeremy Allison, Steve French Cc: linux-cifs, Herbert Xu, Eric Biggers, samba-technical, David Howells, Steve French, keyrings, Linux Crypto Mailing List, Ard Biesheuvel, Denis Kenzior On Wed, 2021-08-18 at 15:08 -0700, Jeremy Allison via samba-technical wrote: > > My 2 cents. Preventing NTLM authentication/signing from working would > be > a negative for the Linux kernel client. I don't mind if that code has > to be isolated inside cifs.ko, but it really needs to keep working, > at least until we have a pluggable client auth in cifs.ko and Samba > that allows the single-server (non AD-Domain) case to keep working > easily. I would echo that, and also just remind folks that MD4 in NTLMSSP is used as a compression only, it has no security value. The security would be the same if the password was compressed with MD4, SHA1 or SHA256 - the security comes from the complexity of the password and the HMAC-MD5 rounds inside NTLMv2. I'll also mention the use of MD4, which is used to re-encrypt a short- term key with the long-term key out of the NTLMv2 scheme. This thankfully is an unchecksumed simple RC4 round of one random value with another, so not subject to known-plaintext attacks here. I know neither MD4 nor HMAC-MD5 is not flavour of the month any more, with good reason, but we would not want to go with way of NFSv4 which is, as I understand it, full Kerberos or bust (so folks choose no protection). Andrew Bartlett -- Andrew Bartlett (he/him) https://samba.org/~abartlet/ Samba Team Member (since 2001) https://samba.org Samba Team Lead, Catalyst IT https://catalyst.net.nz/services/samba Samba Development and Support, Catalyst IT - Expert Open Source Solutions ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-19 3:49 ` Andrew Bartlett @ 2021-08-19 5:18 ` Eric Biggers 2021-08-19 5:23 ` Andrew Bartlett 0 siblings, 1 reply; 19+ messages in thread From: Eric Biggers @ 2021-08-19 5:18 UTC (permalink / raw) To: Andrew Bartlett Cc: Jeremy Allison, Steve French, linux-cifs, Herbert Xu, samba-technical, David Howells, Steve French, keyrings, Linux Crypto Mailing List, Ard Biesheuvel, Denis Kenzior On Thu, Aug 19, 2021 at 03:49:14PM +1200, Andrew Bartlett wrote: > I know neither MD4 nor HMAC-MD5 is not flavour of the month any more, > with good reason, but we would not want to go with way of NFSv4 which > is, as I understand it, full Kerberos or bust (so folks choose no > protection). I'm not sure you understand how embarrassing it is to still be using these algorithms. MD4 has been broken for over 25 years, and better algorithms have been recommended for 29 years. Similarly MD5 has been broken for 16 years and better algorithms have been recommended for 25 years (though granted, HMAC-MD5 is more secure than plain MD5 when properly used). Meanwhile SHA-2 is 20 years old and is still considered secure. So this isn't something that changes every month -- we're talking about no one bothering to do anything in 30 years. Of course, if cryptography isn't actually applicable to the use case, then cryptography shouldn't be used at all. - Eric ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-19 5:18 ` Eric Biggers @ 2021-08-19 5:23 ` Andrew Bartlett 0 siblings, 0 replies; 19+ messages in thread From: Andrew Bartlett @ 2021-08-19 5:23 UTC (permalink / raw) To: Eric Biggers Cc: Jeremy Allison, Steve French, linux-cifs, Herbert Xu, samba-technical, David Howells, Steve French, keyrings, Linux Crypto Mailing List, Ard Biesheuvel, Denis Kenzior On Wed, 2021-08-18 at 22:18 -0700, Eric Biggers wrote: > I'm not sure you understand how embarrassing it is to still be using > these > algorithms. MD4 has been broken for over 25 years, and better > algorithms have > been recommended for 29 years. Similarly MD5 has been broken for 16 > years and > better algorithms have been recommended for 25 years (though granted, > HMAC-MD5 > is more secure than plain MD5 when properly used). Meanwhile SHA-2 > is 20 years > old and is still considered secure. So this isn't something that > changes every > month -- we're talking about no one bothering to do anything in 30 > years. > > Of course, if cryptography isn't actually applicable to the use case, > then > cryptography shouldn't be used at all. I'm sorry that Samba - or the Kernel, you could implement whatever is desired between cifs.ko and kcifsd - hasn't gone it alone to build a new peer-to-peer mechanism, but absent a Samba-only solution Microsoft has been asked and has no intention of updating NTLM, so embarrassing or otherwise this is how it is. Thankfully only the HMAC-MD5 step in what you mention is cryptographically significant, the rest are just very lossy compression algorithms. Andrew Bartlett -- Andrew Bartlett (he/him) https://samba.org/~abartlet/ Samba Team Member (since 2001) https://samba.org Samba Team Lead, Catalyst IT https://catalyst.net.nz/services/samba Samba Development and Support, Catalyst IT - Expert Open Source Solutions ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 16:23 ` Denis Kenzior 2021-08-18 16:47 ` Steve French @ 2021-08-18 21:11 ` ronnie sahlberg 2021-08-18 22:10 ` Ard Biesheuvel 2 siblings, 0 replies; 19+ messages in thread From: ronnie sahlberg @ 2021-08-18 21:11 UTC (permalink / raw) To: Denis Kenzior Cc: Ard Biesheuvel, Linux Crypto Mailing List, Herbert Xu, Eric Biggers, linux-cifs, Steve French, David Howells, keyrings On Thu, Aug 19, 2021 at 2:23 AM Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Ard, > > >> The previous ARC4 removal > >> already caused some headaches [0]. > > > > This is the first time this has been reported on an upstream kernel list. > > > > As you know, I went out of my way to ensure that this removal would > > happen as smoothly as possible, which is why I contributed code to > > both iwd and libell beforehand, and worked with distros to ensure that > > the updated versions would land before the removal of ARC4 from the > > kernel. > > > > It is unfortunate that one of the distros failed to take that into > > account for the backport of a newer kernel to an older distro release, > > but I don't think it is fair to blame that on the process. > > Please don't misunderstand, I don't blame you at all. I was in favor of ARC4 > removal since the kernel AF_ALG implementation was broken and the ell > implementation had to work around that. And you went the extra mile to make > sure the migration was smooth. The reported bug is still a fairly minor > inconvenience in the grand scheme of things. > > But, I'm not in favor of doing the same for MD4... > > > > >> Please note that iwd does use MD4 for MSCHAP > >> and MSCHAPv2 based 802.1X authentication. > >> > > > > Thanks for reporting that. > > > > So what is your timeline for retaining MD4 support in iwd? You are > > aware that it has been broken since 1991, right? Please, consider > > having a deprecation path, so we can at least agree on *some* point in > > time (in 6 months, in 6 years, etc) where we can start culling this > > junk. > > > > That is not something that iwd has any control over though? We have to support > it for as long as there are organizations using TTLS + MD5 or PEAPv0. There > are still surprisingly many today. The same situation exist for cifs. The cifs client depends on md4 in order to authenticate to Windows/Azure/Samba/... cifs servers. And like you we have no control of the servers. Our solution will likely be to fork the md4 code and put a private copy in our module. Maybe you need to do the same. -- ronnie > > Regards, > -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 16:23 ` Denis Kenzior 2021-08-18 16:47 ` Steve French 2021-08-18 21:11 ` ronnie sahlberg @ 2021-08-18 22:10 ` Ard Biesheuvel 2021-08-18 22:22 ` Denis Kenzior 2 siblings, 1 reply; 19+ messages in thread From: Ard Biesheuvel @ 2021-08-18 22:10 UTC (permalink / raw) To: Denis Kenzior Cc: Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings On Wed, 18 Aug 2021 at 18:23, Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Ard, > > >> The previous ARC4 removal > >> already caused some headaches [0]. > > > > This is the first time this has been reported on an upstream kernel list. > > > > As you know, I went out of my way to ensure that this removal would > > happen as smoothly as possible, which is why I contributed code to > > both iwd and libell beforehand, and worked with distros to ensure that > > the updated versions would land before the removal of ARC4 from the > > kernel. > > > > It is unfortunate that one of the distros failed to take that into > > account for the backport of a newer kernel to an older distro release, > > but I don't think it is fair to blame that on the process. > > Please don't misunderstand, I don't blame you at all. I was in favor of ARC4 > removal since the kernel AF_ALG implementation was broken and the ell > implementation had to work around that. And you went the extra mile to make > sure the migration was smooth. The reported bug is still a fairly minor > inconvenience in the grand scheme of things. > > But, I'm not in favor of doing the same for MD4... > Fair enough. > > > >> Please note that iwd does use MD4 for MSCHAP > >> and MSCHAPv2 based 802.1X authentication. > >> > > > > Thanks for reporting that. > > > > So what is your timeline for retaining MD4 support in iwd? You are > > aware that it has been broken since 1991, right? Please, consider > > having a deprecation path, so we can at least agree on *some* point in > > time (in 6 months, in 6 years, etc) where we can start culling this > > junk. > > > > That is not something that iwd has any control over though? We have to support > it for as long as there are organizations using TTLS + MD5 or PEAPv0. There > are still surprisingly many today. > Does that code rely on MD4 as well? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 22:10 ` Ard Biesheuvel @ 2021-08-18 22:22 ` Denis Kenzior 2021-08-18 23:03 ` Steve French 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2021-08-18 22:22 UTC (permalink / raw) To: Ard Biesheuvel Cc: Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings Hi Ard, >> That is not something that iwd has any control over though? We have to support >> it for as long as there are organizations using TTLS + MD5 or PEAPv0. There Ah, my brain said MSCHAP but my fingers typed MD5. >> are still surprisingly many today. >> > > Does that code rely on MD4 as well? > But the answer is yes. Both PEAP and TTLS use MSCHAP or MSCHAPv2 in some form. These are commonly used for Username/Password based WPA(2|3)-Enterprise authentication. Think 'eduroam' for example. MD4 is used to hash the plaintext password, but the hash is sent inside a TLS tunnel, so there's really no immediate crypto weakness concern? At least there's not a replacement on the horizon as far as I know. EAP-PWD has its own problems and I doubt certificate based authentication will overtake username/password any time soon. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 22:22 ` Denis Kenzior @ 2021-08-18 23:03 ` Steve French 2021-08-19 16:56 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Steve French @ 2021-08-18 23:03 UTC (permalink / raw) To: Denis Kenzior Cc: Ard Biesheuvel, Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings On Wed, Aug 18, 2021 at 5:22 PM Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Ard, > > >> That is not something that iwd has any control over though? We have to support > >> it for as long as there are organizations using TTLS + MD5 or PEAPv0. There > > Ah, my brain said MSCHAP but my fingers typed MD5. > > >> are still surprisingly many today. > >> > > > > Does that code rely on MD4 as well? > > > > But the answer is yes. Both PEAP and TTLS use MSCHAP or MSCHAPv2 in some form. > These are commonly used for Username/Password based WPA(2|3)-Enterprise > authentication. Think 'eduroam' for example. Can you give some background here? IIRC MS-CHAPv2 is much worse than the NTLMSSP case in cifs.ko (where RC4/MD5 is used narrowly). Doesn't MS-CHAPv2 depend on DES? -- Thanks, Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 23:03 ` Steve French @ 2021-08-19 16:56 ` Denis Kenzior 0 siblings, 0 replies; 19+ messages in thread From: Denis Kenzior @ 2021-08-19 16:56 UTC (permalink / raw) To: Steve French Cc: Ard Biesheuvel, Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings Hi Steve, >> But the answer is yes. Both PEAP and TTLS use MSCHAP or MSCHAPv2 in some form. >> These are commonly used for Username/Password based WPA(2|3)-Enterprise >> authentication. Think 'eduroam' for example. > > Can you give some background here? IIRC MS-CHAPv2 is much worse than > the NTLMSSP case What background are you looking for? iwd [0] is a wifi management daemon, so we implement various EAP [1] and wifi authentication protocols. > in cifs.ko (where RC4/MD5 is used narrowly). Doesn't MS-CHAPv2 depend on DES? > You are quite correct. MSCHAPv2 also uses DES for generating the responses. EAP with TTLS+MSCHAPv2 and PEAP+MSCHAPv2 are two of the most deployed variants of WPA-Enterprise authentication using Username + Password. Deprecating MD4, MD5, SHA1 or DES would be quite disruptive for us. We are using these through AF_ALG userspace API, so if they're removed, some combination of kernel + iwd version will break. We went through this with ARC4, and while that was justified, I don't think the same justification exists for MD4. [0] https://git.kernel.org/pub/scm/network/wireless/iwd.git [1] https://en.wikipedia.org/wiki/Extensible_Authentication_Protocol Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-18 16:10 ` Ard Biesheuvel 2021-08-18 16:23 ` Denis Kenzior @ 2021-08-19 10:42 ` Jarkko Sakkinen 2021-08-19 17:10 ` Steve French 1 sibling, 1 reply; 19+ messages in thread From: Jarkko Sakkinen @ 2021-08-19 10:42 UTC (permalink / raw) To: Ard Biesheuvel, Denis Kenzior Cc: Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings On Wed, 2021-08-18 at 18:10 +0200, Ard Biesheuvel wrote: > On Wed, 18 Aug 2021 at 16:51, Denis Kenzior <denkenz@gmail.com> > wrote: > > Hi Ard, > > > > On 8/18/21 9:46 AM, Ard Biesheuvel wrote: > > > As discussed on the list [0], MD4 is still being relied upon by > > > the CIFS > > > driver, even though successful attacks on MD4 are as old as Linux > > > itself. > > > > > > So let's move the code into the CIFS driver, and remove it from > > > the > > > crypto API so that it is no longer exposed to other subsystems or > > > to > > > user space via AF_ALG. > > > > > > > Can we please stop removing algorithms from AF_ALG? > > I don't think we can, to be honest. We need to have a deprecation > path > for obsolete and insecure algorithms: the alternative is to keep > supporting a long tail of broken crypto indefinitely. I think you are ignoring the fact that by doing that you might be removing a migration path to more secure algorithms, for some legacy systems. I.e. in some cases this might mean sticking to insecure algorithm *and* old kernel for unnecessary long amount of time because migration is more costly. Perhaps there could be a comman-line parameter or similar to enable legacy crypto? /Jarkko ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-19 10:42 ` Jarkko Sakkinen @ 2021-08-19 17:10 ` Steve French 2021-08-19 20:54 ` ronnie sahlberg 0 siblings, 1 reply; 19+ messages in thread From: Steve French @ 2021-08-19 17:10 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Ard Biesheuvel, Denis Kenzior, Linux Crypto Mailing List, Herbert Xu, Eric Biggers, ronnie sahlberg, linux-cifs, Steve French, David Howells, keyrings, Shyam Prasad N, Andrew Bartlett On Thu, Aug 19, 2021 at 5:42 AM Jarkko Sakkinen <jarkko@kernel.org> wrote: > > On Wed, 2021-08-18 at 18:10 +0200, Ard Biesheuvel wrote: > > On Wed, 18 Aug 2021 at 16:51, Denis Kenzior <denkenz@gmail.com> > > wrote: > > > Hi Ard, > > > > > > On 8/18/21 9:46 AM, Ard Biesheuvel wrote: > > > > As discussed on the list [0], MD4 is still being relied upon by > > > > the CIFS > > > > driver, even though successful attacks on MD4 are as old as Linux > > > > itself. > > > > > > > > So let's move the code into the CIFS driver, and remove it from > > > > the > > > > crypto API so that it is no longer exposed to other subsystems or > > > > to > > > > user space via AF_ALG. > > > > > > > > > > Can we please stop removing algorithms from AF_ALG? > > > > I don't think we can, to be honest. We need to have a deprecation > > path > > for obsolete and insecure algorithms: the alternative is to keep > > supporting a long tail of broken crypto indefinitely. > > I think you are ignoring the fact that by doing that you might be > removing a migration path to more secure algorithms, for some legacy > systems. > > I.e. in some cases this might mean sticking to insecure algorithm *and* > old kernel for unnecessary long amount of time because migration is > more costly. > > Perhaps there could be a comman-line parameter or similar to enable > legacy crypto? There are. For example less secure NTLMv1 is disabled in the build. On the command line "sec=krb5" will use kerberos, and we can move that to be the default, or at least autonegotiate it, but will require some minor changes to cifs-utils to detect if plausible Kerberos ticket is available for this mount before requesting krb5 automatically. But ... we already have parameters to disable SMB1, and in fact if you mount with "mount -t smb3 ..." we won't let you use SMB1 or SMB2 so we get the security advantages of preventing man-in-the-middle attacks during negotiation and encryption etc. In addition, SMB1 can be disabled completely by simply doing "echo 1 > /sys/module/cifs/parameters/disable_legacy_dialects" but even without that, to mount insecurely with SMB1 requires specifying it (vers=1.0) on the command line. In addition requiring the strongest encryption can be set by "echo 1 > /sys/module/parameters/cifs/require_gcm_256" Although moving to a peer to peer auth solution better than NTLMSSP is something important to do ASAP (we should follow up e.g. on making sure we work with the "peer to peer Kerberos" (which is used by Apple for this purpose) see e.g. https://discussions-cn-prz.apple.com/en/thread/252949265) Andrew Bartlett's note explains the bigger picture well: "I would echo that, and also just remind folks that MD4 in NTLMSSP is used as a compression only, it has no security value. The security would be the same if the password was compressed with MD4, SHA1 or SHA256 - the security comes from the complexity of the password and the HMAC-MD5 rounds inside NTLMv2. I'll also mention the use of MD4, which is used to re-encrypt a short- term key with the long-term key out of the NTLMv2 scheme. This thankfully is an unchecksumed simple RC4 round of one random value with another, so not subject to known-plaintext attacks here. I know neither MD4 nor HMAC-MD5 is not flavour of the month any more, with good reason, but we would not want to go with way of NFSv4 which is, as I understand it, full Kerberos or bust (so folks choose no protection)." "Thankfully only the HMAC-MD5 step in what you mention is cryptographically significant, the rest are just very lossy compression algorithms." -- Thanks, Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] crypto: remove MD4 generic shash 2021-08-19 17:10 ` Steve French @ 2021-08-19 20:54 ` ronnie sahlberg 0 siblings, 0 replies; 19+ messages in thread From: ronnie sahlberg @ 2021-08-19 20:54 UTC (permalink / raw) To: Steve French Cc: Jarkko Sakkinen, Ard Biesheuvel, Denis Kenzior, Linux Crypto Mailing List, Herbert Xu, Eric Biggers, linux-cifs, Steve French, David Howells, keyrings, Shyam Prasad N, Andrew Bartlett On Fri, Aug 20, 2021 at 3:10 AM Steve French <smfrench@gmail.com> wrote: > > On Thu, Aug 19, 2021 at 5:42 AM Jarkko Sakkinen <jarkko@kernel.org> wrote: > > > > On Wed, 2021-08-18 at 18:10 +0200, Ard Biesheuvel wrote: > > > On Wed, 18 Aug 2021 at 16:51, Denis Kenzior <denkenz@gmail.com> > > > wrote: > > > > Hi Ard, > > > > > > > > On 8/18/21 9:46 AM, Ard Biesheuvel wrote: > > > > > As discussed on the list [0], MD4 is still being relied upon by > > > > > the CIFS > > > > > driver, even though successful attacks on MD4 are as old as Linux > > > > > itself. > > > > > > > > > > So let's move the code into the CIFS driver, and remove it from > > > > > the > > > > > crypto API so that it is no longer exposed to other subsystems or > > > > > to > > > > > user space via AF_ALG. > > > > > > > > > > > > > Can we please stop removing algorithms from AF_ALG? > > > > > > I don't think we can, to be honest. We need to have a deprecation > > > path > > > for obsolete and insecure algorithms: the alternative is to keep > > > supporting a long tail of broken crypto indefinitely. > > > > I think you are ignoring the fact that by doing that you might be > > removing a migration path to more secure algorithms, for some legacy > > systems. > > > > I.e. in some cases this might mean sticking to insecure algorithm *and* > > old kernel for unnecessary long amount of time because migration is > > more costly. > > > > Perhaps there could be a comman-line parameter or similar to enable > > legacy crypto? > > There are. For example less secure NTLMv1 is disabled in the build. > On the command line "sec=krb5" will use kerberos, and we can move that > to be the default, > or at least autonegotiate it, but will require some minor changes to > cifs-utils to detect if > plausible Kerberos ticket is available for this mount before > requesting krb5 automatically. > > But ... we already have parameters to disable SMB1, and in fact if you > mount with > "mount -t smb3 ..." we won't let you use SMB1 or SMB2 so we get the > security advantages > of preventing man-in-the-middle attacks during negotiation and encryption etc. > In addition, SMB1 can be disabled completely by simply doing > > "echo 1 > /sys/module/cifs/parameters/disable_legacy_dialects" > > but even without that, to mount insecurely with SMB1 requires > specifying it (vers=1.0) on the command line. > > In addition requiring the strongest encryption can be set by > > "echo 1 > /sys/module/parameters/cifs/require_gcm_256" > > > Although moving to a peer to peer auth solution better than NTLMSSP is > something important to do ASAP Agreed. We need a new peer-to-peer authentication mechanism. But this is storage so things move slowly and we can't remove a feature that hundreds of millions of people depend on and break their environments just because "need tickbox". As an example of how long it takes to migrate to a different solution in mass deployed storage technologies just look at SMB1. The whole industry has been working hard and as fast as possible to move away from SMB1 since 2006 and we are still NOT at the stage where we can delete this functionality. Disable it by default for some configurations but delete? No. Even if we get a new peer-to-peer mechanism today, it will take up to 20 years before we will be able to delete MD4 support in the linux kernel for CIFS. Unless we want a wildly disruptive event where we either break storage for hundreds of millions of people or we force them to remain on 5.13 for the next 20 years. Storage is not the same as a cell-phone, or some consumer device where you can drop support or break them every few years. The timelines for migrating in storage is measured in decades. But I guess we will have a loadable module for MD4 in cifs that both the cifs client and the cifs server will be able to use. Other subsystems that have hard dependency on MD4 and can not just break their users can probably also just use the cifs_md4 module too if they want to. At least we can try to avoid a situation where we will have multiple different MD4 implementations for the next decade or two in the kernel. > (we should follow up e.g. on making sure we work with the "peer to > peer Kerberos" (which is used by Apple > for this purpose) see e.g. > https://discussions-cn-prz.apple.com/en/thread/252949265) > > Andrew Bartlett's note explains the bigger picture well: > > "I would echo that, and also just remind folks that MD4 in NTLMSSP is > used as a compression only, it has no security value. The security > would be the same if the password was compressed with MD4, SHA1 or > SHA256 - the security comes from the complexity of the password and the > HMAC-MD5 rounds inside NTLMv2. > > I'll also mention the use of MD4, which is used to re-encrypt a short- > term key with the long-term key out of the NTLMv2 scheme. This > thankfully is an unchecksumed simple RC4 round of one random value with > another, so not subject to known-plaintext attacks here. I know neither > MD4 nor HMAC-MD5 is not flavour of the month any more, > with good reason, but we would not want to go with way of NFSv4 which > is, as I understand it, full Kerberos or bust (so folks choose no > protection)." > > "Thankfully only the HMAC-MD5 step in what you mention is > cryptographically significant, the rest are just very lossy compression > algorithms." > > > > -- > Thanks, > > Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-08-19 20:55 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-18 14:46 [PATCH 0/2] crypto: remove MD4 generic shash Ard Biesheuvel 2021-08-18 14:46 ` [PATCH 1/2] fs/cifs: Incorporate obsolete MD4 crypto code Ard Biesheuvel 2021-08-18 14:46 ` [PATCH 2/2] crypto: md4 - Remove obsolete algorithm Ard Biesheuvel 2021-08-18 14:51 ` [PATCH 0/2] crypto: remove MD4 generic shash Denis Kenzior 2021-08-18 16:10 ` Ard Biesheuvel 2021-08-18 16:23 ` Denis Kenzior 2021-08-18 16:47 ` Steve French 2021-08-18 22:08 ` Jeremy Allison 2021-08-19 3:49 ` Andrew Bartlett 2021-08-19 5:18 ` Eric Biggers 2021-08-19 5:23 ` Andrew Bartlett 2021-08-18 21:11 ` ronnie sahlberg 2021-08-18 22:10 ` Ard Biesheuvel 2021-08-18 22:22 ` Denis Kenzior 2021-08-18 23:03 ` Steve French 2021-08-19 16:56 ` Denis Kenzior 2021-08-19 10:42 ` Jarkko Sakkinen 2021-08-19 17:10 ` Steve French 2021-08-19 20:54 ` ronnie sahlberg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.