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>
},
},
- // #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{
{
sans: []string{},
cn: "foo.com",
},
- expectedError: "leaf doesn't have a SAN extension",
},
// #31: IPv6 addresses work in constraints: roots can permit them as
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) {
}
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)
}
}
- ignoreCN = test.ignoreCN
verifyOpts := VerifyOptions{
Roots: rootPool,
Intermediates: intermediatePool,
for _, key := range keys {
privateKeys.Put(key)
}
- keys = keys[:0]
}
}
"fmt"
"net"
"net/url"
- "os"
"reflect"
"runtime"
"strings"
"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 (
// 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
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
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 {
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:
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
// 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
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,
systemSkip bool
systemLax bool
keyUsages []ExtKeyUsage
- ignoreCN bool
errorCallback func(*testing.T, error)
expectedChains [][]string
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",
// verify error.
name: "CriticalExtLeaf",
leaf: criticalExtLeafWithExt,
- dnsName: "example.com",
intermediates: []string{criticalExtIntermediate},
roots: []string{criticalExtRoot},
currentTime: 1486684488,
// cause a verify error.
name: "CriticalExtIntermediate",
leaf: criticalExtLeaf,
- dnsName: "example.com",
intermediates: []string{criticalExtIntermediateWithExt},
roots: []string{criticalExtRoot},
currentTime: 1486684488,
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",
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"),
},
{
}
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,
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