crypto/x509: error parsing large ASN.1 identifiers

[umlublin]

What version of Go are you using (go version)?

$ go version
go1.17.3 windows/amd64

Does this issue reproduce with the latest release?

Issue observed while connecting to LDAPS serwer with certificate generated by Microsoft Active Directory with Microsoft's specific X509v3 Certificate Policies
error message is "x509: invalid certificate policies"
it comes from parseCertificatePoliciesExtension in x509 parser

Output of "openssl x509 -in my.crt --text"
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            4d:00:04:9b:44:6f:c6:43:9c:d8:f5:3a:00:00:03:00:04:9b:44
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: DC = pl, DC = com, DC = <edited>, CN = <edited>    Subordinate CA
        Validity
            Not Before: Sep 20 11:05:54 2021 GMT
            Not After : Sep 20 11:05:54 2023 GMT
        Subject: CN = <edited>
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                RSA Public-Key: (2048 bit)
                Modulus:
                    00:dd:fd:8b:7e:bd:e7:50:f0:c1:bd:8f:37:d6:e0:
<edited>
                    e8:13:8a:ae:c7:26:73:b5:81:4e:c7:ab:39:2a:ef:
                    fb:9d
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            1.3.6.1.4.1.311.21.7:
                0..&+.....7.........M...".......nK...M...b..e...
            X509v3 Extended Key Usage:
                TLS Web Client Authentication, Signing KDC Response, TLS Web Server Authentication, Microsoft Smartcard Login
            X509v3 Key Usage: critical
                Digital Signature, Key Encipherment
            X509v3 Certificate Policies:
                Policy: 1.3.6.1.4.1.311.21.8.3719450.11115469.11946914.3506198.8878958.75.1492336001.1138714952

            1.3.6.1.4.1.311.21.10:
                010
..+.......0...+......0
..+.......0..
+.....7...
            X509v3 Subject Key Identifier:
                04:7B:E7:F9:21:DB:92:0E:21:DE:70:B2:CD:FC:16:49:0D:11:46:92
<edited>

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=D:\Projekty\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=D:\Projekty\go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.17.3
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=D:\Projekty\go\bin\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\luty4\AppData\Local\Temp\go-build1235732887=/tmp/go-build -gno-record-
gcc-switches

What did you do?

https://play.golang.org/p/WI9bl64Z6wU

What did you expect to see?

**** OID with 4 bytes
Object Identifier: 1.3.6.1.4.1.311.21.8.1492336001
ASN.1 Encoding:   060e2b060104018237150885c7ccfb01
Decode result: true
Object Identifier: 1.3.6.1.4.1.311.21.8.1492336001

What did you see instead?

**** OID with 4 bytes
Object Identifier: 1.3.6.1.4.1.311.21.8.1492336001
ASN.1 Encoding:    060e2b060104018237150885c7ccfb01
Decode result: false
Object Identifier: 

[umlublin]

