Merge pull request #22 from easyops-cn/polish

[X509] sync upstream

smx509/parser.go

@@ -49,9 +49,9 @@ func isPrintable(b byte) bool {
 }
 
 // parseASN1String parses the ASN.1 string types T61String, PrintableString,
-// UTF8String, BMPString, and IA5String. This is mostly copied from the
+// UTF8String, BMPString, IA5String, and NumericString. This is mostly copied
-// respective encoding/asn1.parse... methods, rather than just increasing
+// from the respective encoding/asn1.parse... methods, rather than just
-// the API surface of that package.
+// increasing the API surface of that package.
 func parseASN1String(tag cryptobyte_asn1.Tag, value []byte) (string, error) {
 	switch tag {
 	case cryptobyte_asn1.T61String:
@@ -91,6 +91,13 @@ func parseASN1String(tag cryptobyte_asn1.Tag, value []byte) (string, error) {
 			return "", errors.New("invalid IA5String")
 		}
 		return s, nil
+	case cryptobyte_asn1.Tag(asn1.TagNumericString):
+		for _, b := range value {
+			if !('0' <= b && b <= '9' || b == ' ') {
+				return "", errors.New("invalid NumericString")
+			}
+		}
+		return string(value), nil
 	}
 	return "", fmt.Errorf("unsupported string type: %v", tag)
 }

smx509/pkcs1.go

@@ -0,0 +1,33 @@
+package smx509
+
+import "math/big"
+
+// pkcs1PrivateKey is a structure which mirrors the PKCS #1 ASN.1 for an RSA private key.
+type pkcs1PrivateKey struct {
+	Version int
+	N       *big.Int
+	E       int
+	D       *big.Int
+	P       *big.Int
+	Q       *big.Int
+	// We ignore these values, if present, because rsa will calculate them.
+	Dp   *big.Int `asn1:"optional"`
+	Dq   *big.Int `asn1:"optional"`
+	Qinv *big.Int `asn1:"optional"`
+
+	AdditionalPrimes []pkcs1AdditionalRSAPrime `asn1:"optional,omitempty"`
+}
+
+type pkcs1AdditionalRSAPrime struct {
+	Prime *big.Int
+
+	// We ignore these values because rsa will calculate them.
+	Exp   *big.Int
+	Coeff *big.Int
+}
+
+// pkcs1PublicKey reflects the ASN.1 structure of a PKCS #1 public key.
+type pkcs1PublicKey struct {
+	N *big.Int
+	E int
+}

smx509/pkcs8.go

@@ -10,7 +10,7 @@ import (
 	"github.com/emmansun/gmsm/sm2"
 )
 
-// pkcs8 reflects an ASN.1, PKCS#8 PrivateKey. See
+// pkcs8 reflects an ASN.1, PKCS #8 PrivateKey. See
 // ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-8/pkcs-8v1_2.asn
 // and RFC 5208.
 type pkcs8 struct {
@@ -20,7 +20,7 @@ type pkcs8 struct {
 	// optional attributes omitted.
 }
 
-// ParsePKCS8PrivateKey parses an unencrypted private key in PKCS#8, ASN.1 DER form.
+// ParsePKCS8PrivateKey parses an unencrypted private key in PKCS #8, ASN.1 DER form.
 //
 // It returns a *rsa.PrivateKey, a *ecdsa.PrivateKey, or a ed25519.PrivateKey.
 // More types might be supported in the future.
@@ -57,7 +57,7 @@ func ParsePKCS8PrivateKey(der []byte) (key interface{}, err error) {
 	return key, err
 }
 
-// MarshalPKCS8PrivateKey converts a private key to PKCS#8, ASN.1 DER form.
+// MarshalPKCS8PrivateKey converts a private key to PKCS #8, ASN.1 DER form.
 //
 // The following key types are currently supported: *rsa.PrivateKey, *ecdsa.PrivateKey
 // and ed25519.PrivateKey. Unsupported key types result in an error.

smx509/sec1.go

@@ -11,30 +11,6 @@ import (
 	"github.com/emmansun/gmsm/sm2"
 )
 
-// pkcs1PrivateKey is a structure which mirrors the PKCS#1 ASN.1 for an RSA private key.
-type pkcs1PrivateKey struct {
-	Version int
-	N       *big.Int
-	E       int
-	D       *big.Int
-	P       *big.Int
-	Q       *big.Int
-	// We ignore these values, if present, because rsa will calculate them.
-	Dp   *big.Int `asn1:"optional"`
-	Dq   *big.Int `asn1:"optional"`
-	Qinv *big.Int `asn1:"optional"`
-
-	AdditionalPrimes []pkcs1AdditionalRSAPrime `asn1:"optional,omitempty"`
-}
-
-type pkcs1AdditionalRSAPrime struct {
-	Prime *big.Int
-
-	// We ignore these values because rsa will calculate them.
-	Exp   *big.Int
-	Coeff *big.Int
-}
-
 const ecPrivKeyVersion = 1
 
 // ecPrivateKey reflects an ASN.1 Elliptic Curve Private Key Structure.

smx509/verify.go

@@ -244,6 +244,143 @@ func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) {
 	return mailbox, true
 }
 
