I tried to clarify the comment in the following merge request. Feel free to pull it from there or adjust if it is too verbose or missing some points:

In rCa6a6e94027abf18a51f5f93bf9fb2cfe5496bdf8#74835, @werner wrote:

Why is that not stated in my man page which knows about kernel 3.19? Is that a regression or a RedHat specific patch?

One nit that I overlooked initially is the memory leak, which is fixed with the following patch:

The specs https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf page 10 says specifically:

The test looks good. I hope I changed the API in all the hw optimized implementations.

This still did not seem to help me in making the tests working on Fedora with git master. I am still getting wrong paths to the gpgconf

gpgscm: error running '/root/gnupg/tests/tools/gpgconf': probably not installed

There is a full reproducer and more complete log in https://bugzilla.redhat.com/show_bug.cgi?id=2089075#c11

Thank you for your work on the proposal. I have two comments:

• Do we have some test vector, which can be used in the testsute to test the new API?
• We need to mention the new API in the documentation.

For the record, the changeset in the attached merge request is final and waiting for reviews.

The provided change does not look like fixing the problem for me. The path to gpgconf is still wrong and I am getting the same error both with master and with the patched tarballs:

make[2]: Entering directory '/home/jjelen/devel/gnupg/tests/tpm2dtests'
LC_ALL=C EXEEXT= PATH="../gpgscm:/home/jjelen/.local/bin:/home/jjelen/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin" abs_top_srcdir="/home/jjelen/devel/gnupg" objdir="/home/jjelen/devel/gnupg" TPMSERVER="" SWTPM="/bin/swtpm" SWTPM_IOCTL="/bin/swtpm_ioctl" GNUPG_BUILD_ROOT="/home/jjelen/devel/gnupg/tests" GPGSCM_PATH="/home/jjelen/devel/gnupg/tests/gpgscm" /home/jjelen/devel/gnupg/tests/gpgscm/gpgscm \
  /home/jjelen/devel/gnupg/tests/tpm2dtests/run-tests.scm  
