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

Don't try to override system FIPS mode and use openssl.FIPSCapable #1496

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

qmuntal
Copy link
Member

@qmuntal qmuntal commented Jan 16, 2025

As agreed in https://github.com/microsoft/go-lab/blob/main/docs/adr/0012-remove-gofips.md, we shouldn't try to modify the OpenSSL FIPS mode.

Azure Linux 3 doesn't set fips=yes in the default properties, even when running in kernel FIPS mode. They omit it because the default OpenSSL provider, SCOSSL, is already FIPS compliant, and not setting it allows some non-FIPS algorithms to fall back to the built-in provider (which is also enabled by default). This means we can't use openssl.FIPS() (which checks if fips=yes is set) in AZL3. Fortunately we can use openssl.FIPSCapable() instead, which doesn't check if fips=yes is set.

This has the nice benefit of being more compliant when running in FIPS mode on AZL3, as more non-FIPS approved algorithms (like SHA1) will be provided by the built-in OpenSSL providers instead of falling back to Go.

For #1445.

@qmuntal qmuntal requested review from dagood and mertakman January 16, 2025 16:56
Copy link
Member

@dagood dagood left a comment

Choose a reason for hiding this comment

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

LGTM, in particular the non-symmetry between using openssl.FIPSCapable and openssl.FIPS does make sense here to me with your comments. 👍

@qmuntal qmuntal enabled auto-merge (squash) January 17, 2025 08:17
@qmuntal qmuntal merged commit 542e15d into microsoft/main Jan 17, 2025
35 checks passed
@qmuntal qmuntal deleted the dev/qmuntal/fipspanic branch January 17, 2025 08:44
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