Bug is inside golang.org/x/crypto/cryptobyte/asn1.go
in func (s *String) readBase128Int(out *int) bool {
"if i == 4 {" should be replaced by "if i == 5 {"

[gopherbot]

Change https://golang.org/cl/365674 mentions this issue: Fixes golang/go#49678 x/crypto/cryptobyte/asn1: Error parsing ASN1 identifiers

[heschi]

cc @FiloSottile @agl @katiehockman @rolandshoemaker

[MarekPelka]

Hi, i have the same problem when i try to pull image from our internal docker repository:

docker pull private_repo/myimage:latest
Error response from daemon: Get "https://private_repo/v2/": tls: failed to parse certificate from server: x509: invalid certificate policies

Our private CA certificate’s are added to system. And wget/curl connects to https://private_repo/v2/ without problems.

[umlublin]

@heschi How long can an investigation take?
The operation of one of the banks depends on this fix. We have a "sick" certificate that cannot be changed quickly.
Recompilation of all kubernetes related software is not a solution.

[FiloSottile]

@gopherbot please open a backport issue to Go 1.17. This is a regression due to Go 1.17 changes without workaround that makes it impossible to parse some valid (if a little weird) certificates.

[gopherbot]

Backport issue(s) opened: #50165 (for 1.17).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

[gopherbot]

Change https://golang.org/cl/372274 mentions this issue: [internal-branch.go1.17-vendor] cryptobyte: fix parsing of large ASN.1 OIDs

[odeke-em]

Nice work @umlublin for the report and for the fix too! Thank you @FiloSottile for the cherry picks. @FiloSottile given that we've merged the fix into both master and go1.17-vendor branches, what else is left for us to do to close this issue for Go1.18?

[gopherbot]

Change https://golang.org/cl/373360 mentions this issue: all: update vendored golang.org/x/crypto for cryptobyte fix

[gopherbot]

Change https://golang.org/cl/373361 mentions this issue: [release-branch.go1.17] all: update vendored golang.org/x/crypto for cryptobyte fix

[ncabatoff]

This is a regression due to Go 1.17 changes without workaround that makes it impossible to parse some valid (if a little weird) certificates

@FiloSottile This seems to impact Go 1.16 as well. I haven't tested myself, but we have a bug report in Vault (hashicorp/vault#14122) suggesting as much, and looking at https://github.com/golang/go/blob/release-branch.go1.16/src/vendor/golang.org/x/crypto/cryptobyte/asn1.go#L394 I see the comparison to 4 (instead of 5).

Any chance of a 1.16 backport?

[umlublin]

This is a regression due to Go 1.17 changes without workaround that makes it impossible to parse some valid (if a little weird) certificates

@FiloSottile This seems to impact Go 1.16 as well. I haven't tested myself, but we have a bug report in Vault (hashicorp/vault#14122) suggesting as much, and looking at https://github.com/golang/go/blob/release-branch.go1.16/src/vendor/golang.org/x/crypto/cryptobyte/asn1.go#L394 I see the comparison to 4 (instead of 5).

Any chance of a 1.16 backport?

There is another certificate parser in 1.16, "golang.org/x/crypto/cryptobyte/asn1.go" is not used to parse them.
In 1.17 parser was switched.
Fixing asn1 parser is not needed in this case (hashicorp/vault#14122), but could be needed in another ones.

[FiloSottile]

If this was not a change in behavior in Go 1.17, I wonder why it only started coming up now.

[ncabatoff]

If this was not a change in behavior in Go 1.17, I wonder why it only started coming up now.

I presume @umlublin (and you) are correct and I was just looking at the wrong file. We'll followup with the bug reporter and get more details.

[xinfengliu]

Hi,

I hit the same issue with even larger ID https://oidref.com/1.2.36.20151795998

Tested it with the same program (https://play.golang.org/p/WI9bl64Z6wU) , the test does not pass.

It looks like if any part in the ID is larger than 2^31 - 1, the test fails.

        // decode(encode([]int{1, 2, 36, 20151795998, 3, 1, 1, 1})) // not pass
        // decode(encode([]int{1, 2, 36, 2147483647, 3, 1, 1, 1})) // (2^31 - 1) pass
        decode(encode([]int{1, 2, 36, 2147483648, 3, 1, 1, 1})) // 2^31 not pass

I'm using the latest golang.org/x/crypto@v0.6.0/cryptobyte. Debugging the code shows it fails at https://github.com/golang/crypto/blob/v0.6.0/cryptobyte/asn1.go#L430

Is there anything we can do to get it work?

func (s *String) readBase128Int(out *int) bool {
	ret := 0
	for i := 0; len(*s) > 0; i++ {
		if i == 5 {
			return false
		}
		// Avoid overflowing int on a 32-bit platform.
		// We don't want different behavior based on the architecture.
		if ret >= 1<<(31-7) {
			return false   <= failed
		}
		ret <<= 7
		b := s.read(1)[0]
		ret |= int(b & 0x7f)
		if b&0x80 == 0 {
			*out = ret
			return true
		}
	}
	return false // truncated
}