Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AES-GCM decryption on OpenSSL 1.0.2-fips #111

Merged
merged 6 commits into from
Oct 19, 2023

Conversation

corhere
Copy link
Contributor

@corhere corhere commented Aug 24, 2023

Some of the standard library crypto/cipher AES-GCM tests fail when using OpenSSL 1.0.2 in FIPS mode, but pass with the same library when FIPS mode is disabled. Extend the AES-GCM tests to cover all the failure cases caught by the standard library tests, build OpenSSL 1.0.2 in CI to be FIPS-enabled, and update the bindings so the tests pass.

@corhere corhere force-pushed the fix-aes-gcm-102fips branch from 90b287c to f8037de Compare August 25, 2023 15:10
Copy link
Collaborator

@qmuntal qmuntal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

Some of the standard library crypto/cipher AES-GCM tests fail when using
OpenSSL 1.0.2 in FIPS mode, but pass with the same library when FIPS
mode is disabled. Extend the AES-GCM tests to cover all the failure
cases caught by the standard library tests and build the OpenSSL 1.0.2
library used in CI to be FIPS enabled.

Link the OpenSSL 1.0.2 CI build against FIPS Object Module 2.0.1 rather
than the latest version (2.0.16) as at least one commercially-supported
FIPS validated build of OpenSSL 1.0.2 is known to use that version of
the FIPS Object Module and some of the failures seen with 2.0.1 do not
reproduce with 2.0.16.

Signed-off-by: Cory Snider <csnider@mirantis.com>
Signed-off-by: Cory Snider <csnider@mirantis.com>
Include the OpenSSL error queue in the returned error.

Signed-off-by: Cory Snider <csnider@mirantis.com>
@corhere corhere force-pushed the fix-aes-gcm-102fips branch 2 times, most recently from 527e37f to 7bb4f4b Compare October 17, 2023 19:53
Copy link
Collaborator

@ueno ueno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@qmuntal qmuntal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 7bb4f4b should limit its changes to OpenSSL 1.0.2, other versions shouldn't need the FIPS() call (as per the commit description) to know if an algorithm is supported or not.

@corhere corhere force-pushed the fix-aes-gcm-102fips branch from 7bb4f4b to eb456ec Compare October 18, 2023 18:11
which replaces func version1_1_1_or_above().

Signed-off-by: Cory Snider <csnider@mirantis.com>
OpenSSL 1.0.2 FIPS mode will claim to support the aforementioned
algorithms but will fail with the error "disabled for fips" when one
tries to initialize an EVP context with one.

Signed-off-by: Cory Snider <csnider@mirantis.com>
This lets the compiler statically prove e.g. vPatch >= 0 is a constant
so it can optimize away unnecessary comparisons after inlining and
constant-propagating calls to versionAtOrAbove().

Signed-off-by: Cory Snider <csnider@mirantis.com>
@corhere corhere force-pushed the fix-aes-gcm-102fips branch from eb456ec to 9b5d63e Compare October 18, 2023 18:31
@qmuntal qmuntal merged commit 61234a9 into golang-fips:v2 Oct 19, 2023
8 checks passed
@corhere corhere deleted the fix-aes-gcm-102fips branch October 19, 2023 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants