From cf9e6499e165aa3957c0d83cb88178aa0666fda1 Mon Sep 17 00:00:00 2001 From: Diogo Monica Date: Sun, 19 Jul 2015 01:45:43 -0700 Subject: [PATCH] Addressing comments Signed-off-by: Diogo Monica --- keystoremanager/keystoremanager.go | 72 +++++++++++++++++------------- trustmanager/x509utils.go | 2 +- trustmanager/x509utils_test.go | 2 - 3 files changed, 43 insertions(+), 33 deletions(-) diff --git a/keystoremanager/keystoremanager.go b/keystoremanager/keystoremanager.go index 3cf02431e9..bda36d7e15 100644 --- a/keystoremanager/keystoremanager.go +++ b/keystoremanager/keystoremanager.go @@ -205,7 +205,7 @@ have never seen a certificate for a particular CN, we trust it. If later we see a different certificate for that certificate, we return an ErrValidationFailed error. Note that since we only allow trust data to be downloaded over an HTTPS channel -we are using the current web-of-trust to validate the first download of the certificate +we are using the current public PKI to validate the first download of the certificate adding an extra layer of security over the normal (SSH style) trust model. We shall call this: TOFUS. */ @@ -216,6 +216,13 @@ func (km *KeyStoreManager) ValidateRoot(root *data.Signed, gun string) error { return err } + // Retrieve all the leaf certificates in root for which the CN matches the GUN + allValidCerts, err := validRootLeafCerts(signedRoot, gun) + if err != nil { + logrus.Debugf("error retrieving valid leaf certificates for: %s, %v", gun, err) + return &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"} + } + // Retrieve all the trusted certificates that match this gun certsForCN, err := km.trustedCertificateStore.GetCertificatesByCN(gun) if err != nil { @@ -228,16 +235,8 @@ func (km *KeyStoreManager) ValidateRoot(root *data.Signed, gun string) error { } } - // Retrieve all the leaf certificates in root for which the CN matches the GUN - allValidCerts, err := validRootLeafCerts(signedRoot, gun) - if err != nil { - logrus.Debugf("error retrieving valid leaf certificates for: %s, %v", gun, err) - return &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"} - } - - // If there are certificates for this specific GUN, we already have trusted - // certificates for this GUN. Let's make sure to check that this root is - // signed by them + // If we have certificates that match this specific GUN, let's make sure to + // use them first to validate that this new root is valid. if len(certsForCN) != 0 { logrus.Debugf("found %d valid certificates for %s", len(certsForCN), gun) err = signed.VerifyRoot(root, 0, trustmanager.CertsToKeys(certsForCN)) @@ -259,19 +258,8 @@ func (km *KeyStoreManager) ValidateRoot(root *data.Signed, gun string) error { // the new set of certificates has integrity (self-signed) logrus.Debugf("entering root certificate rotation for: %s", gun) - var rotationFailure bool - // To support root certificate rotation, let trust only the certs present in root - // First, we delete all the old certificates that aren't present in the root - for certID, cert := range certsToRemove(certsForCN, allValidCerts) { - logrus.Debugf("removing certificate with certID: %s", certID) - err = km.trustedCertificateStore.RemoveCert(cert) - if err != nil { - logrus.Debugf("failed to remove trusted certificate with keyID: %s, %v", certID, err) - rotationFailure = true - } - } - - // Now we add all the new certificates (even if they arleady already exist) + // Do root certificate rotation: we trust only the certs present in the new root + // First we add all the new certificates (even if they already exist) for _, cert := range allValidCerts { err := km.trustedCertificateStore.AddCert(cert) if err != nil { @@ -280,21 +268,28 @@ func (km *KeyStoreManager) ValidateRoot(root *data.Signed, gun string) error { continue } logrus.Debugf("error adding new trusted certificate for: %s, %v", gun, err) - rotationFailure = true } // Getting the CertID for debug only, don't care about the error certID, _ := trustmanager.FingerprintCert(cert) - logrus.Debugf("adding trust certificate for with certID %s for %s", certID, gun) + logrus.Debugf("adding trust certificate with certID %s for %s", certID, gun) } - if rotationFailure { - return &ErrRootRotationFail{Reason: "failed to rotate root keys"} + // Now we delete old certificates that aren't present in the new root + for certID, cert := range certsToRemove(certsForCN, allValidCerts) { + logrus.Debugf("removing certificate with certID: %s", certID) + err = km.trustedCertificateStore.RemoveCert(cert) + if err != nil { + logrus.Debugf("failed to remove trusted certificate with keyID: %s, %v", certID, err) + return &ErrRootRotationFail{Reason: "failed to rotate root keys"} + } } logrus.Debugf("Root validation succeeded for %s", gun) return nil } +// validRootLeafCerts returns a list of non-exipired, non-sha1 certificates whoose +// Common-Names match the provided GUN func validRootLeafCerts(root *data.SignedRoot, gun string) ([]*x509.Certificate, error) { // Get a list of all of the leaf certificates present in root allLeafCerts, _ := parseAllCerts(root) @@ -312,11 +307,21 @@ func validRootLeafCerts(root *data.SignedRoot, gun string) ([]*x509.Certificate, logrus.Debugf("error leaf certificate is expired") continue } + + // We don't allow root certificates that use SHA1 + if cert.SignatureAlgorithm == x509.SHA1WithRSA || + cert.SignatureAlgorithm == x509.DSAWithSHA1 || + cert.SignatureAlgorithm == x509.ECDSAWithSHA1 { + + logrus.Debugf("error certificate uses old ciphers") + continue + } + validLeafCerts = append(validLeafCerts, cert) } if len(validLeafCerts) < 1 { - return validLeafCerts, errors.New("no valid leaf certificates found in any of the root keys") + return nil, errors.New("no valid leaf certificates found in any of the root keys") } return validLeafCerts, nil @@ -336,9 +341,16 @@ func parseAllCerts(signedRoot *data.SignedRoot) (map[string]*x509.Certificate, m logrus.Debugf("found the following root keys: %v", rootRoles.KeyIDs) // Iterate over every keyID for the root role inside of roots.json for _, keyID := range rootRoles.KeyIDs { + // check that the key exists in the signed root keys map + key, ok := signedRoot.Signed.Keys[keyID] + if !ok { + logrus.Debugf("error while getting data for keyID: %s", keyID) + continue + } + // Decode all the x509 certificates that were bundled with this // Specific root key - decodedCerts, err := trustmanager.LoadCertBundleFromPEM([]byte(signedRoot.Signed.Keys[keyID].Public())) + decodedCerts, err := trustmanager.LoadCertBundleFromPEM(key.Public()) if err != nil { logrus.Debugf("error while parsing root certificate with keyID: %s, %v", keyID, err) continue diff --git a/trustmanager/x509utils.go b/trustmanager/x509utils.go index 2daef1aaaa..ccbf550f91 100644 --- a/trustmanager/x509utils.go +++ b/trustmanager/x509utils.go @@ -445,7 +445,7 @@ func CertsToKeys(certs []*x509.Certificate) map[string]data.PublicKey { case x509.ECDSA: keyType = data.ECDSAx509Key default: - logrus.Debugf("unknow certificate type found, ignoring") + logrus.Debugf("unknown certificate type found, ignoring") } // Create new the appropriate PublicKey diff --git a/trustmanager/x509utils_test.go b/trustmanager/x509utils_test.go index 545fd07ffd..fc9fe9c203 100644 --- a/trustmanager/x509utils_test.go +++ b/trustmanager/x509utils_test.go @@ -3,7 +3,6 @@ package trustmanager import ( "crypto/rand" "crypto/x509" - "fmt" "strings" "testing" "time" @@ -77,7 +76,6 @@ func TestKeyOperations(t *testing.T) { // Check to see if ED key it is encoded stringEncodedEDKey := string(edPEM) assert.True(t, strings.Contains(stringEncodedEDKey, "-----BEGIN ED25519 PRIVATE KEY-----")) - fmt.Println(stringEncodedEDKey) // Check to see if EC key it is encoded stringEncodedECKey := string(ecPEM)