diff mbox series

[18/18] crypto: dh - accept only approved safe-prime groups in FIPS mode

Message ID 20211201004858.19831-19-nstange@suse.de (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show
Series crypto: dh - infrastructure for NVM in-band auth and FIPS conformance | expand

Commit Message

Nicolai Stange Dec. 1, 2021, 12:48 a.m. UTC
SP800-56Arev3, sec. 5.5.2 ("Assurance of Domain-Parameter Validity")
asserts that an implementation needs to verify domain paramtere validity,
which boils down to either
- the domain parameters corresponding to some known safe-prime group
  explicitly listed to be approved in the document or
- for parameters conforming to a "FIPS 186-type parameter-size set",
  that the implementation needs to perform an explicit domain parameter
  verification, which would require access to the "seed" and "counter"
  values used in their generation.

The latter is not easily feasible and moreover, SP800-56Arev3 states that
safe-prime groups are preferred and that FIPS 186-type parameter sets
should only be supported for backward compatibility, if it all.

Make the dh implementations reject any domain parameters which don't
correspond to any of the approved safe-prime groups in FIPS mode. The
approved safe-prime groups are the ones specified in RFC 7919 and RFC 3526,
and given that all possible values of enum dh_group_id correspond to
either groups from these RFCs or to dh_group_id_unknown, it suffices to
make crypto_dh_decode_key() to reject any parameter set where
->group_id == dh_group_id_unknown.

As this change will effectively render the dh implementation unusable in
FIPS mode if neither of the CRYPTO_DH_GROUPS_RFC7919 or
CRYPTO_DH_GROUPS_RFC3526 Kconfig options enabled, make CRYPTO_DH imply
these two if CRYPTO_FIPS is set.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 crypto/Kconfig     | 2 ++
 crypto/dh_helper.c | 4 ++++
 2 files changed, 6 insertions(+)

Comments

Hannes Reinecke Dec. 1, 2021, 7:34 a.m. UTC | #1
On 12/1/21 1:48 AM, Nicolai Stange wrote:
> SP800-56Arev3, sec. 5.5.2 ("Assurance of Domain-Parameter Validity")
> asserts that an implementation needs to verify domain paramtere validity,
> which boils down to either
> - the domain parameters corresponding to some known safe-prime group
>    explicitly listed to be approved in the document or
> - for parameters conforming to a "FIPS 186-type parameter-size set",
>    that the implementation needs to perform an explicit domain parameter
>    verification, which would require access to the "seed" and "counter"
>    values used in their generation.
> 
> The latter is not easily feasible and moreover, SP800-56Arev3 states that
> safe-prime groups are preferred and that FIPS 186-type parameter sets
> should only be supported for backward compatibility, if it all.
> 
> Make the dh implementations reject any domain parameters which don't
> correspond to any of the approved safe-prime groups in FIPS mode. The
> approved safe-prime groups are the ones specified in RFC 7919 and RFC 3526,
> and given that all possible values of enum dh_group_id correspond to
> either groups from these RFCs or to dh_group_id_unknown, it suffices to
> make crypto_dh_decode_key() to reject any parameter set where
> ->group_id == dh_group_id_unknown.
> 
> As this change will effectively render the dh implementation unusable in
> FIPS mode if neither of the CRYPTO_DH_GROUPS_RFC7919 or
> CRYPTO_DH_GROUPS_RFC3526 Kconfig options enabled, make CRYPTO_DH imply
> these two if CRYPTO_FIPS is set.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>   crypto/Kconfig     | 2 ++
>   crypto/dh_helper.c | 4 ++++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 578711b02bb3..571f2271ad2e 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -229,6 +229,8 @@ menuconfig CRYPTO_DH
>   	select CRYPTO_KPP
>   	select MPILIB
>   	select CRYPTO_RNG_DEFAULT
> +	imply CRYPTO_DH_GROUPS_RFC7919 if CRYPTO_FIPS
> +	imply CRYPTO_DH_GROUPS_RFC3526 if CRYPTO_FIPS
>   	help
>   	  Generic implementation of the Diffie-Hellman algorithm.
>   
> diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
> index cf632beca65e..f30674df0d76 100644
> --- a/crypto/dh_helper.c
> +++ b/crypto/dh_helper.c
> @@ -7,6 +7,7 @@
>   #include <linux/export.h>
>   #include <linux/err.h>
>   #include <linux/string.h>
> +#include <linux/fips.h>
>   #include <crypto/dh.h>
>   #include <crypto/kpp.h>
>   #include <crypto/rng.h>
> @@ -622,6 +623,9 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
>   	    params->g_size > params->p_size)
>   		return -EINVAL;
>   
> +	/* Only safe-prime groups are allowed in FIPS mode. */
> +	if (fips_enabled && params->group_id == dh_group_id_unknown)
> +		return -EINVAL;
>   
>   	return 0;
>   }
> 
That was cheap.
Maybe merge it with the previous patch?

