Only load OpenSSL when in FIPS mode. #207
Conversation
We shouldn't depend on OpenSSL in non-FIPS mode, as it precludes binaries from running in any environment without OpenSSL. This commit moves the FIPS mode check to the beginning of init() and avoids initializing OpenSSL when we know Go standard crypto will be used.
| + fips = "1" | ||
| + } | ||
| + | ||
| + if fips == "" { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
|
lgtm |
|
I known certain customers that are using this, precisely to use openssl even in non-fips mode, because for them it is "faster". Although I personally am not convinced of such benefits. |
We shouldn't depend on OpenSSL in non-FIPS mode, as it precludes binaries from running in any environment without OpenSSL. This commit moves the FIPS mode check to the beginning of init() and avoids initializing OpenSSL when we know Go standard crypto will be used.