[PATCH 0/4] Trivial set of FIPS 140-3 related changes

[PATCH 0/4] Trivial set of FIPS 140-3 related changes

[Nicolai Stange]

Hi all,

these four rather unrelated patches are basically a dump of some of the
more trivial changes required for working towards FIPS 140-3 conformance.

Please pick as you deem appropriate.

Thanks!

Nicolai

Nicolai Stange (4):
  crypto: xts - restrict key lengths to approved values in FIPS mode
  crypto: testmgr - disallow plain cbcmac(aes) in FIPS mode
  crypto: testmgr - disallow plain ghash in FIPS mode
  crypto: testmgr - allow ecdsa-nist-p256 and -p384 in FIPS mode

 crypto/testmgr.c     | 4 ++--
 include/crypto/xts.h | 7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.38.0

[PATCH 1/4] crypto: xts - restrict key lengths to approved values in FIPS mode

[Nicolai Stange]

According to FIPS 140-3 IG C.I., only (total) key lengths of either
256 bits or 512 bits are allowed with xts(aes). Make xts_verify_key() to
reject anything else in FIPS mode.

As xts(aes) is the only approved xts() template instantiation in FIPS mode,
the new restriction implemented in xts_verify_key() effectively only
applies to this particular construction.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 include/crypto/xts.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/crypto/xts.h b/include/crypto/xts.h
index 0f8dba69feb4..a233c1054df2 100644
--- 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

RE: [PATCH 1/4] crypto: xts - restrict key lengths to approved values in FIPS mode

[Elliott, Robert (Servers)]

> 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.

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.

Re: [PATCH 1/4] crypto: xts - restrict key lengths to approved values in FIPS mode

[Nicolai Stange]

"Elliott, Robert (Servers)" <elliott@hpe.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)

Re: [PATCH 1/4] crypto: xts - restrict key lengths to approved values in FIPS mode

[Herbert Xu]

On Wed, Nov 09, 2022 at 11:39:19AM +0100, Nicolai Stange wrote:
>
> Or shall I split out the XTS patch from this series here and post these
> two changes separately then? Herbert, any preferences?

You can do this as a follow-up.

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

RE: [PATCH 1/4] crypto: xts - restrict key lengths to approved values in FIPS mode

[Elliott, Robert (Servers)]

> 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

There's another function in the same file called xts_check_key() 
that is used by some of the hardware drivers:

arch/s390/crypto/paes_s390.c:    * xts_check_key verifies the key length is not odd and makes
 [that references it in the comment but actually calls xts_verify_key in the code]
drivers/crypto/axis/artpec6_crypto.c:   ret = xts_check_key(&cipher->base, key, keylen);
drivers/crypto/cavium/cpt/cptvf_algs.c: err = xts_check_key(tfm, key, keylen);
drivers/crypto/cavium/nitrox/nitrox_skcipher.c: ret = xts_check_key(tfm, key, keylen);
drivers/crypto/ccree/cc_cipher.c:           xts_check_key(tfm, key, keylen)) {
drivers/crypto/marvell/octeontx/otx_cptvf_algs.c:       ret = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);
drivers/crypto/marvell/octeontx2/otx2_cptvf_algs.c:     ret = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);
drivers/crypto/atmel-aes.c:     err = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);

It already has one check qualified by fips_enabled:

        /* ensure that the AES and tweak key are not identical */
        if (fips_enabled && !crypto_memneq(key, key + (keylen / 2), keylen / 2))
                return -EINVAL;

Should that implement the same key length restrictions?

Re: [PATCH 1/4] crypto: xts - restrict key lengths to approved values in FIPS mode

[Nicolai Stange]

"Elliott, Robert (Servers)" <elliott@hpe.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
>
> There's another function in the same file called xts_check_key() 
> that is used by some of the hardware drivers:

Right, thanks for spotting.

