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

Binaries built with opensslcrypto experiment can sometimes fallback to gocrypto, for historical algorithms #1347

Open
xnox opened this issue Oct 2, 2024 · 3 comments
Labels

Comments

@xnox
Copy link

xnox commented Oct 2, 2024

Below uses MD5, but the same logic also applies to DES, 3DES, RC4.
Leaving future algorithms out of scope for this bug report (i.e. Ed25519).

  • Openssl crypto on non-fips hosts usually supports MD5.
  • Some may build openssl without MD5 (no-md5), though no known distributions do this yet
  • There is a variety of openssl FIPS modules
  • Some of them do not contain MD5 with property fips=yes (All 3.0.0+ fips modules)
  • Some of them advertise MD5 as available, but then it is blocked at runtime (1.1.1 from Mainer 2.0, Old Ubuntu).
  • In some of the above scenarios fallback to gocrypto MD5 is indifferent, useful, or undesired.

I would like to argue that fallback to gocrypto is undesired on any fips hosts, for historical algorithms that are being removed.

  • For openssl FIPS 1.1.1 and lower, currently MD5 gets blocked at runtime because openssl module advertises MD5 as available; and toolchain/binaries are using it at runtime; which then fails at runtime.
  • For openssl FIPS 3.0.0 and higher, currently MD5 is advertised as not available at all, and the same binary that previously was blocked from using MD5 - now silently fallsback to gocrypto code, potentially in violation of approved only policy of the system configured openssl.
  • This lead to anomaly that “echo Hello | openssl md5” fails, and go binary that does the same also fails with old FIPS modules, but magically calculates MD5 with modern FIPS modules.
  • This is particularly undesirable for very old algorithms, which has now become obsolete, historical and prohibited. And even marked as unapproved, in the existing Active 140-2 certificates.

Design proposal:

  • Add an additional indicator in the Boring module to indicate if the backend is detected to be in FIPS or non-fips mode.
  • In the legacy algorithms not only check for when boring is enabled, and a given algorithm is available, but also if the module is in fips mode or not
  • As fips mode should not be bypassed without raising a service indicator

Previously filed as a PR at #1327

@xnox
Copy link
Author

xnox commented Oct 2, 2024

Pondering if this is exact opposite of golang-fips/openssl@2cf9b4c

@dagood
Copy link
Member

dagood commented Dec 11, 2024

Sorry we haven't had a chance to discuss this issue further. Noticed it from a notification on the draft PR and think it would be good to copy @qmuntal's message from #1327 (comment) for context on our current thoughts:

We currently aim to be as compatible as possible with upstream Go, which means falling back to Go crypto when necessary. Our stance is that if someone wants to be FIPS compliant then they should vet their code and ensure that only FIPS-approved algos are being used. There are code analysis tools that are good at doing this, such as CodeQL.

This doesn't preclude that in the future we add a way to disable Go fallbacks, for example using requirefips as shown in this PR. But we are not there yet.

@dagood dagood added the fips label Dec 11, 2024
@qmuntal
Copy link
Member

qmuntal commented Dec 11, 2024

Note that upstream added GODEBUG=fips140=only in Go 1.24 to cover the case reported in this issue. See golang/go#70123 for more context. We will try to make it compatible with our backends before Go 1.24 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants