microsoft / go

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

Query from application whether crypto backend is in FIPS mode #999

Open corhere opened 1 year ago

corhere commented 1 year ago

Applications can use the "crypto/boring".Enabled() function to query whether or not a crypto backend is handling cryptographic operations. boring.Enabled() == true implies that the BoringCrypto backend is operating in FIPS mode as that backend unconditionally forces FIPS mode enabled but the same is not true for the OpenSSL and CNG backends unless the application is built with -tags requirefips. There is currently no way for an application to accurately determine the FIPS-mode status of the backend without also requiring that FIPS mode be enabled.

The crypto modules' FIPS-mode status is made available to the crypto backend internals through their respective bindings (openssl.FIPS(), cng.FIPS()) but there is no safe way for the application to access that information. The application can't import "github.com/golang-fips/openssl/v2" to call openssl.FIPS() directly as it would import a separate copy with a different package path which shares no state with the module vendored into the runtime and therefore does not have access to the OpenSSL library handle in use by the crypto backend. Also, the application would fail to build with "multiple definition" link errors from the two conflicting copies of the cgo symbols. There is a hacky way to let the application interrogate the crypto modules' bindings directly, but it is unsafe and brittle:

import "unsafe"

//go:linkname FIPS vendor/github.com/golang-fips/openssl/v2.FIPS
func FIPS() bool

I propose that, when GOEXPERIMENT=*crypto, the following API is made available to applications:

package boring // import "crypto/boring"

// FIPS returns whether the cryptographic backend is operating in FIPS mode.
func FIPS() bool

I will gladly open a PR to implement this if there is interest.

dagood commented 1 year ago

What would this API be used for in practice?

I've also (quite recently!) noticed the lack of this API when writing a demo app that simply reports whether or not the environment actually resulted in FIPS mode being enabled--and I can imagine more important use cases--but I would be interested to hear what you have in mind in particular.

Something we've been avoiding so far is adding new APIs that upstream doesn't implement, to keep source compatibility with upstream Go. But we don't necessarily have to break compat for this:

We haven't talked this through in a team meeting yet, and we should have one tomorrow. Thanks for raising the issue!

corhere commented 1 year ago

I work on a commercial application which has an optional FIPS mode, selectable at startup the same way as apps built with microsoft/go. We currently build it using a private Go+OpenSSL fork which has an API much like what I've proposed. (We happen to spell it "crypto/fips".Enabled(), FWIW.) We use the API for:


It hadn't occurred to me that defining the API when GOEXPERIMENT=boringcrypto would break source compatibility with upstream Go, but you're absolutely right. The set of build tags would be indistinguishable from that of a build using upstream Go with GOEXPERIMENT=boringcrypto but the API surface would be different. There would be no way to write a program which can leverage the API when available without breaking the build on upstream Go with the same build configuration. In light of this, I would like to modify my proposal.

When GOEXPERIMENT=ms.cryptobackendapi is enabled, the following package is made available to applications:

//go:build goexperiment.ms.cryptobackendapi

package backend // import "crypto/backend"

// FIPS returns whether the cryptographic backend is operating in FIPS mode.
func FIPS() bool
$GOEXPERIMENT backend.FIPS()
none compile error
boringcrypto compile error
opensslcrypto compile error
ms.cryptobackendapi false
ms.cryptobackendapi,boringcrypto true
ms.cryptobackendapi,{openssl,cng,system}crypto variable

Pros:

Cons:

For completeness, another possible design would be to define a build tag which is unconditionally satisfied whenever microsoft/go is used as the toolchain, and define the new API unconditionally.

Pros:

Cons:

dagood commented 1 year ago

Thanks for the details! I like the ms.cryptobackendapi proposal, it's hard to think of a downside other than a longer GOEXPERIMENT value in the places where the API is needed.

Always defining a build tag when microsoft/go is being used does seem convenient, but yeah, strays from compatibility by default.

