Refactored fingerprint cert and added better debugging

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

client/cli_crypto_service.go

@@ -92,7 +92,7 @@ func (ccs *RSACryptoService) Sign(keyIDs []string, payload []byte) ([]data.Signa
 			// InitRepo gets a signer that doesn't have access to
 			// the root keys. Continuing here is safe because we
 			// end up not returning any signatures.
-			logrus.Debugf("ignoring error attempting to retrieve RSA key ID: %s, %v", privKey.ID(), err)
+			logrus.Debugf("ignoring error attempting to retrieve RSA key ID: %s, %v", keyid, err)
 			continue
 		}
 
@@ -158,7 +158,7 @@ func (ccs *ECDSACryptoService) Create(role string) (*data.PublicKey, error) {
 		return nil, fmt.Errorf("failed to add key to filestore: %v", err)
 	}
 
-	logrus.Debugf("generated new ECDSA key for role: %s with keyID: %s", role, privKey.ID())
+	logrus.Debugf("generated new ECDSA key for role %s with keyID: %s", role, privKey.ID())
 
 	return data.PublicKeyFromPrivate(*privKey), nil
 }

client/client.go

@@ -142,9 +142,10 @@ func (r *NotaryRepository) Initialize(uSigner *UnlockedSigner) error {
 		return err
 	}
 	r.certificateStore.AddCert(rootCert)
-	logrus.Debugf("Adding certificate with fingerprint: %s to the certificate store.", trustmanager.FingerprintCert(rootCert))
+
 	rootKey := data.NewPublicKey(uSigner.privKey.Cipher(), trustmanager.CertToPEM(rootCert))
 	logrus.Debugf("Linking %s to %s.", rootKey.ID(), uSigner.ID())
