encoding/asn1, crypto/x509: failed to parse Intermediate CA certificate

[sebastien-rosset]

A intermediate CA certificate contains X.509 certificate policy extension with OID 1.2.36.67006527840.666.66.1.1. The fourth oid (67006527840) in that extension is greater than math.MaxInt32, which causes a certificate unmarshaling error. Specifically the error occurs in the asn1.Unmarshal function while trying to unmarshal the certificate policy extension.

The CA cert has been issued by an Internet provider in New Zealand. I am not affiliated with the certifucate authority, and I don't know how that OID was assigned. I cannot force the CA to issue a new CA cert with sub-OIDs that have a value lower than math.MaxInt32; to the best of my understanding there is nothing in the ASN.1 spec that states sub-oids must be lower than 2^31 - 1. I've been able to successfully load the CA certificate using openssl, macOS key chain, and browsers.

I understand it may be challenging to change type ObjectIdentifier []int, but it's also not a good long term option to close this issue.

I saw related issues #30757, #19933, #36881. It looks like in practice there are certificates that have OID values higher than 2^31 - 1. So the reality is golang is unable to process some legitimate certificates.

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

go version go1.14.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/serosset/.cache/go-build"
GOENV="/home/serosset/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/serosset/goroot"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build216516397=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Parse CA certificate:
https://play.golang.org/p/StRBpHZhgDM

Issue narrowed to parsing issue of ASN.1 object identifier:

1. Encode the ASN.1 Object Identifier 1.2.36.67006527840.666.66.1.1 in DER format using the golang asn1.Marshal function.
2. Manually verify the DER encoded data is correct. I used 2 different Linux tools to compare the ASN.1 DER encoding with what is produced by asn1.Marshal.
3. Take the ASN.1 DER encoded byte array and call asn1.Unmarshal.

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

What did you expect to see?

Unmarshal in step (3) should succeed and the same original object identifier should have been retrieved.

Until a long term fix is determined, maybe the golang asn1 (and x509) package documentation should explicitly mention the implementation is not fully compliant with ITU-T Rec X.690, i.e. object identifiers cannot have values higher than 2^31-1. Also, the error message when parsing a certificate is very low-level and does not provide context: asn1: structure error: base 128 integer too large.

What did you see instead?

Step (3) is failing. asn1.Unmarshal fails with error asn1: structure error: base 128 integer too large

[odeke-em]

Thank you for this report @sebastien-rosset!

Kindly pinging the crypto squad @katiehockman @FiloSottile

[rolandshoemaker]

Looks like the original choice made by @agl was to match behavior on all platforms, so the maximum size was limited to 31 bits so that parsing behavior would match on 32 bit and 64 bit builds. In theory this could be supported by simply seeing if casting the parsed int64 -> int caused an overflow and only use the int if it didn't, which was the originally proposed solution to the problem, but this would cause mismatching behavior.

Confusingly this does create a marshal/unmarshal mismatch. asn1.Marshal will happily encode a integer larger than 31 bits, but cannot parse it. It seems like either parsing should be dependent on platform integer size, or marshal/unmarshal should match by limiting encoding to 31 bits on all platforms.

[sebastien-rosset]

Looks like the original choice made by @agl was to match behavior on all platforms, so the maximum size was limited to 31 bits so that parsing behavior would match on 32 bit and 64 bit builds. In theory this could be supported by simply seeing if casting the parsed int64 -> int caused an overflow and only use the int if it didn't, which was the originally proposed solution to the problem, but this would cause mismatching behavior.

The golang ASN.1 implementation correctly implements the ASN.1 INTEGER type, which can have an arbitrary number of bits: An ASN.1 INTEGER can be written to an int, int32, int64, or *big.Int (from the math/big package).

An object identifier is specified in ITU-T recommendation X.208, which has been superseded bu X.680-683. An object identifier value is semantically an ordered list of object identifier component values:

ObjIdComponent ::=
  NameForm |
  NumberForm |
  NameAndNumberForm

NumberForm ::= number | DefinedValue

Where number can have an arbitrary number of digits. It is not limited to 32 bits and it is not limited to 64 bits either. So it seems that to properly implement unmarshaling of object identifiers, the implementation should do something similar to INTEGER unmarshaling, where the value can be represented either as int32,

Confusingly this does create a marshal/unmarshal mismatch. asn1.Marshal will happily encode a integer larger than 31 bits, but cannot parse it. It seems like either parsing should be dependent on platform integer size, or marshal/unmarshal should match by limiting encoding to 31 bits on all platforms.

[rolandshoemaker]

Sure, the problem though is that asn1.ObjectIdentifier was defined as a []int, presumably because in the real world it's quite rare that components are ever >int32, and extremely rare that components are >int64. That isn't to say that this doesn't happen, but for whatever reason that was the decision made.

As such this cannot be changed to something that supports arbitrary integer sizes without changing the definition, which would break a lot of software that assumes the definition is []int.

[sebastien-rosset]

Yes, I understand the challenges of the implementation. However I don't think just because asn1.ObjectIdentifier was defined as a []int, that should mean golang will never be able to parse real-world CA certificates such as the New Zealand example I have provided.

[sebastien-rosset]

I don't think a solution would involve changing the type of ObjectIdentifier. As you and I pointed out, that would break a lot of code. But it seems this could be done in a way similar to what has been done for integers: the asn1.UnmarshalWithParam() function can take int, int32, int64 and big.Int for the val argument. Similarly, two new types ObjectIdentifierInt64 and ObjectIdentifierBigInt could be defined and the function asn1.UnmarshalWithParam() could have a couple more cases under the switch statement. The asn1.Marshal() function would also have to accept these two new types as input. Optionally there could be a wrapper type. That would cover pure ASN.1 operations.

Under the crypto package, I see 80 use of asn1.ObjectIdentifier.

[mikebros]

Hi, @sebastien-rosset. I represent the company that is having this issues. Thank you for posting this issues and looking for a resolution.
Can I ask you to please remove the references to our company name and function as we are not comfortable having us directly referenced in this bug report?
Thanks