qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-17 16:24:42 -07:00
Code Releases Activity

crypto/x509: remove GODEBUG=x509ignoreCN=0 flag

Browse Source 
Common Name and NameConstraintsWithoutSANs are no more.

Fixes #24151 ᕕ(ᐛ)ᕗ

Change-Id: I15058f2a64f981c69e9ee620d3fab00f68967e49
Reviewed-on: https://go-review.googlesource.com/c/go/+/315209
Trust: Filippo Valsorda <filippo@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
This commit is contained in:
Filippo Valsorda 2021-04-29 13:36:08 -04:00
parent b211fe0058
commit 02ce411821
3 changed files with 9 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/crypto/x509/name_constraints_test.go
View File

@ -614,7 +614,8 @@ var nameConstraintsTests = []nameConstraintsTest{
},
},
// #30: without SANs, a certificate with a CN is rejected in a constrained chain.
// #30: without SANs, a certificate with a CN is still accepted in a
// constrained chain, since we ignore the CN in VerifyHostname.
{
roots: []constraintsSpec{
{
@ -630,7 +631,6 @@ var nameConstraintsTests = []nameConstraintsTest{
sans: []string{},
cn: "foo.com",
},
expectedError: "leaf doesn't have a SAN extension",
},
// #31: IPv6 addresses work in constraints: roots can permit them as
@ -1595,26 +1595,6 @@ var nameConstraintsTests = []nameConstraintsTest{
cn: "foo.bar",
},
},
// #85: without SANs, a certificate with a valid CN is accepted in a
// constrained chain if x509ignoreCN is set.
{
roots: []constraintsSpec{
{
ok: []string{"dns:foo.com", "dns:.foo.com"},
},
},
intermediates: [][]constraintsSpec{
{
{},
},
},
leaf: leafSpec{
sans: []string{},
cn: "foo.com",
},
ignoreCN: true,
},
}
func makeConstraintsCACert(constraints constraintsSpec, name string, key *ecdsa.PrivateKey, parent *Certificate, parentKey *ecdsa.PrivateKey) (*Certificate, error) {
@ -1865,10 +1845,6 @@ func parseEKUs(ekuStrs []string) (ekus []ExtKeyUsage, unknowns []asn1.ObjectIden
}
func TestConstraintCases(t *testing.T) {
defer func(savedIgnoreCN bool) {
ignoreCN = savedIgnoreCN
}(ignoreCN)
privateKeys := sync.Pool{
New: func() interface{} {
priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@ -1960,7 +1936,6 @@ func TestConstraintCases(t *testing.T) {
}
}
ignoreCN = test.ignoreCN
verifyOpts := VerifyOptions{
Roots: rootPool,
Intermediates: intermediatePool,
@ -1999,7 +1974,6 @@ func TestConstraintCases(t *testing.T) {
for _, key := range keys {
privateKeys.Put(key)
}
keys = keys[:0]
}
}

64
src/crypto/x509/verify.go
View File

@ -10,7 +10,6 @@ import (
"fmt"
"net"
"net/url"
"os"
"reflect"
"runtime"
"strings"
@ -18,9 +17,6 @@ import (
"unicode/utf8"
)
// ignoreCN disables interpreting Common Name as a hostname. See issue 24151.
var ignoreCN = !strings.Contains(os.Getenv("GODEBUG"), "x509ignoreCN=0")
type InvalidReason int
const (
@ -43,14 +39,7 @@ const (
// NameMismatch results when the subject name of a parent certificate
// does not match the issuer name in the child.
NameMismatch
// NameConstraintsWithoutSANs results when a leaf certificate doesn't
// contain a Subject Alternative Name extension, but a CA certificate
// contains name constraints, and the Common Name can be interpreted as
// a hostname.
//
// This error is only returned when legacy Common Name matching is enabled
// by setting the GODEBUG environment variable to "x509ignoreCN=1". This
// setting might be removed in the future.
// NameConstraintsWithoutSANs is a legacy error and is no longer returned.
NameConstraintsWithoutSANs
// UnconstrainedName results when a CA certificate contains permitted
// name constraints, but leaf certificate contains a name of an
@ -110,15 +99,7 @@ func (h HostnameError) Error() string {
c := h.Certificate
if !c.hasSANExtension() && matchHostnames(c.Subject.CommonName, h.Host) {
if !ignoreCN && !validHostnamePattern(c.Subject.CommonName) {
// This would have validated, if it weren't for the validHostname check on Common Name.
return "x509: Common Name is not a valid hostname: " + c.Subject.CommonName
}
if ignoreCN && validHostnamePattern(c.Subject.CommonName) {
// This would have validated if x509ignoreCN=0 were set.
return "x509: certificate relies on legacy Common Name field, " +
"use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0"
}
return "x509: certificate relies on legacy Common Name field, use SANs instead"
}
var valid string
@ -134,11 +115,7 @@ func (h HostnameError) Error() string {
valid += san.String()
}
} else {
if c.commonNameAsHostname() {
valid = c.Subject.CommonName
} else {
valid = strings.Join(c.DNSNames, ", ")
}
valid = strings.Join(c.DNSNames, ", ")
}
if len(valid) == 0 {
@ -620,15 +597,8 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
leaf = currentChain[0]
}
checkNameConstraints := (certType == intermediateCertificate || certType == rootCertificate) && c.hasNameConstraints()
if checkNameConstraints && leaf.commonNameAsHostname() {
// This is the deprecated, legacy case of depending on the commonName as
// a hostname. We don't enforce name constraints against the CN, but
// VerifyHostname will look for hostnames in there if there are no SANs.
// In order to ensure VerifyHostname will not accept an unchecked name,
// return an error here.
return CertificateInvalidError{c, NameConstraintsWithoutSANs, ""}
} else if checkNameConstraints && leaf.hasSANExtension() {
if (certType == intermediateCertificate || certType == rootCertificate) &&
c.hasNameConstraints() && leaf.hasSANExtension() {
err := forEachSAN(leaf.getSANExtension(), func(tag int, data []byte) error {
switch tag {
case nameTypeEmail:
@ -960,18 +930,6 @@ func validHostname(host string, isPattern bool) bool {
return true
}
// commonNameAsHostname reports whether the Common Name field should be
// considered the hostname that the certificate is valid for. This is a legacy
// behavior, disabled by default or if the Subject Alt Name extension is present.
//
// It applies the strict validHostname check to the Common Name field, so that
// certificates without SANs can still be validated against CAs with name
// constraints if there is no risk the CN would be matched as a hostname.
// See NameConstraintsWithoutSANs and issue 24151.
func (c *Certificate) commonNameAsHostname() bool {
return !ignoreCN && !c.hasSANExtension() && validHostnamePattern(c.Subject.CommonName)
}
func matchExactly(hostA, hostB string) bool {
if hostA == "" || hostA == "." || hostB == "" || hostB == "." {
return false
@ -1046,10 +1004,7 @@ func toLowerCaseASCII(in string) string {
// against the DNSNames field. If the names are valid hostnames, the certificate
// fields can have a wildcard as the left-most label.
//
// The legacy Common Name field is ignored unless it's a valid hostname, the
// certificate doesn't have any Subject Alternative Names, and the GODEBUG
// environment variable is set to "x509ignoreCN=0". Support for Common Name is
// deprecated will be entirely removed in the future.
// Note that the legacy Common Name field is ignored.
func (c *Certificate) VerifyHostname(h string) error {
// IP addresses may be written in [ ].
candidateIP := h
@ -1067,15 +1022,10 @@ func (c *Certificate) VerifyHostname(h string) error {
return HostnameError{c, candidateIP}
}
names := c.DNSNames
if c.commonNameAsHostname() {
names = []string{c.Subject.CommonName}
}
candidateName := toLowerCaseASCII(h) // Save allocations inside the loop.
validCandidateName := validHostnameInput(candidateName)
for _, match := range names {
for _, match := range c.DNSNames {
// Ideally, we'd only match valid hostnames according to RFC 6125 like
// browsers (more or less) do, but in practice Go is used in a wider
// array of contexts and can't even assume DNS resolution. Instead,

66
src/crypto/x509/verify_test.go
View File

@ -30,7 +30,6 @@ type verifyTest struct {
systemSkip bool
systemLax bool
keyUsages []ExtKeyUsage
ignoreCN bool
errorCallback func(*testing.T, error)
expectedChains [][]string
@ -297,8 +296,6 @@ var verifyTests = []verifyTest{
errorCallback: expectNotAuthorizedError,
},
{
// If any SAN extension is present (even one without any DNS
// names), the CN should be ignored.
name: "IgnoreCNWithSANs",
leaf: ignoreCNWithSANLeaf,
dnsName: "foo.example.com",
@ -325,7 +322,6 @@ var verifyTests = []verifyTest{
// verify error.
name: "CriticalExtLeaf",
leaf: criticalExtLeafWithExt,
dnsName: "example.com",
intermediates: []string{criticalExtIntermediate},
roots: []string{criticalExtRoot},
currentTime: 1486684488,
@ -338,7 +334,6 @@ var verifyTests = []verifyTest{
// cause a verify error.
name: "CriticalExtIntermediate",
leaf: criticalExtLeaf,
dnsName: "example.com",
intermediates: []string{criticalExtIntermediateWithExt},
roots: []string{criticalExtRoot},
currentTime: 1486684488,
@ -347,18 +342,6 @@ var verifyTests = []verifyTest{
errorCallback: expectUnhandledCriticalExtension,
},
{
// Test that invalid CN are ignored.
name: "InvalidCN",
leaf: invalidCNWithoutSAN,
dnsName: "foo,invalid",
roots: []string{invalidCNRoot},
currentTime: 1540000000,
systemSkip: true, // does not chain to a system root
errorCallback: expectHostnameError("Common Name is not a valid hostname"),
},
{
// Test that valid CN are respected.
name: "ValidCN",
leaf: validCNWithoutSAN,
dnsName: "foo.example.com",
@ -366,42 +349,6 @@ var verifyTests = []verifyTest{
currentTime: 1540000000,
systemSkip: true, // does not chain to a system root
expectedChains: [][]string{
{"foo.example.com", "Test root"},
},
},
// Replicate CN tests with ignoreCN = true
{
name: "IgnoreCNWithSANs/ignoreCN",
leaf: ignoreCNWithSANLeaf,
dnsName: "foo.example.com",
roots: []string{ignoreCNWithSANRoot},
currentTime: 1486684488,
systemSkip: true, // does not chain to a system root
ignoreCN: true,
errorCallback: expectHostnameError("certificate is not valid for any names"),
},
{
name: "InvalidCN/ignoreCN",
leaf: invalidCNWithoutSAN,
dnsName: "foo,invalid",
roots: []string{invalidCNRoot},
currentTime: 1540000000,
systemSkip: true, // does not chain to a system root
ignoreCN: true,
errorCallback: expectHostnameError("certificate is not valid for any names"),
},
{
name: "ValidCN/ignoreCN",
leaf: validCNWithoutSAN,
dnsName: "foo.example.com",
roots: []string{invalidCNRoot},
currentTime: 1540000000,
systemSkip: true, // does not chain to a system root
ignoreCN: true,
errorCallback: expectHostnameError("certificate relies on legacy Common Name field"),
},
{
@ -503,9 +450,6 @@ func certificateFromPEM(pemBytes string) (*Certificate, error) {
}
func testVerify(t *testing.T, test verifyTest, useSystemRoots bool) {
defer func(savedIgnoreCN bool) { ignoreCN = savedIgnoreCN }(ignoreCN)
ignoreCN = test.ignoreCN
opts := VerifyOptions{
Intermediates: NewCertPool(),
DNSName: test.dnsName,
@ -1589,16 +1533,6 @@ oCGMjNwwCgYIKoZIzj0EAwIDRwAwRAIgDSiwgIn8g1lpruYH0QD1GYeoWVunfmrI
XzZZl0eW/ugCICgOfXeZ2GGy3wIC0352BaC3a8r5AAb2XSGNe+e9wNN6
-----END CERTIFICATE-----`
const invalidCNWithoutSAN = `-----BEGIN CERTIFICATE-----
MIIBJDCBywIUB7q8t9mrDAL+UB1OFaMN5BEWFKIwCgYIKoZIzj0EAwIwFDESMBAG
A1UECwwJVGVzdCByb290MB4XDTE4MDcxMTE4MzUyMVoXDTI4MDcwODE4MzUyMVow
FjEUMBIGA1UEAwwLZm9vLGludmFsaWQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNC
AASnpnwiM6dHfwiTLV9hNS7aRWd28pdzGLABEkoa1bdvQTy7BWn0Bl3/6yunhQtM
90VOgUB6qcYdu7rZuSazylCQMAoGCCqGSM49BAMCA0gAMEUCIQCFlnW2cjxnEqB/
hgSB0t3IZ1DXX4XAVFT85mtFCJPTKgIgYIY+1iimTtrdbpWJzAB2eBwDgIWmWgvr
xfOcLt/vbvo=
-----END CERTIFICATE-----`
const validCNWithoutSAN = `-----BEGIN CERTIFICATE-----
MIIBJzCBzwIUB7q8t9mrDAL+UB1OFaMN5BEWFKQwCgYIKoZIzj0EAwIwFDESMBAG
A1UECwwJVGVzdCByb290MB4XDTE4MDcxMTE4NDcyNFoXDTI4MDcwODE4NDcyNFow