Implement legacy name constraints verification

These changes reproduce the Go 1.14 name constraint verification in the MSP. Without these changes, certificate chains that would fail verification in Go 1.14 would successfully validate in Go 1.15. Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>
hyperledger · Feb 15, 2021 · a1b4d2d · a1b4d2d
1 parent 4321503
commit a1b4d2d
Show file tree

Hide file tree

Showing 2 changed files with 200 additions and 28 deletions.
diff --git a/msp/msp_test.go b/msp/msp_test.go
@@ -294,6 +294,33 @@ func computeSKI(key *ecdsa.PublicKey) []byte {
 	return hash[:]
 }
 
+func TestValidHostname(t *testing.T) {
+	tests := []struct {
+		name  string
+		valid bool
+	}{
+		{"", false},
+		{".", false},
+		{"example.com", true},
+		{"example.com.", true},
+		{"*.example.com", true},
+		{".example.com", false},
+		{"host.*.example.com", false},
+		{"localhost", true},
+		{"-localhost", false},
+		{"Not_Quite.example.com", true},
+		{"weird:colon.example.com", true},
+		{"1-2-3.example.com", true},
+	}
+	for _, tt := range tests {
+		if tt.valid {
+			require.True(t, validHostname(tt.name), "expected %s to be a valid hostname", tt.name)
+		} else {
+			require.False(t, validHostname(tt.name), "expected %s to be an invalid hostname", tt.name)
+		}
+	}
+}
+
 func TestValidateCANameConstraintsMitigation(t *testing.T) {
 	// Prior to Go 1.15, if a signing certificate contains a name constraint, the
 	// leaf certificate does not include a SAN, and the leaf common name looks
@@ -320,9 +347,9 @@ func TestValidateCANameConstraintsMitigation(t *testing.T) {
 		KeyUsage:                    caKeyUsage,
 		SubjectKeyId:                computeSKI(caKey.Public().(*ecdsa.PublicKey)),
 	}
-	certBytes, err := x509.CreateCertificate(rand.Reader, &caTemplate, &caTemplate, caKey.Public(), caKey)
+	caCertBytes, err := x509.CreateCertificate(rand.Reader, &caTemplate, &caTemplate, caKey.Public(), caKey)
 	require.NoError(t, err)
-	ca, err := x509.ParseCertificate(certBytes)
+	ca, err := x509.ParseCertificate(caCertBytes)
 	require.NoError(t, err)
 
 	leafTemplate := x509.Certificate{
@@ -339,37 +366,82 @@ func TestValidateCANameConstraintsMitigation(t *testing.T) {
 	keyBytes, err := x509.MarshalPKCS8PrivateKey(leafKey)
 	require.NoError(t, err)
 
-	caCertPem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certBytes})
+	caCertPem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caCertBytes})
 	leafCertPem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: leafCertBytes})
 	leafKeyPem := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyBytes})
-	fabricMSPConfig := &msp.FabricMSPConfig{
-		Name:      "ConstraintsMSP",
-		RootCerts: [][]byte{caCertPem},
-		SigningIdentity: &msp.SigningIdentityInfo{
-			PublicSigner: leafCertPem,
-			PrivateSigner: &msp.KeyInfo{
-				KeyIdentifier: "Certificate Without SAN",
-				KeyMaterial:   leafKeyPem,
-			},
-		},
-	}
-	mspConfig := &msp.MSPConfig{
-		Config: protoutil.MarshalOrPanic(fabricMSPConfig),
-	}
 
-	ks, err := sw.NewFileBasedKeyStore(nil, filepath.Join(configtest.GetDevMspDir(), "keystore"), true)
-	require.NoError(t, err)
-	cryptoProvider, err := sw.NewDefaultSecurityLevelWithKeystore(ks)
-	require.NoError(t, err)
+	t.Run("VerifyNameConstraintsSingleCert", func(t *testing.T) {
+		for _, der := range [][]byte{caCertBytes, leafCertBytes} {
+			cert, err := x509.ParseCertificate(der)
+			require.NoError(t, err, "failed to parse certificate")
 
-	testMSP, err := NewBccspMspWithKeyStore(MSPv1_0, ks, cryptoProvider)
-	require.NoError(t, err)
+			err = verifyLegacyNameConstraints([]*x509.Certificate{cert})
+			require.NoError(t, err, "single certificate should not trigger legacy constraints")
+		}
+	})
 
