microsoft / go

The Microsoft build of the Go toolset
BSD 3-Clause "New" or "Revised" License
284 stars 27 forks source link

How should devs find usage of FIPS incompatible crypto in their code? #428

Open dagood opened 2 years ago

dagood commented 2 years ago

By default, the OpenSSL-backed crypto implementation in https://github.com/microsoft/go-crypto-openssl falls back to Go crypto when an OpenSSL function is not available. This comes from a crypto board recommendation: https://github.com/microsoft/go/issues/277#issuecomment-969102781. (Note: Just because a function is implemented using OpenSSL doesn't mean the function, or the way the function is used, is FIPS-compliant.)

So, with the goal of checking if an existing codebase will be able to pass FIPS 140-2 certification, how do devs find where fallback happens? Or more broadly, where FIPS compatibility is probably broken?

A possibility is to run the application with a flag that makes the crypto libraries panic when a non-OpenSSL-backed function is executed. This could point out usage in the normal execution path.

You could potentially catch more with a code analysis tool: check for used packages and used funcs. In theory it could also check how the funcs are used--deeper checks than you can do at runtime. (Of course, it's impossible to get perfect analysis, but maybe there are some bad patterns that could be pointed out.) An analyzer also helps for relatively unknown code with tests that might not fully exercise the app's crypto.

/cc @microsoft/golang-compiler We've discussed this kind of thing before, but don't remember where we left it. I didn't spot a tracking issue yet.

qmuntal commented 2 years ago

I'm somehow skeptical about the code analysis tool, crypto API is full of interfaces and indirection layers which makes it difficult to build a tool that infers which types are being passed to lower level crypto functions.

Additionally, we are really just a thin wrapper around OpenSSL (and others in the future), so the issues we can catch at compile time vs the issues OpenSSL will catch at runtime make me thing that investing time in an automatic tool will be only useful for a handful of cases.

Luckily Go already provides a knob to turn off support for non-FIPS TLS configuration, that is importing crypto/tls/fipsonly. Doing that will be good enough for downstream users who do not use any crypto API other than crypto/tls, which basically means using net/http.

For the other cases, we could build an small tool that checks the dependency tree looking for calls to crypto APIs which are not backed by OpenSSL, and add a environment variable that throws a panic if it detects any intent of fallback from go-crypto-openssl to Go crypto.

dagood commented 2 years ago

we could build an small tool that checks the dependency tree looking for calls to crypto APIs which are not backed by OpenSSL

Are you talking about scanning the microsoft/go code here? Or is the idea to start from user code--so the analysis can see exactly what the user code depends on in the context of the Go compiler tooling they're using?

and add a environment variable that throws a panic if it detects any intent of fallback from go-crypto-openssl to Go crypto.

I'm trying to think about how false positives and negatives end up, here. What if a crypto function is used in a way that is backed by OpenSSL, but not FIPS-compliant? (These can be usage scenarios that aren't detectable at runtime, right?) What if some code isn't executed in test runs, but it gets called in some unusual situation in production?


I don't think it's possible to write a particularly accurate tool, but would one make sense that at least points devs at places to start looking? What seems to me like a reasonable scan tool MVP would be one that lists every crypto API call. The dev can look through the list to evaluate. As incremental improvement (if possible--definitely isn't in a lot of cases), the tool could get smarter and stop listing API calls that are definitely good. Or at least, it could give devs notes in the generated list about what potential problems the tool can't prove aren't possible.

This would still miss library/app-specific hand-rolled crypto, but hopefully that's rarer, and would stand out enough to whoever's checking over it for FIPS.

qmuntal commented 2 years ago

Are you talking about scanning the microsoft/go code here? Or is the idea to start from user code--so the analysis can see exactly what the user code depends on in the context of the Go compiler tooling they're using?

I'm talking about scanning the user code. The idea would be to list every crypto API call without judging if the call if compliant or not, except for the few cases that we can, such as sha256.New().

I buy your idea of an MVP which at least sets a starting point to look for possible incompliances.

What if a crypto function is used in a way that is backed by OpenSSL, but not FIPS-compliant?

OpenSSL does its best to ensure all algorithms are used in a FIPS-compliant manner, and if it finds a misuse it will report out. Quote from the OpenSSL 2.0 FIPS User Guide:

The OpenSSL API used with the FIPS Object Module in FIPS mode is designed to return error conditions when an attempt is made to use a non-FIPS algorithm via the OpenSSL API. ... . However, there is no guarantee that the OpenSSL API will always return an error condition in every possible permutation or sequence of API calls that might invoke code relating to non-FIPS algorithms.

The complete security net we can provide would be something like:

  1. Static analysis of user code that detects top level crypto function and ordinary struct methods (aka not interfaces) which are not backed by OpenSSL.
  2. Static analysis of user code that detects all top level crypto functions, ordinary struct methods and interface methods which are backed by OpenSSL but that some of their parameters could be misused, such as ecdsa.Sign, whose hash parameter should be should be the result of hashing a message using a FIPS-compliant hasher.
  3. Opt-in runtime panic if we detect parameters that would produce a fallback to Go crypto, such as rsa.GenerateKey(r io.Reader, bits int) where bits is not 2048 or 3072.
  4. OpenSSL internal FIPS compliance checking (this is already happening).

All layers except the 4th should not have false negatives (aka don't detect a FIPS incompliance), as we know what is backed by OpenSSL, and the 4th will only have false negatives in the rare case where OpenSSL does not catch the FIPS incompliance.

All layers can have false positives, as the user may want to use some crypto functions in a non-FIPSy way. Layer two has more chances of false positives, as the parameter might be FIPS-compliant.

qmuntal commented 2 years ago

For future reference, OpenSSL 3 centralizes most of the FIPS compliance checks in here. Those checks can be disabled either by defining OPENSSL_NO_FIPS_SECURITYCHECKS at build time or by setting fips_security_check_option=0 in the OpenSSL config file.

I haven't found anything similar in OpenSSL 1 yet.