The . in ms. would need some extra patching (some goexperiment code is generated based on the lowercased field names of "internal/goexperiment".Flags), but I believe we could go with MS_CryptoBackendAPI for goexperiment.ms_cryptobackendapi. (Or simply goexperiment.mscryptobackendapi.)

Now that you mention namespacing, I do see how that's important for OSS module authors who want to actually support various forks' FIPS implementations. I suppose we should aim to make a standard on how backends and the related build tags work as part of https://github.com/golang-fips, and until then, namespace our experiments/tags.

package backend // import "crypto/backend"

I was about to suggest the same package name change (vs. adding to boring) until I looked again and saw you already did that. 😄 We need a new build tag and file anyway, so I think it's good to make a new package rather than add to one that implies the API is about boringcrypto.

qmuntal commented 1 year ago

I like to idea of gating the new API behind a specific knob, but I'll use a build tag instead of an experiment.

GOEXPERIMENT is a glorified build tag with a better UX, so it's a neat win for users in terms of usability. On the other hand, I like the simplicity of asking our users to just set GOEXPERIMENT=systemcrypto as a global switch to enable the OpenSSL/CNG backends, and then tweak minor features using -tags. Also, I don't want to make it easy to add/consume Microsoft-specific APIs, the correct path is to submit an upstream API proposal.

In fact, I'll submit an upstream proposal to add FIPS() bool to crypto/boring. Edit: https://github.com/golang/go/issues/61757.

dagood commented 1 year ago

https://github.com/golang/go/issues/61757 is a likely decline. 🙁 I assume small tweaks like calling it func FIPSMode() bool or clarifying in the doc comment would have been mentioned if it helped. (Pretty obvious tweaks to the proposal.) So I don't imagine that's going to happen.

So, current state of this issue: use -tags ms.cryptobackendapi instead of a GOEXPERIMENT, otherwise the same as https://github.com/microsoft/go/issues/999#issuecomment-1664673952.


To help out with build tag usage, we could create a library in https://github.com/golang-fips that contains a simple FIPS() bool API, where the library internally handles the build tags and returns false otherwise. I'm imagining it would smooth things over like this:

Tags FIPS() Compile error
false
goexperiment.boringcrypto true
goexperiment.{cng,openssl}crypto "Missing ms.cryptobackendapi"
goexperiment.{cng,openssl}crypto + ms.cryptobackendapi true/false

(We should probably standardize to get rid of the ms. prefix before posting the library in https://github.com/golang-fips.)

We don't need to do this immediately (or at all) but if the API needs to get friendlier, I think this would be a nice way.

dagood commented 1 year ago

We talked some more about this within our team after the likely decline upstream, and we've determined we don't want to expose it from microsoft/go, either. A few things came up:

  1. We do have a similar cause for caution as upstream. We want to make sure nobody thinks their FIPS compliance work is over once they call the API and get true out of it. Even if the developer knows what true means, it could end up being shown somewhere that misleads someone else.
  2. We haven't had a request for this API from any team within Microsoft.
  3. .NET doesn't have an API for this. We take some cues from how .NET handles crypto, so if they don't provide one, that's a sign that it might not be needed--at least within Microsoft.
    • CryptoConfig.AllowOnlyFipsAlgorithms looks related, but it's different, and it's not even supported in .NET [Core]: it always returns false.
    • Go is used for different types of programs than .NET, so this isn't a perfect indicator. But in general, it seems like if it's reasonably common for programs to change behavior depending on FIPS mode, we would have seen it here, too.
  4. In general, we've avoided adding microsoft/go-specific APIs so far (even behind tags), and would like to continue if possible.

We see that this could be useful when used correctly, but we're pretty wary of it for now. If an urgent need pops up, we think having the //go:linkname workaround is good enough to unblock it without adding an API proactively.

dagood commented 1 week ago

This proposal includes a crypto/fips140.Enabled() API:

Reopening to track.