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

x/crypto/cryptobyte: "x509: invalid key usage" error caused by strict asn1 bit string parsing #70242

Open
martoche opened this issue Nov 7, 2024 · 7 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@martoche
Copy link

martoche commented Nov 7, 2024

Go version

go version go1.23.2 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/martin/Library/Caches/go-build'
GOENV='/Users/martin/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE='git.superg.run'
GOMODCACHE='/Users/martin/go/pkg/mod'
GONOPROXY='git.superg.run/*'
GONOSUMDB='git.superg.run/*'
GOOS='darwin'
GOPATH='/Users/martin/go'
GOPRIVATE='git.superg.run/*'
GOPROXY='https://proxy.golang.org'
GOROOT='/opt/local/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/local/lib/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='off'
GOTELEMETRYDIR='/Users/martin/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='/usr/bin/clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/private/tmp/decode/go.mod'
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/sz/z082qh0j6qvd6m981xc1d3n40000gn/T/go-build3277253954=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I received from my bank a x509 certificate in the PEM format to be used to connect to one of their services.

I passed the certificate to the openssl x509 command-line program to print a textual representation of the certificate: it worked.

Then I tried to parse the certificate in go using x509.ParseCertificate as shown in the following example program:
https://go.dev/play/p/jtIyu3C6fL1

What did you see happen?

The go example program fails with this error:

panic: x509: invalid key usage

goroutine 1 [running]:
main.main()
	/tmp/sandbox2217993318/prog.go:40 +0xf9

Program exited.

What did you expect to see?

Because the openssl command succeeded, I expected the go program to work as well, and to print the parsed certificate.

Here is the output of the openssl command, it doesn't have any warning or error.

$ openssl x509 -in cert.pem -text -noout
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            21:60:51:40:c8:89:7f:46:be:ea:e6:24:40:a8:c8:1a
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C=FR, OU=SSLGROUPE, 0002 493455042, O=BPCE, CN=www.ebics.bpce.fr
        Validity
            Not Before: Apr 13 09:43:09 2021 GMT
            Not After : Apr 12 09:43:09 2026 GMT
        Subject: C=FR, OU=SSLGROUPE, 0002 493455042, O=BPCE, CN=www.ebics.bpce.fr
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    00:c3:e1:61:28:f7:1d:00:f5:3c:f9:b0:ac:16:50:
                    15:ab:24:e9:80:9f:19:92:ad:31:36:74:85:41:e0:
                    c9:74:28:06:3a:fd:b1:08:71:22:b2:39:f2:31:11:
                    38:a1:a8:a5:e7:23:e3:57:bf:0b:29:54:ad:39:d5:
                    6f:c5:0d:6e:ad:c5:37:cd:be:b4:12:ea:d7:81:e7:
                    f0:a3:7c:9e:a8:d6:15:77:c3:c7:0d:d6:d8:eb:53:
                    1b:f4:ce:5c:95:aa:a2:ef:65:e8:44:2d:e6:e8:1d:
                    ab:00:45:26:5b:36:28:95:9c:cf:28:be:12:1e:9a:
                    cd:85:bc:41:f3:9a:68:c3:16:6f:d1:9c:d0:83:cb:
                    c1:75:36:4a:f0:53:38:ea:8e:c3:d6:80:39:8d:83:
                    53:e7:8e:6c:36:8f:ff:d7:ee:e5:18:23:46:30:2e:
                    e1:77:e5:e8:c3:8b:66:49:6e:01:11:34:bf:ca:26:
                    89:79:2b:6a:95:46:ac:1a:cf:cf:7a:04:64:c1:95:
                    ad:4a:26:5b:59:61:b1:e5:23:64:cf:c2:24:bf:7d:
                    33:68:e0:a0:1d:7f:74:cd:ce:03:7e:64:cf:c9:a6:
                    59:07:be:34:c2:39:60:6e:8b:62:e2:22:b2:ec:5f:
                    ff:6d:d1:12:1b:c9:8d:7d:3a:95:a0:d3:b4:64:bf:
                    69:fb
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Subject Key Identifier:
                1C:EE:D7:19:36:7C:FE:C1:FB:5E:21:41:3B:A6:8F:41:91:A4:39:84
            X509v3 Basic Constraints:
                CA:FALSE
            X509v3 Key Usage: critical
                Digital Signature
            X509v3 Extended Key Usage:
                TLS Web Server Authentication, TLS Web Client Authentication
    Signature Algorithm: sha256WithRSAEncryption
    Signature Value:
        4a:67:f4:5e:28:75:0b:e3:31:8f:19:f3:2f:94:54:64:c0:8a:
        fd:27:db:f8:76:5e:96:16:51:9e:87:85:bd:b9:b7:d1:f1:43:
        e7:5c:1f:c1:1d:6b:32:5c:d8:06:a0:fb:12:08:9a:0c:e1:ab:
        e8:24:90:c3:e3:83:40:31:da:21:e2:f2:dc:f7:b1:95:c4:28:
        37:24:92:ea:56:ec:b9:07:9d:ca:86:cc:a0:b7:40:c8:97:f1:
        29:42:79:b5:d8:23:7b:aa:22:fd:0e:33:a6:58:c3:bf:fc:26:
        35:89:e3:d6:96:da:4a:7b:b1:3e:8d:40:b5:f1:2e:8d:67:a6:
        87:4e:f6:32:55:a6:13:4b:19:85:be:b3:e5:e0:ac:6a:06:27:
        6a:c4:8d:a4:89:68:a0:83:3e:c1:77:56:93:29:b9:4f:a5:97:
        0a:cb:f7:87:4e:e6:8c:f4:d6:99:56:e9:8d:b5:2c:a7:48:c5:
        bd:07:2c:47:3e:3a:8d:bd:01:13:12:c6:10:ef:b7:ea:e3:c5:
        c4:73:69:1d:36:d6:91:2f:bf:5f:e7:f3:ec:e6:48:1e:8d:e7:
        13:9c:ef:c1:a3:de:95:22:19:14:ca:c9:9f:ff:de:ab:5b:b6:
        c9:eb:c2:6e:63:80:7f:00:58:ae:7f:b6:ae:e0:1a:93:66:63:
        92:d4:cd:77

