Skip to content

crypto/x509: ParseCertificate allows unsorted SET values in RDNs #73743

New issue
New issue
Open
#73936
Open
crypto/x509: ParseCertificate allows unsorted SET values in RDNs#73743
#73936
Labels
BugReportIssues describing a possible bug in the Go implementation.NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
@printfn

Description

@printfn
printfn
opened on May 16, 2025

Go version

go version go1.24.3 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/user/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/user/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2784876717=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/user/Code/go-test/go.mod'
GOMODCACHE='/home/user/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/user/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/user/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24.3'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Playground: https://go.dev/play/p/OoaeG_APMGI

What did you see happen?

Golang unmarshals incorrectly-sorted ASN.1 SET values. The example certificate has a multi-valued RDN that is incorrectly sorted. This is invalid DER and should not have been parsed.

What did you expect to see?

Golang should have errored when parsing the certificate.

See https://www.itu.int/rec/T-REC-X.690-202102-I/en, section 11.6:

The encodings of the component values of a set-of value shall appear in ascending order, the encodings being compared as
octet strings with the shorter components being padded at their trailing end with 0-octets.

Activity

gabyhelp

gabyhelp commented on May 16, 2025

@gabyhelp
gabyhelp
on May 16, 2025

Related Issues

Related Code Changes

Related Documentation

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

gabyhelp
added
BugReportIssues describing a possible bug in the Go implementation.
on May 16, 2025
mateusz834

mateusz834 commented on May 16, 2025

@mateusz834
mateusz834
on May 16, 2025
Member

Looking at the example you provided, i think that this is more related to crypto/x509 since the x509 parser does not use encoding/asn1, but x/crypto/cryptobyte.

Looking at the parsing code for RDNs, it does not check the ordering of the SET values:

go/src/crypto/x509/parser.go

Lines 141 to 181 in 045b5c1

// parseName parses a DER encoded Name as defined in RFC 5280. We may
// want to export this function in the future for use in crypto/tls.
func parseName(raw cryptobyte.String) (*pkix.RDNSequence, error) {
if !raw.ReadASN1(&raw, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: invalid RDNSequence")
}
var rdnSeq pkix.RDNSequence
for !raw.Empty() {
var rdnSet pkix.RelativeDistinguishedNameSET
var set cryptobyte.String
if !raw.ReadASN1(&set, cryptobyte_asn1.SET) {
return nil, errors.New("x509: invalid RDNSequence")
}
for !set.Empty() {
var atav cryptobyte.String
if !set.ReadASN1(&atav, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: invalid RDNSequence: invalid attribute")
}
var attr pkix.AttributeTypeAndValue
if !atav.ReadASN1ObjectIdentifier(&attr.Type) {
return nil, errors.New("x509: invalid RDNSequence: invalid attribute type")
}
var rawValue cryptobyte.String
var valueTag cryptobyte_asn1.Tag
if !atav.ReadAnyASN1(&rawValue, &valueTag) {
return nil, errors.New("x509: invalid RDNSequence: invalid attribute value")
}
var err error
attr.Value, err = parseASN1String(valueTag, rawValue)
if err != nil {
return nil, fmt.Errorf("x509: invalid RDNSequence: invalid attribute value: %s", err)
}
rdnSet = append(rdnSet, attr)
}
rdnSeq = append(rdnSeq, rdnSet)
}
return &rdnSeq, nil
}

But is also seems like encoding/asn1 accepts such encoding:

func TestTest(t *testing.T) {
	const rdsSeq = "MBYxFDAIBgNVBAoMAUIwCAYDVQQKDAFB"
	bytes, err := base64.RawStdEncoding.DecodeString(rdsSeq)
	if err != nil {
		t.Fatal(err)
	}
	var rdns RDNSequence
	_, err = Unmarshal(bytes, &rdns)
	if err != nil {
		t.Fatal(err)
	}
	t.Log(rdns) // [[{2.5.4.10 B} {2.5.4.10 A}]]
}
mateusz834
added
NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
BugReportIssues describing a possible bug in the Go implementation.
and removed
BugReportIssues describing a possible bug in the Go implementation.
on May 16, 2025
mateusz834
changed the title [-]encoding/asn1: Unmarshal allows unsorted `SET` values[/-] [+]crypto/x509: ParseCertificate allows unsorted `SET` values in RDNs[/+] on May 16, 2025
mateusz834

mateusz834 commented on May 16, 2025

@mateusz834
mateusz834
on May 16, 2025
Member

CC @rolandshoemaker

kumarlokesh
linked a pull request that will close this issue on May 31, 2025
gopherbot

gopherbot commented on May 31, 2025

@gopherbot
gopherbot
on May 31, 2025
Contributor

Change https://go.dev/cl/677796 mentions this issue: crypto/x509: Validate certificates with unsorted SET values in RDNs

rolandshoemaker

rolandshoemaker commented on Jun 2, 2025

@rolandshoemaker
rolandshoemaker
on Jun 2, 2025
Member

I'm not 100% convinced that we need to enforce this property. Generally we try to follow the specs to the letter when we encode certificates, but when parsing them we can be a little more lax. In this particular case, incorrect ordering does not really affect our processing of the certificate at all, so outright rejecting it feels a little extreme.

I am somewhat forgetting the exact context for why we didn't enforce this originally. I suspect (but don't have good evidence on hand) that encoding implementations used to get this wrong, so it was safer to just accept out of order SETs. It may be worthwhile re-checking the current ecosystem to see if there are still certificates which violate this property or not floating around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    BugReportIssues describing a possible bug in the Go implementation.NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @printfn@rolandshoemaker@gopherbot@mateusz834@gabyhelp

      Issue actions
        crypto/x509: ParseCertificate allows unsorted `SET` values in RDNs · Issue #73743 · golang/go