Page MenuHome GnuPG

RSA/DSA keygen modification for FIPS/ACVP testing
Closed, ResolvedPublic


In FIPS we need to accept different key sized than just 2k and 3k. According to NIST.FIPS.186-5 draft, it should be perfectly fine.

For DSA keygen, we need to be able to generate keys based on P and Q parameters for ACVP testing to be able to verify the result.

Event Timeline

Do we really need to support DSA in FIPS mode? I mean standard DSA and not ECDSA.

werner triaged this task as High priority.Oct 8 2021, 3:34 PM

sorry for a confusion. We do not plan to certify DSA so disregard the second part of the patch.

Applied the RSA part.

Aside of DSA should be certified or not...

For the DSA part, yes, I know that there has been a patch for DSA, but it was originally written for older libgcrypt (<= 1.8.4). And it's not relevant to apply it directly to master.

I think that DSA for FIPS 186-3 was fixed in rC30ed9593f632: Fix DSA for FIPS 186-3..

If really needed, we should check the intention/meaning of the DSA part of the patch. At least, removing call of sexp_release (deriveparm) is wrong.

gniibe changed the task status from Open to Testing.Oct 14 2021, 9:28 AM
gniibe added a project: Testing.

( No need to certify the DSA things)

Let me get back to this once more as one of the parts for RSA was initially missed:

diff -up libgcrypt-1.8.4/cipher/rsa.c.fips-keygen libgcrypt-1.8.4/cipher/rsa.c
--- libgcrypt-1.8.4/cipher/rsa.c.fips-keygen	2017-11-23 19:16:58.000000000 +0100
+++ libgcrypt-1.8.4/cipher/rsa.c	2019-02-12 14:29:25.630513971 +0100
@@ -696,7 +696,7 @@ generate_x931 (RSA_secret_key *sk, unsig
   *swapped = 0;
-  if (e_value == 1)   /* Alias for a secure value. */
+  if (e_value == 1 || e_value == 0)   /* Alias for a secure value. */
     e_value = 65537;
   /* Point 1 of section 4.1:  k = 1024 + 256s with S >= 0  */

This was part of the same patch and probably required for CAVS or ACVP testing (probably from some testing vectors that use(d) the value 0 as default secure value), but so far I do not have clear indication if it will be needed or not.

If you do not find this change problematic, it would be great if it could be applied. Otherwise I will drop it and wait what is going to explode.

Adding the case for == 0 only might be problematic, because I don't think it's an alias for a secure value; I think that == 0 means that it's up to libgcrypt to select the value (just like other generate_* functions).

It's OK for me adding the case itself. But comment should be reflected.
Let me write in that way.

Thanks. I did some git archeology and found the first mention of this in the following commit in 2011 without much details:

gniibe removed a project: Testing.