search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.79k stars 17.64k forks source link

boringcrypto: require `CGO_ENABLED=1` when `GOEXPERIMENT=boringcrypto` #68588

Open aead opened 3 months ago

aead commented 3 months ago

Go version

go version go1.22.5 linux/amd64

Output of go env in your module/workspace:

O111MODULE=''
GOARCH='amd64'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/nix/store/16g3awzjv3341s27hkkkv1vk5dw4206m-go-1.22.5/share/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/nix/store/16g3awzjv3341s27hkkkv1vk5dw4206m-go-1.22.5/share/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='0'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2148692384=/tmp/go-build -gno-record-gcc-switches'

What did you do?

The following Go program starts a simple HTTPS server on port :4443 using a self-signed RSA certificate and writes a CPU profile to cpu.pprof: https://go.dev/play/p/6DzPHSlU1r2

Compile this program using the following go build commands on a linux/amd64 machine:

  1. GOEXPERIMENT=boringcrypto CGO_ENABLED=0 go build -o fips-test main.go
  2. GOEXPERIMENT=boringcrypto CGO_ENABLED=1 go build -o fips-test main.go

What did you see happen?

Both commands produce a valid binary without any error. Inspecting both binaries with rsc.io/goversion results in:

  1.   goversion -crypto -m fips-test
  fips-test go1.22.5 X:boringcrypto (standard crypto)
          path   command-line-arguments
          build  -buildmode=exe
          build  -compiler=gc
          build  CGO_ENABLED=0
          build  GOARCH=amd64
          build  GOEXPERIMENT=boringcrypto
          build  GOOS=linux
          build  GOAMD64=v1
  2.  goversion -crypto -m fips-test
 fips-test go1.22.5 X:boringcrypto (boring crypto)
          path   command-line-arguments
          build  -buildmode=exe
          build  -compiler=gc
          build  CGO_ENABLED=1
          build  CGO_CFLAGS=
          build  CGO_CPPFLAGS=
          build  CGO_CXXFLAGS=
          build  CGO_LDFLAGS=
          build  GOARCH=amd64
          build  GOEXPERIMENT=boringcrypto
          build  GOOS=linux
          build  GOAMD64=v1

Looking at the cpu.pprof files also shows that the boringcrypto C implementation is only selected when CGO_ENABLED=1.

go tool pprof cpu.pprof
    File: fips-test
    Type: cpu
    Time: Jul 25, 2024 at 4:33pm (CEST)
    Duration: 13.37s, Total samples = 50ms ( 0.37%)
    Entering interactive mode (type "help" for commands, "o" for options)
    (pprof) top 10
    Showing nodes accounting for 50ms, 100% of 50ms total
     Showing top 10 nodes out of 15
          flat  flat%   sum%        cum   cum%
           20ms 40.00% 40.00%       40ms 80.00%  crypto/internal/bigmod.(*Nat).montgomeryMul
          20ms 40.00% 80.00%       20ms 40.00%  crypto/internal/bigmod.addMulVVW2048
          10ms 20.00%   100%       10ms 20.00%  crypto/internal/bigmod.(*Nat).shiftIn
             0     0%   100%       40ms 80.00%  crypto/internal/bigmod.(*Nat).Exp
             0     0%   100%       10ms 20.00%  crypto/internal/bigmod.(*Nat).Mod
             0     0%   100%       50ms   100%  crypto/rsa.(*PrivateKey).Sign
             0     0%   100%       50ms   100%  crypto/rsa.SignPSS
             0     0%   100%       50ms   100%  crypto/rsa.decrypt
             0     0%   100%       50ms   100%  crypto/rsa.signPSSWithSalt
             0     0%   100%       50ms   100%  crypto/tls.(*Conn).HandshakeContext
 File: fips-test
    Type: cpu
    Time: Jul 25, 2024 at 4:40pm (CEST)
    Duration: 12.57s, Total samples = 50ms (  0.4%)
    Entering interactive mode (type "help" for commands, "o" for options)
    (pprof) top 10
    Showing nodes accounting for 50ms, 100% of 50ms total
    Showing top 10 nodes out of 42
          flat  flat%   sum%        cum   cum%
          30ms 60.00% 60.00%       30ms 60.00%  runtime.cgocall
          10ms 20.00% 80.00%       10ms 20.00%  runtime.memclrNoHeapPointers
          10ms 20.00%   100%       10ms 20.00%  runtime/internal/syscall.Syscall6
             0     0%   100%       10ms 20.00%  bufio.(*Writer).Flush
             0     0%   100%       30ms 60.00%  crypto/internal/boring.(*PrivateKeyRSA).withKey
             0     0%   100%       30ms 60.00%  crypto/internal/boring.SignRSAPSS
             0     0%   100%       30ms 60.00%  crypto/internal/boring.SignRSAPSS.func1
             0     0%   100%       30ms 60.00%  crypto/internal/boring.SignRSAPSS.func1.2
             0     0%   100%       30ms 60.00%  crypto/internal/boring._Cfunc__goboringcrypto_RSA_sign_pss_mgf1
             0     0%   100%       30ms 60.00%  crypto/rsa.(*PrivateKey).Sign

Importing crypto/tls/fipsonly works in both cases and the Go TLS stack actually rejects TLS 1.3 connections (TLS 1.3 has to be disabled for boringcrypto). When obserbing the TLS stack behavior, it seems that the binary uses boringcrypto when CGO_ENABLED=0 even though it actually uses the standard library crypto implementations.

