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

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

Open
dagood opened this issue Feb 11, 2022 · 4 comments
Open
Labels

Comments

@dagood
Copy link
Member

dagood commented Feb 11, 2022

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: #277 (comment). (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.

@dagood dagood added the fips label Feb 11, 2022
@qmuntal
Copy link
Contributor

qmuntal commented Feb 22, 2022

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
Copy link
Member Author

dagood commented Feb 22, 2022

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
Copy link
Contributor

qmuntal commented Feb 23, 2022

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
Copy link
Contributor

qmuntal commented Feb 24, 2022

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.

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

No branches or pull requests

2 participants