Improve error reporting from PCT and make the tests mandatory
Resolves: rhbz#2176145
This commit is contained in:
		
							parent
							
								
									987df146aa
								
							
						
					
					
						commit
						80b16e463d
					
				
							
								
								
									
										145
									
								
								libgcrypt-1.10.0-fips-pct.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										145
									
								
								libgcrypt-1.10.0-fips-pct.patch
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,145 @@ | |||||||
|  | From 2ddeec574bc1ae90bb4242c4ce9ad9e7975a27bd Mon Sep 17 00:00:00 2001 | ||||||
|  | From: Jakub Jelen <jjelen@redhat.com> | ||||||
|  | Date: Wed, 1 Mar 2023 15:42:29 +0100 | ||||||
|  | Subject: [PATCH] ecc: Do not allow skipping tests in FIPS Mode. | ||||||
|  | 
 | ||||||
|  | * cipher/ecc.c (ecc_generate): Do not allow skipping tests PCT tests | ||||||
|  | in FIPS mode. | ||||||
|  | 
 | ||||||
|  | --
 | ||||||
|  | 
 | ||||||
|  | The new FIPS specification requires to run the PCT without any | ||||||
|  | exceptions. | ||||||
|  | 
 | ||||||
|  | GnuPG-bug-id: 6394 | ||||||
|  | Signed-off-by: Jakub Jelen <jjelen@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  cipher/ecc.c | 2 +- | ||||||
|  |  1 file changed, 1 insertion(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/cipher/ecc.c b/cipher/ecc.c
 | ||||||
|  | index 1e80200e..797f2368 100644
 | ||||||
|  | --- a/cipher/ecc.c
 | ||||||
|  | +++ b/cipher/ecc.c
 | ||||||
|  | @@ -677,7 +677,7 @@ ecc_generate (const gcry_sexp_t genparms, gcry_sexp_t *r_skey)
 | ||||||
|  |          log_debug ("ecgen result  using Ed25519+EdDSA\n"); | ||||||
|  |      } | ||||||
|  |   | ||||||
|  | -  if (!(flags & PUBKEY_FLAG_NO_KEYTEST) && fips_mode ())
 | ||||||
|  | +  if (fips_mode ())
 | ||||||
|  |      test_keys_fips (*r_skey); | ||||||
|  |   | ||||||
|  |   leave: | ||||||
|  | -- 
 | ||||||
|  | 2.39.2 | ||||||
|  | 
 | ||||||
|  | From 23a2d1285e35b2eb91bb422609eb1c965c8a9bf6 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: Jakub Jelen <jjelen@redhat.com> | ||||||
|  | Date: Thu, 2 Mar 2023 09:43:44 +0100 | ||||||
|  | Subject: [PATCH] ecc: Make the PCT recoverable in FIPS mode and consistent | ||||||
|  |  with RSA. | ||||||
|  | 
 | ||||||
|  | * cipher/ecc.c (test_keys_fips): Replace calls to log_fatal with | ||||||
|  | return code on error. | ||||||
|  | (ecc_generate): Signal error when PCT fails in FIPS mode. | ||||||
|  | 
 | ||||||
|  | --
 | ||||||
|  | 
 | ||||||
|  | GnuPG-bug-id: 6397 | ||||||
|  | Signed-off-by: Jakub Jelen <jjelen@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  cipher/ecc.c | 36 ++++++++++++++++++++++++++++-------- | ||||||
|  |  1 file changed, 28 insertions(+), 8 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/cipher/ecc.c b/cipher/ecc.c
 | ||||||
|  | index 797f2368..19520db3 100644
 | ||||||
|  | --- a/cipher/ecc.c
 | ||||||
|  | +++ b/cipher/ecc.c
 | ||||||
|  | @@ -101,7 +101,7 @@ static void *progress_cb_data;
 | ||||||
|  |   | ||||||
|  |  /* Local prototypes. */ | ||||||
|  |  static void test_keys (mpi_ec_t ec, unsigned int nbits); | ||||||
|  | -static void test_keys_fips (gcry_sexp_t skey);
 | ||||||
|  | +static int test_keys_fips (gcry_sexp_t skey);
 | ||||||
|  |  static void test_ecdh_only_keys (mpi_ec_t ec, unsigned int nbits, int flags); | ||||||
|  |  static unsigned int ecc_get_nbits (gcry_sexp_t parms); | ||||||
|  |   | ||||||
|  | @@ -308,9 +308,10 @@ test_keys (mpi_ec_t ec, unsigned int nbits)
 | ||||||
|  |  /* We should get here only with the NIST curves as they are the only ones | ||||||
|  |   * having the fips bit set in ecc_domain_parms_t struct so this is slightly | ||||||
|  |   * simpler than the whole ecc_generate function */ | ||||||
|  | -static void
 | ||||||
|  | +static int
 | ||||||