AFAICT, xts_check_key() is the older of the two variants,
xts_verify_key() had been introduced with commit f1c131b45410 ("crypto:
xts - Convert to skcipher"). There had initially only been a single
call from generic crypto/xts.c and the main difference to
xts_check_key() had been that it took a crypto_skcipher for its tfm
argument rather than a plain crypto_tfm as xts_check_key() did.

It seems that over time, xts crypto drivers adopted the newer
xts_verify_key() variant then.

>
> arch/s390/crypto/paes_s390.c:    * xts_check_key verifies the key length is not odd and makes
>  [that references it in the comment but actually calls xts_verify_key in the code]
> drivers/crypto/axis/artpec6_crypto.c:   ret = xts_check_key(&cipher->base, key, keylen);
> drivers/crypto/cavium/cpt/cptvf_algs.c: err = xts_check_key(tfm, key, keylen);
> drivers/crypto/cavium/nitrox/nitrox_skcipher.c: ret = xts_check_key(tfm, key, keylen);
> drivers/crypto/ccree/cc_cipher.c:           xts_check_key(tfm, key, keylen)) {
> drivers/crypto/marvell/octeontx/otx_cptvf_algs.c:       ret = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);
> drivers/crypto/marvell/octeontx2/otx2_cptvf_algs.c:     ret = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);
> drivers/crypto/atmel-aes.c:     err = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);
>
> It already has one check qualified by fips_enabled:
>
>         /* ensure that the AES and tweak key are not identical */
>         if (fips_enabled && !crypto_memneq(key, key + (keylen / 2), keylen / 2))
>                 return -EINVAL;
>
> Should that implement the same key length restrictions?

From a quick glance, all of the above drivers merely convert some
crypto_skcipher to a crypto_tfm before passing it to xts_check_key().

So I think these should all be made to call xts_verify_key() directly
instead, the former xts_check_key() could then get dropped. But that's
more of a cleanup IMO and would probably deserve a separate patch series
on its own.

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)

Re: [PATCH 1/4] crypto: xts - restrict key lengths to approved values in FIPS mode

[Herbert Xu]

On Wed, Nov 09, 2022 at 11:06:17AM +0100, Nicolai Stange wrote:
>
> >From a quick glance, all of the above drivers merely convert some
> crypto_skcipher to a crypto_tfm before passing it to xts_check_key().
> 
> So I think these should all be made to call xts_verify_key() directly
> instead, the former xts_check_key() could then get dropped. But that's
> more of a cleanup IMO and would probably deserve a separate patch series
> on its own.

We should make sure both do the same thing though.  So either
change all the drivers or just change xts_check_key in your patch
in addition to xts_verify_key.

Cheers,
-- 
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

[PATCH 2/4] crypto: testmgr - disallow plain cbcmac(aes) in FIPS mode

[Nicolai Stange]