Cheers,

Hannes
Nicolai Stange Dec. 9, 2021, 9:26 a.m. UTC | #2
Hannes Reinecke <hare@suse.de> writes:

> On 12/1/21 1:48 AM, Nicolai Stange wrote:
>> SP800-56Arev3, sec. 5.5.2 ("Assurance of Domain-Parameter Validity")
>> asserts that an implementation needs to verify domain paramtere validity,
>> which boils down to either
>> - the domain parameters corresponding to some known safe-prime group
>>    explicitly listed to be approved in the document or
>> - for parameters conforming to a "FIPS 186-type parameter-size set",
>>    that the implementation needs to perform an explicit domain parameter
>>    verification, which would require access to the "seed" and "counter"
>>    values used in their generation.
>>
>> The latter is not easily feasible and moreover, SP800-56Arev3 states that
>> safe-prime groups are preferred and that FIPS 186-type parameter sets
>> should only be supported for backward compatibility, if it all.
>>
>> Make the dh implementations reject any domain parameters which don't
>> correspond to any of the approved safe-prime groups in FIPS mode. The
>> approved safe-prime groups are the ones specified in RFC 7919 and RFC 3526,
>> and given that all possible values of enum dh_group_id correspond to
>> either groups from these RFCs or to dh_group_id_unknown, it suffices to
>> make crypto_dh_decode_key() to reject any parameter set where
>> ->group_id == dh_group_id_unknown.
>>
>> As this change will effectively render the dh implementation unusable in
>> FIPS mode if neither of the CRYPTO_DH_GROUPS_RFC7919 or
>> CRYPTO_DH_GROUPS_RFC3526 Kconfig options enabled, make CRYPTO_DH imply
>> these two if CRYPTO_FIPS is set.
>>
>> Signed-off-by: Nicolai Stange <nstange@suse.de>
>> ---
>>   crypto/Kconfig     | 2 ++
>>   crypto/dh_helper.c | 4 ++++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/crypto/Kconfig b/crypto/Kconfig
>> index 578711b02bb3..571f2271ad2e 100644
>> --- a/crypto/Kconfig
>> +++ b/crypto/Kconfig
>> @@ -229,6 +229,8 @@ menuconfig CRYPTO_DH
>>   	select CRYPTO_KPP
>>   	select MPILIB
>>   	select CRYPTO_RNG_DEFAULT
>> +	imply CRYPTO_DH_GROUPS_RFC7919 if CRYPTO_FIPS
>> +	imply CRYPTO_DH_GROUPS_RFC3526 if CRYPTO_FIPS
>>   	help
>>   	  Generic implementation of the Diffie-Hellman algorithm.
>>   diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
>> index cf632beca65e..f30674df0d76 100644
>> --- a/crypto/dh_helper.c
>> +++ b/crypto/dh_helper.c
>> @@ -7,6 +7,7 @@
>>   #include <linux/export.h>
>>   #include <linux/err.h>
>>   #include <linux/string.h>
>> +#include <linux/fips.h>
>>   #include <crypto/dh.h>
>>   #include <crypto/kpp.h>
>>   #include <crypto/rng.h>
>> @@ -622,6 +623,9 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
>>   	    params->g_size > params->p_size)
>>   		return -EINVAL;
>>   +	/* Only safe-prime groups are allowed in FIPS mode. */
>> +	if (fips_enabled && params->group_id == dh_group_id_unknown)
>> +		return -EINVAL;
>>     	return 0;
>>   }
>>
> That was cheap.
> Maybe merge it with the previous patch?

FWIW, I kept this separate in v2: the code change is trivial for sure,
but as this is FIPS related, the premise might be controversial and I
don't want to hide it in a larger patch.

Thanks,

Nicolai
diff mbox series

Patch

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 578711b02bb3..571f2271ad2e 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -229,6 +229,8 @@  menuconfig CRYPTO_DH
 	select CRYPTO_KPP
 	select MPILIB
 	select CRYPTO_RNG_DEFAULT
+	imply CRYPTO_DH_GROUPS_RFC7919 if CRYPTO_FIPS
+	imply CRYPTO_DH_GROUPS_RFC3526 if CRYPTO_FIPS
 	help
 	  Generic implementation of the Diffie-Hellman algorithm.
 
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index cf632beca65e..f30674df0d76 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -7,6 +7,7 @@ 
 #include <linux/export.h>
 #include <linux/err.h>
 #include <linux/string.h>
+#include <linux/fips.h>
 #include <crypto/dh.h>
 #include <crypto/kpp.h>
 #include <crypto/rng.h>
@@ -622,6 +623,9 @@  int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	    params->g_size > params->p_size)
 		return -EINVAL;
 
+	/* Only safe-prime groups are allowed in FIPS mode. */
+	if (fips_enabled && params->group_id == dh_group_id_unknown)
+		return -EINVAL;
 
 	return 0;
 }