[PATCH 1/4] crypto: drbg - ensure most preferred type is FIPS health checked

[PATCH 1/4] crypto: drbg - ensure most preferred type is FIPS health checked

[Dimitri John Ledkov]

drbg supports multiple types of drbg, and multiple parameters of
each. Health check sanity only checks one drbg of a single type. One
can enable all three types of drbg. And instead of checking the most
preferred algorithm (last one wins), it is currently checking first
one instead.

Update ifdef to ensure that healthcheck prefers HMAC, over HASH, over
CTR, last one wins, like all other code and functions.

Fixes: 541af946fe ("crypto: drbg - SP800-90A Deterministic Random Bit Generator")

Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
---
 crypto/drbg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index ff4ebbc68e..2cce18dcfc 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -2018,9 +2018,11 @@ static inline int __init drbg_healthcheck_sanity(void)
 
 #ifdef CONFIG_CRYPTO_DRBG_CTR
 	drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
-#elif defined CONFIG_CRYPTO_DRBG_HASH
+#endif
+#ifdef CONFIG_CRYPTO_DRBG_HASH
 	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
-#else
+#endif
+#ifdef CONFIG_CRYPTO_DRBG_HMAC
 	drbg_convert_tfm_core("drbg_nopr_hmac_sha256", &coreref, &pr);
 #endif
 
-- 
2.34.1

[PATCH 2/4] crypto: drbg - update FIPS CTR self-checks to aes256

[Dimitri John Ledkov]

When originally drbg was introduced FIPS self-checks for all types but
CTR were using the most preferred parameters for each type of
DRBG. Update CTR self-check to use aes256.