cbcmac(aes) may be used only as part of the ccm(aes) construction in FIPS
mode. Since commit d6097b8d5d55 ("crypto: api - allow algs only in specific
constructions in FIPS mode") there's support for using spawns which by
itself are marked as non-approved from approved template instantiations.
So simply mark plain cbcmac(aes) as non-approved in testmgr to block any
attempts of direct instantiations in FIPS mode.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/testmgr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index e2806ef044fd..1ffbe3abb84a 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4501,7 +4501,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 	}, {
 #endif
 		.alg = "cbcmac(aes)",
-		.fips_allowed = 1,
 		.test = alg_test_hash,
 		.suite = {
 			.hash = __VECS(aes_cbcmac_tv_template)
-- 
2.38.0

[PATCH 3/4] crypto: testmgr - disallow plain ghash in FIPS mode

[Nicolai Stange]

ghash may be used only as part of the gcm(aes) construction in FIPS
mode. Since commit d6097b8d5d55 ("crypto: api - allow algs only in specific
constructions in FIPS mode") there's support for using spawns which by
itself are marked as non-approved from approved template instantiations.
So simply mark plain ghash as non-approved in testmgr to block any attempts
of direct instantiations in FIPS mode.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/testmgr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 1ffbe3abb84a..6d91a2acd119 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -5125,7 +5125,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 	}, {
 		.alg = "ghash",
 		.test = alg_test_hash,
-		.fips_allowed = 1,
 		.suite = {
 			.hash = __VECS(ghash_tv_template)
 		}
-- 
2.38.0

[PATCH 4/4] crypto: testmgr - allow ecdsa-nist-p256 and -p384 in FIPS mode

[Nicolai Stange]

The kernel provides implementations of the NIST ECDSA signature
verification primitives. For key sizes of 256 and 384 bits respectively
they are approved and can be enabled in FIPS mode. Do so.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/testmgr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 6d91a2acd119..f641f9c830d8 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -5034,12 +5034,14 @@ static const struct alg_test_desc alg_test_descs[] = {
 	}, {
 		.alg = "ecdsa-nist-p256",
 		.test = alg_test_akcipher,
+		.fips_allowed = 1,
 		.suite = {
 			.akcipher = __VECS(ecdsa_nist_p256_tv_template)
 		}
 	}, {
 		.alg = "ecdsa-nist-p384",
 		.test = alg_test_akcipher,
+		.fips_allowed = 1,
 		.suite = {
 			.akcipher = __VECS(ecdsa_nist_p384_tv_template)
 		}
-- 
2.38.0

Re: [PATCH 0/4] Trivial set of FIPS 140-3 related changes

[Vladis Dronov]

Hi Nicolai, Robert, Herbert, all,

I would like to revive this older upstream email thread. I would like
to address notes from reviewers (namely, Robert) by additional patches
so the whole patchset can be accepted. This should ease our future
kernel work re: FIPS.

The below 2 patches address (I hope) both notes Robert and Herbert have
provided (thanks!). I hope the whole patchset can be accepted then.

Logically my 2 patches should follow [PATCH 1/4] and be patches 2 and 3.
Herbert is it possible to reorder them when accepting?

Thank you! and

Best regards,
Vladis

Vladis Dronov (2):
  crypto: xts - drop xts_check_key()
  crypto: xts - drop redundant xts key check

Re: [PATCH 0/4] Trivial set of FIPS 140-3 related changes

[Eric Biggers]

On Wed, Dec 21, 2022 at 04:24:00PM +0100, Vladis Dronov wrote:
> Hi Nicolai, Robert, Herbert, all,
> 
> I would like to revive this older upstream email thread. I would like
> to address notes from reviewers (namely, Robert) by additional patches
> so the whole patchset can be accepted. This should ease our future
> kernel work re: FIPS.
> 
> The below 2 patches address (I hope) both notes Robert and Herbert have
> provided (thanks!). I hope the whole patchset can be accepted then.
> 
> Logically my 2 patches should follow [PATCH 1/4] and be patches 2 and 3.
> Herbert is it possible to reorder them when accepting?
> 
> Thank you! and
> 
> Best regards,
> Vladis

Please just resend the whole series, with the --base option to git format-patch
used, so that reviewers don't have to try to piece it together.

- Eric

Re: [PATCH 0/4] Trivial set of FIPS 140-3 related changes

[Vladis Dronov]

Hi,

On Wed, Dec 21, 2022 at 9:56 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Dec 21, 2022 at 04:24:00PM +0100, Vladis Dronov wrote:
> > Hi Nicolai, Robert, Herbert, all,
> >
> > I would like to revive this older upstream email thread. I would like
> > to address notes from reviewers (namely, Robert) by additional patches
> > so the whole patchset can be accepted. This should ease our future
> > kernel work re: FIPS.
> >
> > The below 2 patches address (I hope) both notes Robert and Herbert have
> > provided (thanks!). I hope the whole patchset can be accepted then.
> >
> > Logically my 2 patches should follow [PATCH 1/4] and be patches 2 and 3.
> > Herbert is it possible to reorder them when accepting?
> >
> > Thank you! and
> >
> > Best regards,
> > Vladis
>
> Please just resend the whole series, with the --base option to git format-patch
> used, so that reviewers don't have to try to piece it together.

Thank you, Eric, the patchset was resend with a proper ordering:

https://lore.kernel.org/linux-crypto/20221221224111.19254-1-vdronov@redhat.com/T/#t
with a subject: [PATCH 0/6] Trivial set of FIPS 140-3 related changes

Best regards,
Vladis

[PATCH 5/6] crypto: xts - drop xts_check_key()

[Vladis Dronov]

xts_check_key() is obsoleted by xts_verify_key(). Over time XTS crypto
drivers adopted the newer xts_verify_key() variant, but xts_check_key()
is still used by a number of drivers. Switch drivers to use the newer
xts_verify_key() and make a couple of cleanups. This allows us to drop
xts_check_key() completely and avoid redundancy.

Signed-off-by: Vladis Dronov <vdronov@redhat.com>
---
 arch/s390/crypto/paes_s390.c                  |  2 +-
 drivers/crypto/atmel-aes.c                    |  2 +-
 drivers/crypto/axis/artpec6_crypto.c          |  2 +-
 drivers/crypto/cavium/cpt/cptvf_algs.c        |  8 +++----
 .../crypto/cavium/nitrox/nitrox_skcipher.c    |  8 +++----
 drivers/crypto/ccree/cc_cipher.c              |  2 +-
 .../crypto/marvell/octeontx/otx_cptvf_algs.c  |  2 +-
 .../marvell/octeontx2/otx2_cptvf_algs.c       |  2 +-
 include/crypto/xts.h                          | 21 +++----------------
 9 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
index a279b7d23a5e..29dc827e0fe8 100644
--- a/arch/s390/crypto/paes_s390.c
+++ b/arch/s390/crypto/paes_s390.c
@@ -474,7 +474,7 @@ static int xts_paes_set_key(struct crypto_skcipher *tfm, const u8 *in_key,
 		return rc;
 
 	/*
-	 * xts_check_key verifies the key length is not odd and makes
+	 * xts_verify_key verifies the key length is not odd and makes
 	 * sure that the two keys are not the same. This can be done
 	 * on the two protected keys as well
 	 */
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 886bf258544c..130f8bf09a9a 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -1879,7 +1879,7 @@ static int atmel_aes_xts_setkey(struct crypto_skcipher *tfm, const u8 *key,
 	struct atmel_aes_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
 	int err;
 
-	err = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);
+	err = xts_verify_key(tfm, key, keylen);
 	if (err)
 		return err;
 
diff --git a/drivers/crypto/axis/artpec6_crypto.c b/drivers/crypto/axis/artpec6_crypto.c
index 51c66afbe677..f6f41e316dfe 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -1621,7 +1621,7 @@ artpec6_crypto_xts_set_key(struct crypto_skcipher *cipher, const u8 *key,
 		crypto_skcipher_ctx(cipher);
 	int ret;
 
-	ret = xts_check_key(&cipher->base, key, keylen);
+	ret = xts_verify_key(cipher, key, keylen);
 	if (ret)
 		return ret;
 
diff --git a/drivers/crypto/cavium/cpt/cptvf_algs.c b/drivers/crypto/cavium/cpt/cptvf_algs.c
index ce3b91c612f0..6a7760544780 100644
--- a/drivers/crypto/cavium/cpt/cptvf_algs.c
+++ b/drivers/crypto/cavium/cpt/cptvf_algs.c
@@ -232,13 +232,12 @@ static int cvm_decrypt(struct skcipher_request *req)
 static int cvm_xts_setkey(struct crypto_skcipher *cipher, const u8 *key,
 		   u32 keylen)
 {
-	struct crypto_tfm *tfm = crypto_skcipher_tfm(cipher);
-	struct cvm_enc_ctx *ctx = crypto_tfm_ctx(tfm);
+	struct cvm_enc_ctx *ctx = crypto_skcipher_ctx(cipher);
 	int err;
 	const u8 *key1 = key;
 	const u8 *key2 = key + (keylen / 2);
 
-	err = xts_check_key(tfm, key, keylen);
+	err = xts_verify_key(cipher, key, keylen);
 	if (err)
 		return err;
 	ctx->key_len = keylen;
@@ -289,8 +288,7 @@ static int cvm_validate_keylen(struct cvm_enc_ctx *ctx, u32 keylen)
 static int cvm_setkey(struct crypto_skcipher *cipher, const u8 *key,
 		      u32 keylen, u8 cipher_type)
 {
-	struct crypto_tfm *tfm = crypto_skcipher_tfm(cipher);
-	struct cvm_enc_ctx *ctx = crypto_tfm_ctx(tfm);
+	struct cvm_enc_ctx *ctx = crypto_skcipher_ctx(cipher);
 
 	ctx->cipher_type = cipher_type;
 	if (!cvm_validate_keylen(ctx, keylen)) {
diff --git a/drivers/crypto/cavium/nitrox/nitrox_skcipher.c b/drivers/crypto/cavium/nitrox/nitrox_skcipher.c
index 248b4fff1c72..138261dcd032 100644
--- a/drivers/crypto/cavium/nitrox/nitrox_skcipher.c
+++ b/drivers/crypto/cavium/nitrox/nitrox_skcipher.c
@@ -337,12 +337,11 @@ static int nitrox_3des_decrypt(struct skcipher_request *skreq)
 static int nitrox_aes_xts_setkey(struct crypto_skcipher *cipher,
 				 const u8 *key, unsigned int keylen)
 {
-	struct crypto_tfm *tfm = crypto_skcipher_tfm(cipher);
-	struct nitrox_crypto_ctx *nctx = crypto_tfm_ctx(tfm);
+	struct nitrox_crypto_ctx *nctx = crypto_skcipher_ctx(cipher);
 	struct flexi_crypto_context *fctx;
 	int aes_keylen, ret;
 
-	ret = xts_check_key(tfm, key, keylen);
+	ret = xts_verify_key(cipher, key, keylen);
 	if (ret)
 		return ret;
 
@@ -362,8 +361,7 @@ static int nitrox_aes_xts_setkey(struct crypto_skcipher *cipher,
 static int nitrox_aes_ctr_rfc3686_setkey(struct crypto_skcipher *cipher,
 					 const u8 *key, unsigned int keylen)
 {
-	struct crypto_tfm *tfm = crypto_skcipher_tfm(cipher);
-	struct nitrox_crypto_ctx *nctx = crypto_tfm_ctx(tfm);
+	struct nitrox_crypto_ctx *nctx = crypto_skcipher_ctx(cipher);
 	struct flexi_crypto_context *fctx;
 	int aes_keylen;
 
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 309da6334a0a..2cd44d7457a4 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -460,7 +460,7 @@ static int cc_cipher_setkey(struct crypto_skcipher *sktfm, const u8 *key,
 	}
 
 	if (ctx_p->cipher_mode == DRV_CIPHER_XTS &&
-	    xts_check_key(tfm, key, keylen)) {
+	    xts_verify_key(sktfm, key, keylen)) {
 		dev_dbg(dev, "weak XTS key");
 		return -EINVAL;
 	}
diff --git a/drivers/crypto/marvell/octeontx/otx_cptvf_algs.c b/drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
index 01c48ddc4eeb..b9e7433aba8e 100644
--- a/drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
+++ b/drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
@@ -398,7 +398,7 @@ static int otx_cpt_skcipher_xts_setkey(struct crypto_skcipher *tfm,
 	const u8 *key1 = key;
 	int ret;
 
-	ret = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);
+	ret = xts_verify_key(tfm, key, keylen);
 	if (ret)
 		return ret;
 	ctx->key_len = keylen;
diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptvf_algs.c b/drivers/crypto/marvell/octeontx2/otx2_cptvf_algs.c
index 67530e90bbfe..11b7f504bbd7 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cptvf_algs.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cptvf_algs.c
@@ -412,7 +412,7 @@ static int otx2_cpt_skcipher_xts_setkey(struct crypto_skcipher *tfm,
 	const u8 *key1 = key;
 	int ret;
 
-	ret = xts_check_key(crypto_skcipher_tfm(tfm), key, keylen);
+	ret = xts_verify_key(tfm, key, keylen);
 	if (ret)
 		return ret;
 	ctx->key_len = keylen;
diff --git a/include/crypto/xts.h b/include/crypto/xts.h
index a233c1054df2..5a6a2cc89d49 100644
--- a/include/crypto/xts.h
+++ b/include/crypto/xts.h
@@ -8,23 +8,6 @@
 
 #define XTS_BLOCK_SIZE 16
 
-static inline int xts_check_key(struct crypto_tfm *tfm,
-				const u8 *key, unsigned int keylen)
-{
-	/*
-	 * key consists of keys of equal size concatenated, therefore
-	 * the length must be even.
-	 */
-	if (keylen % 2)
-		return -EINVAL;
-
-	/* ensure that the AES and tweak key are not identical */
-	if (fips_enabled && !crypto_memneq(key, key + (keylen / 2), keylen / 2))
-		return -EINVAL;
-
-	return 0;
-}
-
 static inline int xts_verify_key(struct crypto_skcipher *tfm,
 				 const u8 *key, unsigned int keylen)
 {
@@ -42,7 +25,9 @@ static inline int xts_verify_key(struct crypto_skcipher *tfm,
 	if (fips_enabled && keylen != 32 && keylen != 64)
 		return -EINVAL;
 
-	/* ensure that the AES and tweak key are not identical */
+	/* ensure that the AES and tweak key are not identical
+	 * when in FIPS mode or the FORBID_WEAK_KEYS flag is set.
+	 */
 	if ((fips_enabled || (crypto_skcipher_get_flags(tfm) &
 			      CRYPTO_TFM_REQ_FORBID_WEAK_KEYS)) &&
 	    !crypto_memneq(key, key + (keylen / 2), keylen / 2))
-- 
2.37.2

[PATCH 6/6] crypto: xts - drop redundant xts key check

[Vladis Dronov]

xts_fallback_setkey() in xts_aes_set_key() will now enforce key size
rule in FIPS mode when setting up the fallback algorithm keys, which
makes the check in xts_aes_set_key() redundant or unreachable. So just
drop this check.

xts_fallback_setkey() now makes a key size check in xts_verify_key():

xts_fallback_setkey()
  crypto_skcipher_setkey() [ skcipher_setkey_unaligned() ]
    cipher->setkey() { .setkey = xts_setkey }
      xts_setkey()
        xts_verify_key()

Signed-off-by: Vladis Dronov <vdronov@redhat.com>
---
 arch/s390/crypto/aes_s390.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
index 526c3f40f6a2..c773820e4af9 100644
--- a/arch/s390/crypto/aes_s390.c
+++ b/arch/s390/crypto/aes_s390.c
@@ -398,10 +398,6 @@ static int xts_aes_set_key(struct crypto_skcipher *tfm, const u8 *in_key,
 	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;
-
 	/* Pick the correct function code based on the key length */
 	fc = (key_len == 32) ? CPACF_KM_XTS_128 :
 	     (key_len == 64) ? CPACF_KM_XTS_256 : 0;
-- 
2.37.2