crypto/x509: Incorrect TBSCertificateList.Issuer field when using non-pkix.Name-encodable Issuer #53754

cipherboy · 2022-07-08T15:57:06Z

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

$ go version
go version go1.18.2 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

Not relevant.

go env Output

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

What did you do?

Go has historically had issues round-tripping certificates with its pkix.Name field. This has lead to the introduction of RawSubject/RawIssuer on x509.Certificate (see #40876 and others). This is correctly handled during signing, e.g., here.

In #50674, better support was added to the CRL for the raw Issuer field, but both x509.CreateRevocationList(...) and x509.Certificate.CreateCRL(...) still re-use the (incorrectly) parsed pkix.Name over the RawSubject form.

See also related Hashicorp Vault issues:

For example:

openssl req -x509 -newkey rsa -keyout key.pem -out cert.pem -days 365 -nodes -subj '/C=US/ST=California/L=San Francisco/O=Internet Widgets, Inc./OU=WWW/CN=Root/emailAddress=admin@example.com' -sha256 -addext basicConstraints=CA:TRUE -addext "keyUsage = digitalSignature, keyEncipherment, dataEncipherment, cRLSign, keyCertSign"

https://go.dev/play/p/Ht0mgZcMn7w

and

openssl req -x509 -newkey rsa -keyout key.pem -out cert.pem -days 365 -nodes -subj '/C=US/ST=California/L=San Francisco/O=Internet Widgets, Inc./OU=WWW/CN=Root/emailAddress=admin@example.com' -sha256 -addext basicConstraints=CA:TRUE -addext "keyUsage = digitalSignature, keyEncipherment, dataEncipherment, cRLSign, keyCertSign" -utf8

https://go.dev/play/p/0nMCWyMrXs6

What did you expect to see?

Subject of the issuer should be correctly preserved in the CRL's Issuer field.

What did you see instead?

The Email address was incorrectly elided from the CRL's Issuer field and the types were converted from UTF8 to PrintableString in the second example.

I was originally thinking it would be sufficient to round-trip from issuer.RawSubject into a pkix.RDNSequence, but this doesn't solve the UTF8 issue:

    x509_test.go:2705: Expected issuer.RawSubject, parsedCRL.RawIssuer to be the same; wanted: 30819a310b30090603550406130255533113301106035504080c0a43616c69666f726e69613116301406035504070c0d53616e204672616e636973636f311f301d060355040a0c16496e7465726e657420576964676574732c20496e632e310c300a060355040b0c03575757310d300b06035504030c04526f6f743120301e06092a864886f70d010901161161646d696e406578616d706c652e636f6d, got: 30819a310b3009060355040613025553311330110603550408130a43616c69666f726e6961311630140603550407130d53616e204672616e636973636f311f301d060355040a1316496e7465726e657420576964676574732c20496e632e310c300a060355040b1303575757310d300b06035504031304526f6f743120301e06092a864886f70d0109010c1161646d696e406578616d706c652e636f6d

Notably, some implementations like OpenSSL will treat the two as unequal still due to different type encodings.

Additionally, other implementations like OpenJDK will fail to consider this CRL for checking revocation status. I believe .NET and Microsoft's AIA-based verification behaves similarly, but fails closed rather than open.

I think this means we'd have to rework away from using pkix.TBSCertificateList to another type similar to tbsCertificate in order to use asn1.RawValue? Let me know if that's the right approach and I can file a PR for that.

The text was updated successfully, but these errors were encountered:

heschi · 2022-07-08T16:44:05Z

cc @golang/security

rolandshoemaker · 2022-07-08T17:34:23Z

Yup, because pkix.TBSCertificateList cannot be changed we'll need to transition to using an internal (private) type that just uses an asn1.RawValue and do the same trick that CreateCertificate does to pass through a raw issuer (and probably deprecate pkix.TBSCertificateList.) x509.Certificate.CreateCRL shouldn't be updated, as it is deprecated.

Feel free to send a CL if you'd like, but since we are currently in the release freeze we won't be able to land it until the tree re-opens in August.

Per discussion with Roland Shoemaker, this updates x509.CreateRevocationList to mirror the behavior of x509.CreateCertificate, creating an internal struct for the ASN.1 encoding of the CRL. This allows us to switch the Issuer field type to asn1.RawValue, bypassing the round-tripping issues of pkix.Name in most scenarios. Per linked ticket, this resolves issues where a non-Go created certificate can be used to create CRLs which aren't correctly attested to that certificate. In rare cases where the CRL issuer is validated against the certificate's issuer (such as the linked JDK example), this can result in failing to check this CRL for the candidate certificate. Resolves: golang#53754 Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

Per discussion with Roland Shoemaker, this updates x509.CreateRevocationList to mirror the behavior of x509.CreateCertificate, creating an internal struct for the ASN.1 encoding of the CRL. This allows us to switch the Issuer field type to asn1.RawValue, bypassing the round-tripping issues of pkix.Name in most scenarios. Per linked ticket, this resolves issues where a non-Go created certificate can be used to create CRLs which aren't correctly attested to that certificate. In rare cases where the CRL issuer is validated against the certificate's issuer (such as the linked JDK example), this can result in failing to check this CRL for the candidate certificate. Fixes golang#53754 Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

Per discussion with Roland Shoemaker, this updates x509.CreateRevocationList to mirror the behavior of x509.CreateCertificate, creating an internal struct for the ASN.1 encoding of the CRL. This allows us to switch the Issuer field type to asn1.RawValue, bypassing the round-tripping issues of pkix.Name in most scenarios. Per linked ticket, this resolves issues where a non-Go created certificate can be used to create CRLs which aren't correctly attested to that certificate. In rare cases where the CRL issuer is validated against the certificate's issuer (such as the linked JDK example), this can result in failing to check this CRL for the candidate certificate. Fixes golang#53754

gopherbot · 2022-07-21T16:54:08Z

Change https://go.dev/cl/418834 mentions this issue: crypto/x509: create CRLs with Issuer.RawSubject

cipherboy · 2022-08-05T13:29:02Z

Hey @rolandshoemaker! Congrats on the 1.19 GA :-) Any chance I could get some eyes on the patch set? Thanks!