+// domainToReverseLabels converts a textual domain name like foo.example.com to
+// the list of labels in reverse order, e.g. ["com", "example", "foo"].
+func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) {
+	for len(domain) > 0 {
+		if i := strings.LastIndexByte(domain, '.'); i == -1 {
+			reverseLabels = append(reverseLabels, domain)
+			domain = ""
+		} else {
+			reverseLabels = append(reverseLabels, domain[i+1:])
+			domain = domain[:i]
+		}
+	}
+
+	if len(reverseLabels) > 0 && len(reverseLabels[0]) == 0 {
+		// An empty label at the end indicates an absolute value.
+		return nil, false
+	}
+
+	for _, label := range reverseLabels {
+		if len(label) == 0 {
+			// Empty labels are otherwise invalid.
+			return nil, false
+		}
+
+		for _, c := range label {
+			if c < 33 || c > 126 {
+				// Invalid character.
+				return nil, false
+			}
+		}
+	}
+
+	return reverseLabels, true
+}
+
+func matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, error) {
+	// If the constraint contains an @, then it specifies an exact mailbox
+	// name.
+	if strings.Contains(constraint, "@") {
+		constraintMailbox, ok := parseRFC2821Mailbox(constraint)
+		if !ok {
+			return false, fmt.Errorf("x509: internal error: cannot parse constraint %q", constraint)
+		}
+		return mailbox.local == constraintMailbox.local && strings.EqualFold(mailbox.domain, constraintMailbox.domain), nil
+	}
+
+	// Otherwise the constraint is like a DNS constraint of the domain part
+	// of the mailbox.
+	return matchDomainConstraint(mailbox.domain, constraint)
+}
+
+func matchURIConstraint(uri *url.URL, constraint string) (bool, error) {
+	// From RFC 5280, Section 4.2.1.10:
+	// “a uniformResourceIdentifier that does not include an authority
+	// component with a host name specified as a fully qualified domain
+	// name (e.g., if the URI either does not include an authority
+	// component or includes an authority component in which the host name
+	// is specified as an IP address), then the application MUST reject the
+	// certificate.”
+
+	host := uri.Host
+	if len(host) == 0 {
+		return false, fmt.Errorf("URI with empty host (%q) cannot be matched against constraints", uri.String())
+	}
+
+	if strings.Contains(host, ":") && !strings.HasSuffix(host, "]") {
+		var err error
+		host, _, err = net.SplitHostPort(uri.Host)
+		if err != nil {
+			return false, err
+		}
+	}
+
+	if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") ||
+		net.ParseIP(host) != nil {
+		return false, fmt.Errorf("URI with IP (%q) cannot be matched against constraints", uri.String())
+	}
+
+	return matchDomainConstraint(host, constraint)
+}
+
+func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) {
+	if len(ip) != len(constraint.IP) {
+		return false, nil
+	}
+
+	for i := range ip {
+		if mask := constraint.Mask[i]; ip[i]&mask != constraint.IP[i]&mask {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
+func matchDomainConstraint(domain, constraint string) (bool, error) {
+	// The meaning of zero length constraints is not specified, but this
+	// code follows NSS and accepts them as matching everything.
+	if len(constraint) == 0 {
+		return true, nil
+	}
+
+	domainLabels, ok := domainToReverseLabels(domain)
+	if !ok {
+		return false, fmt.Errorf("x509: internal error: cannot parse domain %q", domain)
+	}
+
+	// RFC 5280 says that a leading period in a domain name means that at
+	// least one label must be prepended, but only for URI and email
+	// constraints, not DNS constraints. The code also supports that
+	// behaviour for DNS constraints.
+
+	mustHaveSubdomains := false
+	if constraint[0] == '.' {
+		mustHaveSubdomains = true
+		constraint = constraint[1:]
+	}
+
+	constraintLabels, ok := domainToReverseLabels(constraint)
+	if !ok {
+		return false, fmt.Errorf("x509: internal error: cannot parse domain %q", constraint)
+	}
+
+	if len(domainLabels) < len(constraintLabels) ||
+		(mustHaveSubdomains && len(domainLabels) == len(constraintLabels)) {
+		return false, nil
+	}
+
+	for i, constraintLabel := range constraintLabels {
+		if !strings.EqualFold(constraintLabel, domainLabels[i]) {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
 // checkNameConstraints checks that c permits a child certificate to claim the
 // given name, of type nameType. The argument parsedName contains the parsed
 // form of name, suitable for passing to the match function. The total number
@@ -304,41 +441,6 @@ func (c *Certificate) checkNameConstraints(count *int,
 	return nil
 }
 
-// domainToReverseLabels converts a textual domain name like foo.example.com to
-// the list of labels in reverse order, e.g. ["com", "example", "foo"].
-func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) {
-	for len(domain) > 0 {
-		if i := strings.LastIndexByte(domain, '.'); i == -1 {
-			reverseLabels = append(reverseLabels, domain)
-			domain = ""
-		} else {
-			reverseLabels = append(reverseLabels, domain[i+1:])
-			domain = domain[:i]
-		}
-	}
-
-	if len(reverseLabels) > 0 && len(reverseLabels[0]) == 0 {
-		// An empty label at the end indicates an absolute value.
-		return nil, false
-	}
-
-	for _, label := range reverseLabels {
-		if len(label) == 0 {
-			// Empty labels are otherwise invalid.
-			return nil, false
-		}
-
-		for _, c := range label {
-			if c < 33 || c > 126 {
-				// Invalid character.
-				return nil, false
-			}
-		}
-	}
-
-	return reverseLabels, true
-}
-
 // isValid performs validity checks on c given that it is a candidate to append
 // to the chain in currentChain.
 func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *VerifyOptions) error {
@@ -501,6 +603,12 @@ func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *V
 // the name being validated. Note that DirectoryName constraints are not
 // supported.
 //
+// Name constraint validation follows the rules from RFC 5280, with the
+// addition that DNS name constraints may use the leading period format
+// defined for emails and URIs. When a constraint has a leading period
+// it indicates that at least one additional label must be prepended to
+// the constrained name to be considered valid.
+//
 // Extended Key Usage values are enforced nested down a chain, so an intermediate
 // or root that enumerates EKUs prevents a leaf from asserting an EKU not in that
 // list. (While this is not specified, it is common practice in order to limit
@@ -523,8 +631,8 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e
 		}
 	}
 
-	// Use Windows's own verification and chain building.
+	// Use platform verifiers, where available, if Roots is from SystemCertPool.
-	if opts.Roots == nil && runtime.GOOS == "windows" {
+	if runtime.GOOS == "windows" {
 		if opts.Roots == nil {
 			return c.systemVerify(&opts)
 		}
@@ -600,7 +708,7 @@ func appendToFreshChain(chain []*Certificate, cert *Certificate) []*Certificate
 }
 
 // maxChainSignatureChecks is the maximum number of CheckSignatureFrom calls
-// that an invocation of buildChains will (tranistively) make. Most chains are
+// that an invocation of buildChains will (transitively) make. Most chains are
 // less than 15 certificates long, so this leaves space for multiple chains and
 // for failed checks due to different intermediates having the same Subject.
 const maxChainSignatureChecks = 100
@@ -713,7 +821,7 @@ func validHostname(host string, isPattern bool) bool {
 				continue
 			}
 			if c == '_' {
-				// Not valid characters in hostnames, but commonly
+				// Not a valid character in hostnames, but commonly
 				// found in deployments outside the WebPKI.
 				continue
 			}
@@ -835,6 +943,7 @@ func (c *Certificate) VerifyHostname(h string) error {
 			}
 		}
 	}
+
 	return x509.HostnameError{Certificate: c.asX509(), Host: h}
 }
 
@@ -899,105 +1008,3 @@ NextCert:
 
 	return true
 }
