[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87h6z8e7jc.fsf@suse.de>
Date: Wed, 09 Nov 2022 11:39:19 +0100
From: Nicolai Stange <nstange@...e.de>
To: "Elliott, Robert (Servers)" <elliott@....com>
Cc: Nicolai Stange <nstange@...e.de>,
Herbert Xu <herbert@...dor.apana.org.au>,
"David S. Miller" <davem@...emloft.net>,
Vladis Dronov <vdronov@...hat.com>,
Stephan Mueller <smueller@...onox.de>,
"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] crypto: xts - restrict key lengths to approved
values in FIPS mode
"Elliott, Robert (Servers)" <elliott@....com> writes:
>> diff --git a/include/crypto/xts.h b/include/crypto/xts.h
> ...
>> @@ -35,6 +35,13 @@ static inline int xts_verify_key(struct crypto_skcipher
>> *tfm,
>> if (keylen % 2)
>> return -EINVAL;
>>
>> + /*
>> + * In FIPS mode only a combined key length of either 256 or
>> + * 512 bits is allowed, c.f. FIPS 140-3 IG C.I.
>> + */
>> + if (fips_enabled && keylen != 32 && keylen != 64)
>> + return -EINVAL;
>> +
>> /* ensure that the AES and tweak key are not identical */
>> if ((fips_enabled || (crypto_skcipher_get_flags(tfm) &
>> CRYPTO_TFM_REQ_FORBID_WEAK_KEYS)) &&
>> --
>> 2.38.0
>
> arch/s390/crypto/aes_s390.c has similar lines:
>
> static int xts_aes_set_key(struct crypto_skcipher *tfm, const u8 *in_key,
> unsigned int key_len)
> {
> struct s390_xts_ctx *xts_ctx = crypto_skcipher_ctx(tfm);
> unsigned long fc;
> int err;
>
> err = xts_fallback_setkey(tfm, in_key, key_len);
> if (err)
> return err;
>
> /* In fips mode only 128 bit or 256 bit keys are valid */
> if (fips_enabled && key_len != 32 && key_len != 64)
> return -EINVAL;
>
>
> xts_fallback_setkey will now enforce that rule when setting up the
> fallback algorithm keys, which makes the xts_aes_set_key check
> unreachable.
Good finding!
>
> If that fallback setup were not present, then a call to xts_verify_key
> might be preferable to enforce any other rules like the WEAK_KEYS
> rule.
>
So if this patch here would get accepted, I'd propose to remove the then
dead code from aes_s390 afterwards and make an explicit call to
xts_verify_key() instead.
Or shall I split out the XTS patch from this series here and post these
two changes separately then? Herbert, any preferences?
Thanks!
Nicolai
--
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
(HRB 36809, AG Nürnberg)
Powered by blists - more mailing lists