cipherboy · 2022-10-31T15:08:57Z

@rolandshoemaker Bumping this, any chance you could look at the attached CL?

Per discussion with Roland Shoemaker, this updates x509.CreateRevocationList to mirror the behavior of x509.CreateCertificate, creating an internal struct for the ASN.1 encoding of the CRL. This allows us to switch the Issuer field type to asn1.RawValue, bypassing the round-tripping issues of pkix.Name in most scenarios. Per linked ticket, this resolves issues where a non-Go created certificate can be used to create CRLs which aren't correctly attested to that certificate. In rare cases where the CRL issuer is validated against the certificate's issuer (such as the linked JDK example), this can result in failing to check this CRL for the candidate certificate. Addresses review feedback by Roland Shoemaker on 2022/11/01. Fixes golang#53754 Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

cipherboy · 2022-11-03T15:28:40Z

Thank you for the reviews @rolandshoemaker and @bradfitz!! Glad this'll make 1.20 :-)

Per discussion with Roland Shoemaker, this updates x509.CreateRevocationList to mirror the behavior of x509.CreateCertificate, creating an internal struct for the ASN.1 encoding of the CRL. This allows us to switch the Issuer field type to asn1.RawValue, bypassing the round-tripping issues of pkix.Name in most scenarios. Per linked ticket, this resolves issues where a non-Go created certificate can be used to create CRLs which aren't correctly attested to that certificate. In rare cases where the CRL issuer is validated against the certificate's issuer (such as the linked JDK example), this can result in failing to check this CRL for the candidate certificate. Fixes golang#53754 Change-Id: If0adc053c081d6fb0b1ce47324c877eb2429a51f GitHub-Last-Rev: 033115d GitHub-Pull-Request: golang#53985 Reviewed-on: https://go-review.googlesource.com/c/go/+/418834 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Roland Shoemaker <roland@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Roland Shoemaker <roland@golang.org>

heschi added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 8, 2022

heschi added this to the Go1.20 milestone Jul 8, 2022

This was referenced Jul 18, 2022

crypto/x509: Incorrect TBSCertificateList.Issuer field when using non-pkix.Name-encodable Issuer [1.18 backport] #53943

Closed

crypto/x509: Incorrect TBSCertificateList.Issuer field when using non-pkix.Name-encodable Issuer [1.19 backport] #53944

Closed

cipherboy mentioned this issue Jul 21, 2022

crypto/x509: create CRLs with Issuer.RawSubject #53985

Closed

cipherboy mentioned this issue Sep 23, 2022

Add note about issuer naming and CRLs hashicorp/vault#17298

Merged

hc-github-team-secure-vault-core mentioned this issue Sep 23, 2022

Backport of Add note about issuer naming and CRLs into release/1.12.x hashicorp/vault#17299

Merged

dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 2, 2022

gopherbot closed this as completed in a367981 Nov 3, 2022

golang locked and limited conversation to collaborators Nov 3, 2023

gopherbot added the FrozenDueToAge label Nov 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto/x509: Incorrect TBSCertificateList.Issuer field when using non-pkix.Name-encodable Issuer #53754

crypto/x509: Incorrect TBSCertificateList.Issuer field when using non-pkix.Name-encodable Issuer #53754

cipherboy commented Jul 8, 2022 •

edited

heschi commented Jul 8, 2022

rolandshoemaker commented Jul 8, 2022

gopherbot commented Jul 21, 2022

cipherboy commented Aug 5, 2022

cipherboy commented Oct 31, 2022

cipherboy commented Nov 3, 2022

crypto/x509: Incorrect TBSCertificateList.Issuer field when using non-pkix.Name-encodable Issuer #53754

crypto/x509: Incorrect TBSCertificateList.Issuer field when using non-pkix.Name-encodable Issuer #53754

Comments

cipherboy commented Jul 8, 2022 • edited

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

heschi commented Jul 8, 2022

rolandshoemaker commented Jul 8, 2022

gopherbot commented Jul 21, 2022

cipherboy commented Aug 5, 2022

cipherboy commented Oct 31, 2022

cipherboy commented Nov 3, 2022

cipherboy commented Jul 8, 2022 •

edited

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

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