align x509 implementation

smx509/cert_pool.go

@@ -1,59 +1,109 @@
 package smx509
 
 import (
+	"bytes"
+	"crypto/sha256"
 	"encoding/pem"
-	"errors"
+	"sync"
-	"runtime"
 )
 
+type sum224 [sha256.Size224]byte
+
 // CertPool is a set of certificates.
 type CertPool struct {
-	bySubjectKeyId map[string][]int
+	byName map[string][]int // cert.RawSubject => index into lazyCerts
-	byName         map[string][]int
+
-	certs          []*Certificate
+	// lazyCerts contains funcs that return a certificate,
+	// lazily parsing/decompressing it as needed.
+	lazyCerts []lazyCert
+
+	// haveSum maps from sum224(cert.Raw) to true. It's used only
+	// for AddCert duplicate detection, to avoid CertPool.contains
+	// calls in the AddCert path (because the contains method can
+	// call getCert and otherwise negate savings from lazy getCert
+	// funcs).
+	haveSum map[sum224]bool
+
+	// systemPool indicates whether this is a special pool derived from the
+	// system roots. If it includes additional roots, it requires doing two
+	// verifications, one using the roots provided by the caller, and one using
+	// the system platform verifier.
+	systemPool bool
+}
+
+// lazyCert is minimal metadata about a Cert and a func to retrieve it
+// in its normal expanded *Certificate form.
+type lazyCert struct {
+	// rawSubject is the Certificate.RawSubject value.
+	// It's the same as the CertPool.byName key, but in []byte
+	// form to make CertPool.Subjects (as used by crypto/tls) do
+	// fewer allocations.
+	rawSubject []byte
+
+	// getCert returns the certificate.
+	//
+	// It is not meant to do network operations or anything else
+	// where a failure is likely; the func is meant to lazily
+	// parse/decompress data that is already known to be good. The
+	// error in the signature primarily is meant for use in the
+	// case where a cert file existed on local disk when the program
+	// started up is deleted later before it's read.
+	getCert func() (*Certificate, error)
 }
 
 // NewCertPool returns a new, empty CertPool.
 func NewCertPool() *CertPool {
 	return &CertPool{
-		bySubjectKeyId: make(map[string][]int),
+		byName:  make(map[string][]int),
-		byName:         make(map[string][]int),
+		haveSum: make(map[sum224]bool),
 	}
 }
 
+// len returns the number of certs in the set.
+// A nil set is a valid empty set.
+func (s *CertPool) len() int {
+	if s == nil {
+		return 0
+	}
+	return len(s.lazyCerts)
+}
+
+// cert returns cert index n in s.
+func (s *CertPool) cert(n int) (*Certificate, error) {
+	return s.lazyCerts[n].getCert()
+}
+
 func (s *CertPool) copy() *CertPool {
 	p := &CertPool{
-		bySubjectKeyId: make(map[string][]int, len(s.bySubjectKeyId)),
+		byName:     make(map[string][]int, len(s.byName)),
-		byName:         make(map[string][]int, len(s.byName)),
+		lazyCerts:  make([]lazyCert, len(s.lazyCerts)),
-		certs:          make([]*Certificate, len(s.certs)),
+		haveSum:    make(map[sum224]bool, len(s.haveSum)),
-	}
+		systemPool: s.systemPool,
-	for k, v := range s.bySubjectKeyId {
-		indexes := make([]int, len(v))
-		copy(indexes, v)
-		p.bySubjectKeyId[k] = indexes
 	}
 	for k, v := range s.byName {
 		indexes := make([]int, len(v))
 		copy(indexes, v)
 		p.byName[k] = indexes
 	}
-	copy(p.certs, s.certs)
+	for k := range s.haveSum {
+		p.haveSum[k] = true
+	}
+	copy(p.lazyCerts, s.lazyCerts)
 	return p
 }
 
 // SystemCertPool returns a copy of the system cert pool.
 //
-// Any mutations to the returned pool are not written to disk and do
+// On Unix systems other than macOS the environment variables SSL_CERT_FILE and
-// not affect any other pool returned by SystemCertPool.
+// SSL_CERT_DIR can be used to override the system default locations for the SSL
+// certificate file and SSL certificate files directory, respectively. The
+// latter can be a colon-separated list.
 //
