crypto tls panic when pulling container images
Opened this issue · 14 comments
When we try to pull container images in fips enabled environments, we see a crypto tls panic.
panic: tls: HKDF-Extract invocation failed unexpectedly
goroutine 221 [running]:
crypto/tls.(*cipherSuiteTLS13).extract(0x5651729bbfe0, {0x0?, 0x1?, 0x1?}, {0x0, 0x0, 0x0})
go-go1.22.2/src/crypto/tls/key_schedule.go:100 +0x285
crypto/tls.(*clientHandshakeStateTLS13).establishHandshakeKeys(0xc0003bdbd0)
go-go1.22.2/src/crypto/tls/handshake_client_tls13.go:388 +0xd3
crypto/tls.(*clientHandshakeStateTLS13).handshake(0xc0003bdbd0)
go-go1.22.2/src/crypto/tls/handshake_client_tls13.go:90 +0x2bb
crypto/tls.(*Conn).clientHandshake(0xc000634708, {0x5651722c75f8, 0xc000696f00})
go-go1.22.2/src/crypto/tls/handshake_client.go:265 +0x594
crypto/tls.(*Conn).handshakeContext(0xc000634708, {0x5651722c75c0, 0xc000810db0})
go-go1.22.2/src/crypto/tls/conn.go:1553 +0x3cb
crypto/tls.(*Conn).HandshakeContext(...)
go-go1.22.2/src/crypto/tls/conn.go:1493
net/http.(*persistConn).addTLS.func2()
go-go1.22.2/src/net/http/transport.go:1573 +0x6e
created by net/http.(*persistConn).addTLS in goroutine 217
go-go1.22.2/src/net/http/transport.go:1569 +0x309
I see the hashToMD function not able to find a match in the hash.Hash type switch here. and returning nil.
I instrumented a debug print statement inside the hashToMD function.
fmt.Printf("actual h type=%T\n", h)
and i see h type=*sha256.digest
But before the img pull command I also see cases of h type=*openssl.sha256Hash which seems to pass the type switch in hashToMD function. Not sure which type is expected or both.
If I skip the type switch and hard code "return cryptoHashToMD(crypto.SHA256)" , the panic disappears.
The code function flow seems to be
ExtractHKDF -> newHKDF -> hashToMD -> h type=*sha256.digest -> no match in type switch.
I have confirmed that exact same panic also occurs with go v1.21.x.
Thank you for looking into it! I tried that with a simple TLS client (compiled with the Go compiler built from the go1.22.2-1-openssl-fips, but it works under under GOLANG_FIPS=1 OPENSSL_FORCE_FIPS_MODE=1:
sh-5.1$ GOLANG_FIPS=1 OPENSSL_FORCE_FIPS_MODE=1 ./client --server-name fedoraproject.org --address fedoraproject.org:443
2024/05/15 08:12:04 Connected to 152.19.134.198:443 with: VersionTLS13 [TLS_AES_256_GCM_SHA384]ExtractHKDF -> newHKDF -> hashToMD -> h type=*sha256.digest -> no match in type switch.
Would it be possible to set a breakpoint in crypto.sha256.New() to see how it ends up with a non-boring hash implementation?
Would it be possible to set a breakpoint in crypto.sha256.New() to see how it ends up with a non-boring hash implementation?
I can definitely try the non-boring path. But first, let me share a script that helps reproduce the error in fips enabled env. see attached.
You would need a linux amd64 machine (fedora/rh/centos shud work). Please let me know if the script doesn't work in ur env and i would be happy to make necessary modifications to it. Please rm the txt extension, github wont let me attach otherwise.
@sipasing thanks for opening this issue and for the reproducer! As I mentioned over slack (and @ueno mentioned on this thread) somehow a non-FIPS (non-boring) hash is being selected and passed down into the FIPS module. Setting a breakpoint as @ueno mentioned could be helpful for us to understand how we're getting into this situation.
Got it. On it. Do u have any preference on gdb or delve ? Or a simple debug print is sufficient ?
Got it. On it. Do u have any preference on gdb or delve ? Or a simple debug print is sufficient ?
Delve is preferred, a print statement could work as well.
I added these print statement inside src/crypto/sha256/sha256.go ?
func New() hash.Hash {
+ fmt.Println("crypt.sha256.New")
+ _, file, no, ok := runtime.Caller(1)
+ if ok {
+ fmt.Printf("called from %s#%d\n", file, no)
+ }
if boring.Enabled() {
+ fmt.Println("boring.Enabled")
return boring.NewSHA256()
}
+
+ fmt.Println("new digest")
d := new(digest)
d.Reset()
and i get
quay.io/quay/busybox:latest: resolving |--------------------------------------|
elapsed: 0.3 s total: 0.0 B (0.0 B/s)
crypt.sha256.New
called from go-go1.21.7/src/vendor/github.com/golang-fips/openssl-fips/openssl/hmac.go#64
boring.Enabled
right before the panic.
For delve, I would need to attach to the go binary and then execute the ctr img pull command ? . working on it.
To add to previous comment, hmac.go#64 is
func NewHMAC(h func() hash.Hash, key []byte) hash.Hash {
64 ch := h()
65 md := hashToMD(ch)
to find the caller of NewHMAC, I further added another printf inside hmac.go#64
func NewHMAC(h func() hash.Hash, key []byte) hash.Hash {
+ fmt.Println("openssl.NewHMAC")
+ _, file, no, ok := runtime.Caller(1)
+ if ok {
+ fmt.Printf("called from %s#%d\n", file, no)
+ }
and i get
openssl.NewHMAC
called from go-go1.21.7/src/crypto/hmac/hmac.go#131
crypt.sha256.New
called from go-go1.21.7/src/vendor/github.com/golang-fips/openssl-fips/openssl/hmac.go#70
boring.Enabled
hmac.go#131 is
129 func New(h func() hash.Hash, key []byte) hash.Hash {
130 if boring.Enabled() {
131 hm := boring.NewHMAC(h, key)
Delve output
dlv exec ./ctr -- image pull quay.io/quay/busybox:latest
Type 'help' for list of commands.
(dlv) b myLoopingStuff src/crypto/sha256/sha256.go:153
Breakpoint myLoopingStuff set at 0x55e7df6e8346 for crypto/sha256.New() go-go1.21.7/src/crypto/sha256/sha256.go:153
(dlv) continue
quay.io/quay/busybox:latest: resolving |--------------------------------------|
elapsed: 0.1 s total: 0.0 B (0.0 B/s)
quay.io/quay/busybox:latest: resolving |--------------------------------------|
elapsed: 0.2 s total: 0.0 B (0.0 B/s)
src/crypto/hmac/hmac.New
called from go-go1.21.7/src/crypto/tls/prf.go#28
boring.Enabled
openssl.NewHMAC
called from go-go1.21.7/src/crypto/hmac/hmac.go#139
> [myLoopingStuff] crypto/sha256.New() go-go1.21.7/src/crypto/sha256/sha256.go:153 (hits goroutine(183):1 total:1) (PC: 0x55e7df6e8346)
148: // New returns a new hash.Hash computing the SHA256 checksum. The Hash
149: // also implements encoding.BinaryMarshaler and
150: // encoding.BinaryUnmarshaler to marshal and unmarshal the internal
151: // state of the hash.
152: func New() hash.Hash {
=> 153: fmt.Println("crypt.sha256.New")
154: _, file, no, ok := runtime.Caller(1)
155: if ok {
156: fmt.Printf("called from %s#%d\n", file, no)
157: }
158: if boring.Enabled() {
(dlv) bt
0 0x000055e7df6e8346 in crypto/sha256.New
at go-go1.21.7/src/crypto/sha256/sha256.go:153
1 0x000055e7df5f00a6 in vendor/github.com/golang-fips/openssl-fips/openssl.NewHMAC
at go-go1.21.7/src/vendor/github.com/golang-fips/openssl-fips/openssl/hmac.go:70
2 0x000055e7df72f4e2 in crypto/hmac.New
at go-go1.21.7/src/crypto/hmac/hmac.go:139
3 0x000055e7df788278 in crypto/tls.pHash
at go-go1.21.7/src/crypto/tls/prf.go:28
4 0x000055e7df788ec9 in crypto/tls.prf12.func1
at go-go1.21.7/src/crypto/tls/prf.go:73
5 0x000055e7df789644 in crypto/tls.extMasterFromPreMasterSecret
at go-go1.21.7/src/crypto/tls/prf.go:123
6 0x000055e7df767c26 in crypto/tls.(*clientHandshakeState).doFullHandshake
at go-go1.21.7/src/crypto/tls/handshake_client.go:658
7 0x000055e7df76642a in crypto/tls.(*clientHandshakeState).handshake
at go-go1.21.7/src/crypto/tls/handshake_client.go:495
8 0x000055e7df764006 in crypto/tls.(*Conn).clientHandshake
at go-go1.21.7/src/crypto/tls/handshake_client.go:276
9 0x000055e7df797085 in crypto/tls.(*Conn).clientHandshake-fm
at <autogenerated>:1
10 0x000055e7df760212 in crypto/tls.(*Conn).handshakeContext
at go-go1.21.7/src/crypto/tls/conn.go:1552
11 0x000055e7df75fb37 in crypto/tls.(*Conn).HandshakeContext
at go-go1.21.7/src/crypto/tls/conn.go:1492
12 0x000055e7df893cbd in net/http.(*persistConn).addTLS.func2
at go-go1.21.7/src/net/http/transport.go:1555
13 0x000055e7df281661 in runtime.goexit
at go-go1.21.7/src/runtime/asm_amd64.s:1650
Also right before the panic, dlv shows hash from github.com/minio/sha256-simd.digest.
(dlv) b go-go1.21.7/src/vendor/github.com/golang-fips/openssl-fips/openssl/hmac.go:23
18: )
19:
20: // hashToMD converts a hash.Hash implementation from this package
21: // to a BoringCrypto *C.GO_EVP_MD.
22: func hashToMD(h hash.Hash) *C.GO_EVP_MD {
=> 23: switch h.(type) {
24: case *sha1Hash:
25: return C._goboringcrypto_EVP_sha1()
26: case *sha224Hash:
27: return C._goboringcrypto_EVP_sha224()
28: case *sha256Hash:
(dlv) p h
hash.Hash(*github.com/minio/sha256-simd.digest) *{
h: [8]uint32 [1779033703,3144134277,1013904242,2773480762,1359893119,2600822924,528734635,1541459225],
x: [64]uint8 [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],
nx: 0,
len: 0,}
(dlv) c
Ah, ok. So given that last comment it seems that the hash is of type *github.com/minio/sha256-simd.digest which is not produced within the bounds of the FIPS module (e.g. not from OpenSSL), so that's why the panic is happening. The error message could be better here, but the behavior is correct.
Overall this is not something we can support. All crypto, including hashes, must be generated by the OpenSSL FIPS module. Upstream Go boringcrypto has the same constraints. I'd suggest to open a bug against the library or application that pulls in minio/sha256-simd and suggest to add Go boringcrypto support via build tags similarly to how it has been implemented in minio-go, for example:
Thanks for the pointers. I am still investigating ways in which I can manually replace hashes generated by minio/sha256-simd in containerd (the application) with OpenSSL hashes.
I kept this ticket open because I wanted to improve on the error messages. Currently there is a panic generated , but maybe we can improve it to also say that the hash is not supported or something to that effect ??