search

microsoft / go

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

`go tool cover` doesn't work with FIPS mode enabled on Mariner 2.0 #1253

Open olishenton opened 1 week ago

olishenton commented 1 week ago

The Go runtime/coverage library uses an MD5 hash in its metadata calculations (code for reference). The "vendor crypto backends" patch moves this to use EVP_DigestInit_ex on a fresh evp ctx when using OpenSSL as the backend (code for reference).

On Mariner 2.0, use of md5 is is explicitly disabled in FIPS mode (code for reference) unless EVP_MD_CTX_FLAG_NON_FIPS_ALLOW is set on the evp ctx.

Resultant stack trace:

panic: EVP_DigestInit_ex
openssl error(s):
error:060800C8:digital envelope routines:EVP_DigestInit_ex:disabled for FIPS
    crypto/evp/digest.c:135

goroutine 1 [running]:
vendor/github.com/golang-fips/openssl/v2.newEvpHash(0x2, 0x10, 0x40)
    /usr/local/go/src/vendor/github.com/golang-fips/openssl/v2/hash.go:132 +0x12e
vendor/github.com/golang-fips/openssl/v2.NewMD5(...)
    /usr/local/go/src/vendor/github.com/golang-fips/openssl/v2/hash.go:248
crypto/internal/backend.NewMD5(...)
    /usr/local/go/src/crypto/internal/backend/openssl_linux.go:135
crypto/md5.New()
    /usr/local/go/src/crypto/md5/md5.go:104 +0x3a
runtime/coverage.prepareForMetaEmit()
    /usr/local/go/src/runtime/coverage/emit.go:214 +0x197

Of course, this isn't that big a deal, since code coverage behaviour with FIPS mode disabled should be the same as if enabled, but it would be nice if this worked.

I don't remember what the FIPS implementations are like on other distros' OpenSSL releases, but wouldn't be surprised if this is a more general problem. Also worth remarking that Mariner 3.0 will move to using SymCrypt as the provider for FIPS mode which will heavily change this picture - in all expecting this Issue to be a LiveWith.

dagood commented 1 week ago

Interesting, it seems to me the Microsoft Go OpenSSL backend should always pass EVP_MD_CTX_FLAG_NON_FIPS_ALLOW in order to meet our current compatibility goals, because there's no API that existing apps are already using to indicate when non-FIPS algorithms should be usable.

Of course, each app must be reviewed for FIPS compliance, but the goal of Microsoft Go is that if it is determined that the use is compliant, no source change is required for it to build and run successfully.

Yeah, other distros of FIPS-compliant Go that I've seen don't go for total compat and total OpenSSL usage, and this kind of panic might even be considered desirable. We use OpenSSL in more places than might be considered necessary (in order to follow a Microsoft internal policy), and we allow non-FIPS usage of algorithms while using the OpenSSL backend for compat reasons.

@microsoft/golang-compiler

The "vendor crypto backends" patch moves this to use EVP_DigestInit_ex on a fresh evp ctx

The phrasing doesn't seem like the way I'd describe it, so pointing out a few things in case you didn't realize: the vendoring patch is just grabbing the source for https://github.com/golang-fips/openssl. The backend foundation patch wires up the md5 packages to use a backend (if selected): https://github.com/microsoft/go/blob/035727a366d2759706ef90343b733b6782db917f/patches/0002-Add-crypto-backend-foundation.patch#L883-L885

And the OpenSSL backend patch wires them together: https://github.com/microsoft/go/blob/035727a366d2759706ef90343b733b6782db917f/patches/0004-Add-OpenSSL-crypto-backend.patch#L332