-// New changes in the system cert pool might not be reflected
+// Any mutations to the returned pool are not written to disk and do not affect
-// in subsequent calls.
+// any other pool returned by SystemCertPool.
+//
+// New changes in the system cert pool might not be reflected in subsequent calls.
 func SystemCertPool() (*CertPool, error) {
-	if runtime.GOOS == "windows" {
-		// Issue 16736, 18609:
-		return nil, errors.New("crypto/x509: system root pool is not available on Windows")
-	}
-
 	if sysRoots := systemRootsPool(); sysRoots != nil {
 		return sysRoots.copy(), nil
 	}
@@ -62,19 +112,44 @@ func SystemCertPool() (*CertPool, error) {
 }
 
 // findPotentialParents returns the indexes of certificates in s which might
-// have signed cert. The caller must not modify the returned slice.
+// have signed cert.
-func (s *CertPool) findPotentialParents(cert *Certificate) []int {
+func (s *CertPool) findPotentialParents(cert *Certificate) []*Certificate {
 	if s == nil {
 		return nil
 	}
 
-	var candidates []int
+	// consider all candidates where cert.Issuer matches cert.Subject.
-	if len(cert.AuthorityKeyId) > 0 {
+	// when picking possible candidates the list is built in the order
-		candidates = s.bySubjectKeyId[string(cert.AuthorityKeyId)]
+	// of match plausibility as to save cycles in buildChains:
+	//   AKID and SKID match
+	//   AKID present, SKID missing / AKID missing, SKID present
+	//   AKID and SKID don't match
+	var matchingKeyID, oneKeyID, mismatchKeyID []*Certificate
+	for _, c := range s.byName[string(cert.RawIssuer)] {
+		candidate, err := s.cert(c)
+		if err != nil {
+			continue
+		}
+		kidMatch := bytes.Equal(candidate.SubjectKeyId, cert.AuthorityKeyId)
+		switch {
+		case kidMatch:
+			matchingKeyID = append(matchingKeyID, candidate)
+		case (len(candidate.SubjectKeyId) == 0 && len(cert.AuthorityKeyId) > 0) ||
+			(len(candidate.SubjectKeyId) > 0 && len(cert.AuthorityKeyId) == 0):
+			oneKeyID = append(oneKeyID, candidate)
+		default:
+			mismatchKeyID = append(mismatchKeyID, candidate)
+		}
 	}
-	if len(candidates) == 0 {
+
-		candidates = s.byName[string(cert.RawIssuer)]
+	found := len(matchingKeyID) + len(oneKeyID) + len(mismatchKeyID)
+	if found == 0 {
+		return nil
 	}
+	candidates := make([]*Certificate, 0, found)
+	candidates = append(candidates, matchingKeyID...)
+	candidates = append(candidates, oneKeyID...)
+	candidates = append(candidates, mismatchKeyID...)
 	return candidates
 }
 
@@ -82,15 +157,7 @@ func (s *CertPool) contains(cert *Certificate) bool {
 	if s == nil {
 		return false
 	}
-
+	return s.haveSum[sha256.Sum224(cert.Raw)]
-	candidates := s.byName[string(cert.RawSubject)]
-	for _, c := range candidates {
-		if s.certs[c].Equal(cert) {
-			return true
-		}
-	}
-
-	return false
 }
 
 // AddCert adds a certificate to a pool.
@@ -98,21 +165,32 @@ func (s *CertPool) AddCert(cert *Certificate) {
 	if cert == nil {
 		panic("adding nil Certificate to CertPool")
 	}
+	s.addCertFunc(sha256.Sum224(cert.Raw), string(cert.RawSubject), func() (*Certificate, error) {
+		return cert, nil
+	})
+}
+
+// addCertFunc adds metadata about a certificate to a pool, along with
+// a func to fetch that certificate later when needed.
+//
+// The rawSubject is Certificate.RawSubject and must be non-empty.
+// The getCert func may be called 0 or more times.
+func (s *CertPool) addCertFunc(rawSum224 sum224, rawSubject string, getCert func() (*Certificate, error)) {
+	if getCert == nil {
+		panic("getCert can't be nil")
+	}
 
 	// Check that the certificate isn't being added twice.
-	if s.contains(cert) {
+	if s.haveSum[rawSum224] {
 		return
 	}
 
-	n := len(s.certs)
+	s.haveSum[rawSum224] = true
-	s.certs = append(s.certs, cert)
+	s.lazyCerts = append(s.lazyCerts, lazyCert{
-
+		rawSubject: []byte(rawSubject),
-	if len(cert.SubjectKeyId) > 0 {
+		getCert:    getCert,
-		keyId := string(cert.SubjectKeyId)
+	})
-		s.bySubjectKeyId[keyId] = append(s.bySubjectKeyId[keyId], n)
+	s.byName[rawSubject] = append(s.byName[rawSubject], len(s.lazyCerts)-1)
-	}
-	name := string(cert.RawSubject)
-	s.byName[name] = append(s.byName[name], n)
 }
 
 // AppendCertsFromPEM attempts to parse a series of PEM encoded certificates.
@@ -132,24 +210,38 @@ func (s *CertPool) AppendCertsFromPEM(pemCerts []byte) (ok bool) {
 			continue
 		}
 
-		cert, err := ParseCertificate(block.Bytes)
+		certBytes := block.Bytes
+		cert, err := ParseCertificate(certBytes)
 		if err != nil {
 			continue
 		}
-
+		var lazyCert struct {
-		s.AddCert(cert)
+			sync.Once
+			v *Certificate
+		}
+		s.addCertFunc(sha256.Sum224(cert.Raw), string(cert.RawSubject), func() (*Certificate, error) {
+			lazyCert.Do(func() {
+				// This can't fail, as the same bytes already parsed above.
+				lazyCert.v, _ = ParseCertificate(certBytes)
+				certBytes = nil
+			})
+			return lazyCert.v, nil
+		})
 		ok = true
 	}
 
-	return
+	return ok
 }
 
 // Subjects returns a list of the DER-encoded subjects of
 // all of the certificates in the pool.
+//
+// Deprecated: if s was returned by SystemCertPool, Subjects
+// will not include the system roots.
 func (s *CertPool) Subjects() [][]byte {
-	res := make([][]byte, len(s.certs))
+	res := make([][]byte, s.len())
-	for i, c := range s.certs {
+	for i, lc := range s.lazyCerts {
-		res[i] = c.RawSubject
+		res[i] = lc.rawSubject
 	}
 	return res
-}
+}

smx509/pkcs8.go

@@ -65,8 +65,6 @@ func ParsePKCS8PrivateKey(der []byte) (key interface{}, err error) {
 // This kind of key is commonly encoded in PEM blocks of type "PRIVATE KEY".
 func MarshalPKCS8PrivateKey(key interface{}) ([]byte, error) {
 	switch k := key.(type) {
-	case *ecdsa.PrivateKey:
-		return marshalPKCS8ECPrivateKey(k)
 	case *sm2.PrivateKey:
 		return marshalPKCS8ECPrivateKey(&k.PrivateKey)
 	}

smx509/root_aix.go

@@ -4,3 +4,8 @@ package smx509
 var certFiles = []string{
 	"/var/ssl/certs/ca-bundle.crt",
 }
+
+// Possible directories with certificate files; all will be read.
+var certDirectories = []string{
+	"/var/ssl/certs",
+}

smx509/root_bsd.go

@@ -10,3 +10,10 @@ var certFiles = []string{
 	"/usr/local/share/certs/ca-root-nss.crt", // DragonFly
 	"/etc/openssl/certs/ca-certificates.crt", // NetBSD
 }
+
+// Possible directories with certificate files; all will be read.
+var certDirectories = []string{
+	"/etc/ssl/certs",         // FreeBSD 12.2+
+	"/usr/local/share/certs", // FreeBSD
+	"/etc/openssl/certs",     // NetBSD
+}

smx509/root_js.go

@@ -5,3 +5,6 @@ package smx509
 
 // Possible certificate files; stop after finding one.
 var certFiles = []string{}
+
+// Possible directories with certificate files; all will be read.
+var certDirectories = []string{}

smx509/root_linux.go

@@ -9,3 +9,10 @@ var certFiles = []string{
 	"/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", // CentOS/RHEL 7
 	"/etc/ssl/cert.pem",                                 // Alpine Linux
 }
+
+// Possible directories with certificate files; all will be read.
+var certDirectories = []string{
+	"/etc/ssl/certs",               // SLES10/SLES11, https://golang.org/issue/12139
+	"/etc/pki/tls/certs",           // Fedora/RHEL
+	"/system/etc/security/cacerts", // Android
+}

smx509/root_plan9.go

@@ -4,7 +4,6 @@
 package smx509
 
 import (
-	"io/ioutil"
 	"os"
 )
 
@@ -21,7 +20,7 @@ func loadSystemRoots() (*CertPool, error) {
 	roots := NewCertPool()
 	var bestErr error
 	for _, file := range certFiles {
-		data, err := ioutil.ReadFile(file)
+		data, err := os.ReadFile(file)
 		if err == nil {
 			roots.AppendCertsFromPEM(data)
 			return roots, nil

smx509/root_solaris.go

@@ -6,3 +6,8 @@ var certFiles = []string{
 	"/etc/ssl/certs/ca-certificates.crt", // Joyent SmartOS
 	"/etc/ssl/cacert.pem",                // OmniOS
 }
+
+// Possible directories with certificate files; all will be read.
+var certDirectories = []string{
+	"/etc/certs/CA",
+}

smx509/root_unix.go

@@ -4,21 +4,12 @@
 package smx509
 
 import (
-	"io/ioutil"
+	"io/fs"
 	"os"
+	"path/filepath"
+	"strings"
 )
 
-// Possible directories with certificate files; stop after successfully
-// reading at least one file from a directory.
-var certDirectories = []string{
-	"/etc/ssl/certs",               // SLES10/SLES11, https://golang.org/issue/12139
-	"/system/etc/security/cacerts", // Android
-	"/usr/local/share/certs",       // FreeBSD
-	"/etc/pki/tls/certs",           // Fedora/RHEL
-	"/etc/openssl/certs",           // NetBSD
-	"/var/ssl/certs",               // AIX
-}
-
 const (
 	// certFileEnv is the environment variable which identifies where to locate
 	// the SSL certificate file. If set this overrides the system default.
@@ -43,7 +34,7 @@ func loadSystemRoots() (*CertPool, error) {
 
 	var firstErr error
 	for _, file := range files {
-		data, err := ioutil.ReadFile(file)
+		data, err := os.ReadFile(file)
 		if err == nil {
 			roots.AppendCertsFromPEM(data)
 			break
@@ -55,32 +46,58 @@ func loadSystemRoots() (*CertPool, error) {
 
 	dirs := certDirectories
 	if d := os.Getenv(certDirEnv); d != "" {
-		dirs = []string{d}
+		// OpenSSL and BoringSSL both use ":" as the SSL_CERT_DIR separator.
+		// See:
+		//  * https://golang.org/issue/35325
+		//  * https://www.openssl.org/docs/man1.0.2/man1/c_rehash.html
+		dirs = strings.Split(d, ":")
 	}
 
 	for _, directory := range dirs {
-		fis, err := ioutil.ReadDir(directory)
+		fis, err := readUniqueDirectoryEntries(directory)
 		if err != nil {
 			if firstErr == nil && !os.IsNotExist(err) {
 				firstErr = err
 			}
 			continue
 		}
-		rootsAdded := false
 		for _, fi := range fis {
-			data, err := ioutil.ReadFile(directory + "/" + fi.Name())
+			data, err := os.ReadFile(directory + "/" + fi.Name())
-			if err == nil && roots.AppendCertsFromPEM(data) {
+			if err == nil {
-				rootsAdded = true
+				roots.AppendCertsFromPEM(data)
 			}
 		}
-		if rootsAdded {
-			return roots, nil
-		}
 	}
 
-	if len(roots.certs) > 0 || firstErr == nil {
+	if roots.len() > 0 || firstErr == nil {
 		return roots, nil
 	}
 
 	return nil, firstErr
 }
+
+// readUniqueDirectoryEntries is like os.ReadDir but omits
+// symlinks that point within the directory.
+func readUniqueDirectoryEntries(dir string) ([]fs.DirEntry, error) {
+	files, err := os.ReadDir(dir)
+	if err != nil {
+		return nil, err
+	}
+	uniq := files[:0]
+	for _, f := range files {
+		if !isSameDirSymlink(f, dir) {
+			uniq = append(uniq, f)
+		}
+	}
+	return uniq, nil
+}
+
+// isSameDirSymlink reports whether fi in dir is a symlink with a
+// target not containing a slash.
+func isSameDirSymlink(f fs.DirEntry, dir string) bool {
+	if f.Type()&fs.ModeSymlink == 0 {
+		return false
+	}
+	target, err := os.Readlink(filepath.Join(dir, f.Name()))
+	return err == nil && !strings.Contains(target, "/")
+}

smx509/root_windows.go

@@ -7,6 +7,10 @@ import (
 	"unsafe"
 )
 
+func loadSystemRoots() (*CertPool, error) {
+	return &CertPool{systemPool: true}, nil
+}
+
 // Creates a new *syscall.CertContext representing the leaf certificate in an in-memory
 // certificate store containing itself and all of the intermediate certificates specified
 // in the opts.Intermediates CertPool.
@@ -35,7 +39,11 @@ func createStoreContext(leaf *Certificate, opts *VerifyOptions) (*syscall.CertCo
 	}
 
 	if opts.Intermediates != nil {
-		for _, intermediate := range opts.Intermediates.certs {
+		for i := 0; i < opts.Intermediates.len(); i++ {
+			intermediate, err := opts.Intermediates.cert(i)
+			if err != nil {
+				return nil, err
+			}
 			ctx, err := syscall.CertCreateCertificateContext(syscall.X509_ASN_ENCODING|syscall.PKCS_7_ASN_ENCODING, &intermediate.Raw[0], uint32(len(intermediate.Raw)))
 			if err != nil {
 				return nil, err
@@ -125,9 +133,9 @@ func checkChainSSLServerPolicy(c *Certificate, chainCtx *syscall.CertChainContex
 	if status.Error != 0 {
 		switch status.Error {
 		case syscall.CERT_E_EXPIRED:
-			return x509.CertificateInvalidError{&c.Certificate, x509.Expired, ""}
+			return x509.CertificateInvalidError{Cert: &c.Certificate, Reason: x509.Expired, Detail: ""}
 		case syscall.CERT_E_CN_NO_MATCH:
-			return x509.HostnameError{&c.Certificate, opts.DNSName}
+			return x509.HostnameError{Certificate: &c.Certificate, Host: opts.DNSName}
 		case syscall.CERT_E_UNTRUSTEDROOT:
 			return UnknownAuthorityError{c, nil, nil}
 		default:
@@ -148,6 +156,44 @@ func init() {
 	}
 }
 
+func verifyChain(c *Certificate, chainCtx *syscall.CertChainContext, opts *VerifyOptions) (chain []*Certificate, err error) {
+	err = checkChainTrustStatus(c, chainCtx)
+	if err != nil {
+		return nil, err
+	}
+
+	if opts != nil && len(opts.DNSName) > 0 {
+		err = checkChainSSLServerPolicy(c, chainCtx, opts)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	chain, err = extractSimpleChain(chainCtx.Chains, int(chainCtx.ChainCount))
+	if err != nil {
+		return nil, err
+	}
+	if len(chain) == 0 {
+		return nil, errors.New("x509: internal error: system verifier returned an empty chain")
+	}
+
+	// Mitigate CVE-2020-0601, where the Windows system verifier might be
+	// tricked into using custom curve parameters for a trusted root, by
+	// double-checking all ECDSA signatures. If the system was tricked into
+	// using spoofed parameters, the signature will be invalid for the correct
+	// ones we parsed. (We don't support custom curves ourselves.)
+	for i, parent := range chain[1:] {
+		if parent.PublicKeyAlgorithm != x509.ECDSA {
+			continue
+		}
+		if err := parent.CheckSignature(chain[i].SignatureAlgorithm,
+			chain[i].RawTBSCertificate, chain[i].Signature); err != nil {
+			return nil, err
+		}
+	}
+	return chain, nil
+}
+
 // systemVerify is like Verify, except that it uses CryptoAPI calls
 // to build certificate chains and verify them.
 func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) {
@@ -173,11 +219,6 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
 		if oid, ok := windowsExtKeyUsageOIDs[eku]; ok {
 			oids = append(oids, &oid[0])
 		}
-		// Like the standard verifier, accept SGC EKUs as equivalent to ServerAuth.
-		if eku == x509.ExtKeyUsageServerAuth {
-			oids = append(oids, &syscall.OID_SERVER_GATED_CRYPTO[0])
-			oids = append(oids, &syscall.OID_SGC_NETSCAPE[0])
-		}
 	}
 	if oids != nil {
 		para.RequestedUsage.Type = syscall.USAGE_MATCH_TYPE_OR
@@ -195,109 +236,39 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
 		verifyTime = &ft
 	}
 
-	// CertGetCertificateChain will traverse Windows's root stores
+	// The default is to return only the highest quality chain,
-	// in an attempt to build a verified certificate chain. Once
+	// setting this flag will add additional lower quality contexts.
-	// it has found a verified chain, it stops. MSDN docs on
+	// These are returned in the LowerQualityChains field.
-	// CERT_CHAIN_CONTEXT:
+	const CERT_CHAIN_RETURN_LOWER_QUALITY_CONTEXTS = 0x00000080
-	//
+
-	//   When a CERT_CHAIN_CONTEXT is built, the first simple chain
+	// CertGetCertificateChain will traverse Windows's root stores in an attempt to build a verified certificate chain
-	//   begins with an end certificate and ends with a self-signed
+	var topCtx *syscall.CertChainContext
-	//   certificate. If that self-signed certificate is not a root
+	err = syscall.CertGetCertificateChain(syscall.Handle(0), storeCtx, verifyTime, storeCtx.Store, para, CERT_CHAIN_RETURN_LOWER_QUALITY_CONTEXTS, 0, &topCtx)
-	//   or otherwise trusted certificate, an attempt is made to
-	//   build a new chain. CTLs are used to create the new chain
-	//   beginning with the self-signed certificate from the original
-	//   chain as the end certificate of the new chain. This process
-	//   continues building additional simple chains until the first
-	//   self-signed certificate is a trusted certificate or until
-	//   an additional simple chain cannot be built.
-	//
-	// The result is that we'll only get a single trusted chain to
-	// return to our caller.
-	var chainCtx *syscall.CertChainContext
-	err = syscall.CertGetCertificateChain(syscall.Handle(0), storeCtx, verifyTime, storeCtx.Store, para, 0, 0, &chainCtx)
 	if err != nil {
 		return nil, err
 	}
-	defer syscall.CertFreeCertificateChain(chainCtx)
+	defer syscall.CertFreeCertificateChain(topCtx)
 
-	err = checkChainTrustStatus(c, chainCtx)
+	chain, topErr := verifyChain(c, topCtx, opts)
-	if err != nil {
+	if topErr == nil {
-		return nil, err
+		chains = append(chains, chain)
 	}
 
-	if opts != nil && len(opts.DNSName) > 0 {
+	if lqCtxCount := topCtx.LowerQualityChainCount; lqCtxCount > 0 {
-		err = checkChainSSLServerPolicy(c, chainCtx, opts)
+		lqCtxs := (*[1 << 20]*syscall.CertChainContext)(unsafe.Pointer(topCtx.LowerQualityChains))[:lqCtxCount:lqCtxCount]
-		if err != nil {
-			return nil, err
-		}
-	}
 
-	chain, err := extractSimpleChain(chainCtx.Chains, int(chainCtx.ChainCount))
+		for _, ctx := range lqCtxs {
-	if err != nil {
+			chain, err := verifyChain(c, ctx, opts)
-		return nil, err
+			if err == nil {
-	}
+				chains = append(chains, chain)
-	if len(chain) < 1 {
-		return nil, errors.New("x509: internal error: system verifier returned an empty chain")
-	}
-
-	// Mitigate CVE-2020-0601, where the Windows system verifier might be
-	// tricked into using custom curve parameters for a trusted root, by
-	// double-checking all ECDSA signatures. If the system was tricked into
-	// using spoofed parameters, the signature will be invalid for the correct
-	// ones we parsed. (We don't support custom curves ourselves.)
-	for i, parent := range chain[1:] {
-		if parent.PublicKeyAlgorithm != x509.ECDSA {
-			continue
-		}
-		if err := parent.CheckSignature(chain[i].SignatureAlgorithm,
-			chain[i].RawTBSCertificate, chain[i].Signature); err != nil {
-			return nil, err
-		}
-	}
-
-	return [][]*Certificate{chain}, nil
-}
-
-func loadSystemRoots() (*CertPool, error) {
-	// TODO: restore this functionality on Windows. We tried to do
-	// it in Go 1.8 but had to revert it. See Issue 18609.
-	// Returning (nil, nil) was the old behavior, prior to CL 30578.
-	// The if statement here avoids vet complaining about
-	// unreachable code below.
-	if true {
-		return nil, nil
-	}
-
-	const CRYPT_E_NOT_FOUND = 0x80092004
-
-	store, err := syscall.CertOpenSystemStore(0, syscall.StringToUTF16Ptr("ROOT"))
-	if err != nil {
-		return nil, err
-	}
-	defer syscall.CertCloseStore(store, 0)
-
-	roots := NewCertPool()
-	var cert *syscall.CertContext
-	for {
-		cert, err = syscall.CertEnumCertificatesInStore(store, cert)
-		if err != nil {
-			if errno, ok := err.(syscall.Errno); ok {
-				if errno == CRYPT_E_NOT_FOUND {
-					break
-				}
 			}
-			return nil, err
-		}
-		if cert == nil {
-			break
-		}
-		// Copy the buf, since ParseCertificate does not create its own copy.
-		buf := (*[1 << 20]byte)(unsafe.Pointer(cert.EncodedCert))[:cert.Length:cert.Length]
-		buf2 := make([]byte, cert.Length)
-		copy(buf2, buf)
-		if c, err := ParseCertificate(buf2); err == nil {
-			roots.AddCert(c)
 		}
 	}
-	return roots, nil
+
+	if len(chains) == 0 {
+		// Return the error from the highest quality context.
+		return nil, topErr
+	}
+
+	return chains, nil
 }

smx509/sec1.go

@@ -88,13 +88,10 @@ func MarshalSM2PrivateKey(key *sm2.PrivateKey) ([]byte, error) {
 // marshalECPrivateKey marshals an EC private key into ASN.1, DER format and
 // sets the curve ID to the given OID, or omits it if OID is nil.
 func marshalECPrivateKeyWithOID(key *ecdsa.PrivateKey, oid asn1.ObjectIdentifier) ([]byte, error) {
-	privateKeyBytes := key.D.Bytes()
+	privateKey := make([]byte, (key.Curve.Params().N.BitLen()+7)/8)
-	paddedPrivateKey := make([]byte, (key.Curve.Params().N.BitLen()+7)/8)
-	copy(paddedPrivateKey[len(paddedPrivateKey)-len(privateKeyBytes):], privateKeyBytes)
-
 	return asn1.Marshal(ecPrivateKey{
 		Version:       1,
-		PrivateKey:    paddedPrivateKey,
+		PrivateKey:    key.D.FillBytes(privateKey),
 		NamedCurveOID: oid,
 		PublicKey:     asn1.BitString{Bytes: elliptic.Marshal(key.Curve, key.X, key.Y)},
 	})

smx509/verify.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"net"
 	"net/url"
-	"os"
 	"reflect"
 	"runtime"
 	"strings"
@@ -15,41 +14,6 @@ import (
 	"unicode/utf8"
 )
 
-// ignoreCN disables interpreting Common Name as a hostname. See issue 24151.
-var ignoreCN = strings.Contains(os.Getenv("GODEBUG"), "x509ignoreCN=1")
-
-// CertificateInvalidError results when an odd error occurs. Users of this
-// library probably want to handle all these errors uniformly.
-type CertificateInvalidError struct {
-	Cert   *Certificate
-	Reason x509.InvalidReason
-	Detail string
-}
-
-func (e CertificateInvalidError) Error() string {
-	switch e.Reason {
-	case x509.NotAuthorizedToSign:
-		return "x509: certificate is not authorized to sign other certificates"
-	case x509.Expired:
-		return "x509: certificate has expired or is not yet valid: " + e.Detail
-	case x509.CANotAuthorizedForThisName:
-		return "x509: a root or intermediate certificate is not authorized to sign for this name: " + e.Detail
-	case x509.CANotAuthorizedForExtKeyUsage:
-		return "x509: a root or intermediate certificate is not authorized for an extended key usage: " + e.Detail
-	case x509.TooManyIntermediates:
-		return "x509: too many intermediates for path length constraint"
-	case x509.IncompatibleUsage:
-		return "x509: certificate specifies an incompatible key usage"
-	case x509.NameMismatch:
-		return "x509: issuer name does not match subject from issuing certificate"
-	case x509.NameConstraintsWithoutSANs:
-		return "x509: issuer has name constraints but leaf doesn't have a SAN extension"
-	case x509.UnconstrainedName:
-		return "x509: issuer has name constraints but leaf contains unknown or unconstrained name: " + e.Detail
-	}
-	return "x509: unknown error"
-}
-
 // UnknownAuthorityError results when the certificate issuer is unknown
 type UnknownAuthorityError struct {
 	Cert *Certificate
@@ -77,19 +41,6 @@ func (e UnknownAuthorityError) Error() string {
 	return s
 }
 
-// SystemRootsError results when we fail to load the system root certificates.
-type SystemRootsError struct {
-	Err error
-}
-
-func (se SystemRootsError) Error() string {
-	msg := "x509: failed to load system roots and no roots provided"
-	if se.Err != nil {
-		return msg + "; " + se.Err.Error()
-	}
-	return msg
-}
-
 // errNotParsed is returned when a certificate without ASN.1 contents is
 // verified. Platform-specific verification needs the ASN.1 contents.
 var errNotParsed = errors.New("x509: missing ASN.1 contents; use ParseCertificate")
@@ -295,18 +246,18 @@ func (c *Certificate) checkNameConstraints(count *int,
 
 	*count += excludedValue.Len()
 	if *count > maxConstraintComparisons {
-		return CertificateInvalidError{c, x509.TooManyConstraints, ""}
+		return x509.CertificateInvalidError{&c.Certificate, x509.TooManyConstraints, ""}
 	}
 
 	for i := 0; i < excludedValue.Len(); i++ {
 		constraint := excludedValue.Index(i).Interface()
 		match, err := match(parsedName, constraint)
 		if err != nil {
-			return CertificateInvalidError{c, x509.CANotAuthorizedForThisName, err.Error()}
+			return x509.CertificateInvalidError{&c.Certificate, x509.CANotAuthorizedForThisName, err.Error()}
 		}
 
 		if match {
-			return CertificateInvalidError{c, x509.CANotAuthorizedForThisName, fmt.Sprintf("%s %q is excluded by constraint %q", nameType, name, constraint)}
+			return x509.CertificateInvalidError{&c.Certificate, x509.CANotAuthorizedForThisName, fmt.Sprintf("%s %q is excluded by constraint %q", nameType, name, constraint)}
 		}
 	}
 
@@ -314,7 +265,7 @@ func (c *Certificate) checkNameConstraints(count *int,
 
 	*count += permittedValue.Len()
 	if *count > maxConstraintComparisons {
-		return CertificateInvalidError{c, x509.TooManyConstraints, ""}
+		return x509.CertificateInvalidError{&c.Certificate, x509.TooManyConstraints, ""}
 	}
 
 	ok := true
@@ -323,7 +274,7 @@ func (c *Certificate) checkNameConstraints(count *int,
 
 		var err error
 		if ok, err = match(parsedName, constraint); err != nil {
-			return CertificateInvalidError{c, x509.CANotAuthorizedForThisName, err.Error()}
+			return x509.CertificateInvalidError{&c.Certificate, x509.CANotAuthorizedForThisName, err.Error()}
 		}
 
 		if ok {
@@ -332,7 +283,7 @@ func (c *Certificate) checkNameConstraints(count *int,
 	}
 
 	if !ok {
-		return CertificateInvalidError{c, x509.CANotAuthorizedForThisName, fmt.Sprintf("%s %q is not permitted by any constraint", nameType, name)}
+		return x509.CertificateInvalidError{&c.Certificate, x509.CANotAuthorizedForThisName, fmt.Sprintf("%s %q is not permitted by any constraint", nameType, name)}
 	}
 
 	return nil
@@ -383,7 +334,7 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
 	if len(currentChain) > 0 {
 		child := currentChain[len(currentChain)-1]
 		if !bytes.Equal(child.RawIssuer, c.RawSubject) {
-			return CertificateInvalidError{c, x509.NameMismatch, ""}
+			return x509.CertificateInvalidError{&c.Certificate, x509.NameMismatch, ""}
 		}
 	}
 
@@ -392,14 +343,14 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
 		now = time.Now()
 	}
 	if now.Before(c.NotBefore) {
-		return CertificateInvalidError{
+		return x509.CertificateInvalidError{
-			Cert:   c,
+			Cert:   &c.Certificate,
 			Reason: x509.Expired,
 			Detail: fmt.Sprintf("current time %s is before %s", now.Format(time.RFC3339), c.NotBefore.Format(time.RFC3339)),
 		}
 	} else if now.After(c.NotAfter) {
-		return CertificateInvalidError{
+		return x509.CertificateInvalidError{
-			Cert:   c,
+			Cert:   &c.Certificate,
 			Reason: x509.Expired,
 			Detail: fmt.Sprintf("current time %s is after %s", now.Format(time.RFC3339), c.NotAfter.Format(time.RFC3339)),
 		}
@@ -419,15 +370,8 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
 		leaf = currentChain[0]
 	}
 
-	checkNameConstraints := (certType == intermediateCertificate || certType == rootCertificate) && c.hasNameConstraints()
+	if (certType == intermediateCertificate || certType == rootCertificate) &&
-	if checkNameConstraints && leaf.commonNameAsHostname() {
+		c.hasNameConstraints() && leaf.hasSANExtension() {
-		// 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, x509.NameConstraintsWithoutSANs, ""}
-	} else if checkNameConstraints && leaf.hasSANExtension() {
 		err := forEachSAN(leaf.getSANExtension(), func(tag int, data []byte) error {
 			switch tag {
 			case nameTypeEmail:
@@ -514,13 +458,13 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
 	// encryption key could only be used for Diffie-Hellman key agreement.
 
 	if certType == intermediateCertificate && (!c.BasicConstraintsValid || !c.IsCA) {
-		return CertificateInvalidError{c, x509.NotAuthorizedToSign, ""}
+		return x509.CertificateInvalidError{&c.Certificate, x509.NotAuthorizedToSign, ""}
 	}
 
 	if c.BasicConstraintsValid && c.MaxPathLen >= 0 {
 		numIntermediates := len(currentChain) - 1
 		if numIntermediates > c.MaxPathLen {
-			return CertificateInvalidError{c, x509.TooManyIntermediates, ""}
+			return x509.CertificateInvalidError{&c.Certificate, x509.TooManyIntermediates, ""}
 		}
 	}
 
@@ -554,23 +498,36 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
 	if len(c.Raw) == 0 {
 		return nil, errNotParsed
 	}
-	if opts.Intermediates != nil {
+	for i := 0; i < opts.Intermediates.len(); i++ {
-		for _, intermediate := range opts.Intermediates.certs {
+		c, err := opts.Intermediates.cert(i)
-			if len(intermediate.Raw) == 0 {
+		if err != nil {
-				return nil, errNotParsed
+			return nil, fmt.Errorf("crypto/x509: error fetching intermediate: %w", err)
-			}
+		}
+		if len(c.Raw) == 0 {
+			return nil, errNotParsed
 		}
 	}
 
 	// Use Windows's own verification and chain building.
 	if opts.Roots == nil && runtime.GOOS == "windows" {
-		return c.systemVerify(&opts)
+		if opts.Roots == nil {
+			return c.systemVerify(&opts)
+		}
+		if opts.Roots != nil && opts.Roots.systemPool {
+			platformChains, err := c.systemVerify(&opts)
+			// If the platform verifier succeeded, or there are no additional
+			// roots, return the platform verifier result. Otherwise, continue
+			// with the Go verifier.
+			if err == nil || opts.Roots.len() == 0 {
+				return platformChains, err
+			}
+		}
 	}
 
 	if opts.Roots == nil {
 		opts.Roots = systemRootsPool()
 		if opts.Roots == nil {
-			return nil, SystemRootsError{systemRootsErr}
+			return nil, x509.SystemRootsError{Err: systemRootsErr}
 		}
 	}
 
@@ -614,7 +571,7 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
 	}
 
 	if len(chains) == 0 {
-		return nil, CertificateInvalidError{c, x509.IncompatibleUsage, ""}
+		return nil, x509.CertificateInvalidError{&c.Certificate, x509.IncompatibleUsage, ""}
 	}
 
 	return chains, nil
@@ -684,11 +641,11 @@ func (c *Certificate) buildChains(cache map[*Certificate][][]*Certificate, curre
 		}
 	}
 
-	for _, rootNum := range opts.Roots.findPotentialParents(c) {
+	for _, root := range opts.Roots.findPotentialParents(c) {
-		considerCandidate(rootCertificate, opts.Roots.certs[rootNum])
+		considerCandidate(rootCertificate, root)
 	}
-	for _, intermediateNum := range opts.Intermediates.findPotentialParents(c) {
+	for _, intermediate := range opts.Intermediates.findPotentialParents(c) {
-		considerCandidate(intermediateCertificate, opts.Intermediates.certs[intermediateNum])
+		considerCandidate(intermediateCertificate, intermediate)
 	}
 
 	if len(chains) > 0 {
@@ -701,11 +658,16 @@ func (c *Certificate) buildChains(cache map[*Certificate][][]*Certificate, curre
 	return
 }
 
+func validHostnamePattern(host string) bool { return validHostname(host, true) }
+func validHostnameInput(host string) bool   { return validHostname(host, false) }
+
 // validHostname reports whether host is a valid hostname that can be matched or
 // matched against according to RFC 6125 2.2, with some leniency to accommodate
 // legacy values.
-func validHostname(host string) bool {
+func validHostname(host string, isPattern bool) bool {
-	host = strings.TrimSuffix(host, ".")
+	if !isPattern {
+		host = strings.TrimSuffix(host, ".")
+	}
 
 	if len(host) == 0 {
 		return false
@@ -716,7 +678,7 @@ func validHostname(host string) bool {
 			// Empty label.
 			return false
 		}
-		if i == 0 && part == "*" {
+		if isPattern && i == 0 && part == "*" {
 			// Only allow full left-most wildcards, as those are the only ones
 			// we match, and matching literal '*' characters is probably never
 			// the expected behavior.
@@ -735,7 +697,7 @@ func validHostname(host string) bool {
 			if c == '-' && j != 0 {
 				continue
 			}
-			if c == '_' || c == ':' {
+			if c == '_' {
 				// Not valid characters in hostnames, but commonly
 				// found in deployments outside the WebPKI.
 				continue
@@ -747,21 +709,16 @@ func validHostname(host string) bool {
 	return true
 }
 
-// commonNameAsHostname reports whether the Common Name field should be
+func matchExactly(hostA, hostB string) bool {
-// considered the hostname that the certificate is valid for. This is a legacy
+	if hostA == "" || hostA == "." || hostB == "" || hostB == "." {
-// behavior, disabled if the Subject Alt Name extension is present.
+		return false
-//
+	}
-// It applies the strict validHostname check to the Common Name field, so that
+	return toLowerCaseASCII(hostA) == toLowerCaseASCII(hostB)
-// 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() && validHostname(c.Subject.CommonName)
 }
 
 func matchHostnames(pattern, host string) bool {
-	host = strings.TrimSuffix(host, ".")
+	pattern = toLowerCaseASCII(pattern)
-	pattern = strings.TrimSuffix(pattern, ".")
+	host = toLowerCaseASCII(strings.TrimSuffix(host, "."))
 
 	if len(pattern) == 0 || len(host) == 0 {
 		return false
@@ -820,6 +777,13 @@ func toLowerCaseASCII(in string) string {
 
 // VerifyHostname returns nil if c is a valid certificate for the named host.
 // Otherwise it returns an error describing the mismatch.
+//
+// IP addresses can be optionally enclosed in square brackets and are checked
+// against the IPAddresses field. Other names are checked case insensitively
+// against the DNSNames field. If the names are valid hostnames, the certificate
+// fields can have a wildcard as the left-most label.
+//
+// Note that the legacy Common Name field is ignored.
 func (c *Certificate) VerifyHostname(h string) error {
 	// IP addresses may be written in [ ].
 	candidateIP := h
@@ -837,20 +801,25 @@ func (c *Certificate) VerifyHostname(h string) error {
 		return x509.HostnameError{&c.Certificate, candidateIP}
 	}
 
-	lowered := toLowerCaseASCII(h)
+	candidateName := toLowerCaseASCII(h) // Save allocations inside the loop.
+	validCandidateName := validHostnameInput(candidateName)
 
-	if c.commonNameAsHostname() {
+	for _, match := range c.DNSNames {
-		if matchHostnames(toLowerCaseASCII(c.Subject.CommonName), lowered) {
+		// Ideally, we'd only match valid hostnames according to RFC 6125 like
-			return nil
+		// 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,
-	} else {
+		// always allow perfect matches, and only apply wildcard and trailing
-		for _, match := range c.DNSNames {
+		// dot processing to valid hostnames.
-			if matchHostnames(toLowerCaseASCII(match), lowered) {
+		if validCandidateName && validHostnamePattern(match) {
+			if matchHostnames(match, candidateName) {
+				return nil
+			}
+		} else {
+			if matchExactly(match, candidateName) {
 				return nil
 			}
 		}
 	}
-
 	return x509.HostnameError{&c.Certificate, h}
 }
 

smx509/x509.go

@@ -3,7 +3,6 @@ package smx509
 import (
 	"bytes"
 	"crypto"
-	"crypto/dsa"
 	"crypto/ecdsa"
 	"crypto/ed25519"
 	"crypto/elliptic"
@@ -19,8 +18,6 @@ import (
 	"math/big"
 	"net"
 	"net/url"
-	"strconv"
-	"strings"
 	"time"
 	"unicode/utf8"
 
@@ -680,647 +677,6 @@ type distributionPointName struct {
 	RelativeName pkix.RDNSequence `asn1:"optional,tag:1"`
 }
 
-func parsePublicKey(algo x509.PublicKeyAlgorithm, keyData *publicKeyInfo) (interface{}, error) {
-	asn1Data := keyData.PublicKey.RightAlign()
-	switch algo {
-	case x509.RSA:
-		// RSA public keys must have a NULL in the parameters.
-		// See RFC 3279, Section 2.3.1.
-		if !bytes.Equal(keyData.Algorithm.Parameters.FullBytes, asn1.NullBytes) {
-			return nil, errors.New("x509: RSA key missing NULL parameters")
-		}
-
-		p := new(pkcs1PublicKey)
-		rest, err := asn1.Unmarshal(asn1Data, p)
-		if err != nil {
-			return nil, err
-		}
-		if len(rest) != 0 {
-			return nil, errors.New("x509: trailing data after RSA public key")
-		}
-
-		if p.N.Sign() <= 0 {
-			return nil, errors.New("x509: RSA modulus is not a positive number")
-		}
-		if p.E <= 0 {
-			return nil, errors.New("x509: RSA public exponent is not a positive number")
-		}
-
-		pub := &rsa.PublicKey{
-			E: p.E,
-			N: p.N,
-		}
-		return pub, nil
-	case x509.DSA:
-		var p *big.Int
-		rest, err := asn1.Unmarshal(asn1Data, &p)
-		if err != nil {
-			return nil, err
-		}
-		if len(rest) != 0 {
-			return nil, errors.New("x509: trailing data after DSA public key")
-		}
-		paramsData := keyData.Algorithm.Parameters.FullBytes
-		params := new(dsaAlgorithmParameters)
-		rest, err = asn1.Unmarshal(paramsData, params)
-		if err != nil {
-			return nil, err
-		}
-		if len(rest) != 0 {
-			return nil, errors.New("x509: trailing data after DSA parameters")
-		}
-		if p.Sign() <= 0 || params.P.Sign() <= 0 || params.Q.Sign() <= 0 || params.G.Sign() <= 0 {
-			return nil, errors.New("x509: zero or negative DSA parameter")
-		}
-		pub := &dsa.PublicKey{
-			Parameters: dsa.Parameters{
-				P: params.P,
-				Q: params.Q,
-				G: params.G,
-			},
-			Y: p,
-		}
-		return pub, nil
-	case x509.ECDSA:
-		paramsData := keyData.Algorithm.Parameters.FullBytes
-		namedCurveOID := new(asn1.ObjectIdentifier)
-		rest, err := asn1.Unmarshal(paramsData, namedCurveOID)
-		if err != nil {
-			return nil, errors.New("x509: failed to parse ECDSA parameters as named curve")
-		}
-		if len(rest) != 0 {
-			return nil, errors.New("x509: trailing data after ECDSA parameters")
-		}
-		namedCurve := namedCurveFromOID(*namedCurveOID)
-		if namedCurve == nil {
-			return nil, errors.New("x509: unsupported elliptic curve")
-		}
-		x, y := elliptic.Unmarshal(namedCurve, asn1Data)
-		if x == nil {
-			return nil, errors.New("x509: failed to unmarshal elliptic curve point")
-		}
-		pub := &ecdsa.PublicKey{
-			Curve: namedCurve,
-			X:     x,
-			Y:     y,
-		}
-		return pub, nil
-	case x509.Ed25519:
-		// RFC 8410, Section 3
-		// > For all of the OIDs, the parameters MUST be absent.
-		if len(keyData.Algorithm.Parameters.FullBytes) != 0 {
-			return nil, errors.New("x509: Ed25519 key encoded with illegal parameters")
-		}
-		if len(asn1Data) != ed25519.PublicKeySize {
-			return nil, errors.New("x509: wrong Ed25519 public key size")
-		}
-		pub := make([]byte, ed25519.PublicKeySize)
-		copy(pub, asn1Data)
-		return ed25519.PublicKey(pub), nil
-	default:
-		return nil, nil
-	}
-}
-
-func forEachSAN(extension []byte, callback func(tag int, data []byte) error) error {
-	// RFC 5280, 4.2.1.6
-
-	// SubjectAltName ::= GeneralNames
-	//
-	// GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName
-	//
-	// GeneralName ::= CHOICE {
-	//      otherName                       [0]     OtherName,
-	//      rfc822Name                      [1]     IA5String,
-	//      dNSName                         [2]     IA5String,
-	//      x400Address                     [3]     ORAddress,
-	//      directoryName                   [4]     Name,
-	//      ediPartyName                    [5]     EDIPartyName,
-	//      uniformResourceIdentifier       [6]     IA5String,
-	//      iPAddress                       [7]     OCTET STRING,
-	//      registeredID                    [8]     OBJECT IDENTIFIER }
-	var seq asn1.RawValue
-	rest, err := asn1.Unmarshal(extension, &seq)
-	if err != nil {
-		return err
-	} else if len(rest) != 0 {
-		return errors.New("x509: trailing data after X.509 extension")
-	}
-	if !seq.IsCompound || seq.Tag != 16 || seq.Class != 0 {
-		return asn1.StructuralError{Msg: "bad SAN sequence"}
-	}
-
-	rest = seq.Bytes
-	for len(rest) > 0 {
-		var v asn1.RawValue
-		rest, err = asn1.Unmarshal(rest, &v)
-		if err != nil {
-			return err
-		}
-
-		if err := callback(v.Tag, v.Bytes); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-func parseSANExtension(value []byte) (dnsNames, emailAddresses []string, ipAddresses []net.IP, uris []*url.URL, err error) {
-	err = forEachSAN(value, func(tag int, data []byte) error {
-		switch tag {
-		case nameTypeEmail:
-			emailAddresses = append(emailAddresses, string(data))
-		case nameTypeDNS:
-			dnsNames = append(dnsNames, string(data))
-		case nameTypeURI:
-			uri, err := url.Parse(string(data))
-			if err != nil {
-				return fmt.Errorf("x509: cannot parse URI %q: %s", string(data), err)
-			}
-			if len(uri.Host) > 0 {
-				if _, ok := domainToReverseLabels(uri.Host); !ok {
-					return fmt.Errorf("x509: cannot parse URI %q: invalid domain", string(data))
-				}
-			}
-			uris = append(uris, uri)
-		case nameTypeIP:
-			switch len(data) {
-			case net.IPv4len, net.IPv6len:
-				ipAddresses = append(ipAddresses, data)
-			default:
-				return errors.New("x509: cannot parse IP address of length " + strconv.Itoa(len(data)))
-			}
-		}
-
-		return nil
-	})
-
-	return
-}
-
-// isValidIPMask reports whether mask consists of zero or more 1 bits, followed by zero bits.
-func isValidIPMask(mask []byte) bool {
-	seenZero := false
-
-	for _, b := range mask {
-		if seenZero {
-			if b != 0 {
-				return false
-			}
-
-			continue
-		}
-
-		switch b {
-		case 0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe:
-			seenZero = true
-		case 0xff:
-		default:
-			return false
-		}
-	}
-
-	return true
-}
-
-func parseNameConstraintsExtension(out *x509.Certificate, e pkix.Extension) (unhandled bool, err error) {
-	// RFC 5280, 4.2.1.10
-
-	// NameConstraints ::= SEQUENCE {
-	//      permittedSubtrees       [0]     GeneralSubtrees OPTIONAL,
-	//      excludedSubtrees        [1]     GeneralSubtrees OPTIONAL }
-	//
-	// GeneralSubtrees ::= SEQUENCE SIZE (1..MAX) OF GeneralSubtree
-	//
-	// GeneralSubtree ::= SEQUENCE {
-	//      base                    GeneralName,
-	//      minimum         [0]     BaseDistance DEFAULT 0,
-	//      maximum         [1]     BaseDistance OPTIONAL }
-	//
-	// BaseDistance ::= INTEGER (0..MAX)
-
-	outer := cryptobyte.String(e.Value)
-	var toplevel, permitted, excluded cryptobyte.String
-	var havePermitted, haveExcluded bool
-	if !outer.ReadASN1(&toplevel, cryptobyte_asn1.SEQUENCE) ||
-		!outer.Empty() ||
-		!toplevel.ReadOptionalASN1(&permitted, &havePermitted, cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) ||
-		!toplevel.ReadOptionalASN1(&excluded, &haveExcluded, cryptobyte_asn1.Tag(1).ContextSpecific().Constructed()) ||
-		!toplevel.Empty() {
-		return false, errors.New("x509: invalid NameConstraints extension")
-	}
-
-	if !havePermitted && !haveExcluded || len(permitted) == 0 && len(excluded) == 0 {
-		// From RFC 5280, Section 4.2.1.10:
-		//   “either the permittedSubtrees field
-		//   or the excludedSubtrees MUST be
-		//   present”
-		return false, errors.New("x509: empty name constraints extension")
-	}
-
-	getValues := func(subtrees cryptobyte.String) (dnsNames []string, ips []*net.IPNet, emails, uriDomains []string, err error) {
-		for !subtrees.Empty() {
-			var seq, value cryptobyte.String
-			var tag cryptobyte_asn1.Tag
-			if !subtrees.ReadASN1(&seq, cryptobyte_asn1.SEQUENCE) ||
-				!seq.ReadAnyASN1(&value, &tag) {
-				return nil, nil, nil, nil, fmt.Errorf("x509: invalid NameConstraints extension")
-			}
-
-			var (
-				dnsTag   = cryptobyte_asn1.Tag(2).ContextSpecific()
-				emailTag = cryptobyte_asn1.Tag(1).ContextSpecific()
-				ipTag    = cryptobyte_asn1.Tag(7).ContextSpecific()
-				uriTag   = cryptobyte_asn1.Tag(6).ContextSpecific()
-			)
-
-			switch tag {
-			case dnsTag:
-				domain := string(value)
-				if err := isIA5String(domain); err != nil {
-					return nil, nil, nil, nil, errors.New("x509: invalid constraint value: " + err.Error())
-				}
-
-				trimmedDomain := domain
-				if len(trimmedDomain) > 0 && trimmedDomain[0] == '.' {
-					// constraints can have a leading
-					// period to exclude the domain
-					// itself, but that's not valid in a
-					// normal domain name.
-					trimmedDomain = trimmedDomain[1:]
-				}
-				if _, ok := domainToReverseLabels(trimmedDomain); !ok {
-					return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse dnsName constraint %q", domain)
-				}
-				dnsNames = append(dnsNames, domain)
-
-			case ipTag:
-				l := len(value)
-				var ip, mask []byte
-
-				switch l {
-				case 8:
-					ip = value[:4]
-					mask = value[4:]
-
-				case 32:
-					ip = value[:16]
-					mask = value[16:]
-
-				default:
-					return nil, nil, nil, nil, fmt.Errorf("x509: IP constraint contained value of length %d", l)
-				}
-
-				if !isValidIPMask(mask) {
-					return nil, nil, nil, nil, fmt.Errorf("x509: IP constraint contained invalid mask %x", mask)
-				}
-
-				ips = append(ips, &net.IPNet{IP: net.IP(ip), Mask: net.IPMask(mask)})
-
-			case emailTag:
-				constraint := string(value)
-				if err := isIA5String(constraint); err != nil {
-					return nil, nil, nil, nil, errors.New("x509: invalid constraint value: " + err.Error())
-				}
-
-				// If the constraint contains an @ then
-				// it specifies an exact mailbox name.
-				if strings.Contains(constraint, "@") {
-					if _, ok := parseRFC2821Mailbox(constraint); !ok {
-						return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse rfc822Name constraint %q", constraint)
-					}
-				} else {
-					// Otherwise it's a domain name.
-					domain := constraint
-					if len(domain) > 0 && domain[0] == '.' {
-						domain = domain[1:]
-					}
-					if _, ok := domainToReverseLabels(domain); !ok {
-						return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse rfc822Name constraint %q", constraint)
-					}
-				}
-				emails = append(emails, constraint)
-
-			case uriTag:
-				domain := string(value)
-				if err := isIA5String(domain); err != nil {
-					return nil, nil, nil, nil, errors.New("x509: invalid constraint value: " + err.Error())
-				}
-
-				if net.ParseIP(domain) != nil {
-					return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse URI constraint %q: cannot be IP address", domain)
-				}
-
-				trimmedDomain := domain
-				if len(trimmedDomain) > 0 && trimmedDomain[0] == '.' {
-					// constraints can have a leading
-					// period to exclude the domain itself,
-					// but that's not valid in a normal
-					// domain name.
-					trimmedDomain = trimmedDomain[1:]
-				}
-				if _, ok := domainToReverseLabels(trimmedDomain); !ok {
-					return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse URI constraint %q", domain)
-				}
-				uriDomains = append(uriDomains, domain)
-
-			default:
-				unhandled = true
-			}
-		}
-
-		return dnsNames, ips, emails, uriDomains, nil
-	}
-
-	if out.PermittedDNSDomains, out.PermittedIPRanges, out.PermittedEmailAddresses, out.PermittedURIDomains, err = getValues(permitted); err != nil {
-		return false, err
-	}
-	if out.ExcludedDNSDomains, out.ExcludedIPRanges, out.ExcludedEmailAddresses, out.ExcludedURIDomains, err = getValues(excluded); err != nil {
-		return false, err
-	}
-	out.PermittedDNSDomainsCritical = e.Critical
-
-	return unhandled, nil
-}
-
-func parseCertificate(in *certificate) (*x509.Certificate, error) {
-	out := new(x509.Certificate)
-	out.Raw = in.Raw
-	out.RawTBSCertificate = in.TBSCertificate.Raw
-	out.RawSubjectPublicKeyInfo = in.TBSCertificate.PublicKey.Raw
-	out.RawSubject = in.TBSCertificate.Subject.FullBytes
-	out.RawIssuer = in.TBSCertificate.Issuer.FullBytes
-
-	out.Signature = in.SignatureValue.RightAlign()
-	out.SignatureAlgorithm = getSignatureAlgorithmFromAI(in.TBSCertificate.SignatureAlgorithm)
-
-	out.PublicKeyAlgorithm =
-		getPublicKeyAlgorithmFromOID(in.TBSCertificate.PublicKey.Algorithm.Algorithm)
-	var err error
-	out.PublicKey, err = parsePublicKey(out.PublicKeyAlgorithm, &in.TBSCertificate.PublicKey)
-	if err != nil {
-		return nil, err
-	}
-
-	out.Version = in.TBSCertificate.Version + 1
-	out.SerialNumber = in.TBSCertificate.SerialNumber
-
-	var issuer, subject pkix.RDNSequence
-	if rest, err := asn1.Unmarshal(in.TBSCertificate.Subject.FullBytes, &subject); err != nil {
-		return nil, err
-	} else if len(rest) != 0 {
-		return nil, errors.New("x509: trailing data after X.509 subject")
-	}
-	if rest, err := asn1.Unmarshal(in.TBSCertificate.Issuer.FullBytes, &issuer); err != nil {
-		return nil, err
-	} else if len(rest) != 0 {
-		return nil, errors.New("x509: trailing data after X.509 subject")
-	}
-
-	out.Issuer.FillFromRDNSequence(&issuer)
-	out.Subject.FillFromRDNSequence(&subject)
-
-	out.NotBefore = in.TBSCertificate.Validity.NotBefore
-	out.NotAfter = in.TBSCertificate.Validity.NotAfter
-
-	for _, e := range in.TBSCertificate.Extensions {
-		out.Extensions = append(out.Extensions, e)
-		unhandled := false
-
-		if len(e.Id) == 4 && e.Id[0] == 2 && e.Id[1] == 5 && e.Id[2] == 29 {
-			switch e.Id[3] {
-			case 15:
-				// RFC 5280, 4.2.1.3
-				var usageBits asn1.BitString
-				if rest, err := asn1.Unmarshal(e.Value, &usageBits); err != nil {
-					return nil, err
-				} else if len(rest) != 0 {
-					return nil, errors.New("x509: trailing data after X.509 KeyUsage")
-				}
-
-				var usage int
-				for i := 0; i < 9; i++ {
-					if usageBits.At(i) != 0 {
-						usage |= 1 << uint(i)
-					}
-				}
-				out.KeyUsage = x509.KeyUsage(usage)
-
-			case 19:
-				// RFC 5280, 4.2.1.9
-				var constraints basicConstraints
-				if rest, err := asn1.Unmarshal(e.Value, &constraints); err != nil {
-					return nil, err
-				} else if len(rest) != 0 {
-					return nil, errors.New("x509: trailing data after X.509 BasicConstraints")
-				}
-
-				out.BasicConstraintsValid = true
-				out.IsCA = constraints.IsCA
-				out.MaxPathLen = constraints.MaxPathLen
-				out.MaxPathLenZero = out.MaxPathLen == 0
-				// TODO: map out.MaxPathLen to 0 if it has the -1 default value? (Issue 19285)
-			case 17:
-				out.DNSNames, out.EmailAddresses, out.IPAddresses, out.URIs, err = parseSANExtension(e.Value)
-				if err != nil {
-					return nil, err
-				}
-
-				if len(out.DNSNames) == 0 && len(out.EmailAddresses) == 0 && len(out.IPAddresses) == 0 && len(out.URIs) == 0 {
-					// If we didn't parse anything then we do the critical check, below.
-					unhandled = true
-				}
-
-			case 30:
-				unhandled, err = parseNameConstraintsExtension(out, e)
-				if err != nil {
-					return nil, err
-				}
-
-			case 31:
-				// RFC 5280, 4.2.1.13
-
-				// CRLDistributionPoints ::= SEQUENCE SIZE (1..MAX) OF DistributionPoint
-				//
-				// DistributionPoint ::= SEQUENCE {
-				//     distributionPoint       [0]     DistributionPointName OPTIONAL,
-				//     reasons                 [1]     ReasonFlags OPTIONAL,
-				//     cRLIssuer               [2]     GeneralNames OPTIONAL }
-				//
-				// DistributionPointName ::= CHOICE {
-				//     fullName                [0]     GeneralNames,
-				//     nameRelativeToCRLIssuer [1]     RelativeDistinguishedName }
-
-				var cdp []distributionPoint
-				if rest, err := asn1.Unmarshal(e.Value, &cdp); err != nil {
-					return nil, err
-				} else if len(rest) != 0 {
-					return nil, errors.New("x509: trailing data after X.509 CRL distribution point")
-				}
-
-				for _, dp := range cdp {
-					// Per RFC 5280, 4.2.1.13, one of distributionPoint or cRLIssuer may be empty.
-					if len(dp.DistributionPoint.FullName) == 0 {
-						continue
-					}
-
-					for _, fullName := range dp.DistributionPoint.FullName {
-						if fullName.Tag == 6 {
-							out.CRLDistributionPoints = append(out.CRLDistributionPoints, string(fullName.Bytes))
-						}
-					}
-				}
-
-			case 35:
-				// RFC 5280, 4.2.1.1
-				var a authKeyId
-				if rest, err := asn1.Unmarshal(e.Value, &a); err != nil {
-					return nil, err
-				} else if len(rest) != 0 {
-					return nil, errors.New("x509: trailing data after X.509 authority key-id")
-				}
-				out.AuthorityKeyId = a.Id
-
-			case 37:
-				// RFC 5280, 4.2.1.12.  Extended Key Usage
-
-				// id-ce-extKeyUsage OBJECT IDENTIFIER ::= { id-ce 37 }
-				//
-				// ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId
-				//
-				// KeyPurposeId ::= OBJECT IDENTIFIER
-
-				var keyUsage []asn1.ObjectIdentifier
-				if rest, err := asn1.Unmarshal(e.Value, &keyUsage); err != nil {
-					return nil, err
-				} else if len(rest) != 0 {
-					return nil, errors.New("x509: trailing data after X.509 ExtendedKeyUsage")
-				}
-
-				for _, u := range keyUsage {
-					if extKeyUsage, ok := extKeyUsageFromOID(u); ok {
-						out.ExtKeyUsage = append(out.ExtKeyUsage, extKeyUsage)
-					} else {
-						out.UnknownExtKeyUsage = append(out.UnknownExtKeyUsage, u)
-					}
-				}
-
-			case 14:
-				// RFC 5280, 4.2.1.2
-				var keyid []byte
-				if rest, err := asn1.Unmarshal(e.Value, &keyid); err != nil {
-					return nil, err
-				} else if len(rest) != 0 {
-					return nil, errors.New("x509: trailing data after X.509 key-id")
-				}
-				out.SubjectKeyId = keyid
-
-			case 32:
-				// RFC 5280 4.2.1.4: Certificate Policies
-				var policies []policyInformation
-				if rest, err := asn1.Unmarshal(e.Value, &policies); err != nil {
-					return nil, err
-				} else if len(rest) != 0 {
-					return nil, errors.New("x509: trailing data after X.509 certificate policies")
-				}
-				out.PolicyIdentifiers = make([]asn1.ObjectIdentifier, len(policies))
-				for i, policy := range policies {
-					out.PolicyIdentifiers[i] = policy.Policy
-				}
-
-			default:
-				// Unknown extensions are recorded if critical.
-				unhandled = true
-			}
-		} else if e.Id.Equal(oidExtensionAuthorityInfoAccess) {
-			// RFC 5280 4.2.2.1: Authority Information Access
-			var aia []authorityInfoAccess
-			if rest, err := asn1.Unmarshal(e.Value, &aia); err != nil {
-				return nil, err
-			} else if len(rest) != 0 {
-				return nil, errors.New("x509: trailing data after X.509 authority information")
-			}
-
-			for _, v := range aia {
-				// GeneralName: uniformResourceIdentifier [6] IA5String
-				if v.Location.Tag != 6 {
-					continue
-				}
-				if v.Method.Equal(oidAuthorityInfoAccessOcsp) {
-					out.OCSPServer = append(out.OCSPServer, string(v.Location.Bytes))
-				} else if v.Method.Equal(oidAuthorityInfoAccessIssuers) {
-					out.IssuingCertificateURL = append(out.IssuingCertificateURL, string(v.Location.Bytes))
-				}
-			}
-		} else {
-			// Unknown extensions are recorded if critical.
-			unhandled = true
-		}
-
-		if e.Critical && unhandled {
-			out.UnhandledCriticalExtensions = append(out.UnhandledCriticalExtensions, e.Id)
-		}
-	}
-
-	return out, nil
-}
-
-// ParseCertificate parses a single certificate from the given ASN.1 DER data.
-func ParseCertificate(asn1Data []byte) (*Certificate, error) {
-	var cert certificate
-	rest, err := asn1.Unmarshal(asn1Data, &cert)
-	if err != nil {
-		return nil, err
-	}
-	if len(rest) > 0 {
-		return nil, asn1.SyntaxError{Msg: "trailing data"}
-	}
-
-	var result *x509.Certificate
-	result, err = parseCertificate(&cert)
-
-	if err != nil {
-		return nil, err
-	}
-	return &Certificate{*result}, nil
-}
-
-func ParseCertificatePEM(data []byte) (*Certificate, error) {
-	block, _ := pem.Decode(data)
-	if block == nil {
-		return nil, errors.New("failed to decode PEM block containing CSR")
-	}
-	return ParseCertificate(block.Bytes)
-}
-
-// ParseCertificates parses one or more certificates from the given ASN.1 DER
-// data. The certificates must be concatenated with no intermediate padding.
-func ParseCertificates(asn1Data []byte) ([]*Certificate, error) {
-	var v []*certificate
-
-	for len(asn1Data) > 0 {
-		cert := new(certificate)
-		var err error
-		asn1Data, err = asn1.Unmarshal(asn1Data, cert)
-		if err != nil {
-			return nil, err
-		}
-		v = append(v, cert)
-	}
-
-	ret := make([]*Certificate, len(v))
-	for i, ci := range v {
-		cert, err := parseCertificate(ci)
-		if err != nil {
-			return nil, err
-		}
-		ret[i] = &Certificate{*cert}
-	}
-
-	return ret, nil
-}
-
 func reverseBitsInAByte(in byte) byte {
 	b1 := in>>4 | in<<4
 	b2 := b1>>2&0x33 | b1<<2&0xcc
@@ -1422,7 +778,7 @@ func isIA5String(s string) error {
 	return nil
 }
 
-func buildCertExtensions(template *x509.Certificate, subjectIsEmpty bool, authorityKeyId []byte) (ret []pkix.Extension, err error) {
+func buildCertExtensions(template *x509.Certificate, subjectIsEmpty bool, authorityKeyId, subjectKeyId []byte) (ret []pkix.Extension, err error) {
 	ret = make([]pkix.Extension, 10 /* maximum number of elements. */)
 	n := 0
 
@@ -1452,9 +808,9 @@ func buildCertExtensions(template *x509.Certificate, subjectIsEmpty bool, author
 		n++
 	}
 
-	if len(template.SubjectKeyId) > 0 && !oidInExtensions(oidExtensionSubjectKeyId, template.ExtraExtensions) {
+	if len(subjectKeyId) > 0 && !oidInExtensions(oidExtensionSubjectKeyId, template.ExtraExtensions) {
 		ret[n].Id = oidExtensionSubjectKeyId
-		ret[n].Value, err = asn1.Marshal(template.SubjectKeyId)
+		ret[n].Value, err = asn1.Marshal(subjectKeyId)
 		if err != nil {
 			return
 		}
@@ -1878,14 +1234,20 @@ func CreateCertificate(rand io.Reader, template, parent *x509.Certificate, pub,
 	if !ok {
 		return nil, errors.New("x509: certificate private key does not implement crypto.Signer")
 	}
+
+	if template.SerialNumber == nil {
+		return nil, errors.New("x509: no SerialNumber given")
+	}
+
+	if template.BasicConstraintsValid && !template.IsCA && template.MaxPathLen != -1 && (template.MaxPathLen != 0 || template.MaxPathLenZero) {
+		return nil, errors.New("x509: only CAs are allowed to specify MaxPathLen")
+	}
+
 	hashFunc, signatureAlgorithm, err := signingParamsForPublicKey(key.Public(), template.SignatureAlgorithm)
 	if err != nil {
 		return nil, err
 	}
 
-	if template.SerialNumber == nil {
-		return nil, errors.New("x509: no SerialNumber given")
-	}
 	publicKeyBytes, publicKeyAlgorithm, err := marshalPublicKey(pub)
 	if err != nil {
 		return nil, err
@@ -1921,18 +1283,13 @@ func CreateCertificate(rand io.Reader, template, parent *x509.Certificate, pub,
 		Equal(crypto.PublicKey) bool
 	}
 
-	/*
+	if privPub, ok := key.Public().(privateKey); !ok {
-		if privPub, ok := key.Public().(privateKey); !ok {
+		return nil, errors.New("x509: internal error: supported public key does not implement Equal")
-			return nil, errors.New("x509: internal error: supported public key does not implement Equal")
+	} else if parent.PublicKey != nil && !privPub.Equal(parent.PublicKey) {
-		} else if parent.PublicKey != nil && !privPub.Equal(parent.PublicKey) {
-			return nil, errors.New("x509: provided PrivateKey doesn't match parent's PublicKey")
-		}
-	*/
-	if privPub, ok := key.Public().(privateKey); ok && parent.PublicKey != nil && !privPub.Equal(parent.PublicKey) {
 		return nil, errors.New("x509: provided PrivateKey doesn't match parent's PublicKey")
 	}
 
-	extensions, err := buildCertExtensions(template, bytes.Equal(asn1Subject, emptyASN1Subject), authorityKeyId)
+	extensions, err := buildCertExtensions(template, bytes.Equal(asn1Subject, emptyASN1Subject), authorityKeyId, subjectKeyId)
 	if err != nil {
 		return nil, err
 	}
@@ -1986,14 +1343,19 @@ func CreateCertificate(rand io.Reader, template, parent *x509.Certificate, pub,
 	if err != nil {
 		return nil, err
 	}
+
 	// Check the signature to ensure the crypto.Signer behaved correctly.
-	// We skip this check if the signature algorithm is MD5WithRSA as we
+	sigAlg := getSignatureAlgorithmFromAI(signatureAlgorithm)
-	// only support this algorithm for signing, and not verification.
+	switch sigAlg {
-	if sigAlg := getSignatureAlgorithmFromAI(signatureAlgorithm); sigAlg != x509.MD5WithRSA {
+	case x509.MD5WithRSA, x509.SHA1WithRSA, x509.ECDSAWithSHA1:
+		// We skip the check if the signature algorithm is only supported for
+		// signing, not verification.
+	default:
 		if err := checkSignature(sigAlg, c.Raw, signature, key.Public()); err != nil {
 			return nil, fmt.Errorf("x509: signature over certificate returned by signer is invalid: %w", err)
 		}
 	}
+
 	return signedCert, nil
 }