+
 	err = r.rootKeyStore.Link(uSigner.ID(), rootKey.ID())
 	if err != nil {
 		return err
@@ -163,6 +164,8 @@ func (r *NotaryRepository) Initialize(uSigner *UnlockedSigner) error {
 	}
 
 	timestampKey := data.NewPublicKey(parsedKey.Cipher(), parsedKey.Public())
+	logrus.Debugf("got remote %s timestamp key with keyID: %s", parsedKey.Cipher(), timestampKey.ID())
+	logrus.Debugf("timestamp key: %s", rawTSKey)
 
 	targetsKey, err := r.signer.Create("targets")
 	if err != nil {
@@ -539,27 +542,32 @@ func (r *NotaryRepository) validateRoot(root *data.Signed) error {
 	}
 
 	certs := make(map[string]*data.PublicKey)
-	for _, fingerprint := range rootSigned.Roles["root"].KeyIDs {
+	for _, keyID := range rootSigned.Roles["root"].KeyIDs {
 		// TODO(dlaw): currently assuming only one cert contained in
 		// public key entry. Need to fix when we want to pass in chains.
-		k, _ := pem.Decode([]byte(rootSigned.Keys[fingerprint].Public()))
+		k, _ := pem.Decode([]byte(rootSigned.Keys[keyID].Public()))
 		decodedCerts, err := x509.ParseCertificates(k.Bytes)
 		if err != nil {
+			logrus.Debugf("error while parsing root certificate with keyID: %s, %v", keyID, err)
 			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 := trustmanager.FingerprintCert(leafCert)
+		leafID, err := trustmanager.FingerprintCert(leafCert)
+		if err != nil {
+			logrus.Debugf("error while fingerprinting root certificate with keyID: %s, %v", keyID, err)
+			continue
+		}
 
 		// 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 = r.certificateStore.GetCertificateByFingerprint(leafID)
+		_, err = r.certificateStore.GetCertificateByKeyID(leafID)
 		if err == nil && leafCert.Subject.CommonName == r.gun {
-			certs[fingerprint] = rootSigned.Keys[fingerprint]
+			certs[keyID] = rootSigned.Keys[keyID]
 		}
 
 		// Check to see if this leafCertificate has a chain to one of the Root CAs
@@ -567,7 +575,7 @@ func (r *NotaryRepository) validateRoot(root *data.Signed) error {
 		certList := []*x509.Certificate{leafCert}
 		err = trustmanager.Verify(r.caStore, r.gun, certList)
 		if err == nil {
-			certs[fingerprint] = rootSigned.Keys[fingerprint]
+			certs[keyID] = rootSigned.Keys[keyID]
 		}
 	}
 

client/client_test.go

@@ -98,7 +98,8 @@ func TestInitRepo(t *testing.T) {
 	certificates := repo.certificateStore.GetCertificates()
 	assert.Len(t, certificates, 1, "unexpected number of certificates")
 
-	certID := trustmanager.FingerprintCert(certificates[0])
+	certID, err := trustmanager.FingerprintCert(certificates[0])
+	assert.NoError(t, err, "unable to fingerprint the certificate")
 
 	actualDest, err := os.Readlink(filepath.Join(tempBaseDir, "private", "root_keys", certID+".key"))
 	assert.NoError(t, err, "missing symlink to root key")

cmd/notary/keys.go

@@ -64,7 +64,7 @@ func keysRemove(cmd *cobra.Command, args []string) {
 	gunOrID := args[0]
 
 	// Try to retrieve the ID from the CA store.
-	cert, err := caStore.GetCertificateByFingerprint(gunOrID)
+	cert, err := caStore.GetCertificateByKeyID(gunOrID)
 	if err == nil {
 		fmt.Printf("Removing: ")
 		printCert(cert)
@@ -78,7 +78,7 @@ func keysRemove(cmd *cobra.Command, args []string) {
 	}
 
 	// Try to retrieve the ID from the Certificate store.
-	cert, err = certificateStore.GetCertificateByFingerprint(gunOrID)
+	cert, err = certificateStore.GetCertificateByKeyID(gunOrID)
 	if err == nil {
 		fmt.Printf("Removing: ")
 		printCert(cert)
@@ -216,17 +216,21 @@ func keysGenerate(cmd *cobra.Command, args []string) {
 
 func printCert(cert *x509.Certificate) {
 	timeDifference := cert.NotAfter.Sub(time.Now())
-	fingerprint := trustmanager.FingerprintCert(cert)
+	keyID, err := trustmanager.FingerprintCert(cert)
-	fmt.Printf("%s %s (expires in: %v days)\n", cert.Subject.CommonName, fingerprint, math.Floor(timeDifference.Hours()/24))
+	if err != nil {
+		fatalf("could not fingerprint certificate: %v", err)
+	}
+
+	fmt.Printf("%s %s (expires in: %v days)\n", cert.Subject.CommonName, keyID, math.Floor(timeDifference.Hours()/24))
 }
 
 func printKey(keyPath string) {
 	keyPath = strings.TrimSuffix(keyPath, filepath.Ext(keyPath))
 	keyPath = strings.TrimPrefix(keyPath, viper.GetString("privDir"))
 
-	fingerprint := filepath.Base(keyPath)
+	keyID := filepath.Base(keyPath)
 	gun := filepath.Dir(keyPath)[1:]
-	fmt.Printf("%s %s\n", gun, fingerprint)
+	fmt.Printf("%s %s\n", gun, keyID)
 }
 
 func askConfirm() bool {

trustmanager/x509filestore.go

@@ -70,17 +70,19 @@ func (s X509FileStore) AddCert(cert *x509.Certificate) error {
 // addNamedCert allows adding a certificate while controling the filename it gets
 // stored under. If the file does not exist on disk, saves it.
 func (s X509FileStore) addNamedCert(cert *x509.Certificate) error {
-	fingerprint := fingerprintCert(cert)
+	fileName, keyID, err := fileName(cert)
-	logrus.Debug("Adding cert with fingerprint: ", fingerprint)
+	if err != nil {
+		return err
+	}
+
+	logrus.Debug("Adding cert with keyID: ", keyID)
 	// Validate if we already loaded this certificate before
-	if _, ok := s.fingerprintMap[fingerprint]; ok {
+	if _, ok := s.fingerprintMap[keyID]; ok {
 		return errors.New("certificate already in the store")
 	}
 
 	// Convert certificate to PEM
 	certBytes := CertToPEM(cert)
-	// Compute FileName
-	fileName := fileName(cert)
 
 	// Save the file to disk if not already there.
 	if _, err := os.Stat(s.fileStore.GetPath(fileName)); os.IsNotExist(err) {
@@ -92,11 +94,11 @@ func (s X509FileStore) addNamedCert(cert *x509.Certificate) error {
 	}
 
 	// We wrote the certificate succcessfully, add it to our in-memory storage
-	s.fingerprintMap[fingerprint] = cert
+	s.fingerprintMap[keyID] = cert
-	s.fileMap[fingerprint] = fileName
+	s.fileMap[keyID] = fileName
 
 	name := string(cert.RawSubject)
-	s.nameMap[name] = append(s.nameMap[name], fingerprint)
+	s.nameMap[name] = append(s.nameMap[name], keyID)
 
 	return nil
 }
@@ -107,10 +109,13 @@ func (s X509FileStore) RemoveCert(cert *x509.Certificate) error {
 		return errors.New("removing nil Certificate from X509Store")
 	}
 
-	fingerprint := fingerprintCert(cert)
+	keyID, err := fingerprintCert(cert)
-	delete(s.fingerprintMap, fingerprint)
+	if err != nil {
-	filename := s.fileMap[fingerprint]
+		return err
-	delete(s.fileMap, fingerprint)
+	}
+	delete(s.fingerprintMap, keyID)
+	filename := s.fileMap[keyID]
+	delete(s.fileMap, keyID)
 
 	name := string(cert.RawSubject)
 
@@ -118,7 +123,7 @@ func (s X509FileStore) RemoveCert(cert *x509.Certificate) error {
 	fpList := s.nameMap[name]
 	newfpList := fpList[:0]
 	for _, x := range fpList {
-		if x != fingerprint {
+		if x != keyID {
 			newfpList = append(newfpList, x)
 		}
 	}
@@ -174,15 +179,15 @@ func (s X509FileStore) GetCertificatePool() *x509.CertPool {
 	return pool
 }
 
-// GetCertificateByFingerprint returns the certificate that matches a certain kID or error
+// GetCertificateByKeyID returns the certificate that matches a certain keyID or error
-func (s X509FileStore) GetCertificateByFingerprint(hexkID string) (*x509.Certificate, error) {
+func (s X509FileStore) GetCertificateByKeyID(keyID string) (*x509.Certificate, error) {
 	// If it does not look like a hex encoded sha256 hash, error
-	if len(hexkID) != 64 {
+	if len(keyID) != 64 {
 		return nil, errors.New("invalid Subject Key Identifier")
 	}
 
 	// Check to see if this subject key identifier exists
-	if cert, ok := s.fingerprintMap[CertID(hexkID)]; ok {
+	if cert, ok := s.fingerprintMap[CertID(keyID)]; ok {
 		return cert, nil
 
 	}
@@ -207,6 +212,11 @@ func (s X509FileStore) GetVerifyOptions(dnsName string) (x509.VerifyOptions, err
 	return opts, nil
 }
 
-func fileName(cert *x509.Certificate) string {
+func fileName(cert *x509.Certificate) (string, CertID, error) {
-	return path.Join(cert.Subject.CommonName, FingerprintCert(cert))
+	keyID, err := fingerprintCert(cert)
+	if err != nil {
+		return "", "", err
+	}
+
+	return path.Join(cert.Subject.CommonName, string(keyID)), keyID, nil
 }

trustmanager/x509memstore.go

@@ -47,11 +47,14 @@ func (s X509MemStore) AddCert(cert *x509.Certificate) error {
 		return errors.New("certificate failed validation")
 	}
 
-	fingerprint := fingerprintCert(cert)
+	keyID, err := fingerprintCert(cert)
+	if err != nil {
+		return err
+	}
 
-	s.fingerprintMap[fingerprint] = cert
+	s.fingerprintMap[keyID] = cert
 	name := string(cert.RawSubject)
-	s.nameMap[name] = append(s.nameMap[name], fingerprint)
+	s.nameMap[name] = append(s.nameMap[name], keyID)
 
 	return nil
 }
@@ -62,15 +65,18 @@ func (s X509MemStore) RemoveCert(cert *x509.Certificate) error {
 		return errors.New("removing nil Certificate to X509Store")
 	}
 
-	fingerprint := fingerprintCert(cert)
+	keyID, err := fingerprintCert(cert)
-	delete(s.fingerprintMap, fingerprint)
+	if err != nil {
+		return err
+	}
+	delete(s.fingerprintMap, keyID)
 	name := string(cert.RawSubject)
 
 	// Filter the fingerprint out of this name entry
 	fpList := s.nameMap[name]
 	newfpList := fpList[:0]
 	for _, x := range fpList {
-		if x != fingerprint {
+		if x != keyID {
 			newfpList = append(newfpList, x)
 		}
 	}
@@ -139,15 +145,15 @@ func (s X509MemStore) GetCertificatePool() *x509.CertPool {
 	return pool
 }
 
-// GetCertificateByFingerprint returns the certificate that matches a certain kID or error
+// GetCertificateByKeyID returns the certificate that matches a certain keyID or error
-func (s X509MemStore) GetCertificateByFingerprint(hexkID string) (*x509.Certificate, error) {
+func (s X509MemStore) GetCertificateByKeyID(keyID string) (*x509.Certificate, error) {
 	// If it does not look like a hex encoded sha256 hash, error
-	if len(hexkID) != 64 {
+	if len(keyID) != 64 {
 		return nil, errors.New("invalid Subject Key Identifier")
 	}
 
 	// Check to see if this subject key identifier exists
-	if cert, ok := s.fingerprintMap[CertID(hexkID)]; ok {
+	if cert, ok := s.fingerprintMap[CertID(keyID)]; ok {
 		return cert, nil
 
 	}

trustmanager/x509memstore_test.go

@@ -106,20 +106,20 @@ func TestRemoveCert(t *testing.T) {
 	}
 }
 
-func TestInexistentGetCertificateByFingerprint(t *testing.T) {
+func TestInexistentGetCertificateByKeyID(t *testing.T) {
 	store := NewX509MemStore()
 	err := store.AddCertFromFile("../fixtures/notary/root-ca.crt")
 	if err != nil {
 		t.Fatalf("failed to load certificate from file: %v", err)
 	}
 
-	_, err = store.GetCertificateByFingerprint("4d06afd30b8bed131d2a84c97d00b37f422021598bfae34285ce98e77b708b5a")
+	_, err = store.GetCertificateByKeyID("4d06afd30b8bed131d2a84c97d00b37f422021598bfae34285ce98e77b708b5a")
 	if err == nil {
 		t.Fatalf("no error returned for inexistent certificate")
 	}
 }
 
-func TestGetCertificateByFingerprint(t *testing.T) {
+func TestGetCertificateByKeyID(t *testing.T) {
 	b, err := ioutil.ReadFile("../fixtures/notary/root-ca.crt")
 	if err != nil {
 		t.Fatalf("couldn't load fixture: %v", err)
@@ -138,12 +138,15 @@ func TestGetCertificateByFingerprint(t *testing.T) {
 		t.Fatalf("failed to load certificate from PEM: %v", err)
 	}
 
-	certFingerprint := FingerprintCert(cert)
+	keyID, err := FingerprintCert(cert)
+	if err != nil {
+		t.Fatalf("failed to fingerprint the certificate: %v", err)
+	}
 
 	// Tries to retrieve cert by Subject Key IDs
-	_, err = store.GetCertificateByFingerprint(certFingerprint)
+	_, err = store.GetCertificateByKeyID(keyID)
 	if err != nil {
-		t.Fatalf("expected certificate in store: %s", certFingerprint)
+		t.Fatalf("expected certificate in store: %s", keyID)
 	}
 }
 

trustmanager/x509store.go

@@ -14,7 +14,7 @@ type X509Store interface {
 	AddCertFromPEM(pemCerts []byte) error
 	AddCertFromFile(filename string) error
 	RemoveCert(cert *x509.Certificate) error
-	GetCertificateByFingerprint(fingerprint string) (*x509.Certificate, error)
+	GetCertificateByKeyID(keyID string) (*x509.Certificate, error)
 	GetCertificates() []*x509.Certificate
 	GetCertificatePool() *x509.CertPool
 	GetVerifyOptions(dnsName string) (x509.VerifyOptions, error)

trustmanager/x509utils.go

@@ -87,11 +87,16 @@ func LoadCertFromPEM(pemBytes []byte) (*x509.Certificate, error) {
 }
 
 // FingerprintCert returns a TUF compliant fingerprint for a X509 Certificate
-func FingerprintCert(cert *x509.Certificate) string {
+func FingerprintCert(cert *x509.Certificate) (string, error) {
-	return string(fingerprintCert(cert))
+	certID, err := fingerprintCert(cert)
+	if err != nil {
+		return "", err
+	}
+
+	return string(certID), nil
 }
 
-func fingerprintCert(cert *x509.Certificate) CertID {
+func fingerprintCert(cert *x509.Certificate) (CertID, error) {
 	block := pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}
 	pemdata := pem.EncodeToMemory(&block)
 
@@ -102,14 +107,15 @@ func fingerprintCert(cert *x509.Certificate) CertID {
 	case x509.ECDSA:
 		keyType = "ECDSA"
 	default:
-		logrus.Debug("error while fingerprinting certificate. Got Unknown key type.")
+		return "", fmt.Errorf("error while fingerprinting certificate. Got Unknown key type.")
 	}
 
-	logrus.Debugf("certificate fingerprint of key type: %s", keyType)
 	// Create new TUF Key so we can compute the TUF-compliant CertID
 	tufKey := data.NewTUFKey(keyType, pemdata, nil)
 
-	return CertID(tufKey.ID())
+	logrus.Debugf("certificate fingerprint generated for key type %s: %s", keyType, tufKey.ID())
+
+	return CertID(tufKey.ID()), nil
 }
 
 // loadCertsFromDir receives a store AddCertFromFile for each certificate found
@@ -211,7 +217,14 @@ func GenerateRSAKey(random io.Reader, bits int) (*data.PrivateKey, error) {
 		return nil, fmt.Errorf("could not generate private key: %v", err)
 	}
 
-	return RSAToPrivateKey(rsaPrivKey)
+	tufPrivKey, err := RSAToPrivateKey(rsaPrivKey)
+	if err != nil {
+		return nil, err
+	}
+
+	logrus.Debugf("generated RSA key with keyID: %s", tufPrivKey.ID())
+
+	return tufPrivKey, nil
 }
 
 // RSAToPrivateKey converts an rsa.Private key to a TUF data.PrivateKey type
@@ -237,7 +250,14 @@ func GenerateECDSAKey(random io.Reader) (*data.PrivateKey, error) {
 		return nil, err
 	}
 
-	return ECDSAToPrivateKey(ecdsaPrivKey)
+	tufPrivKey, err := ECDSAToPrivateKey(ecdsaPrivKey)
+	if err != nil {
+		return nil, err
+	}
+
+	logrus.Debugf("generated ECDSA key with keyID: %s", tufPrivKey.ID())
+
+	return tufPrivKey, nil
 }
 
 // ECDSAToPrivateKey converts an rsa.Private key to a TUF data.PrivateKey type