What did you expect to see?

This seems like a "footgun" in the sense that people might try to build a binary that uses a FIPS 140-2 certified module to meet compliance requirements but accidentally have set CGO_ENABLED=0 in their build environment.

While it might seem obvious that GOEXPERIMENT=boringcrypto demands CGO_ENABLED=1, I was not able to find any documentation for this. Also there are blog posts not mentioning this in any way. For example: https://medium.com/cyberark-engineering/navigating-fips-compliance-for-go-applications-libraries-integration-and-security-42ac87eec40b

I'm aware that building Go binaries with boringcrypto as crypto backend is not officially supported. However, requiring CGO_ENABLED=1 does not seem to have any downside.

gabyhelp commented 3 months ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

seankhliao commented 3 months ago

cc @golang/security

xnox commented 1 month ago

Just use github.com/microsoft/go systemcrypto experiment with build tag to require FIPS. It produces binaries that will use openssl at runtime, with any FIPS provider available on any host OS, which all them provide (Ubuntu Pro FIPS, RHEL UBI, SuSe, Chainguard).

Unlike boringcrypto experiment, systemcrypto experiment correctly blocks unapproved usage; and prevents miss compilation by failing at build time and/or binary startup. Such that all of the above mentioned pitfalls are resolved.

Starting from trivial ones like forgetting to enable cgo (or turning it off by accident), to blocking md5, to more nuinanced caveats - like preventing silently falling back to uncertified gocrypto.

aead commented 1 month ago

@xnox The point of issue is that the boringcrypto experiment has an "implicit" dependency on cgo which is not enforced.

Just use github.com/microsoft/go systemcrypto experiment with build tag to require FIPS.

Using an "unofficial" fork of the Go standard library is not a viable solution for everyone.

xnox commented 4 weeks ago

@xnox The point of issue is that the boringcrypto experiment has an "implicit" dependency on cgo which is not enforced.

Just use github.com/microsoft/go systemcrypto experiment with build tag to require FIPS.

Using an "unofficial" fork of the Go standard library is not a viable solution for everyone.

heh, and "official" fork officially said no support for boringcrypto experiment and no guarantees. Compared with support and responsiveness from multiple enterprise distributions and a tech giant that cares about FIPS on the other side.

Note that in microsoft/go fork, in addition to openssl and CNG backends the boringcrypto backend is also available, and improved upon. So even with microsoft/go you can continue to use boringcrypto if you so wish.

In https://github.com/microsoft/go/blob/84de1c58814b14711dcda4d9d11557a68bdf4188/patches/0007-Add-backend-code-gen.patch#L507

There is generated conflict file, that has init, that panics missbuilt binaries in boringcrypto mode. Specifically when all the conditions are not met. This resolves this issue.

That particular chunk of code can be vendored into any project, and you can self safe guard against this issue with a panic upon init, by checking the same set of tags.

alexisromagnoli commented 2 weeks ago

@xnox you wrote:

Starting from trivial ones like forgetting to enable cgo (or turning it off by accident), to blocking md5, to more nuinanced caveats - like preventing silently falling back to uncertified gocrypto.

I've built a test app. using "github.com/microsoft/go systemcrypto experiment with build tag to require FIPS", and also added to it:

import (
    "crypto/md5"
      ...
}

func myMD5() {
    // Data to hash
    data := "Hello, this is some data to hash using MD5"

    // Create an MD5 hash
    hash := md5.New()
    hash.Write([]byte(data))
    hashBytes := hash.Sum(nil)

    // Convert the hash to a hexadecimal string for display
    hashString := hex.EncodeToString(hashBytes)

    // Print the MD5 hash
    fmt.Printf("MD5 hash of '%s': %s\n", data, hashString)
}

func main() {
        myMD5()
   ...
}

and it didn't block md5.

Here they say:

There is not yet any way to configure the crypto APIs to panic instead of falling back to standard Go crypto.

Are you sure about this? Or maybe, I'm missing something. TIA!

FiloSottile commented 2 weeks ago

Discussions of third-party forks such as github.com/microsoft/go are off topic. This issue is about a safety limitation in the (not officially supported) Go+BoringCrypto mode (which is unlikely to see much investment due to #69536).

xnox commented 2 weeks ago

There is not yet any way to configure the crypto APIs to panic instead of falling back to standard Go crypto.

Are you sure about this? Or maybe, I'm missing something.

the blocking depends on the policy of the FIPS module at runtime, and fallbacks. For example, today you typically need openssl default library context to be loaded with openssl default & fips providers, and default query properties set to fips=yes, and the module activated and passing selftests. Then MD5 will be blocked, as per policy of the module you are using, meaning you need to use an up to date 140-3 module. It's not the toolchain that enforces policy, but the relevant runtime FIPS module. FIPS is not about what apis you call; but about whether or not a fips module is in play and what it returns.

All of the above is out of scope for this bug report - please consult with your security officer, and your FIPS policies at runtime. Or contact your FIPS module vendor (i.e. Ubuntu Pro FIPS, RHEL UBI Fips, Chainguard FIPS Images, Suse FIPS, etc).

aead commented 2 weeks ago

Thanks @FiloSottile - closing this then. Hope the validation process is not too painful 🤞

FiloSottile commented 2 weeks ago

To be clear I was saying that discussions of third-party forks are off topic, not the original issue about Go+BoringCrypto.

Reopening to keep track of this just in case we do decide to clean up Go+BoringCrypto (or to close once we drop it).