* [PATCH 0/3] crypto: authenc - fix key parsing
@ 2018-12-17 7:23 Eric Biggers
2018-12-17 7:23 ` [PATCH 1/3] crypto: authenc - fix parsing key with misaligned rta_len Eric Biggers
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Eric Biggers @ 2018-12-17 7:23 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: Rob Rice, Raveendra Padasalagi, Gilad Ben-Yossef, Yael Chemla
Fix incorrect validation of the key passed to "authenc" AEADs that
allowed crashing the kernel via AF_ALG. The real fix is in patch 1, but
two drivers had to be converted to use crypto_authenc_extractkeys() too.
Please note: for the bcm and ccree driver changes I haven't re-run the
self-tests, as I don't have that hardware. Please do so if you can.
Eric Biggers (3):
crypto: authenc - fix parsing key with misaligned rta_len
crypto: bcm - convert to use crypto_authenc_extractkeys()
crypto: ccree - convert to use crypto_authenc_extractkeys()
crypto/authenc.c | 14 ++++++++---
drivers/crypto/Kconfig | 1 +
drivers/crypto/bcm/cipher.c | 44 ++++++++++------------------------
drivers/crypto/ccree/cc_aead.c | 40 +++++++++++++++----------------
4 files changed, 44 insertions(+), 55 deletions(-)
--
2.19.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] crypto: authenc - fix parsing key with misaligned rta_len
2018-12-17 7:23 [PATCH 0/3] crypto: authenc - fix key parsing Eric Biggers
@ 2018-12-17 7:23 ` Eric Biggers
2018-12-17 7:23 ` [PATCH 2/3] crypto: bcm - convert to use crypto_authenc_extractkeys() Eric Biggers
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Eric Biggers @ 2018-12-17 7:23 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: Rob Rice, Raveendra Padasalagi, Gilad Ben-Yossef, Yael Chemla
From: Eric Biggers <ebiggers@google.com>
Keys for "authenc" AEADs are formatted as an rtattr containing a 4-byte
'enckeylen', followed by an authentication key and an encryption key.
crypto_authenc_extractkeys() parses the key to find the inner keys.
However, it fails to consider the case where the rtattr's payload is
longer than 4 bytes but not 4-byte aligned, and where the key ends
before the next 4-byte aligned boundary. In this case, 'keylen -=
RTA_ALIGN(rta->rta_len);' underflows to a value near UINT_MAX. This
causes a buffer overread and crash during crypto_ahash_setkey().
Fix it by restricting the rtattr payload to the expected size.
Reproducer using AF_ALG:
#include <linux/if_alg.h>
#include <linux/rtnetlink.h>
#include <sys/socket.h>
int main()
{
int fd;
struct sockaddr_alg addr = {
.salg_type = "aead",
.salg_name = "authenc(hmac(sha256),cbc(aes))",
};
struct {
struct rtattr attr;
__be32 enckeylen;
char keys[1];
} __attribute__((packed)) key = {
.attr.rta_len = sizeof(key),
.attr.rta_type = 1 /* CRYPTO_AUTHENC_KEYA_PARAM */,
};
fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
bind(fd, (void *)&addr, sizeof(addr));
setsockopt(fd, SOL_ALG, ALG_SET_KEY, &key, sizeof(key));
}
It caused:
BUG: unable to handle kernel paging request at ffff88007ffdc000
PGD 2e01067 P4D 2e01067 PUD 2e04067 PMD 2e05067 PTE 0
Oops: 0000 [#1] SMP
CPU: 0 PID: 883 Comm: authenc Not tainted 4.20.0-rc1-00108-g00c9fe37a7f27 #13
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-20181126_142135-anatol 04/01/2014
RIP: 0010:sha256_ni_transform+0xb3/0x330 arch/x86/crypto/sha256_ni_asm.S:155
[...]
Call Trace:
sha256_ni_finup+0x10/0x20 arch/x86/crypto/sha256_ssse3_glue.c:321
crypto_shash_finup+0x1a/0x30 crypto/shash.c:178
shash_digest_unaligned+0x45/0x60 crypto/shash.c:186
crypto_shash_digest+0x24/0x40 crypto/shash.c:202
hmac_setkey+0x135/0x1e0 crypto/hmac.c:66
crypto_shash_setkey+0x2b/0xb0 crypto/shash.c:66
shash_async_setkey+0x10/0x20 crypto/shash.c:223
crypto_ahash_setkey+0x2d/0xa0 crypto/ahash.c:202
crypto_authenc_setkey+0x68/0x100 crypto/authenc.c:96
crypto_aead_setkey+0x2a/0xc0 crypto/aead.c:62
aead_setkey+0xc/0x10 crypto/algif_aead.c:526
alg_setkey crypto/af_alg.c:223 [inline]
alg_setsockopt+0xfe/0x130 crypto/af_alg.c:256
__sys_setsockopt+0x6d/0xd0 net/socket.c:1902
__do_sys_setsockopt net/socket.c:1913 [inline]
__se_sys_setsockopt net/socket.c:1910 [inline]
__x64_sys_setsockopt+0x1f/0x30 net/socket.c:1910
do_syscall_64+0x4a/0x180 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Fixes: e236d4a89a2f ("[CRYPTO] authenc: Move enckeylen into key itself")
Cc: <stable@vger.kernel.org> # v2.6.25+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/authenc.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/crypto/authenc.c b/crypto/authenc.c
index 37f54d1b2f669..4be293a4b5f0f 100644
--- a/crypto/authenc.c
+++ b/crypto/authenc.c
@@ -58,14 +58,22 @@ int crypto_authenc_extractkeys(struct crypto_authenc_keys *keys, const u8 *key,
return -EINVAL;
if (rta->rta_type != CRYPTO_AUTHENC_KEYA_PARAM)
return -EINVAL;
- if (RTA_PAYLOAD(rta) < sizeof(*param))
+
+ /*
+ * RTA_OK() didn't align the rtattr's payload when validating that it
+ * fits in the buffer. Yet, the keys should start on the next 4-byte
+ * aligned boundary. To avoid confusion, require that the rtattr
+ * payload be exactly the param struct, which has a 4-byte aligned size.
+ */
+ if (RTA_PAYLOAD(rta) != sizeof(*param))
return -EINVAL;
+ BUILD_BUG_ON(sizeof(*param) % RTA_ALIGNTO);
param = RTA_DATA(rta);
keys->enckeylen = be32_to_cpu(param->enckeylen);
- key += RTA_ALIGN(rta->rta_len);
- keylen -= RTA_ALIGN(rta->rta_len);
+ key += rta->rta_len;
+ keylen -= rta->rta_len;
if (keylen < keys->enckeylen)
return -EINVAL;
--
2.19.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] crypto: bcm - convert to use crypto_authenc_extractkeys()
2018-12-17 7:23 [PATCH 0/3] crypto: authenc - fix key parsing Eric Biggers
2018-12-17 7:23 ` [PATCH 1/3] crypto: authenc - fix parsing key with misaligned rta_len Eric Biggers
@ 2018-12-17 7:23 ` Eric Biggers
2018-12-17 7:23 ` [PATCH 3/3] crypto: ccree " Eric Biggers
2019-01-10 14:03 ` [PATCH 0/3] crypto: authenc - fix key parsing Herbert Xu
3 siblings, 0 replies; 5+ messages in thread
From: Eric Biggers @ 2018-12-17 7:23 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: Rob Rice, Raveendra Padasalagi, Gilad Ben-Yossef, Yael Chemla
From: Eric Biggers <ebiggers@google.com>
Convert the bcm crypto driver to use crypto_authenc_extractkeys() so
that it picks up the fix for broken validation of rtattr::rta_len.
This also fixes the DES weak key check to actually be done on the right
key. (It was checking the authentication key, not the encryption key...)
Fixes: 9d12ba86f818 ("crypto: brcm - Add Broadcom SPU driver")
Cc: <stable@vger.kernel.org> # v4.11+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/crypto/Kconfig | 1 +
drivers/crypto/bcm/cipher.c | 44 +++++++++++--------------------------
2 files changed, 14 insertions(+), 31 deletions(-)
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index d80751d48cf1e..04ab0f5d272ee 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -692,6 +692,7 @@ config CRYPTO_DEV_BCM_SPU
depends on ARCH_BCM_IPROC
depends on MAILBOX
default m
+ select CRYPTO_AUTHENC
select CRYPTO_DES
select CRYPTO_MD5
select CRYPTO_SHA1
diff --git a/drivers/crypto/bcm/cipher.c b/drivers/crypto/bcm/cipher.c
index 2ce3a16d3d10f..14fb60d73162d 100644
--- a/drivers/crypto/bcm/cipher.c
+++ b/drivers/crypto/bcm/cipher.c
@@ -2845,44 +2845,28 @@ static int aead_authenc_setkey(struct crypto_aead *cipher,
struct spu_hw *spu = &iproc_priv.spu;
struct iproc_ctx_s *ctx = crypto_aead_ctx(cipher);
struct crypto_tfm *tfm = crypto_aead_tfm(cipher);
- struct rtattr *rta = (void *)key;
- struct crypto_authenc_key_param *param;
- const u8 *origkey = key;
- const unsigned int origkeylen = keylen;
-
- int ret = 0;
+ struct crypto_authenc_keys keys;
+ int ret;
flow_log("%s() aead:%p key:%p keylen:%u\n", __func__, cipher, key,
keylen);
flow_dump(" key: ", key, keylen);
- if (!RTA_OK(rta, keylen))
- goto badkey;
- if (rta->rta_type != CRYPTO_AUTHENC_KEYA_PARAM)
- goto badkey;
- if (RTA_PAYLOAD(rta) < sizeof(*param))
+ ret = crypto_authenc_extractkeys(&keys, key, keylen);
+ if (ret)
goto badkey;
- param = RTA_DATA(rta);
- ctx->enckeylen = be32_to_cpu(param->enckeylen);
-
- key += RTA_ALIGN(rta->rta_len);
- keylen -= RTA_ALIGN(rta->rta_len);
-
- if (keylen < ctx->enckeylen)
- goto badkey;
- if (ctx->enckeylen > MAX_KEY_SIZE)
+ if (keys.enckeylen > MAX_KEY_SIZE ||
+ keys.authkeylen > MAX_KEY_SIZE)
goto badkey;
- ctx->authkeylen = keylen - ctx->enckeylen;
-
- if (ctx->authkeylen > MAX_KEY_SIZE)
- goto badkey;
+ ctx->enckeylen = keys.enckeylen;
+ ctx->authkeylen = keys.authkeylen;
- memcpy(ctx->enckey, key + ctx->authkeylen, ctx->enckeylen);
+ memcpy(ctx->enckey, keys.enckey, keys.enckeylen);
/* May end up padding auth key. So make sure it's zeroed. */
memset(ctx->authkey, 0, sizeof(ctx->authkey));
- memcpy(ctx->authkey, key, ctx->authkeylen);
+ memcpy(ctx->authkey, keys.authkey, keys.authkeylen);
switch (ctx->alg->cipher_info.alg) {
case CIPHER_ALG_DES:
@@ -2890,7 +2874,7 @@ static int aead_authenc_setkey(struct crypto_aead *cipher,
u32 tmp[DES_EXPKEY_WORDS];
u32 flags = CRYPTO_TFM_RES_WEAK_KEY;
- if (des_ekey(tmp, key) == 0) {
+ if (des_ekey(tmp, keys.enckey) == 0) {
if (crypto_aead_get_flags(cipher) &
CRYPTO_TFM_REQ_WEAK_KEY) {
crypto_aead_set_flags(cipher, flags);
@@ -2905,7 +2889,7 @@ static int aead_authenc_setkey(struct crypto_aead *cipher,
break;
case CIPHER_ALG_3DES:
if (ctx->enckeylen == (DES_KEY_SIZE * 3)) {
- const u32 *K = (const u32 *)key;
+ const u32 *K = (const u32 *)keys.enckey;
u32 flags = CRYPTO_TFM_RES_BAD_KEY_SCHED;
if (!((K[0] ^ K[2]) | (K[1] ^ K[3])) ||
@@ -2956,9 +2940,7 @@ static int aead_authenc_setkey(struct crypto_aead *cipher,
ctx->fallback_cipher->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK;
ctx->fallback_cipher->base.crt_flags |=
tfm->crt_flags & CRYPTO_TFM_REQ_MASK;
- ret =
- crypto_aead_setkey(ctx->fallback_cipher, origkey,
- origkeylen);
+ ret = crypto_aead_setkey(ctx->fallback_cipher, key, keylen);
if (ret) {
flow_log(" fallback setkey() returned:%d\n", ret);
tfm->crt_flags &= ~CRYPTO_TFM_RES_MASK;
--
2.19.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] crypto: ccree - convert to use crypto_authenc_extractkeys()
2018-12-17 7:23 [PATCH 0/3] crypto: authenc - fix key parsing Eric Biggers
2018-12-17 7:23 ` [PATCH 1/3] crypto: authenc - fix parsing key with misaligned rta_len Eric Biggers
2018-12-17 7:23 ` [PATCH 2/3] crypto: bcm - convert to use crypto_authenc_extractkeys() Eric Biggers
@ 2018-12-17 7:23 ` Eric Biggers
2019-01-10 14:03 ` [PATCH 0/3] crypto: authenc - fix key parsing Herbert Xu
3 siblings, 0 replies; 5+ messages in thread
From: Eric Biggers @ 2018-12-17 7:23 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: Rob Rice, Raveendra Padasalagi, Gilad Ben-Yossef, Yael Chemla
From: Eric Biggers <ebiggers@google.com>
Convert the ccree crypto driver to use crypto_authenc_extractkeys() so
that it picks up the fix for broken validation of rtattr::rta_len.
Fixes: ff27e85a85bb ("crypto: ccree - add AEAD support")
Cc: <stable@vger.kernel.org> # v4.17+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/crypto/ccree/cc_aead.c | 40 ++++++++++++++++------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/crypto/ccree/cc_aead.c b/drivers/crypto/ccree/cc_aead.c
index f2643cda45db9..a3527c00b29a9 100644
--- a/drivers/crypto/ccree/cc_aead.c
+++ b/drivers/crypto/ccree/cc_aead.c
@@ -549,13 +549,12 @@ static int cc_aead_setkey(struct crypto_aead *tfm, const u8 *key,
unsigned int keylen)
{
struct cc_aead_ctx *ctx = crypto_aead_ctx(tfm);
- struct rtattr *rta = (struct rtattr *)key;
struct cc_crypto_req cc_req = {};
- struct crypto_authenc_key_param *param;
struct cc_hw_desc desc[MAX_AEAD_SETKEY_SEQ];
- int rc = -EINVAL;
unsigned int seq_len = 0;
struct device *dev = drvdata_to_dev(ctx->drvdata);
+ const u8 *enckey, *authkey;
+ int rc;
dev_dbg(dev, "Setting key in context @%p for %s. key=%p keylen=%u\n",
ctx, crypto_tfm_alg_name(crypto_aead_tfm(tfm)), key, keylen);
@@ -563,35 +562,33 @@ static int cc_aead_setkey(struct crypto_aead *tfm, const u8 *key,
/* STAT_PHASE_0: Init and sanity checks */
if (ctx->auth_mode != DRV_HASH_NULL) { /* authenc() alg. */
- if (!RTA_OK(rta, keylen))
- goto badkey;
- if (rta->rta_type != CRYPTO_AUTHENC_KEYA_PARAM)
- goto badkey;
- if (RTA_PAYLOAD(rta) < sizeof(*param))
- goto badkey;
- param = RTA_DATA(rta);
- ctx->enc_keylen = be32_to_cpu(param->enckeylen);
- key += RTA_ALIGN(rta->rta_len);
- keylen -= RTA_ALIGN(rta->rta_len);
- if (keylen < ctx->enc_keylen)
+ struct crypto_authenc_keys keys;
+
+ rc = crypto_authenc_extractkeys(&keys, key, keylen);
+ if (rc)
goto badkey;
- ctx->auth_keylen = keylen - ctx->enc_keylen;
+ enckey = keys.enckey;
+ authkey = keys.authkey;
+ ctx->enc_keylen = keys.enckeylen;
+ ctx->auth_keylen = keys.authkeylen;
if (ctx->cipher_mode == DRV_CIPHER_CTR) {
/* the nonce is stored in bytes at end of key */
+ rc = -EINVAL;
if (ctx->enc_keylen <
(AES_MIN_KEY_SIZE + CTR_RFC3686_NONCE_SIZE))
goto badkey;
/* Copy nonce from last 4 bytes in CTR key to
* first 4 bytes in CTR IV
*/
- memcpy(ctx->ctr_nonce, key + ctx->auth_keylen +
- ctx->enc_keylen - CTR_RFC3686_NONCE_SIZE,
- CTR_RFC3686_NONCE_SIZE);
+ memcpy(ctx->ctr_nonce, enckey + ctx->enc_keylen -
+ CTR_RFC3686_NONCE_SIZE, CTR_RFC3686_NONCE_SIZE);
/* Set CTR key size */
ctx->enc_keylen -= CTR_RFC3686_NONCE_SIZE;
}
} else { /* non-authenc - has just one key */
+ enckey = key;
+ authkey = NULL;
ctx->enc_keylen = keylen;
ctx->auth_keylen = 0;
}
@@ -603,13 +600,14 @@ static int cc_aead_setkey(struct crypto_aead *tfm, const u8 *key,
/* STAT_PHASE_1: Copy key to ctx */
/* Get key material */
- memcpy(ctx->enckey, key + ctx->auth_keylen, ctx->enc_keylen);
+ memcpy(ctx->enckey, enckey, ctx->enc_keylen);
if (ctx->enc_keylen == 24)
memset(ctx->enckey + 24, 0, CC_AES_KEY_SIZE_MAX - 24);
if (ctx->auth_mode == DRV_HASH_XCBC_MAC) {
- memcpy(ctx->auth_state.xcbc.xcbc_keys, key, ctx->auth_keylen);
+ memcpy(ctx->auth_state.xcbc.xcbc_keys, authkey,
+ ctx->auth_keylen);
} else if (ctx->auth_mode != DRV_HASH_NULL) { /* HMAC */
- rc = cc_get_plain_hmac_key(tfm, key, ctx->auth_keylen);
+ rc = cc_get_plain_hmac_key(tfm, authkey, ctx->auth_keylen);
if (rc)
goto badkey;
}
--
2.19.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] crypto: authenc - fix key parsing
2018-12-17 7:23 [PATCH 0/3] crypto: authenc - fix key parsing Eric Biggers
` (2 preceding siblings ...)
2018-12-17 7:23 ` [PATCH 3/3] crypto: ccree " Eric Biggers
@ 2019-01-10 14:03 ` Herbert Xu
3 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2019-01-10 14:03 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto, Rob Rice, Raveendra Padasalagi, Gilad Ben-Yossef,
Yael Chemla
On Sun, Dec 16, 2018 at 11:23:21PM -0800, Eric Biggers wrote:
> Fix incorrect validation of the key passed to "authenc" AEADs that
> allowed crashing the kernel via AF_ALG. The real fix is in patch 1, but
> two drivers had to be converted to use crypto_authenc_extractkeys() too.
>
> Please note: for the bcm and ccree driver changes I haven't re-run the
> self-tests, as I don't have that hardware. Please do so if you can.
>
> Eric Biggers (3):
> crypto: authenc - fix parsing key with misaligned rta_len
> crypto: bcm - convert to use crypto_authenc_extractkeys()
> crypto: ccree - convert to use crypto_authenc_extractkeys()
>
> crypto/authenc.c | 14 ++++++++---
> drivers/crypto/Kconfig | 1 +
> drivers/crypto/bcm/cipher.c | 44 ++++++++++------------------------
> drivers/crypto/ccree/cc_aead.c | 40 +++++++++++++++----------------
> 4 files changed, 44 insertions(+), 55 deletions(-)
All applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-10 14:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-17 7:23 [PATCH 0/3] crypto: authenc - fix key parsing Eric Biggers
2018-12-17 7:23 ` [PATCH 1/3] crypto: authenc - fix parsing key with misaligned rta_len Eric Biggers
2018-12-17 7:23 ` [PATCH 2/3] crypto: bcm - convert to use crypto_authenc_extractkeys() Eric Biggers
2018-12-17 7:23 ` [PATCH 3/3] crypto: ccree " Eric Biggers
2019-01-10 14:03 ` [PATCH 0/3] crypto: authenc - fix key parsing Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).