| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <VI1PR0402MB34855E46247C86307CFE871898CB0@VI1PR0402MB3485.eurprd04.prod.outlook.com>
Date: Fri, 19 Jul 2019 14:38:25 +0000
From: Horia Geanta <horia.geanta@....com>
To: Iuliana Prodan <iuliana.prodan@....com>,
Herbert Xu <herbert@...dor.apana.org.au>,
Aymen Sghaier <aymen.sghaier@....com>
CC: "David S. Miller" <davem@...emloft.net>,
"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH v2 04/14] crypto: caam - check key length
On 7/19/2019 2:58 AM, Iuliana Prodan wrote:
> Check key length to solve the extra tests that expect -EINVAL to be
> returned when the key size is not valid.
>
> Validated AES keylen for skcipher and ahash.
>
Also aead was updated.
> The check_aes_keylen function is added in a common file, to be used
> also for caam/qi and caam/qi2.
>
[...]
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index 28d55a0..6ac59b1 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
[...]
> @@ -683,10 +691,17 @@ static int rfc4106_setkey(struct crypto_aead *aead,
> {
> struct caam_ctx *ctx = crypto_aead_ctx(aead);
> struct device *jrdev = ctx->jrdev;
> + int err;
>
> if (keylen < 4)
> return -EINVAL;
>
This is no longer needed, check_aes_keylen() catches this case too.
> + err = check_aes_keylen(keylen - 4);
> + if (err) {
> + crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return err;
> + }
> +
> print_hex_dump_debug("key in @"__stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
>
> @@ -707,10 +722,17 @@ static int rfc4543_setkey(struct crypto_aead *aead,
> {
> struct caam_ctx *ctx = crypto_aead_ctx(aead);
> struct device *jrdev = ctx->jrdev;
> + int err;
>
> if (keylen < 4)
> return -EINVAL;
>
Same here, check_aes_keylen() handles this case.
> + err = check_aes_keylen(keylen - 4);
> + if (err) {
> + crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return err;
> + }
> +
> print_hex_dump_debug("key in @"__stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
>
[...]
> diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
> index 66531d6..46097e3 100644
> --- a/drivers/crypto/caam/caamalg_qi.c
> +++ b/drivers/crypto/caam/caamalg_qi.c
> @@ -385,6 +385,12 @@ static int gcm_setkey(struct crypto_aead *aead,
> struct device *jrdev = ctx->jrdev;
> int ret;
>
> + ret = check_aes_keylen(keylen);
> + if (ret) {
> + crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return ret;
> + }
> +
> print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
>
> @@ -483,6 +489,12 @@ static int rfc4106_setkey(struct crypto_aead *aead,
> if (keylen < 4)
> return -EINVAL;
>
Same here, check_aes_keylen() handles this case.
> + ret = check_aes_keylen(keylen - 4);
> + if (ret) {
> + crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return ret;
> + }
> +
> print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
>
> @@ -585,6 +597,12 @@ static int rfc4543_setkey(struct crypto_aead *aead,
> if (keylen < 4)
> return -EINVAL;
>
Same here, check_aes_keylen() handles this case.
> + ret = check_aes_keylen(keylen - 4);
> + if (ret) {
> + crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return ret;
> + }
> +
> print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
>
[...]
> +static int des_skcipher_setkey(struct crypto_skcipher *skcipher,
> + const u8 *key, unsigned int keylen)
> +{
> + u32 tmp[DES_EXPKEY_WORDS];
> +
> + if (!des_ekey(tmp, key) && (crypto_skcipher_get_flags(skcipher) &
CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI needs to select CRYPTO_DES,
such that des_ekey is available.
> + CRYPTO_TFM_REQ_FORBID_WEAK_KEYS)) {
> + crypto_skcipher_set_flags(skcipher,
> + CRYPTO_TFM_RES_WEAK_KEY);
> + return -EINVAL;
> + }
> +
> + return skcipher_setkey(skcipher, key, keylen, 0);
> }
>
> static int xts_skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
[...]
> diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
> index bc370af..da4abf1 100644
> --- a/drivers/crypto/caam/caamalg_qi2.c
> +++ b/drivers/crypto/caam/caamalg_qi2.c
[...]
> @@ -817,10 +823,17 @@ static int rfc4106_setkey(struct crypto_aead *aead,
> {
> struct caam_ctx *ctx = crypto_aead_ctx(aead);
> struct device *dev = ctx->dev;
> + int ret;
>
> if (keylen < 4)
> return -EINVAL;
>
Same here, check_aes_keylen() handles this case.
> + ret = check_aes_keylen(keylen - 4);
> + if (ret) {
> + crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return ret;
> + }
> +
> print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
>
> @@ -911,10 +924,17 @@ static int rfc4543_setkey(struct crypto_aead *aead,
> {
> struct caam_ctx *ctx = crypto_aead_ctx(aead);
> struct device *dev = ctx->dev;
> + int ret;
>
> if (keylen < 4)
> return -EINVAL;
>
Same here, check_aes_keylen() handles this case.
> + ret = check_aes_keylen(keylen - 4);
> + if (ret) {
> + crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return ret;
> + }
> +
> print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
>
[...]
> +static int chacha20_skcipher_setkey(struct crypto_skcipher *skcipher,
> + const u8 *key, unsigned int keylen)
> +{
> + return skcipher_setkey(skcipher, key, keylen, 0);
> +}
Missing check for key length.
> +
> +static int des_skcipher_setkey(struct crypto_skcipher *skcipher,
> + const u8 *key, unsigned int keylen)
> +{
> + u32 tmp[DES3_EDE_EXPKEY_WORDS];
> + struct crypto_tfm *tfm = crypto_skcipher_tfm(skcipher);
> +
> + if (keylen == DES3_EDE_KEY_SIZE &&
> + __des3_ede_setkey(tmp, &tfm->crt_flags, key, DES3_EDE_KEY_SIZE)) {
> + return -EINVAL;
> + }
> +
> + if (!des_ekey(tmp, key) && (crypto_skcipher_get_flags(skcipher) &
CRYPTO_DEV_FSL_DPAA2_CAAM needs to select CRYPTO_DES,
such that __des3_ede_setkey and des_ekey are available.
> + CRYPTO_TFM_REQ_FORBID_WEAK_KEYS)) {
> + crypto_skcipher_set_flags(skcipher,
> + CRYPTO_TFM_RES_WEAK_KEY);
> + return -EINVAL;
> + }
> +
> + return skcipher_setkey(skcipher, key, keylen, 0);
> }
[...]
> diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
> index 73abefa..2ec4bad 100644
> --- a/drivers/crypto/caam/caamhash.c
> +++ b/drivers/crypto/caam/caamhash.c
> @@ -62,6 +62,7 @@
> #include "desc_constr.h"
> #include "jr.h"
> #include "error.h"
> +#include "common_if.h"
> #include "sg_sw_sec4.h"
> #include "key_gen.h"
> #include "caamhash_desc.h"
> @@ -501,6 +502,9 @@ static int axcbc_setkey(struct crypto_ahash *ahash, const u8 *key,
> struct caam_hash_ctx *ctx = crypto_ahash_ctx(ahash);
> struct device *jrdev = ctx->jrdev;
>
> + if (keylen != AES_KEYSIZE_128)
> + return -EINVAL;
> +
CRYPTO_TFM_RES_BAD_KEY_LEN flag should be set on the error path.
> memcpy(ctx->key, key, keylen);
> dma_sync_single_for_device(jrdev, ctx->key_dma, keylen, DMA_TO_DEVICE);
> ctx->adata.keylen = keylen;
> @@ -515,6 +519,13 @@ static int acmac_setkey(struct crypto_ahash *ahash, const u8 *key,
> unsigned int keylen)
> {
> struct caam_hash_ctx *ctx = crypto_ahash_ctx(ahash);
> + int err;
> +
> + err = check_aes_keylen(keylen);
> + if (err) {
> + crypto_ahash_set_flags(ahash, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return err;
> + }
>
> /* key is immediate data for all cmac shared descriptors */
> ctx->adata.key_virt = key;
Horia
Powered by blists - more mailing lists