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.

Sign up for GitHub

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

Only load OpenSSL when in FIPS mode. #207

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Only load OpenSSL when in FIPS mode. #207

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions patches/018-fix-std-crypto.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
diff --git a/src/crypto/internal/backend/openssl.go b/src/crypto/internal/backend/openssl.go
index 3d3a9a36ee..b7a65a1f6e 100644
--- a/src/crypto/internal/backend/openssl.go
+++ b/src/crypto/internal/backend/openssl.go
@@ -25,6 +25,21 @@ var enabled bool
var knownVersions = [...]string{"3", "1.1", "11", "111", "1.0.2", "1.0.0", "10"}

func init() {
+ // 0: FIPS opt-out: abort the process if it is enabled and can't be disabled.
+ // 1: FIPS required: abort the process if it is not enabled and can't be enabled.
+ // other values: do not override OpenSSL configured FIPS mode.
+ var fips string
+ if v, ok := syscall.Getenv("GOLANG_FIPS"); ok {
+ fips = v
+ } else if hostFIPSModeEnabled() {
+ // System configuration can only force FIPS mode.
+ fips = "1"
+ }
+
+ if fips == "" {
Copy link
Collaborator

@ueno ueno Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

According to the comment above, should this check against any other value than "0" nor "1", not only ""?

I'm also not sure what to do with "GOLANG_FIPS=0"; should it just return or go through the openssl.SetFIPS path below?

Copy link

Choose a reason for hiding this comment

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

We will want to keep functionality to explicitly disable fips mode via openssl, so I suggest keeping GOLANG_FIPS=0 to use openssl, but turn off FIPS mode.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ueno We were originally talking about including an option to use openssl as the crypto backend without using FIPS, so Derek may have been setting us up for that in the future with the fips == "0" path. However, currently that path will still use standard crypto, because other bits aren't implemented and boring.Enabled() will still return false. So my initial thought was to just remove the fips == "0" branch and nest all of the openssl initialization under fips == "1", since I think with the recent direction of the project it's questionable we will ever fully implement non-FIPS openssl as a backend. Then @rphillips suggested lifting the FIPS mode check and skipping the rest, which I like as a good middle ground to keep things the same until we can discuss with Derek whether we still intend to implement GOLANG_FIPS=0. But I'm also open to just removing the fips == "0" branch now and then add it back if things change later. Either way it won't matter what happens with GOLANG_FIPS=0, but you're right, other values will need to be handled too, good catch.

Copy link
Collaborator

@ueno ueno Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that leaving the current logic would be safer, though on Fedora/CentOS Stream/RHEL, the current GOLANG_FIPS=0 behavior is mostly identical to GOLANG_FIPS=1 OPENSSL_FORCE_FIPS_MODE=0.

+ return
+ }
+
version, _ := syscall.Getenv("GO_OPENSSL_VERSION_OVERRIDE")
if version == "" {
var fallbackVersion string
@@ -49,16 +64,6 @@ func init() {
if err := openssl.Init(version); err != nil {
panic("opensslcrypto: can't initialize OpenSSL " + version + ": " + err.Error())
}
- // 0: FIPS opt-out: abort the process if it is enabled and can't be disabled.
- // 1: FIPS required: abort the process if it is not enabled and can't be enabled.
- // other values: do not override OpenSSL configured FIPS mode.
- var fips string
- if v, ok := syscall.Getenv("GOLANG_FIPS"); ok {
- fips = v
- } else if hostFIPSModeEnabled() {
- // System configuration can only force FIPS mode.
- fips = "1"
- }
switch fips {
case "0":
if openssl.FIPS() {
Loading