-	err = testMSP.Setup(mspConfig)
-	require.Error(t, err)
-	var cie x509.CertificateInvalidError
-	require.True(t, errors.As(err, &cie))
-	require.Equal(t, x509.NameConstraintsWithoutSANs, cie.Reason)
+	t.Run("VerifyNameConstraints", func(t *testing.T) {
+		var certs []*x509.Certificate
+		for _, der := range [][]byte{leafCertBytes, caCertBytes} {
+			cert, err := x509.ParseCertificate(der)
+			require.NoError(t, err, "failed to parse certificate")
+			certs = append(certs, cert)
+		}
+
+		err = verifyLegacyNameConstraints(certs)
+		require.Error(t, err, "certificate chain should trigger legacy constraints")
+		var cie x509.CertificateInvalidError
+		require.True(t, errors.As(err, &cie))
+		require.Equal(t, x509.NameConstraintsWithoutSANs, cie.Reason)
+	})
+
+	t.Run("VerifyNameConstraintsWithSAN", func(t *testing.T) {
+		caCert, err := x509.ParseCertificate(caCertBytes)
+		require.NoError(t, err)
+
+		leafTemplate := leafTemplate
+		leafTemplate.DNSNames = []string{"localhost"}
+
+		leafCertBytes, err := x509.CreateCertificate(rand.Reader, &leafTemplate, caCert, leafKey.Public(), caKey)
+		require.NoError(t, err)
+
+		leafCert, err := x509.ParseCertificate(leafCertBytes)
+		require.NoError(t, err)
+
+		err = verifyLegacyNameConstraints([]*x509.Certificate{leafCert, caCert})
+		require.NoError(t, err, "signer with name constraints and leaf with SANs should be valid")
+	})
+
+	t.Run("ValidationAtSetup", func(t *testing.T) {
+		fabricMSPConfig := &msp.FabricMSPConfig{
+			Name:      "ConstraintsMSP",
+			RootCerts: [][]byte{caCertPem},
+			SigningIdentity: &msp.SigningIdentityInfo{
+				PublicSigner: leafCertPem,
+				PrivateSigner: &msp.KeyInfo{
+					KeyIdentifier: "Certificate Without SAN",
+					KeyMaterial:   leafKeyPem,
+				},
+			},
+		}
+		mspConfig := &msp.MSPConfig{
+			Config: protoutil.MarshalOrPanic(fabricMSPConfig),
+		}
+
+		ks, err := sw.NewFileBasedKeyStore(nil, filepath.Join(configtest.GetDevMspDir(), "keystore"), true)
+		require.NoError(t, err)
+		cryptoProvider, err := sw.NewDefaultSecurityLevelWithKeystore(ks)
+		require.NoError(t, err)
+
+		testMSP, err := NewBccspMspWithKeyStore(MSPv1_0, ks, cryptoProvider)
+		require.NoError(t, err)
+
+		err = testMSP.Setup(mspConfig)
+		require.Error(t, err)
+		var cie x509.CertificateInvalidError
+		require.True(t, errors.As(err, &cie))
+		require.Equal(t, x509.NameConstraintsWithoutSANs, cie.Reason)
+	})
 }
 
 func TestIsWellFormed(t *testing.T) {

diff --git a/msp/mspimpl.go b/msp/mspimpl.go
@@ -10,8 +10,10 @@ import (
 	"bytes"
 	"crypto/x509"
 	"crypto/x509/pkix"
+	"encoding/asn1"
 	"encoding/hex"
 	"encoding/pem"
+	"strings"
 
 	"github.com/golang/protobuf/proto"
 	m "github.com/hyperledger/fabric-protos-go/msp"
@@ -735,9 +737,107 @@ func (msp *bccspmsp) getUniqueValidationChain(cert *x509.Certificate, opts x509.
 		return nil, errors.Errorf("this MSP only supports a single validation chain, got %d", len(validationChains))
 	}
 
+	// Make the additional verification checks that were done in Go 1.14.
+	err = verifyLegacyNameConstraints(validationChains[0])
+	if err != nil {
+		return nil, errors.WithMessage(err, "the supplied identity is not valid")
+	}
+
 	return validationChains[0], nil
 }
 
+var (
+	oidExtensionSubjectAltName  = asn1.ObjectIdentifier{2, 5, 29, 17}
+	oidExtensionNameConstraints = asn1.ObjectIdentifier{2, 5, 29, 30}
+)
+
+// verifyLegacyNameConstraints exercises the name constraint validation rules
+// that were part of the certificate verification process in Go 1.14.
+//
+// If a signing certificate contains a name constratint, the leaf certificate
+// does not include SAN extensions, and the leaf's common name looks like a
+// host name, the validation would fail with an x509.CertificateInvalidError
+// and a rason of x509.NameConstraintsWithoutSANs.
+func verifyLegacyNameConstraints(chain []*x509.Certificate) error {
+	if len(chain) < 2 {
+		return nil
+	}
+
+	// Leaf certificates with SANs are fine.
+	if oidInExtensions(oidExtensionSubjectAltName, chain[0].Extensions) {
+		return nil
+	}
+	// Leaf certificates without a hostname in CN are fine.
+	if !validHostname(chain[0].Subject.CommonName) {
+		return nil
+	}
+	// If an intermediate or root have a name constraint, validation
+	// would fail in Go 1.14.
+	for _, c := range chain[1:] {
+		if oidInExtensions(oidExtensionNameConstraints, c.Extensions) {
+			return x509.CertificateInvalidError{Cert: chain[0], Reason: x509.NameConstraintsWithoutSANs}
+		}
+	}
+	return nil
+}
+
+func oidInExtensions(oid asn1.ObjectIdentifier, exts []pkix.Extension) bool {
+	for _, ext := range exts {
+		if ext.Id.Equal(oid) {
+			return true
+		}
+	}
+	return 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.
+//
+// This implementation is sourced from the standaard library.
+func validHostname(host string) bool {
+	host = strings.TrimSuffix(host, ".")
+
+	if len(host) == 0 {
+		return false
+	}
+
+	for i, part := range strings.Split(host, ".") {
+		if part == "" {
+			// Empty label.
+			return false
+		}
+		if 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.
+			continue
+		}
+		for j, c := range part {
+			if 'a' <= c && c <= 'z' {
+				continue
+			}
+			if '0' <= c && c <= '9' {
+				continue
+			}
+			if 'A' <= c && c <= 'Z' {
+				continue
+			}
+			if c == '-' && j != 0 {
+				continue
+			}
+			if c == '_' || c == ':' {
+				// Not valid characters in hostnames, but commonly
+				// found in deployments outside the WebPKI.
+				continue
+			}
+			return false
+		}
+	}
+
+	return true
+}
+
 func (msp *bccspmsp) getValidationChain(cert *x509.Certificate, isIntermediateChain bool) ([]*x509.Certificate, error) {
 	validationChain, err := msp.getUniqueValidationChain(cert, msp.getValidityOptsForCert(cert))
 	if err != nil {