Fixed verification to use exact match and fallback to CA + tests

Signed-off-by: Diogo Monica <diogo@docker.com>

cmd/notary/cli_crypto_service.go

@@ -36,9 +36,9 @@ func (ccs *cliCryptoService) Create(role string) (*data.PublicKey, error) {
 	block := pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}
 	pemdata := pem.EncodeToMemory(&block)
 
-	// If this key has the role root, save it as a trusted certificate on our caStore
+	// If this key has the role root, save it as a trusted certificate on our certificateStore
 	if role == "root" {
-		caStore.AddCertFromPEM(pemdata)
+		certificateStore.AddCertFromPEM(pemdata)
 	}
 
 	return data.NewPublicKey("RSA", string(pemdata)), nil
@@ -85,7 +85,6 @@ func (ccs *cliCryptoService) Sign(keyIDs []string, payload []byte) ([]data.Signa
 
 //TODO (diogo): Add support for EC P384
 func generateKeyAndCert(gun string) (crypto.PrivateKey, *x509.Certificate, error) {
-
 	// Generates a new RSA key
 	key, err := rsa.GenerateKey(rand.Reader, 2048)
 	if err != nil {

cmd/notary/keys.go

@@ -66,7 +66,7 @@ func keysRemove(cmd *cobra.Command, args []string) {
 	//TODO (diogo): Validate Global Unique Name. We probably want to reject 1 char GUNs.
 	gunOrID := args[0]
 
-	// Try to retreive the ID from the CA store.
+	// Try to retrieve the ID from the CA store.
 	cert, err := caStore.GetCertificateBykID(gunOrID)
 	if err == nil {
 		fmt.Printf("Removing: ")
@@ -80,6 +80,20 @@ func keysRemove(cmd *cobra.Command, args []string) {
 		return
 	}
 
+	// Try to retrieve the ID from the Certificate store.
+	cert, err = certificateStore.GetCertificateBykID(gunOrID)
+	if err == nil {
+		fmt.Printf("Removing: ")
+		printCert(cert)
+
+		// If the ID is found, remove it.
+		err = certificateStore.RemoveCert(cert)
+		if err != nil {
+			fatalf("failed to remove certificate from KeyStore")
+		}
+		return
+	}
+
 	// We didn't find a certificate with this ID, let's try to see if we can find keys.
 	keyList := privKeyStore.ListGUN(gunOrID)
 	if len(keyList) < 1 {
@@ -140,10 +154,16 @@ func keysTrust(cmd *cobra.Command, args []string) {
 		fatalf("aborting action.")
 	}
 
+	err = nil
+	if cert.IsCA {
 		err = caStore.AddCert(cert)
+	} else {
+		err = certificateStore.AddCert(cert)
+	}
 	if err != nil {
 		fatalf("error adding certificate from file: %v", err)
 	}
+
 	fmt.Printf("Adding: ")
 	printCert(cert)
 
@@ -155,12 +175,19 @@ func keysList(cmd *cobra.Command, args []string) {
 		os.Exit(1)
 	}
 
-	fmt.Println("# Trusted Certificates:")
+	fmt.Println("# Trusted CAs:")
 	trustedCAs := caStore.GetCertificates()
 	for _, c := range trustedCAs {
 		printCert(c)
 	}
 
+	fmt.Println("")
+	fmt.Println("# Trusted Certificates:")
+	trustedCerts := certificateStore.GetCertificates()
+	for _, c := range trustedCerts {
+		printCert(c)
+	}
+
 	fmt.Println("")
 	fmt.Println("# Signing keys: ")
 	for _, k := range privKeyStore.List() {
@@ -185,7 +212,7 @@ func keysGenerate(cmd *cobra.Command, args []string) {
 		fatalf("could not generate key: %v", err)
 	}
 
-	caStore.AddCert(cert)
+	certificateStore.AddCert(cert)
 	fingerprint := trustmanager.FingerprintCert(cert)
 	fmt.Println("Generated new keypair with ID: ", string(fingerprint))
 }

cmd/notary/main.go

@@ -19,11 +19,12 @@ const configFileName string = "config"
 
 // Default paths should end with a '/' so directory creation works correctly
 const configPath string = ".docker/trust/"
-const trustDir string = configPath + "repository_certificates/"
+const trustDir string = configPath + "trusted_certificates/"
 const privDir string = configPath + "private/"
 const tufDir string = configPath + "tuf/"
 
 var caStore trustmanager.X509Store
+var certificateStore trustmanager.X509Store
 var privKeyStore trustmanager.FileStore
 
 var rawOutput bool
@@ -67,10 +68,21 @@ func init() {
 	finalPrivDir := viper.GetString("privDir")
 
 	// Load all CAs that aren't expired and don't use SHA1
-	// We could easily add "return cert.IsCA && cert.BasicConstraintsValid" in order
-	// to have only valid CA certificates being loaded
 	caStore, err = trustmanager.NewX509FilteredFileStore(finalTrustDir, func(cert *x509.Certificate) bool {
-		return time.Now().Before(cert.NotAfter) &&
+		return cert.IsCA && cert.BasicConstraintsValid && cert.SubjectKeyId != nil &&
+			time.Now().Before(cert.NotAfter) &&
+			cert.SignatureAlgorithm != x509.SHA1WithRSA &&
+			cert.SignatureAlgorithm != x509.DSAWithSHA1 &&
+			cert.SignatureAlgorithm != x509.ECDSAWithSHA1
+	})
+	if err != nil {
+		fatalf("could not create X509FileStore: %v", err)
+	}
+
+	// Load all individual (non-CA) certificates that aren't expired and don't use SHA1
+	certificateStore, err = trustmanager.NewX509FilteredFileStore(finalTrustDir, func(cert *x509.Certificate) bool {
+		return !cert.IsCA &&
+			time.Now().Before(cert.NotAfter) &&
 			cert.SignatureAlgorithm != x509.SHA1WithRSA &&
 			cert.SignatureAlgorithm != x509.DSAWithSHA1 &&
 			cert.SignatureAlgorithm != x509.ECDSAWithSHA1

cmd/notary/tuf.go

@@ -13,6 +13,7 @@ import (
 	"path/filepath"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/docker/notary/trustmanager"
 	"github.com/endophage/gotuf"
 	"github.com/endophage/gotuf/client"
 	"github.com/endophage/gotuf/data"
@@ -443,9 +444,6 @@ func saveRepo(repo *tuf.TufRepo, filestore store.MetadataStore) error {
 
 func bootstrapClient(gun string, remote store.RemoteStore, repo *tuf.TufRepo, kdb *keys.KeyDB) (*client.Client, error) {
 	rootJSON, err := remote.GetMeta("root", 5<<20)
-	if err != nil {
-		return nil, err
-	}
 	root := &data.Signed{}
 	err = json.Unmarshal(rootJSON, root)
 	if err != nil {
@@ -466,6 +464,31 @@ func bootstrapClient(gun string, remote store.RemoteStore, repo *tuf.TufRepo, kd
 	), nil
 }
 
+/*
+validateRoot iterates over every root key included in the TUF data and attempts
+to validate the certificate by first checking for an exact match on the certificate
+store, and subsequently trying to find a valid chain on the caStore.
+
+Example TUF Content for root role:
+"roles" : {
+  "root" : {
+    "threshold" : 1,
+      "keyids" : [
+        "e6da5c303d572712a086e669ecd4df7b785adfc844e0c9a7b1f21a7dfc477a38"
+      ]
+  },
+ ...
+}
+
+Example TUF Content for root key:
+"e6da5c303d572712a086e669ecd4df7b785adfc844e0c9a7b1f21a7dfc477a38" : {
+	"keytype" : "RSA",
+	"keyval" : {
+	  "private" : "",
+	  "public" : "Base64-encoded, PEM encoded x509 Certificate"
+	}
+}
+*/
 func validateRoot(gun string, root *data.Signed) error {
 	rootSigned := &data.Root{}
 	err := json.Unmarshal(root.Signed, rootSigned)
@@ -474,19 +497,36 @@ func validateRoot(gun string, root *data.Signed) error {
 	}
 	certs := make(map[string]*data.PublicKey)
 	for _, kID := range rootSigned.Roles["root"].KeyIDs {
-		// TODO: currently assuming only one cert contained in
+		// TODO(dlaw): currently assuming only one cert contained in
-		// public key entry
+		// public key entry. Need to fix when we want to pass in chains.
 		k, _ := pem.Decode([]byte(rootSigned.Keys["kid"].Public()))
-		rootCert, err := x509.ParseCertificates(k.Bytes)
+		decodedCerts, err := x509.ParseCertificates(k.Bytes)
-		if err != nil {
-			continue
-		}
-		err = caStore.Verify(gun, rootCert[0])
 		if err != nil {
 			continue
 		}
+
+		// TODO(diogo): Assuming that first certificate is the leaf-cert. Need to
+		// iterate over all decodedCerts and find a non-CA one (should be the last).
+		leafCert := decodedCerts[0]
+		leafID := string(trustmanager.FingerprintCert(leafCert))
+
+		// Check to see if there is an exact match of this certificate.
+		// Checking the CommonName is not required since ID is calculated over
+		// Cert.Raw. It's included to prevent breaking logic with changes of how the
+		// ID gets computed.
+		_, err = certificateStore.GetCertificateBykID(leafID)
+		if err == nil && leafCert.Subject.CommonName == gun {
 			certs[kID] = rootSigned.Keys[kID]
 		}
+
+		// Check to see if this leafCertificate has a chain to one of the Root CAs
+		// of our CA Store.
+		certList := []*x509.Certificate{leafCert}
+		err = trustmanager.Verify(caStore, gun, certList)
+		if err == nil {
+			certs[kID] = rootSigned.Keys[kID]
+		}
+	}
 	_, err = signed.VerifyRoot(root, 0, certs, 1)
 	if err != nil {
 		// failed to validate the signatures against the certificates

trustmanager/filestore_test.go

@@ -182,7 +182,6 @@ func TestListGUN(t *testing.T) {
 		// Since we're generating this manually we need to add the extension '.'
 		fileName := fmt.Sprintf("%s-%s.%s", testName, strconv.Itoa(i), testExt)
 		expectedFilePath = filepath.Join(tempBaseDir, fileName)
-		fmt.Println(expectedFilePath)
 		_, err = generateRandomFile(expectedFilePath, perms)
 		if err != nil {
 			t.Fatalf("failed to generate random file: %v", err)

trustmanager/x509filestore.go

@@ -203,28 +203,6 @@ func (s X509FileStore) GetVerifyOptions(dnsName string) (x509.VerifyOptions, err
 	return opts, nil
 }
 
-func (s X509FileStore) Verify(dnsName string, certs ...*x509.Certificate) error {
-	// If we have no Certificates loaded return error (we don't want to rever to using
-	// system CAs).
-	if len(s.fingerprintMap) == 0 {
-		return errors.New("no root CAs available")
-	}
-
-	// TODO: determine which cert in rootCerts is the leaf and add
-	// the intermediates to verifyOpts.Intermediates
-	opts := x509.VerifyOptions{
-		DNSName: dnsName,
-		Roots:   s.GetCertificatePool(),
-	}
-
-	// TODO: assuming only one cert ever passed and that it's the leaf
-	chains, err := certs[0].Verify(opts)
-	if len(chains) == 0 || err != nil {
-		return errors.New("Certificate did not verify")
-	}
-	return nil
-}
-
 func fileName(cert *x509.Certificate) string {
 	return path.Join(cert.Subject.CommonName, string(FingerprintCert(cert)))
 }

trustmanager/x509memstore.go

@@ -171,27 +171,3 @@ func (s X509MemStore) GetVerifyOptions(dnsName string) (x509.VerifyOptions, erro
 
 	return opts, nil
 }
-
-// TODO: Create a parent Store object that implements the shared methods
-// and gets embedded into this and the X509MemoryStore
-func (s X509MemStore) Verify(dnsName string, certs ...*x509.Certificate) error {
-	// If we have no Certificates loaded return error (we don't want to rever to using
-	// system CAs).
-	if len(s.fingerprintMap) == 0 {
-		return errors.New("no root CAs available")
-	}
-
-	// TODO: determine which cert in rootCerts is the leaf and add
-	// the intermediates to verifyOpts.Intermediates
-	opts := x509.VerifyOptions{
-		DNSName: dnsName,
-		Roots:   s.GetCertificatePool(),
-	}
-
-	// TODO: assuming only one cert ever passed and that it's the leaf
-	chains, err := certs[0].Verify(opts)
-	if len(chains) == 0 || err != nil {
-		return errors.New("Certificate did not verify")
-	}
-	return nil
-}

trustmanager/x509memstore_test.go

@@ -140,7 +140,7 @@ func TestGetCertificateBykID(t *testing.T) {
 
 	certFingerprint := FingerprintCert(cert)
 
-	// Tries to retreive cert by Subject Key IDs
+	// Tries to retrieve cert by Subject Key IDs
 	_, err = store.GetCertificateBykID(string(certFingerprint))
 	if err != nil {
 		t.Fatalf("expected certificate in store: %s", certFingerprint)

trustmanager/x509store.go

@@ -1,6 +1,10 @@
 package trustmanager
 
-import "crypto/x509"
+import (
+	"crypto/x509"
+	"errors"
+	"fmt"
+)
 
 const certExtension string = "crt"
 
@@ -14,7 +18,6 @@ type X509Store interface {
 	GetCertificates() []*x509.Certificate
 	GetCertificatePool() *x509.CertPool
 	GetVerifyOptions(dnsName string) (x509.VerifyOptions, error)
-	Verify(dnsName string, certs ...*x509.Certificate) error
 }
 
 type CertID string
@@ -34,3 +37,62 @@ type ValidatorFunc func(cert *x509.Certificate) bool
 func (vf ValidatorFunc) Validate(cert *x509.Certificate) bool {
 	return vf(cert)
 }
+
+// Verify operates on an X509Store and validates the existence of a chain of trust
+// between a leafCertificate and a CA present inside of the X509 Store.
+// It requires at least two certificates in certList, a leaf Certificate and an
+// intermediate CA certificate.
+func Verify(s X509Store, dnsName string, certList []*x509.Certificate) error {
+	// If we have no Certificates loaded return error (we don't want to revert to using
+	// system CAs).
+	if len(s.GetCertificates()) == 0 {
+		return errors.New("no root CAs available")
+	}
+
+	// At a minimum we should be provided a leaf cert and an intermediate.
+	if len(certList) < 2 {
+		return errors.New("certificate and at least one intermediate needed")
+	}
+
+	// Get the VerifyOptions from the keystore for a base dnsName
+	opts, err := s.GetVerifyOptions(dnsName)
+	if err != nil {
+		return err
+	}
+
+	// Create a Certificate Pool for our intermediate certificates
+	intPool := x509.NewCertPool()
+	var leafCert *x509.Certificate
+
+	// Iterate through all the certificates
+	for _, c := range certList {
+		// If the cert is a CA, we add it to the intermediates pool. If not, we call
+		// it the leaf cert
+		if c.IsCA {
+			intPool.AddCert(c)
+			continue
+		}
+		// Certificate is not a CA, it must be our leaf certificate.
+		// If we already found one, bail with error
+		if leafCert != nil {
+			return errors.New("more than one leaf certificate found")
+		}
+		leafCert = c
+	}
+
+	// We exited the loop with no leaf certificates
+	if leafCert == nil {
+		return errors.New("no leaf certificates found")
+	}
+
+	// We have one leaf certificate and at least one intermediate. Lets add this
+	// Cert Pool as the Intermediates list on our VerifyOptions
+	opts.Intermediates = intPool
+
+	// Finally, let's call Verify on our leafCert with our fully configured options
+	chains, err := leafCert.Verify(opts)
+	if len(chains) == 0 || err != nil {
+		return fmt.Errorf("certificate validation failed not verify: %v", err)
+	}
+	return nil
+}

trustmanager/x509store_test.go

@@ -0,0 +1,155 @@
+package trustmanager
+
+import (
+	"crypto/x509"
+	"fmt"
+	"testing"
+)
+
+func TestVerifyLeafSuccessfully(t *testing.T) {
+	// Get root certificate
+	rootCA, err := LoadCertFromFile("../fixtures/notary/root-ca.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Get intermediate certificate
+	intermediateCA, err := LoadCertFromFile("../fixtures/notary/ca.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Get leaf certificate
+	leafCert, err := LoadCertFromFile("../fixtures/notary/secure.docker.com.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Create a store and add the CA root
+	store := NewX509MemStore()
+	err = store.AddCert(rootCA)
+	if err != nil {
+		t.Fatalf("failed to load certificate from file: %v", err)
+	}
+
+	// Get our certList with Leaf Cert and Intermediate
+	certList := []*x509.Certificate{leafCert, intermediateCA}
+
+	// Get the VerifyOptions from our Store
+	opts, err := store.GetVerifyOptions("secure.docker.com")
+	fmt.Println(opts)
+
+	// Try to find a valid chain for cert
+	err = Verify(store, "secure.docker.com", certList)
+	if err != nil {
+		t.Fatalf("expected to find a valid chain for this certificate: %v", err)
+	}
+}
+
+func TestVerifyLeafSuccessfullyWithMultipleIntermediates(t *testing.T) {
+	// Get root certificate
+	rootCA, err := LoadCertFromFile("../fixtures/notary/root-ca.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Get intermediate certificate
+	intermediateCA, err := LoadCertFromFile("../fixtures/notary/ca.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Get leaf certificate
+	leafCert, err := LoadCertFromFile("../fixtures/notary/secure.docker.com.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Create a store and add the CA root
+	store := NewX509MemStore()
+	err = store.AddCert(rootCA)
+	if err != nil {
+		t.Fatalf("failed to load certificate from file: %v", err)
+	}
+
+	// Get our certList with Leaf Cert and Intermediate
+	certList := []*x509.Certificate{leafCert, intermediateCA, intermediateCA, rootCA}
+
+	// Get the VerifyOptions from our Store
+	opts, err := store.GetVerifyOptions("secure.docker.com")
+	fmt.Println(opts)
+
+	// Try to find a valid chain for cert
+	err = Verify(store, "secure.docker.com", certList)
+	if err != nil {
+		t.Fatalf("expected to find a valid chain for this certificate: %v", err)
+	}
+}
+
+func TestVerifyLeafWithNoIntermediate(t *testing.T) {
+	// Get root certificate
+	rootCA, err := LoadCertFromFile("../fixtures/notary/root-ca.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Get leaf certificate
+	leafCert, err := LoadCertFromFile("../fixtures/notary/secure.docker.com.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Create a store and add the CA root
+	store := NewX509MemStore()
+	err = store.AddCert(rootCA)
+	if err != nil {
+		t.Fatalf("failed to load certificate from file: %v", err)
+	}
+
+	// Get our certList with Leaf Cert and Intermediate
+	certList := []*x509.Certificate{leafCert, leafCert}
+
+	// Get the VerifyOptions from our Store
+	opts, err := store.GetVerifyOptions("secure.docker.com")
+	fmt.Println(opts)
+
+	// Try to find a valid chain for cert
+	err = Verify(store, "secure.docker.com", certList)
+	if err == nil {
+		t.Fatalf("expected error due to more than one leaf certificate")
+	}
+}
+
+func TestVerifyLeafWithNoLeaf(t *testing.T) {
+	// Get root certificate
+	rootCA, err := LoadCertFromFile("../fixtures/notary/root-ca.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Get intermediate certificate
+	intermediateCA, err := LoadCertFromFile("../fixtures/notary/ca.crt")
+	if err != nil {
+		t.Fatalf("couldn't load fixture: %v", err)
+	}
+
+	// Create a store and add the CA root
+	store := NewX509MemStore()
+	err = store.AddCert(rootCA)
+	if err != nil {
+		t.Fatalf("failed to load certificate from file: %v", err)
+	}
+
+	// Get our certList with Leaf Cert and Intermediate
+	certList := []*x509.Certificate{intermediateCA, intermediateCA}
+
+	// Get the VerifyOptions from our Store
+	opts, err := store.GetVerifyOptions("secure.docker.com")
+	fmt.Println(opts)
+
+	// Try to find a valid chain for cert
+	err = Verify(store, "secure.docker.com", certList)
+	if err == nil {
+		t.Fatalf("expected error due to no leafs provided")
+	}
+}