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.

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

crypto/x509: malformed signature algorithm identifier error while version and serialNumber swapped #70269

Open
dulanshuangqiao opened this issue Nov 9, 2024 · 2 comments
Open

crypto/x509: malformed signature algorithm identifier error while version and serialNumber swapped #70269

dulanshuangqiao opened this issue Nov 9, 2024 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@dulanshuangqiao
Copy link

Go version

go version go1.23.2 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='/snap/go/10730'

GOSUMDB='sum.golang.org'

GOTMPDIR=''

GOTOOLCHAIN='auto'

GOTOOLDIR='/snap/go/10730/pkg/tool/linux_amd64'

GOVCS=''

GOVERSION='go1.23.2'

GODEBUG=''

GOTELEMETRY='local'

GOTELEMETRYDIR='/home/liu/.config/go/telemetry'

GCCGO='gccgo'

GOAMD64='v1'

AR='ar'

CC='gcc'

CXX='g++'

CGO_ENABLED='1'

GOMOD='/dev/null'

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 -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build578094757=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Use x509.ParseCertificate to parse the der certificate.
The version and serial number of the certificate are swapped.
image
case.zip

What did you see happen?

Parse error:malformed signature algorithm identifier

What did you expect to see?

Swap the positions of serialnumber and version, and the value of serialnumber is the same as version. Since the type of version is OPTIONAL[0] + INTEGER, and the type of serialnumber is INTEGER, the invalid version should be checked first instead of throwing an alformed signature algorithm identifier.
@gabyhelp
Copy link

gabyhelp commented Nov 9, 2024

Related Issues

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

@mateusz834 mateusz834 changed the title ANS1 Parse crypto/x509: malformed signature algorithm identifier error while version and serialNumber swapped Nov 9, 2024
@mateusz834
Copy link
Member

The TBSCertificate is your certificate looks like this: serialNumber, version, signature. The spec does not allow this kind of order of fields:

RFC 5280:

    TBSCertificate  ::=  SEQUENCE  {
        version         [0]  EXPLICIT Version DEFAULT v1,
        serialNumber         CertificateSerialNumber,
        signature            AlgorithmIdentifier,
        -- (...)
    }

The version is checked first, but it is optional (has a DEFAULT), thus we are skipping it (because of an unexpected tag). After that the parser is still at the same spot, it does not skip the badly-placed serialNumber field, so now the sertialNumber parsing succeeds. After that, the AlgorithmIdentifier parsing fails, because it contains an unexpected tag (badly placed version). I think that the error is correct.

See the implementation for reference:

go/src/crypto/x509/parser.go

Lines 829 to 883 in 5123f38

func parseCertificate(der []byte) (*Certificate, error) {
cert := &Certificate{}
input := cryptobyte.String(der)
// we read the SEQUENCE including length and tag bytes so that
// we can populate Certificate.Raw, before unwrapping the
// SEQUENCE so it can be operated on
if !input.ReadASN1Element(&input, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed certificate")
}
cert.Raw = input
if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed certificate")
}
var tbs cryptobyte.String
// do the same trick again as above to extract the raw
// bytes for Certificate.RawTBSCertificate
if !input.ReadASN1Element(&tbs, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed tbs certificate")
}
cert.RawTBSCertificate = tbs
if !tbs.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed tbs certificate")
}
if !tbs.ReadOptionalASN1Integer(&cert.Version, cryptobyte_asn1.Tag(0).Constructed().ContextSpecific(), 0) {
return nil, errors.New("x509: malformed version")
}
if cert.Version < 0 {
return nil, errors.New("x509: malformed version")
}
// for backwards compat reasons Version is one-indexed,
// rather than zero-indexed as defined in 5280
cert.Version++
if cert.Version > 3 {
return nil, errors.New("x509: invalid version")
}
serial := new(big.Int)
if !tbs.ReadASN1Integer(serial) {
return nil, errors.New("x509: malformed serial number")
}
if serial.Sign() == -1 {
if x509negativeserial.Value() != "1" {
return nil, errors.New("x509: negative serial number")
} else {
x509negativeserial.IncNonDefault()
}
}
cert.SerialNumber = serial
var sigAISeq cryptobyte.String
if !tbs.ReadASN1(&sigAISeq, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed signature algorithm identifier")

@mateusz834 mateusz834 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 9, 2024
@gabyhelp gabyhelp mentioned this issue Feb 7, 2025
Closed
@gabyhelp gabyhelp mentioned this issue Mar 3, 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
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

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