Fixes: 541af946fe ("crypto: drbg - SP800-90A Deterministic Random Bit Generator")
Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
---
 crypto/drbg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 2cce18dcfc..b120e2866b 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1478,8 +1478,8 @@ static int drbg_generate(struct drbg_state *drbg,
 			err = alg_test("drbg_pr_hmac_sha256",
 				       "drbg_pr_hmac_sha256", 0, 0);
 		else if (drbg->core->flags & DRBG_CTR)
-			err = alg_test("drbg_pr_ctr_aes128",
-				       "drbg_pr_ctr_aes128", 0, 0);
+			err = alg_test("drbg_pr_ctr_aes256",
+				       "drbg_pr_ctr_aes256", 0, 0);
 		else
 			err = alg_test("drbg_pr_sha256",
 				       "drbg_pr_sha256", 0, 0);
@@ -2017,7 +2017,7 @@ static inline int __init drbg_healthcheck_sanity(void)
 		return 0;
 
 #ifdef CONFIG_CRYPTO_DRBG_CTR
-	drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
+	drbg_convert_tfm_core("drbg_nopr_ctr_aes256", &coreref, &pr);
 #endif
 #ifdef CONFIG_CRYPTO_DRBG_HASH
 	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
-- 
2.34.1

Re: [PATCH 2/4] crypto: drbg - update FIPS CTR self-checks to aes256

[Stephan Mueller]

Am Sonntag, 29. Oktober 2023, 21:48:21 CET schrieb Dimitri John Ledkov:

Hi Dimitri,

> When originally drbg was introduced FIPS self-checks for all types but
> CTR were using the most preferred parameters for each type of
> DRBG. Update CTR self-check to use aes256.
> 
> Fixes: 541af946fe ("crypto: drbg - SP800-90A Deterministic Random Bit
> Generator") Signed-off-by: Dimitri John Ledkov
> <dimitri.ledkov@canonical.com>



> ---
>  crypto/drbg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/crypto/drbg.c b/crypto/drbg.c
> index 2cce18dcfc..b120e2866b 100644
> --- a/crypto/drbg.c
> +++ b/crypto/drbg.c
> @@ -1478,8 +1478,8 @@ static int drbg_generate(struct drbg_state *drbg,
>  			err = alg_test("drbg_pr_hmac_sha256",
>  				       "drbg_pr_hmac_sha256", 0, 0);
>  		else if (drbg->core->flags & DRBG_CTR)
> -			err = alg_test("drbg_pr_ctr_aes128",
> -				       "drbg_pr_ctr_aes128", 0, 0);
> +			err = alg_test("drbg_pr_ctr_aes256",
> +				       "drbg_pr_ctr_aes256", 0, 0);
>  		else
>  			err = alg_test("drbg_pr_sha256",
>  				       "drbg_pr_sha256", 0, 0);
> @@ -2017,7 +2017,7 @@ static inline int __init drbg_healthcheck_sanity(void)
> return 0;
> 
>  #ifdef CONFIG_CRYPTO_DRBG_CTR
> -	drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
> +	drbg_convert_tfm_core("drbg_nopr_ctr_aes256", &coreref, &pr);
>  #endif
>  #ifdef CONFIG_CRYPTO_DRBG_HASH
>  	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);

I am not sure again whether this warrants a "Fixes" tag, because the first 
modification is in a commented-out code section and the latter again intends 
to test the DRBG thresholds and thus just needs "a" DRBG.

Anyhow:

Reviewed-by: Stephan Mueller <smueller@chronox.de>


Ciao
Stephan

Re: [PATCH 2/4] crypto: drbg - update FIPS CTR self-checks to aes256

[Dimitri John Ledkov]

On Mon, 30 Oct 2023 at 12:23, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Sonntag, 29. Oktober 2023, 21:48:21 CET schrieb Dimitri John Ledkov:
>
> Hi Dimitri,
>
> > When originally drbg was introduced FIPS self-checks for all types but
> > CTR were using the most preferred parameters for each type of
> > DRBG. Update CTR self-check to use aes256.
> >
> > Fixes: 541af946fe ("crypto: drbg - SP800-90A Deterministic Random Bit
> > Generator") Signed-off-by: Dimitri John Ledkov
> > <dimitri.ledkov@canonical.com>
>
>
>
> > ---
> >  crypto/drbg.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/crypto/drbg.c b/crypto/drbg.c
> > index 2cce18dcfc..b120e2866b 100644
> > --- a/crypto/drbg.c
> > +++ b/crypto/drbg.c
> > @@ -1478,8 +1478,8 @@ static int drbg_generate(struct drbg_state *drbg,
> >                       err = alg_test("drbg_pr_hmac_sha256",
> >                                      "drbg_pr_hmac_sha256", 0, 0);
> >               else if (drbg->core->flags & DRBG_CTR)
> > -                     err = alg_test("drbg_pr_ctr_aes128",
> > -                                    "drbg_pr_ctr_aes128", 0, 0);
> > +                     err = alg_test("drbg_pr_ctr_aes256",
> > +                                    "drbg_pr_ctr_aes256", 0, 0);
> >               else
> >                       err = alg_test("drbg_pr_sha256",
> >                                      "drbg_pr_sha256", 0, 0);
> > @@ -2017,7 +2017,7 @@ static inline int __init drbg_healthcheck_sanity(void)
> > return 0;
> >
> >  #ifdef CONFIG_CRYPTO_DRBG_CTR
> > -     drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
> > +     drbg_convert_tfm_core("drbg_nopr_ctr_aes256", &coreref, &pr);
> >  #endif
> >  #ifdef CONFIG_CRYPTO_DRBG_HASH
> >       drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
>
> I am not sure again whether this warrants a "Fixes" tag, because the first
> modification is in a commented-out code section and the latter again intends
> to test the DRBG thresholds and thus just needs "a" DRBG.
>

Agreed, it is more of a reference. But yes, I don't believe this needs
to trigger stable backports - because yes it is like dead code, or
fips only code which majority kernels don't do. Maybe I should
reference that commit in description only?

> Anyhow:
>
> Reviewed-by: Stephan Mueller <smueller@chronox.de>
>
>
> Ciao
> Stephan
>
>

-- 
okurrr,

Dimitri

Re: [PATCH 2/4] crypto: drbg - update FIPS CTR self-checks to aes256

[Stephan Mueller]

Am Montag, 30. Oktober 2023, 11:30:32 CET schrieb Dimitri John Ledkov:

Hi Dimitri,

> On Mon, 30 Oct 2023 at 12:23, Stephan Mueller <smueller@chronox.de> wrote:
> > Am Sonntag, 29. Oktober 2023, 21:48:21 CET schrieb Dimitri John Ledkov:
> > 
> > Hi Dimitri,
> > 
> > > When originally drbg was introduced FIPS self-checks for all types but
> > > CTR were using the most preferred parameters for each type of
> > > DRBG. Update CTR self-check to use aes256.
> > > 
> > > Fixes: 541af946fe ("crypto: drbg - SP800-90A Deterministic Random Bit
> > > Generator") Signed-off-by: Dimitri John Ledkov
> > > <dimitri.ledkov@canonical.com>
> > > 
> > > 
> > > 
> > > ---
> > > 
> > >  crypto/drbg.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/crypto/drbg.c b/crypto/drbg.c
> > > index 2cce18dcfc..b120e2866b 100644
> > > --- a/crypto/drbg.c
> > > +++ b/crypto/drbg.c
> > > @@ -1478,8 +1478,8 @@ static int drbg_generate(struct drbg_state *drbg,
> > > 
> > >                       err = alg_test("drbg_pr_hmac_sha256",
> > >                       
> > >                                      "drbg_pr_hmac_sha256", 0, 0);
> > >               
> > >               else if (drbg->core->flags & DRBG_CTR)
> > > 
> > > -                     err = alg_test("drbg_pr_ctr_aes128",
> > > -                                    "drbg_pr_ctr_aes128", 0, 0);
> > > +                     err = alg_test("drbg_pr_ctr_aes256",
> > > +                                    "drbg_pr_ctr_aes256", 0, 0);
> > > 
> > >               else
> > >               
> > >                       err = alg_test("drbg_pr_sha256",
> > >                       
> > >                                      "drbg_pr_sha256", 0, 0);
> > > 
> > > @@ -2017,7 +2017,7 @@ static inline int __init
> > > drbg_healthcheck_sanity(void) return 0;
> > > 
> > >  #ifdef CONFIG_CRYPTO_DRBG_CTR
> > > 
> > > -     drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
> > > +     drbg_convert_tfm_core("drbg_nopr_ctr_aes256", &coreref, &pr);
> > > 
> > >  #endif
> > >  #ifdef CONFIG_CRYPTO_DRBG_HASH
> > >  
> > >       drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
> > 
> > I am not sure again whether this warrants a "Fixes" tag, because the first
> > modification is in a commented-out code section and the latter again
> > intends to test the DRBG thresholds and thus just needs "a" DRBG.
> 
> Agreed, it is more of a reference. But yes, I don't believe this needs
> to trigger stable backports - because yes it is like dead code, or
> fips only code which majority kernels don't do. Maybe I should
> reference that commit in description only?

I think this would be good to avoid the need to trigger backports for this 
"trivial" patch.

Thanks a lot.
> 
> > Anyhow:
> > 
> > Reviewed-by: Stephan Mueller <smueller@chronox.de>
> > 
> > 
> > Ciao
> > Stephan


Ciao
Stephan

[PATCH 3/4] crypto: drbg - ensure drbg hmac sha512 is used in FIPS selftests

[Dimitri John Ledkov]

Update code comment, self test & healthcheck to use HMAC SHA512,
instead of HMAC SHA256. These changes are in dead-code, or FIPS
enabled code-paths only and have not effect on usual kernel builds.

On systems booting in FIPS mode that has the effect of switch sanity
selftest to HMAC sha512 based (which has been the default DRBG).

Fixes: 9b7b94683a ("crypto: DRBG - switch to HMAC SHA512 DRBG as default DRBG")
Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
---
 crypto/drbg.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index b120e2866b..99666193d9 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -111,9 +111,9 @@
  * as stdrng. Each DRBG receives an increasing cra_priority values the later
  * they are defined in this array (see drbg_fill_array).
  *
- * HMAC DRBGs are favored over Hash DRBGs over CTR DRBGs, and
- * the SHA256 / AES 256 over other ciphers. Thus, the favored
- * DRBGs are the latest entries in this array.
+ * HMAC DRBGs are favored over Hash DRBGs over CTR DRBGs, and the
+ * HMAC-SHA512 / SHA256 / AES 256 over other ciphers. Thus, the
+ * favored DRBGs are the latest entries in this array.
  */
 static const struct drbg_core drbg_cores[] = {
 #ifdef CONFIG_CRYPTO_DRBG_CTR
@@ -1475,8 +1475,8 @@ static int drbg_generate(struct drbg_state *drbg,
 		int err = 0;
 		pr_devel("DRBG: start to perform self test\n");
 		if (drbg->core->flags & DRBG_HMAC)
-			err = alg_test("drbg_pr_hmac_sha256",
-				       "drbg_pr_hmac_sha256", 0, 0);
+			err = alg_test("drbg_pr_hmac_sha512",
+				       "drbg_pr_hmac_sha512", 0, 0);
 		else if (drbg->core->flags & DRBG_CTR)
 			err = alg_test("drbg_pr_ctr_aes256",
 				       "drbg_pr_ctr_aes256", 0, 0);
@@ -2023,7 +2023,7 @@ static inline int __init drbg_healthcheck_sanity(void)
 	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
 #endif
 #ifdef CONFIG_CRYPTO_DRBG_HMAC
-	drbg_convert_tfm_core("drbg_nopr_hmac_sha256", &coreref, &pr);
+	drbg_convert_tfm_core("drbg_nopr_hmac_sha512", &coreref, &pr);
 #endif
 
 	drbg = kzalloc(sizeof(struct drbg_state), GFP_KERNEL);
-- 
2.34.1

Re: [PATCH 3/4] crypto: drbg - ensure drbg hmac sha512 is used in FIPS selftests

[Stephan Mueller]

Am Sonntag, 29. Oktober 2023, 21:48:22 CET schrieb Dimitri John Ledkov:

Hi Dimitri,

> Update code comment, self test & healthcheck to use HMAC SHA512,
> instead of HMAC SHA256. These changes are in dead-code, or FIPS
> enabled code-paths only and have not effect on usual kernel builds.
> 
> On systems booting in FIPS mode that has the effect of switch sanity
> selftest to HMAC sha512 based (which has been the default DRBG).
> 
> Fixes: 9b7b94683a ("crypto: DRBG - switch to HMAC SHA512 DRBG as default
> DRBG") Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
> ---
>  crypto/drbg.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/crypto/drbg.c b/crypto/drbg.c
> index b120e2866b..99666193d9 100644
> --- a/crypto/drbg.c
> +++ b/crypto/drbg.c
> @@ -111,9 +111,9 @@
>   * as stdrng. Each DRBG receives an increasing cra_priority values the
> later * they are defined in this array (see drbg_fill_array).
>   *
> - * HMAC DRBGs are favored over Hash DRBGs over CTR DRBGs, and
> - * the SHA256 / AES 256 over other ciphers. Thus, the favored
> - * DRBGs are the latest entries in this array.
> + * HMAC DRBGs are favored over Hash DRBGs over CTR DRBGs, and the
> + * HMAC-SHA512 / SHA256 / AES 256 over other ciphers. Thus, the
> + * favored DRBGs are the latest entries in this array.
>   */
>  static const struct drbg_core drbg_cores[] = {
>  #ifdef CONFIG_CRYPTO_DRBG_CTR
> @@ -1475,8 +1475,8 @@ static int drbg_generate(struct drbg_state *drbg,
>  		int err = 0;
>  		pr_devel("DRBG: start to perform self test\n");
>  		if (drbg->core->flags & DRBG_HMAC)
> -			err = alg_test("drbg_pr_hmac_sha256",
> -				       "drbg_pr_hmac_sha256", 0, 0);
> +			err = alg_test("drbg_pr_hmac_sha512",
> +				       "drbg_pr_hmac_sha512", 0, 0);
>  		else if (drbg->core->flags & DRBG_CTR)
>  			err = alg_test("drbg_pr_ctr_aes256",
>  				       "drbg_pr_ctr_aes256", 0, 0);
> @@ -2023,7 +2023,7 @@ static inline int __init drbg_healthcheck_sanity(void)
> drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
>  #endif
>  #ifdef CONFIG_CRYPTO_DRBG_HMAC
> -	drbg_convert_tfm_core("drbg_nopr_hmac_sha256", &coreref, &pr);
> +	drbg_convert_tfm_core("drbg_nopr_hmac_sha512", &coreref, &pr);
>  #endif
> 
>  	drbg = kzalloc(sizeof(struct drbg_state), GFP_KERNEL);

Reviewed-by: Stephan Mueller <smueller@chronox.de>

Ciao
Stephan

[PATCH 4/4] crypto: drbg - Remove SHA1 from drbg

[Dimitri John Ledkov]

SP800-90C 3rd draft states that SHA-1 will be removed from all
specifications, including drbg by end of 2030. Given kernels built
today will be operating past that date, start complying with upcoming
requirements.

No functional change, as SHA-256 / SHA-512 based DRBG have always been
the preferred ones.

Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
---
 crypto/drbg.c    | 16 ----------------
 crypto/testmgr.c | 25 ++++---------------------
 2 files changed, 4 insertions(+), 37 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 99666193d9..bccadaedcf 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -139,12 +139,6 @@ static const struct drbg_core drbg_cores[] = {
 #endif /* CONFIG_CRYPTO_DRBG_CTR */
 #ifdef CONFIG_CRYPTO_DRBG_HASH
 	{
-		.flags = DRBG_HASH | DRBG_STRENGTH128,
-		.statelen = 55, /* 440 bits */
-		.blocklen_bytes = 20,
-		.cra_name = "sha1",
-		.backend_cra_name = "sha1",
-	}, {
 		.flags = DRBG_HASH | DRBG_STRENGTH256,
 		.statelen = 111, /* 888 bits */
 		.blocklen_bytes = 48,
@@ -166,12 +160,6 @@ static const struct drbg_core drbg_cores[] = {
 #endif /* CONFIG_CRYPTO_DRBG_HASH */
 #ifdef CONFIG_CRYPTO_DRBG_HMAC
 	{
-		.flags = DRBG_HMAC | DRBG_STRENGTH128,
-		.statelen = 20, /* block length of cipher */
-		.blocklen_bytes = 20,
-		.cra_name = "hmac_sha1",
-		.backend_cra_name = "hmac(sha1)",
-	}, {
 		.flags = DRBG_HMAC | DRBG_STRENGTH256,
 		.statelen = 48, /* block length of cipher */
 		.blocklen_bytes = 48,
@@ -648,8 +636,6 @@ MODULE_ALIAS_CRYPTO("drbg_pr_hmac_sha384");
 MODULE_ALIAS_CRYPTO("drbg_nopr_hmac_sha384");
 MODULE_ALIAS_CRYPTO("drbg_pr_hmac_sha256");
 MODULE_ALIAS_CRYPTO("drbg_nopr_hmac_sha256");
-MODULE_ALIAS_CRYPTO("drbg_pr_hmac_sha1");
-MODULE_ALIAS_CRYPTO("drbg_nopr_hmac_sha1");
 
 /* update function of HMAC DRBG as defined in 10.1.2.2 */
 static int drbg_hmac_update(struct drbg_state *drbg, struct list_head *seed,
@@ -768,8 +754,6 @@ MODULE_ALIAS_CRYPTO("drbg_pr_sha384");
 MODULE_ALIAS_CRYPTO("drbg_nopr_sha384");
 MODULE_ALIAS_CRYPTO("drbg_pr_sha256");
 MODULE_ALIAS_CRYPTO("drbg_nopr_sha256");
-MODULE_ALIAS_CRYPTO("drbg_pr_sha1");
-MODULE_ALIAS_CRYPTO("drbg_nopr_sha1");
 
 /*
  * Increment buffer
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 216878c8bc..209b21ef79 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4849,14 +4849,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.drbg = __VECS(drbg_nopr_ctr_aes256_tv_template)
 		}
-	}, {
-		/*
-		 * There is no need to specifically test the DRBG with every
-		 * backend cipher -- covered by drbg_nopr_hmac_sha256 test
-		 */
-		.alg = "drbg_nopr_hmac_sha1",
-		.fips_allowed = 1,
-		.test = alg_test_null,
 	}, {
 		.alg = "drbg_nopr_hmac_sha256",
 		.test = alg_test_drbg,
@@ -4865,7 +4857,10 @@ static const struct alg_test_desc alg_test_descs[] = {
 			.drbg = __VECS(drbg_nopr_hmac_sha256_tv_template)
 		}
 	}, {
-		/* covered by drbg_nopr_hmac_sha256 test */
+		/*
+		 * There is no need to specifically test the DRBG with every
+		 * backend cipher -- covered by drbg_nopr_hmac_sha512 test
+		 */
 		.alg = "drbg_nopr_hmac_sha384",
 		.test = alg_test_null,
 	}, {
@@ -4875,10 +4870,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.drbg = __VECS(drbg_nopr_hmac_sha512_tv_template)
 		}
-	}, {
-		.alg = "drbg_nopr_sha1",
-		.fips_allowed = 1,
-		.test = alg_test_null,
 	}, {
 		.alg = "drbg_nopr_sha256",
 		.test = alg_test_drbg,
@@ -4910,10 +4901,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.alg = "drbg_pr_ctr_aes256",
 		.fips_allowed = 1,
 		.test = alg_test_null,
-	}, {
-		.alg = "drbg_pr_hmac_sha1",
-		.fips_allowed = 1,
-		.test = alg_test_null,
 	}, {
 		.alg = "drbg_pr_hmac_sha256",
 		.test = alg_test_drbg,
@@ -4929,10 +4916,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.alg = "drbg_pr_hmac_sha512",
 		.test = alg_test_null,
 		.fips_allowed = 1,
-	}, {
-		.alg = "drbg_pr_sha1",
-		.fips_allowed = 1,
-		.test = alg_test_null,
 	}, {
 		.alg = "drbg_pr_sha256",
 		.test = alg_test_drbg,
-- 
2.34.1

Re: [PATCH 4/4] crypto: drbg - Remove SHA1 from drbg

[Stephan Mueller]

Am Sonntag, 29. Oktober 2023, 21:48:23 CET schrieb Dimitri John Ledkov:

Hi Dimitri,

> SP800-90C 3rd draft states that SHA-1 will be removed from all
> specifications, including drbg by end of 2030. Given kernels built
> today will be operating past that date, start complying with upcoming
> requirements.
> 
> No functional change, as SHA-256 / SHA-512 based DRBG have always been
> the preferred ones.

Reviewed-by: Stephan Mueller <smueller@chronox.de>

Ciao
Stephan

Re: [PATCH 1/4] crypto: drbg - ensure most preferred type is FIPS health checked

[Stephan Mueller]

Am Sonntag, 29. Oktober 2023, 21:48:20 CET schrieb Dimitri John Ledkov:

Hi Dimitri,

> drbg supports multiple types of drbg, and multiple parameters of
> each. Health check sanity only checks one drbg of a single type. One
> can enable all three types of drbg. And instead of checking the most
> preferred algorithm (last one wins), it is currently checking first
> one instead.

The purpose of the sanity check is to make sure the various thresholds are 
effective. For this, you need "a" DRBG, no matter which one.
> 
> Update ifdef to ensure that healthcheck prefers HMAC, over HASH, over
> CTR, last one wins, like all other code and functions.

I can see that this patch makes the code more consistent with the rest. Yet, I 
would doubt the "Fixes" indicator below is needed, though.

Anyhow:

Reviewed-by: Stephan Mueller <smueller@chronox.de>

Ciao
Stephan

[PATCH v2 0/4] drbg small fixes

[Dimitri John Ledkov]

This is v2 update of the
https://lore.kernel.org/linux-crypto/5821221.9qqs2JS0CK@tauon.chronox.de/T/#u
patch series.

Added Review-by Stephan, and changed patch descriptions to drop Fixes:
metadata and explicitely mention that backporting this patches to
stable series will not bring any benefits per se (as they patch dead
code, fips_enabled only code, that doesn't affect certification).

Dimitri John Ledkov (4):
  crypto: drbg - ensure most preferred type is FIPS health checked
  crypto: drbg - update FIPS CTR self-checks to aes256
  crypto: drbg - ensure drbg hmac sha512 is used in FIPS selftests
  crypto: drbg - Remove SHA1 from drbg

 crypto/drbg.c    | 40 +++++++++++++---------------------------
 crypto/testmgr.c | 25 ++++---------------------
 2 files changed, 17 insertions(+), 48 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] crypto: drbg - ensure most preferred type is FIPS health checked

[Dimitri John Ledkov]

drbg supports multiple types of drbg, and multiple parameters of
each. Health check sanity only checks one drbg of a single type. One
can enable all three types of drbg. And instead of checking the most
preferred algorithm (last one wins), it is currently checking first
one instead.

Update ifdef to ensure that healthcheck prefers HMAC, over HASH, over
CTR, last one wins, like all other code and functions.

This patch updates code from 541af946fe ("crypto: drbg - SP800-90A
Deterministic Random Bit Generator"), but is not interesting to
cherry-pick for stable updates, because it doesn't affect regular
builds, nor has any tangible effect on FIPS certifcation.

Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
Reviewed-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index ff4ebbc68e..2cce18dcfc 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -2018,9 +2018,11 @@ static inline int __init drbg_healthcheck_sanity(void)
 
 #ifdef CONFIG_CRYPTO_DRBG_CTR
 	drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
-#elif defined CONFIG_CRYPTO_DRBG_HASH
+#endif
+#ifdef CONFIG_CRYPTO_DRBG_HASH
 	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
-#else
+#endif
+#ifdef CONFIG_CRYPTO_DRBG_HMAC
 	drbg_convert_tfm_core("drbg_nopr_hmac_sha256", &coreref, &pr);
 #endif
 
-- 
2.34.1

[PATCH v2 2/4] crypto: drbg - update FIPS CTR self-checks to aes256

[Dimitri John Ledkov]

When originally drbg was introduced FIPS self-checks for all types but
CTR were using the most preferred parameters for each type of
DRBG. Update CTR self-check to use aes256.

This patch updates code from 541af946fe ("crypto: drbg - SP800-90A
Deterministic Random Bit Generator"), but is not interesting to
cherry-pick for stable updates, because it doesn't affect regular
builds, nor has any tangible effect on FIPS certifcation.

Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
Reviewed-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 2cce18dcfc..b120e2866b 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1478,8 +1478,8 @@ static int drbg_generate(struct drbg_state *drbg,
 			err = alg_test("drbg_pr_hmac_sha256",
 				       "drbg_pr_hmac_sha256", 0, 0);
 		else if (drbg->core->flags & DRBG_CTR)
-			err = alg_test("drbg_pr_ctr_aes128",
-				       "drbg_pr_ctr_aes128", 0, 0);
+			err = alg_test("drbg_pr_ctr_aes256",
+				       "drbg_pr_ctr_aes256", 0, 0);
 		else
 			err = alg_test("drbg_pr_sha256",
 				       "drbg_pr_sha256", 0, 0);
@@ -2017,7 +2017,7 @@ static inline int __init drbg_healthcheck_sanity(void)
 		return 0;
 
 #ifdef CONFIG_CRYPTO_DRBG_CTR
-	drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
+	drbg_convert_tfm_core("drbg_nopr_ctr_aes256", &coreref, &pr);
 #endif
 #ifdef CONFIG_CRYPTO_DRBG_HASH
 	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
-- 
2.34.1

[PATCH v2 3/4] crypto: drbg - ensure drbg hmac sha512 is used in FIPS selftests

[Dimitri John Ledkov]

Update code comment, self test & healthcheck to use HMAC SHA512,
instead of HMAC SHA256. These changes are in dead-code, or FIPS
enabled code-paths only and have not effect on usual kernel builds.

On systems booting in FIPS mode that has the effect of switch sanity
selftest to HMAC sha512 based (which has been the default DRBG).

This patch updates code from 9b7b94683a ("crypto: DRBG - switch to
HMAC SHA512 DRBG as default DRBG"), but is not interesting to
cherry-pick for stable updates, because it doesn't affect regular
builds, nor has any tangible effect on FIPS certifcation.

Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
Reviewed-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index b120e2866b..99666193d9 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -111,9 +111,9 @@
  * as stdrng. Each DRBG receives an increasing cra_priority values the later
  * they are defined in this array (see drbg_fill_array).
  *
- * HMAC DRBGs are favored over Hash DRBGs over CTR DRBGs, and
- * the SHA256 / AES 256 over other ciphers. Thus, the favored
- * DRBGs are the latest entries in this array.
+ * HMAC DRBGs are favored over Hash DRBGs over CTR DRBGs, and the
+ * HMAC-SHA512 / SHA256 / AES 256 over other ciphers. Thus, the
+ * favored DRBGs are the latest entries in this array.
  */
 static const struct drbg_core drbg_cores[] = {
 #ifdef CONFIG_CRYPTO_DRBG_CTR
@@ -1475,8 +1475,8 @@ static int drbg_generate(struct drbg_state *drbg,
 		int err = 0;
 		pr_devel("DRBG: start to perform self test\n");
 		if (drbg->core->flags & DRBG_HMAC)
-			err = alg_test("drbg_pr_hmac_sha256",
-				       "drbg_pr_hmac_sha256", 0, 0);
+			err = alg_test("drbg_pr_hmac_sha512",
+				       "drbg_pr_hmac_sha512", 0, 0);
 		else if (drbg->core->flags & DRBG_CTR)
 			err = alg_test("drbg_pr_ctr_aes256",
 				       "drbg_pr_ctr_aes256", 0, 0);
@@ -2023,7 +2023,7 @@ static inline int __init drbg_healthcheck_sanity(void)
 	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
 #endif
 #ifdef CONFIG_CRYPTO_DRBG_HMAC
-	drbg_convert_tfm_core("drbg_nopr_hmac_sha256", &coreref, &pr);
+	drbg_convert_tfm_core("drbg_nopr_hmac_sha512", &coreref, &pr);
 #endif
 
 	drbg = kzalloc(sizeof(struct drbg_state), GFP_KERNEL);
-- 
2.34.1

[PATCH v2 4/4] crypto: drbg - Remove SHA1 from drbg

[Dimitri John Ledkov]

SP800-90C 3rd draft states that SHA-1 will be removed from all
specifications, including drbg by end of 2030. Given kernels built
today will be operating past that date, start complying with upcoming
requirements.

No functional change, as SHA-256 / SHA-512 based DRBG have always been
the preferred ones.

Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
Reviewed-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c    | 16 ----------------
 crypto/testmgr.c | 25 ++++---------------------
 2 files changed, 4 insertions(+), 37 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 99666193d9..bccadaedcf 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -139,12 +139,6 @@ static const struct drbg_core drbg_cores[] = {
 #endif /* CONFIG_CRYPTO_DRBG_CTR */
 #ifdef CONFIG_CRYPTO_DRBG_HASH
 	{
-		.flags = DRBG_HASH | DRBG_STRENGTH128,
-		.statelen = 55, /* 440 bits */
-		.blocklen_bytes = 20,
-		.cra_name = "sha1",
-		.backend_cra_name = "sha1",
-	}, {
 		.flags = DRBG_HASH | DRBG_STRENGTH256,
 		.statelen = 111, /* 888 bits */
 		.blocklen_bytes = 48,
@@ -166,12 +160,6 @@ static const struct drbg_core drbg_cores[] = {
 #endif /* CONFIG_CRYPTO_DRBG_HASH */
 #ifdef CONFIG_CRYPTO_DRBG_HMAC
 	{
-		.flags = DRBG_HMAC | DRBG_STRENGTH128,
-		.statelen = 20, /* block length of cipher */
-		.blocklen_bytes = 20,
-		.cra_name = "hmac_sha1",
-		.backend_cra_name = "hmac(sha1)",
-	}, {
 		.flags = DRBG_HMAC | DRBG_STRENGTH256,
 		.statelen = 48, /* block length of cipher */
 		.blocklen_bytes = 48,
@@ -648,8 +636,6 @@ MODULE_ALIAS_CRYPTO("drbg_pr_hmac_sha384");
 MODULE_ALIAS_CRYPTO("drbg_nopr_hmac_sha384");
 MODULE_ALIAS_CRYPTO("drbg_pr_hmac_sha256");
 MODULE_ALIAS_CRYPTO("drbg_nopr_hmac_sha256");
-MODULE_ALIAS_CRYPTO("drbg_pr_hmac_sha1");
-MODULE_ALIAS_CRYPTO("drbg_nopr_hmac_sha1");
 
 /* update function of HMAC DRBG as defined in 10.1.2.2 */
 static int drbg_hmac_update(struct drbg_state *drbg, struct list_head *seed,
@@ -768,8 +754,6 @@ MODULE_ALIAS_CRYPTO("drbg_pr_sha384");
 MODULE_ALIAS_CRYPTO("drbg_nopr_sha384");
 MODULE_ALIAS_CRYPTO("drbg_pr_sha256");
 MODULE_ALIAS_CRYPTO("drbg_nopr_sha256");
-MODULE_ALIAS_CRYPTO("drbg_pr_sha1");
-MODULE_ALIAS_CRYPTO("drbg_nopr_sha1");
 
 /*
  * Increment buffer
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 216878c8bc..209b21ef79 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4849,14 +4849,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.drbg = __VECS(drbg_nopr_ctr_aes256_tv_template)
 		}
-	}, {
-		/*
-		 * There is no need to specifically test the DRBG with every
-		 * backend cipher -- covered by drbg_nopr_hmac_sha256 test
-		 */
-		.alg = "drbg_nopr_hmac_sha1",
-		.fips_allowed = 1,
-		.test = alg_test_null,
 	}, {
 		.alg = "drbg_nopr_hmac_sha256",
 		.test = alg_test_drbg,
@@ -4865,7 +4857,10 @@ static const struct alg_test_desc alg_test_descs[] = {
 			.drbg = __VECS(drbg_nopr_hmac_sha256_tv_template)
 		}
 	}, {
-		/* covered by drbg_nopr_hmac_sha256 test */
+		/*
+		 * There is no need to specifically test the DRBG with every
+		 * backend cipher -- covered by drbg_nopr_hmac_sha512 test
+		 */
 		.alg = "drbg_nopr_hmac_sha384",
 		.test = alg_test_null,
 	}, {
@@ -4875,10 +4870,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.drbg = __VECS(drbg_nopr_hmac_sha512_tv_template)
 		}
-	}, {
-		.alg = "drbg_nopr_sha1",
-		.fips_allowed = 1,
-		.test = alg_test_null,
 	}, {
 		.alg = "drbg_nopr_sha256",
 		.test = alg_test_drbg,
@@ -4910,10 +4901,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.alg = "drbg_pr_ctr_aes256",
 		.fips_allowed = 1,
 		.test = alg_test_null,
-	}, {
-		.alg = "drbg_pr_hmac_sha1",
-		.fips_allowed = 1,
-		.test = alg_test_null,
 	}, {
 		.alg = "drbg_pr_hmac_sha256",
 		.test = alg_test_drbg,
@@ -4929,10 +4916,6 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.alg = "drbg_pr_hmac_sha512",
 		.test = alg_test_null,
 		.fips_allowed = 1,
-	}, {
-		.alg = "drbg_pr_sha1",
-		.fips_allowed = 1,
-		.test = alg_test_null,
 	}, {
 		.alg = "drbg_pr_sha256",
 		.test = alg_test_drbg,
-- 
2.34.1

Re: [PATCH v2 0/4] drbg small fixes

[Herbert Xu]

On Mon, Oct 30, 2023 at 02:05:12PM +0200, Dimitri John Ledkov wrote:
> This is v2 update of the
> https://lore.kernel.org/linux-crypto/5821221.9qqs2JS0CK@tauon.chronox.de/T/#u
> patch series.
> 
> Added Review-by Stephan, and changed patch descriptions to drop Fixes:
> metadata and explicitely mention that backporting this patches to
> stable series will not bring any benefits per se (as they patch dead
> code, fips_enabled only code, that doesn't affect certification).
> 
> Dimitri John Ledkov (4):
>   crypto: drbg - ensure most preferred type is FIPS health checked
>   crypto: drbg - update FIPS CTR self-checks to aes256
>   crypto: drbg - ensure drbg hmac sha512 is used in FIPS selftests
>   crypto: drbg - Remove SHA1 from drbg
> 
>  crypto/drbg.c    | 40 +++++++++++++---------------------------
>  crypto/testmgr.c | 25 ++++---------------------
>  2 files changed, 17 insertions(+), 48 deletions(-)
> 
> -- 
> 2.34.1

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