|  |  test_keys_fips (gcry_sexp_t skey) | ||||||
|  |  { | ||||||
|  | +  int result = -1; /* Default to failure */
 | ||||||
|  |    gcry_md_hd_t hd = NULL; | ||||||
|  |    const char *data_tmpl = "(data (flags rfc6979) (hash %s %b))"; | ||||||
|  |    gcry_sexp_t sig = NULL; | ||||||
|  | @@ -323,18 +324,27 @@ test_keys_fips (gcry_sexp_t skey)
 | ||||||
|  |    /* Open MD context and feed the random data in */ | ||||||
|  |    rc = _gcry_md_open (&hd, GCRY_MD_SHA256, 0); | ||||||
|  |    if (rc) | ||||||
|  | -    log_fatal ("ECDSA operation: failed to initialize MD context: %s\n", gpg_strerror (rc));
 | ||||||
|  | +    {
 | ||||||
|  | +      log_error ("ECDSA operation: failed to initialize MD context: %s\n", gpg_strerror (rc));
 | ||||||
|  | +      goto leave;
 | ||||||
|  | +    }
 | ||||||
|  |    _gcry_md_write (hd, plaintext, sizeof(plaintext)); | ||||||
|  |   | ||||||
|  |    /* Sign the data */ | ||||||
|  |    rc = _gcry_pk_sign_md (&sig, data_tmpl, hd, skey, NULL); | ||||||
|  |    if (rc) | ||||||
|  | -    log_fatal ("ECDSA operation: signing failed: %s\n", gpg_strerror (rc));
 | ||||||
|  | +    {
 | ||||||
|  | +      log_error ("ECDSA operation: signing failed: %s\n", gpg_strerror (rc));
 | ||||||
|  | +      goto leave;
 | ||||||
|  | +    }
 | ||||||
|  |   | ||||||
|  |    /* Verify this signature.  */ | ||||||
|  |    rc = _gcry_pk_verify_md (sig, data_tmpl, hd, skey, NULL); | ||||||
|  |    if (rc) | ||||||
|  | -    log_fatal ("ECDSA operation: verification failed: %s\n", gpg_strerror (rc));
 | ||||||
|  | +    {
 | ||||||
|  | +      log_error ("ECDSA operation: verification failed: %s\n", gpg_strerror (rc));
 | ||||||
|  | +      goto leave;
 | ||||||
|  | +    }
 | ||||||
|  |   | ||||||
|  |    /* Modify the data and check that the signing fails.  */ | ||||||
|  |    _gcry_md_reset(hd); | ||||||
|  | @@ -342,10 +352,16 @@ test_keys_fips (gcry_sexp_t skey)
 | ||||||
|  |    _gcry_md_write (hd, plaintext, sizeof(plaintext)); | ||||||
|  |    rc = _gcry_pk_verify_md (sig, data_tmpl, hd, skey, NULL); | ||||||
|  |    if (rc != GPG_ERR_BAD_SIGNATURE) | ||||||
|  | -    log_fatal ("ECDSA operation: signature verification worked on modified data\n");
 | ||||||
|  | +    {
 | ||||||
|  | +      log_error ("ECDSA operation: signature verification worked on modified data\n");
 | ||||||
|  | +      goto leave;
 | ||||||
|  | +    }
 | ||||||
|  |   | ||||||
|  | +  result = 0;
 | ||||||
|  | +leave:
 | ||||||
|  |    _gcry_md_close (hd); | ||||||
|  |    sexp_release (sig); | ||||||
|  | +  return result;
 | ||||||
|  |  } | ||||||
|  |   | ||||||
|  |   | ||||||
|  | @@ -677,8 +693,12 @@ ecc_generate (const gcry_sexp_t genparms, gcry_sexp_t *r_skey)
 | ||||||
|  |          log_debug ("ecgen result  using Ed25519+EdDSA\n"); | ||||||
|  |      } | ||||||
|  |   | ||||||
|  | -  if (fips_mode ())
 | ||||||
|  | -    test_keys_fips (*r_skey);
 | ||||||
|  | +  if (fips_mode () && test_keys_fips (*r_skey))
 | ||||||
|  | +    {
 | ||||||
|  | +      sexp_release (*r_skey); r_skey = NULL;
 | ||||||
|  | +      fips_signal_error ("self-test after key generation failed");
 | ||||||
|  | +      rc = GPG_ERR_SELFTEST_FAILED;
 | ||||||
|  | +    }
 | ||||||
|  |   | ||||||
|  |   leave: | ||||||
|  |    mpi_free (public); | ||||||
|  | -- 
 | ||||||
|  | 2.39.2 | ||||||
|  | 
 | ||||||
| @ -53,6 +53,9 @@ Patch15: libgcrypt-1.10.0-fips-x931.patch | |||||||
| Patch16: libgcrypt-1.10.0-fips-rsa-pss.patch | Patch16: libgcrypt-1.10.0-fips-rsa-pss.patch | ||||||
| # https://dev.gnupg.org/T6376 | # https://dev.gnupg.org/T6376 | ||||||
| Patch17: libgcrypt-1.10.0-fips-indicator-md-hmac.patch | Patch17: libgcrypt-1.10.0-fips-indicator-md-hmac.patch | ||||||
|  | # https://dev.gnupg.org/T6394 | ||||||
|  | # https://dev.gnupg.org/T6397 | ||||||
|  | Patch18: libgcrypt-1.10.0-fips-pct.patch | ||||||
| 
 | 
 | ||||||
| %global gcrylibdir %{_libdir} | %global gcrylibdir %{_libdir} | ||||||
| %global gcrysoname libgcrypt.so.20 | %global gcrysoname libgcrypt.so.20 | ||||||
| @ -103,6 +106,7 @@ applications using libgcrypt. | |||||||
| %patch15 -p1 | %patch15 -p1 | ||||||
| %patch16 -p1 | %patch16 -p1 | ||||||
| %patch17 -p1 | %patch17 -p1 | ||||||
|  | %patch18 -p1 | ||||||
| 
 | 
 | ||||||
| %build | %build | ||||||
| # This package has a configure test which uses ASMs, but does not link the | # This package has a configure test which uses ASMs, but does not link the | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user