gpgscm: error running '/home/jjelen/devel/gnupg/tests/tools/gpgconf': probably not installed
(wait-process "/home/jjelen/devel/gnupg/tests/tools/gpgconf" 2693795 #t): Configuration error

AFAIK the above case has a lot of wiggle room to fit one PID and the surrounded string into 400 bytes and even if it would need to truncate, it would write terminating character, at least on Linux:

Ok. Thank you for the clarification. I will drop the second part and keep only the FIPS change in the patch. Merge request already updated.

I was pointed by Daiki to the following patch in Fedora binutils, which allows listing the fdo packaging metadata, but it does not list any other unknown objects and unfortunately fails hard:

I have one follow-up is that the readelf chokes on the integrity note for some reason:

$ readelf -n /usr/lib64/libgcrypt.so.20.4.1
Displaying notes found in: .note.fdo.integrity
  Owner                Data size 	Description
  FDO                  0x00000020	Unknown note type: (0x8e2afeca)

I assume this is just because the readelf does not know this type. I see this type was initially proposed by Daiki, but I did not find any other sources for this magic number so before filling bugs for readelf, do we have some doc why the 0xcafe2a8e is used?

Adding

GPG_TTY=$(tty)
export GPG_TTY

makes this working so thank you for the pointer.

the -v does not show more useful info on the gpg side:

# gpg2 --quick-gen-key admin
About to create a key for:
    "admin"

You are combining two concepts here -- the KDF and the AEAD cipher itself (at least from the FIPS terminology). I would like to avoid mixing these two together in the new API. If you would like to implement the SSH/TLS KDF, I would suggest to use the kdf API you already have. Then we are here left only with a new geniv API to implement. In the T4873 I mentioned example how it is now used in libssh using libgcrypt, which implements the iv increment outside of the libgcrypt:

The mails from these days still contain the following header:

List-Post: <mailto:gnupg-devel@lists.gnupg.org>

which is probably causing the mail client directing the mails to this address. Is there a way to change or or make it an alias so it is easier for people to use the mailing list without finding this issue?

I think this is not urgent as we are able to FIPS certify libgcrypt without that, but the modern protocols and algorithm use this and if we want to use libgcrypt to implement these in FIPS compliant way, we certainly need something like that.

For the record, there is a new report on the mailing list about this module on MacOS:

Thanks. Looks good to me.

In T5512#153650, @Jakuje wrote:

This is my draft for the FIPS indicator KDF. I think we do not need to keep the original GCRYCTL_FIPS_SERVICE_INDICATOR if we replace it also in the tests. This will also need some tests and documentation update.

libgcrypt-fips-indicator-kdf.patch3 KBDownload

I went through the documentation related to FIPS and updated some wording to match reality. It will probably require still some more work.

This is my draft for the FIPS indicator KDF. I think we do not need to keep the original GCRYCTL_FIPS_SERVICE_INDICATOR if we replace it also in the tests. This will also need some tests and documentation update.

The previous comment should have come to the T5600. Sorry for the noise.

Sorry for resurrecting the done task, but I got a message from @pmgdeb who noticed there is mismatch between parenthesis in the --with-fips-module-version help string. The attached patch fixes the issue and add proper help text.

Sorry for resurrecting the done task, but I got a message from @pmgdeb who noticed there is mismatch between parenthesis in the --with-fips-module-version help string. The attached patch fixes the issue and add proper help text.

Thanks. Looks good to me (both merged changes and the above proposal). In addition to the changes proposed above, we certainly need to update the documentation about this, probably also the FSM diagram.

We talked today about the renaming the current "linux" entropy module to "oldlinux" would make sense.

Thank you. Tested locally that it does what it is supposed to do and all tests passed for me as expected.

Reading through the changes, the content and usage of the getentropy looks good.

the random daemon is still part of the configure.ac and the undefined _gcry_daemon_initialize_basics() and _gcry_daemon_randomize() is still used under the USE_RANDOM_DAEMON guard in several places. I think at least the following cases should be removed too (or the configure check to be modified to throw error or warning):

It turned out that the new *.inp files are not part of the release tarball, which makes the tests from generated tarball fail. The attached patch should fix this issue.

Sorry for the noise. There were couple of other places which I missed initially and which are covered in the v2 patch which follows:

It turns out together with rCe96980022e5e some tests are failing in FIPS mode. The attached patch should handle the failures.

I have just a note about this issue, that it would be helpful to exercise this new API in some tests. Right now, only the old API is tested.

It turns out that the asymmetric key operations are not yet properly enforced with the .disabled flag. While the other key crypto usually has some "open" api, where this can be simply captured, the pubkey API has several entry points and the "test_algo" is not enough to check for disabled key types.

Thank you. My local tests (in emulated fips mode and normal mode) do not show any errors with current master branch.

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

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

I went through some more testing and noticed one missing file in the release tarball, that prevents building libgcrypt now. Should be fixed with the attached patch.

I did go through a bit more testing too and the selftests still initialize and use the secure memory (and the t-secmem fails in FIPS mode if we invoke selftests from constructor). Now from run_random_selftests() -> _gcry_random_selftest() -> drbg_healthcheck() -> _gcry_rngdrbg_healthcheck_one(). So this means that we either need to de-initialize secure memory after the constructor selftests or prevent its initialization as I suggested in some of the previous comments.

I do not like the idea of using the get_config interface for this. It should be easily usable by applications to check for single cipher/mode so int/bool return values would be preferred against the string ones (which are now used in the get_config). I am not sure if getting all the configuration in one string blob would be any use (except for some auditing) either.

(forgot to upload the patch to the last comment)

I am fine with either way. The memcmp variant is probably cleaner to make sure all works as expected in all cases.

Thank you. Extending the semantics of GCRYCTL_CLOSE_RANDOM_DEVICE sounds good to me. I think the deinit functions were created initially especially not to change the semantics of existing code using GCRYCTL_CLOSE_RANDOM_DEVICE, but I agree that it will probably not be an issue.

With just implicit indicators, we would have to block all non-approved cipher modes and kdfs including the OCB mode and skcrypt, which would probably make gnupg2 unusable in FIPS mode, which is not our intention.

I just wanted to add one more note that i just found out that the tests --disable-hwf or gcry_control GCRYCTL_DISABLE_HWF have no effect in case the global_init() is called from constructor.

Thank you for merging the important parts of the patches and implementing similar stuff for DSA. You are right that DSA is supported in the 140-3 specs so it is fine to keep it enabled with the keylength constraints.

Implicit indicators mean that we need to go through the all algorithms and verify that they work if they have approved key sizes/parameters and do not work when they do not.

If I read it right, the version 3.1.0 adds the pthread requirement. Using 3.0.2 should be fine for us.

The most of the stuff about boot blocking was discussed in the bug https://bugzilla.redhat.com/show_bug.cgi?id=1569393 (private). There were some bugs in our patches, but also some issue in the kernel that locked the boot process (in FIPS mode).

OK. Sorry for the noise. I got a clarification that the test is no longer needed so closing this issue.

From the FIPS Certs draft for RHEL 8.5, I have the following sentence:

Fair enough. Unfortunately, the separation is not completely clear from the dist git history, so please, excuse any inaccuracies I will provide here. I will try to reference particular bugs so we can get back to them if needed:

At this moment, we agreed on keeping the current behavior and not allowing the SHA1 for verification either. But we might need to revisit that in the future if this will cause issues. Or we might go the way of switching the service to non-fips if needed, rather than creating some more middle ground.

Thank you for having a look into that. The change looks fine, but I need to get some clarification about what "Legacy use" means for "Digital signature verification" in the Table 8 of https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf

In T5433#151041, @gniibe wrote:

Sorry, I was wrong. We don't need any changes.

When using gcry_pk_hash_sign and gcry_pk_hash_verify, approved digest algos are guaranteed when FIPS enabled.

Yes, it's a user of the function who supplies HD (handle for hash). (I had wrong assumption HD could be with non-approved digest algo.) But it is needed for the user to enable the HD and to feed message beforehand. At that stage, non-approved digest algo must fail.

In T5617#150908, @gniibe wrote:

OK, let us start discussion by applying the patch first.

I have wondered if introducing another state in FSM would be needed, because:

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

Hi, was there any update on this? I found the following bug [0] in libgcrypt, which we solved [1] with using poll ages ago.

Thanks. This looks good to me.

I tried to generate a tarball from master and I failed to build the hmac256 binary because the hmac256.h was not packaged into the dist tarball in master. If hmac256 should be standalone binary, I propose it should not need have a separate header file:

I have a draft, which results in the following "API" of the name-version:

I had in my mind something like this:

We ran the coverity again with the new 2.3.1 release and there are couple of new stuff that I probably missed in the initial review.

Thank you. On the first sight, it looks reasonable, but I would like to experiment with it a bit to see all use cases are covered.

Thanks. I think we are good here. If we will decide to pursuate the brainpool switch, I will open a new issue.

Oh, my bad. I probably used wrong git command. Uploaded now the patches themselves:

I have one more patch set to improve FIPS testing in test/curves.c. In the past, it was basically skipped altogether in FIPS mode. This implements more fine-grained selection of what is being tested. This is the first part.

I added couple of minor comments. I hope they went into somewhere.