-
-func matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, error) {
-	// If the constraint contains an @, then it specifies an exact mailbox
-	// name.
-	if strings.Contains(constraint, "@") {
-		constraintMailbox, ok := parseRFC2821Mailbox(constraint)
-		if !ok {
-			return false, fmt.Errorf("x509: internal error: cannot parse constraint %q", constraint)
-		}
-		return mailbox.local == constraintMailbox.local && strings.EqualFold(mailbox.domain, constraintMailbox.domain), nil
-	}
-
-	// Otherwise the constraint is like a DNS constraint of the domain part
-	// of the mailbox.
-	return matchDomainConstraint(mailbox.domain, constraint)
-}
-
-func matchURIConstraint(uri *url.URL, constraint string) (bool, error) {
-	// From RFC 5280, Section 4.2.1.10:
-	// “a uniformResourceIdentifier that does not include an authority
-	// component with a host name specified as a fully qualified domain
-	// name (e.g., if the URI either does not include an authority
-	// component or includes an authority component in which the host name
-	// is specified as an IP address), then the application MUST reject the
-	// certificate.”
-
-	host := uri.Host
-	if len(host) == 0 {
-		return false, fmt.Errorf("URI with empty host (%q) cannot be matched against constraints", uri.String())
-	}
-
-	if strings.Contains(host, ":") && !strings.HasSuffix(host, "]") {
-		var err error
-		host, _, err = net.SplitHostPort(uri.Host)
-		if err != nil {
-			return false, err
-		}
-	}
-
-	if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") ||
-		net.ParseIP(host) != nil {
-		return false, fmt.Errorf("URI with IP (%q) cannot be matched against constraints", uri.String())
-	}
-
-	return matchDomainConstraint(host, constraint)
-}
-
-func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) {
-	if len(ip) != len(constraint.IP) {
-		return false, nil
-	}
-
-	for i := range ip {
-		if mask := constraint.Mask[i]; ip[i]&mask != constraint.IP[i]&mask {
-			return false, nil
-		}
-	}
-
-	return true, nil
-}
-
-func matchDomainConstraint(domain, constraint string) (bool, error) {
-	// The meaning of zero length constraints is not specified, but this
-	// code follows NSS and accepts them as matching everything.
-	if len(constraint) == 0 {
-		return true, nil
-	}
-
-	domainLabels, ok := domainToReverseLabels(domain)
-	if !ok {
-		return false, fmt.Errorf("x509: internal error: cannot parse domain %q", domain)
-	}
-
-	// RFC 5280 says that a leading period in a domain name means that at
-	// least one label must be prepended, but only for URI and email
-	// constraints, not DNS constraints. The code also supports that
-	// behaviour for DNS constraints.
-
-	mustHaveSubdomains := false
-	if constraint[0] == '.' {
-		mustHaveSubdomains = true
-		constraint = constraint[1:]
-	}
-
-	constraintLabels, ok := domainToReverseLabels(constraint)
-	if !ok {
-		return false, fmt.Errorf("x509: internal error: cannot parse domain %q", constraint)
-	}
-
-	if len(domainLabels) < len(constraintLabels) ||
-		(mustHaveSubdomains && len(domainLabels) == len(constraintLabels)) {
-		return false, nil
-	}
-
-	for i, constraintLabel := range constraintLabels {
-		if !strings.EqualFold(constraintLabel, domainLabels[i]) {
-			return false, nil
-		}
-	}
-
-	return true, nil
-}

