Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Closed
dulanshuangqiao opened this issue Oct 28, 2024 · 7 comments
Closed

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

dulanshuangqiao opened this issue Oct 28, 2024 · 7 comments
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@dulanshuangqiao
Copy link

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
Copy link

Related Issues and Documentation

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

@seankhliao
Copy link
Member

seankhliao commented Oct 28, 2024 •
edited
Loading

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

@seankhliao seankhliao changed the title Invalid extension judgment crypto/x509: ParseCertificate should error on invalid tag for PolicyMappings Oct 28, 2024
@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 28, 2024
@dulanshuangqiao
Copy link
Author

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
Copy link
Member

seankhliao commented Nov 3, 2024 •
edited
Loading

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

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Nov 3, 2024
@dulanshuangqiao
Copy link
Author

dulanshuangqiao commented Nov 4, 2024 •
edited
Loading

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
Copy link
Author

参见#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
Copy link
Member

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

This was referenced Nov 9, 2024
Open
Open
@gabyhelp gabyhelp mentioned this issue Nov 30, 2024
Open
@gabyhelp gabyhelp mentioned this issue Jan 31, 2025
Closed
@gabyhelp gabyhelp mentioned this issue Mar 25, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@seankhliao @dulanshuangqiao @gabyhelp