crypto/x509: ParseCertificate should error on invalid tag for PolicyMappings

[dulanshuangqiao]

Go version

go version go1.18.1 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/liu/.cache/go-build"
GOENV="/home/liu/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/liu/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/liu/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.18"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.18/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
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-build3649475886=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Use x509.ParseCertificate to parse the der certificate.

What did you see happen?

For my test case, I changed the correct aki extension to the wrong tag (30—>39). The parsing result was an invalid authority key identifier error. However, the same modification to the PolicyMappings extension (30—>35) did not throw the same error.

What did you expect to see?

RFC5280 stipulates that both extensions are SEQUENCE sequences, which are encoded as 30 under ASN1 rules. I provide my test case in the attachment (the unmodified tag is named 1, and the modified tag is named 2)
Test Cases.zip

[gabyhelp]

Related Issues and Documentation

• repeated extension #70039 (closed)
• crypto/x509: the ParseRevocationList() doesn't seem to populate the AuthorityKeyId correctly #57461 (closed)
• crypto/x509: error parsing large ASN.1 identifiers #49678 (closed)
• crypto/x509: go.1.15 onwards CreateCertificate signed by CA cert adds unparseable ASN1 blocks under x509.Certificate.Extensions #47526 (closed)
• encoding/asn1, crypto/x509: failed to parse Intermediate CA certificate #38737
• crypto/x509: wrong value of RevocationList.AuthorityKeyId #67571 (closed)
• crypto does not recognize the extension "authority key identifier" #43868 (closed)

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

[seankhliao]

1.18 isn't a supported release, does this reproduce on a supported version?
Also please include code.

[dulanshuangqiao]

1.18 isn't a supported release, does this reproduce on a supported version? Also please include code.

I reproduced this problem in go version go1.23.2 linux/amd64
For the test cases I provided, aki_y and pm_y are both correct tags: 0x30. I changed both to 0x40 to get aki_n and pm_n. The results of executing this code for four test cases are:
aki_n.der Error: parsing x509: invalid authority key identifier
aki_y.der Converted
pm_n.der Converted
pm_y.der Converted
This result is different. Why is the wrong tag bit in PolicyMappings not detected as an invalid extension like aki?
Test cases and code are provided in the attachment
TestCases&Code.zip

[seankhliao]

See #68484, right now Go has no support for PolicyMappings so it just ignores it (unless it's marked critical).

[dulanshuangqiao]

See #68484, right now Go has no support for PolicyMappings so it just ignores it (unless it's marked critical).

According to your reply, I tested it again with the wrong tag PolicyMappings marked as critical.
It doesn't just ignore it (unless it's marked critical) as you said.
Even though it is marked as a critical extension golang does not check it properly.
I think this is a feature request.
I attached the test case used.
Cert.zip

[dulanshuangqiao]

参见#68484，目前Go不支持PolicyMappings，所以它只是忽略它（除非它被标记为关键）。

参见#68484，目前 Go 不支持 PolicyMappings，所以它只是忽略它（除非它被标记为关键）。

根据您的回复，我再次测试了它，并将错误的 PolicyMappings 标记为关键。 它不会像您所说的那样直接忽略它（除非它被标记为关键）。 即使它被标记为关键扩展，golang 也不会正确检查它。 我认为这是一个功能请求。 我附上了使用的测试用例 。Cert.zip

I noticed that you closed this issue before I responded, but I thought it was a feature request, so I responded again in the closed issue after retesting.
Looking forward to your response

[seankhliao]

the issue I linked is the feature request to implement it.