smx509/x509.go

@@ -35,6 +35,8 @@ type pkixPublicKey struct {
 }
 
 // ParsePKIXPublicKey parses a public key in PKIX, ASN.1 DER form.
+// The encoded public key is a SubjectPublicKeyInfo structure
+// (see RFC 5280, Section 4.1).
 //
 // It returns a *rsa.PublicKey, *dsa.PublicKey, *ecdsa.PublicKey, or
 // ed25519.PublicKey. More types might be supported in the future.
@@ -119,6 +121,8 @@ func marshalPublicKey(pub interface{}) (publicKeyBytes []byte, publicKeyAlgorith
 }
 
 // MarshalPKIXPublicKey converts a public key to PKIX, ASN.1 DER form.
+// The encoded public key is a SubjectPublicKeyInfo structure
+// (see RFC 5280, Section 4.1).
 //
 // The following key types are currently supported: *rsa.PublicKey, *ecdsa.PublicKey
 // and ed25519.PublicKey. Unsupported key types result in an error.
@@ -238,12 +242,6 @@ const (
 	Ed25519 = x509.Ed25519
 )
 
-// pkcs1PublicKey reflects the ASN.1 structure of a PKCS#1 public key.
-type pkcs1PublicKey struct {
-	N *big.Int
-	E int
-}
-
 var signatureAlgorithmDetails = []struct {
 	algo       SignatureAlgorithm
 	name       string
@@ -1256,7 +1254,7 @@ func signingParamsForPublicKey(pub interface{}, requestedSigAlgo SignatureAlgori
 var emptyASN1Subject = []byte{0x30, 0}
 
 // CreateCertificate creates a new X.509 v3 certificate based on a template.
-// The following members of template are used:
+// The following members of template are currently used:
 //
 //  - AuthorityKeyId
 //  - BasicConstraintsValid
@@ -1293,7 +1291,7 @@ var emptyASN1Subject = []byte{0x30, 0}
 //
 // The certificate is signed by parent. If parent is equal to template then the
 // certificate is self-signed. The parameter pub is the public key of the
-// signee and priv is the private key of the signer.
+// certificate to be generated and priv is the private key of the signer.
 //
 // The returned slice is the certificate in DER encoding.
 //
@@ -1304,6 +1302,9 @@ var emptyASN1Subject = []byte{0x30, 0}
 // The AuthorityKeyId will be taken from the SubjectKeyId of parent, if any,
 // unless the resulting certificate is self-signed. Otherwise the value from
 // template will be used.
+//
+// If SubjectKeyId from template is empty and the template is a CA, SubjectKeyId
+// will be generated from the hash of the public key.
 func CreateCertificate(rand io.Reader, template, parent *x509.Certificate, pub, priv interface{}) ([]byte, error) {
 	key, ok := priv.(crypto.Signer)
 	if !ok {
@@ -1395,6 +1396,7 @@ func CreateCertificate(rand io.Reader, template, parent *x509.Certificate, pub,
 		h.Write(signed)
 		signed = h.Sum(nil)
 	}
+
 	var signerOpts crypto.SignerOpts = hashFunc
 	if template.SignatureAlgorithm != 0 && isRSAPSS(template.SignatureAlgorithm) {
 		signerOpts = &rsa.PSSOptions{
@@ -1449,6 +1451,9 @@ func ParseDERCRL(derBytes []byte) (*pkix.CertificateList, error) {
 
 // CreateCRL returns a DER encoded CRL, signed by this Certificate, that
 // contains the given list of revoked certificates.
+//
+// Note: this method does not generate an RFC 5280 conformant X.509 v2 CRL.
+// To generate a standards compliant CRL, use CreateRevocationList instead.
 func (c *Certificate) CreateCRL(rand io.Reader, priv interface{}, revokedCerts []pkix.RevokedCertificate, now, expiry time.Time) (crlBytes []byte, err error) {
 	key, ok := priv.(crypto.Signer)
 	if !ok {
@@ -1532,7 +1537,7 @@ type certificateRequest struct {
 	SignatureValue     asn1.BitString
 }
 
-// oidExtensionRequest is a PKCS#9 OBJECT IDENTIFIER that indicates requested
+// oidExtensionRequest is a PKCS #9 OBJECT IDENTIFIER that indicates requested
 // extensions in a CSR.
 var oidExtensionRequest = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 14}
 
@@ -1624,6 +1629,7 @@ func CreateCertificateRequest(rand io.Reader, template *x509.CertificateRequest,
 	if !ok {
 		return nil, errors.New("x509: certificate private key does not implement crypto.Signer")
 	}
+
 	var hashFunc crypto.Hash
 	var sigAlgo pkix.AlgorithmIdentifier
 	hashFunc, sigAlgo, err = signingParamsForPublicKey(key.Public(), template.SignatureAlgorithm)