Prioritize FIPS-enabled versions when automatically selecting a version #53
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,9 +24,6 @@ import ( | |
| var ( | ||
| providerNameFips = C.CString("fips") | ||
| providerNameDefault = C.CString("default") | ||
| ) | ||
|
|
||
| var ( | ||
|
|
@@ -126,26 +123,33 @@ func loadLibrary(version string) (unsafe.Pointer, error) { | |
| } | ||
| return handle, nil | ||
| } | ||
| var fallbackHandle unsafe.Pointer | ||
| for _, v := range knownVersions { | ||
| handle := dlopen(v) | ||
| if handle == nil { | ||
| continue | ||
| } | ||
| if C.go_openssl_fips_enabled(handle) == 1 { | ||
| // Found a FIPS enabled version, use it. | ||
| if fallbackHandle != nil { | ||
| // If we found a FIPS enabled version but we already have a fallback | ||
| // version, close the fallback version. | ||
| C.dlclose(fallbackHandle) | ||
| } | ||
| return handle, nil | ||
| } | ||
| if fallbackHandle == nil { | ||
| // Remember the first version that exists but is not FIPS enabled | ||
| // in case we don't find any FIPS enabled version. | ||
| fallbackHandle = handle | ||
| } else { | ||
| C.dlclose(handle) | ||
| } | ||
| } | ||
| if fallbackHandle != nil { | ||
| return fallbackHandle, nil | ||
| } | ||
| return nil, errors.New("openssl: can't load libcrypto.so using any known version suffix") | ||
| } | ||
|
|
||
| // FIPS returns true if OpenSSL is running in FIPS mode, else returns false. | ||
|
|
@@ -159,53 +163,49 @@ func FIPS() bool { | |
| } | ||
| // EVP_default_properties_is_fips_enabled can return true even if the FIPS provider isn't loaded, | ||
| // it is only based on the default properties. | ||
| return C.go_openssl_OSSL_PROVIDER_available(nil, providerNameFips) == 1 | ||
| default: | ||
| panic(errUnsuportedVersion()) | ||
| } | ||
| } | ||
|
|
||
| // SetFIPS enables or disables FIPS mode. | ||
| // | ||
| // On OpenSSL 3, the `fips` provider is loaded if enabled is true, | ||
| // else the `default` provider is loaded. | ||
| func SetFIPS(enabled bool) error { | ||
| var mode C.int | ||
| if enabled { | ||
| mode = C.int(1) | ||
| } else { | ||
| mode = C.int(0) | ||
| } | ||
| switch vMajor { | ||
| case 1: | ||
| if C.go_openssl_FIPS_mode_set(mode) != 1 { | ||
| return newOpenSSLError("openssl: FIPS_mode_set") | ||
| } | ||
| return nil | ||
| case 3: | ||
| var provName *C.char | ||
| if enabled { | ||
| provName = providerNameFips | ||
| } else { | ||
| provName = providerNameDefault | ||
| } | ||
| // Check if provName is not loaded. | ||
| if C.go_openssl_OSSL_PROVIDER_available(nil, provName) == 0 { | ||
| // If not, fallback to provName provider. | ||
|
There was a problem hiding this comment. This doesn't seem to line up with the behavior mentioned in the There was a problem hiding this comment. Did that on purpose, but forgot to remove the comment. I now think that supporting FIPS providers not named There was a problem hiding this comment. Notice that I've implemented things a bit different in golang-fips/openssl#66. That's because I don't want to break the go-crypto-openssl API, in case we decide to backport this. |
||
| if C.go_openssl_OSSL_PROVIDER_load(nil, provName) == nil { | ||
| return newOpenSSLError("openssl: OSSL_PROVIDER_load") | ||
| } | ||
| // Make sure we now have a provider available. | ||
| if C.go_openssl_OSSL_PROVIDER_available(nil, provName) == 0 { | ||
| return fail("SetFIPS(" + strconv.FormatBool(enabled) + ") not supported") | ||
| } | ||
| } | ||
| if C.go_openssl_EVP_default_properties_enable_fips(nil, mode) != 1 { | ||
| return newOpenSSLError("openssl: EVP_default_properties_enable_fips") | ||
| } | ||
| return nil | ||
| default: | ||
|
|
@@ -281,6 +281,7 @@ func bnToBig(bn C.GO_BIGNUM_PTR) BigInt { | |
| // output depends on the input. noescape is inlined and currently | ||
| // compiles down to zero instructions. | ||
| // USE CAREFULLY! | ||
| // | ||
| //go:nosplit | ||
| func noescape(p unsafe.Pointer) unsafe.Pointer { | ||
| x := uintptr(p) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, on second look, I think this might be going too deep? If systemwide/opensslwide FIPS isn't enabled, it seems like this means that even if FIPS mode is supported, this will return
0, so this version of OpenSSL won't get picked. So if we e.g. have 3.0 that doesn't support FIPS and 1.0.2 that supports FIPS but FIPS isn't enabled by default, we would pick 3.0 as the "fallback", not have FIPS, and (e.g.) fail to turn on FIPS mode later when we really should have been able to.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that was may intention, pick the systemwide enabled OpenSSL version, not just one that is FIPS-capable but not enabled (should have documented it better in the PR description). IMO it makes more sense this way, production environments that require FIPS must have it enable systemwide, as not all tooling and framework provides something like
GOFIPS=1.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, yeah, that makes sense.
My next thought would be "what if you want to test FIPS mode on a non-FIPS machine with multiple OpenSSL versions etc." but in that case, setting
GO_OPENSSL_VERSION_OVERRIDEis perfectly fine if the fallback isn't the right version--and it's probably something you would end up doing anyway to test each version intentionally. 😄