I've tracked down the problem by comparing the execution of my go program in the delve debugger with the execution of the openssl in the lldb debugger.

The error in go happens in the asn1 parser, specifically in the ReadASN1BitString function, at this very specific line:
https://cs.opensource.google/go/x/crypto/+/71ed71b4faf97caafd1863fed003e9ac311f10ee:cryptobyte/asn1.go;l=561

The result of the expression bytes[len(bytes)-1]&(1<<paddingBits-1) is not equal to 0, so the ReadASN1BitString function returns false (ie: an error occured).

(In my specific example, paddingBits is equal to 5 and bytes is equal to [136])

My understanding is that the go authors decided that if the padded part of the bit string is not equal to zero, then the function should fail.

In openssl, the equivalent line is this one:
https://github.com/openssl/openssl/blob/e54526413d5ef7c665e25f552f2f01d4352bd33d/crypto/asn1/a_bitstr.c#L121

We see that the openssl library doesn't behave like the go library: instead of failing if the padded part of the bit string is not equal to zero, it sets the value of the padded part to zero.

(In my specific example, 136 & (0xff << 5) == 128).

The rust authors seem to have made the same choice as openssl. See the comment that says The "unused bits" are set to 0. in this file:
https://docs.rs/der/latest/src/der/asn1/bit_string.rs.html#56

If my analysis is correct, I suggest to relax the ASN1 bit string parser by allowing non-zero padding and then setting it to zero like openssl and rust does.

@gopherbot gopherbot added this to the Unreleased milestone Nov 7, 2024
@martoche
Copy link
Author

martoche commented Nov 7, 2024

Also, running openssl asn1parse doesn't show any warning or error:
$ openssl asn1parse -i -in cert.pem -dump

@mateusz834
Copy link
Member

mateusz834 commented Nov 7, 2024

For some reason it seems like ReadASN1BitStringAsBytes is not doing this check.

https://github.com/golang/crypto/blob/71ed71b4faf97caafd1863fed003e9ac311f10ee/cryptobyte/asn1.go#L573-L585

EDIT: actually it is fine "It is an error if the BIT STRING is not a whole number of bytes"

@dr2chase
Copy link
Contributor

dr2chase commented Nov 7, 2024

@FiloSottile @golang/security PTAL, the problem is nicely explained.

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 7, 2024
@mateusz834
Copy link
Member

My understanding is that the go authors decided that if the padded part of the bit string is not equal to zero, then the function should fail.

Actualy, the spec authors decided that. DER requires unused bits to be set to zero, only BER allows that.

X.690:

11.2 Unused bits
11.2.1 Each unused bit in the final octet of the encoding of a bit string value shall be set to zero

@zpavlinovic
Copy link
Contributor

My manual bisection suggests that, if this is indeed a bug, it is present since at least go1.17.

@mateusz834
Copy link
Member

@zpavlinovic ParseCertificate was rewritten then, mainly a change from encoding/asn1 to x/crypto/cryptobyte

https://go.dev/doc/go1.17

ParseCertificate has been rewritten, and now consumes ~70% fewer resources. The observable behavior when processing WebPKI certificates has not otherwise changed, except for error messages.

But interestingly, the encoding/ans1 still has the same check as the x/crypto/cryptobyte.

if paddingBits > 7 ||
len(bytes) == 1 && paddingBits > 0 ||
bytes[len(bytes)-1]&((1<<bytes[0])-1) != 0 {
err = SyntaxError{"invalid padding bits in BIT STRING"}
return
}

https://github.com/golang/crypto/blob/71ed71b4faf97caafd1863fed003e9ac311f10ee/cryptobyte/asn1.go#L557-L563

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants