diff --git a/certs/certs.go b/certs/certs.go index d8ba0d9255..4b81543b3e 100644 --- a/certs/certs.go +++ b/certs/certs.go @@ -4,6 +4,7 @@ import ( "crypto/x509" "errors" "fmt" + "strings" "time" "github.com/Sirupsen/logrus" @@ -36,6 +37,18 @@ func (err ErrRootRotationFail) Error() string { return fmt.Sprintf("could not rotate trust to a new trusted root: %s", err.Reason) } +func prettyFormatCertIDs(certs []*x509.Certificate) string { + ids := make([]string, 0, len(certs)) + for _, cert := range certs { + id, err := trustmanager.FingerprintCert(cert) + if err != nil { + id = fmt.Sprintf("[Error %s]", err) + } + ids = append(ids, id) + } + return strings.Join(ids, ", ") +} + /* ValidateRoot receives a new root, validates its correctness and attempts to do root key rotation if needed. @@ -71,14 +84,14 @@ func ValidateRoot(certStore trustmanager.X509Store, root *data.Signed, gun strin } // Retrieve all the leaf certificates in root for which the CN matches the GUN - allValidCerts, err := validRootLeafCerts(signedRoot, gun) + certsFromRoot, 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 := certStore.GetCertificatesByCN(gun) + trustedCerts, err := certStore.GetCertificatesByCN(gun) if err != nil { // If the error that we get back is different than ErrNoCertificatesFound // we couldn't check if there are any certificates with this CN already @@ -91,9 +104,10 @@ func ValidateRoot(certStore trustmanager.X509Store, root *data.Signed, gun strin // 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 root certificates for %s", len(certsForCN), gun) - err = signed.VerifyRoot(root, 0, trustmanager.CertsToKeys(certsForCN)) + if len(trustedCerts) != 0 { + logrus.Debugf("found %d valid root certificates for %s: %s", len(trustedCerts), gun, + prettyFormatCertIDs(trustedCerts)) + err = signed.VerifyRoot(root, 0, trustmanager.CertsToKeys(trustedCerts)) if err != nil { logrus.Debugf("failed to verify TUF data for: %s, %v", gun, err) return &ErrValidationFail{Reason: "failed to validate data with current trusted certificates"} @@ -103,7 +117,7 @@ func ValidateRoot(certStore trustmanager.X509Store, root *data.Signed, gun strin } // Validate the integrity of the new root (does it have valid signatures) - err = signed.VerifyRoot(root, 0, trustmanager.CertsToKeys(allValidCerts)) + err = signed.VerifyRoot(root, 0, trustmanager.CertsToKeys(certsFromRoot)) if err != nil { logrus.Debugf("failed to verify TUF data for: %s, %v", gun, err) return &ErrValidationFail{Reason: "failed to validate integrity of roots"} @@ -116,7 +130,7 @@ func ValidateRoot(certStore trustmanager.X509Store, root *data.Signed, gun strin // 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 { + for _, cert := range certsFromRoot { err := certStore.AddCert(cert) if err != nil { // If the error is already exists we don't fail the rotation @@ -129,7 +143,12 @@ func ValidateRoot(certStore trustmanager.X509Store, root *data.Signed, gun strin } // Now we delete old certificates that aren't present in the new root - for certID, cert := range certsToRemove(certsForCN, allValidCerts) { + oldCertsToRemove, err := certsToRemove(trustedCerts, certsFromRoot) + if err != nil { + logrus.Debugf("inconsistency when removing old certificates: %v", err) + return err + } + for certID, cert := range oldCertsToRemove { logrus.Debugf("removing certificate with certID: %s", certID) err = certStore.RemoveCert(cert) if err != nil { @@ -142,8 +161,9 @@ func ValidateRoot(certStore trustmanager.X509Store, root *data.Signed, gun strin return nil } -// validRootLeafCerts returns a list of non-exipired, non-sha1 certificates whose -// Common-Names match the provided GUN +// validRootLeafCerts returns a list of non-expired, non-sha1 certificates +// found in root whose Common-Names match the provided GUN. Note that this +// "validity" alone does not imply any measure of trust. 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) @@ -180,7 +200,8 @@ func validRootLeafCerts(root *data.SignedRoot, gun string) ([]*x509.Certificate, return nil, errors.New("no valid leaf certificates found in any of the root keys") } - logrus.Debugf("found %d valid leaf certificates for %s", len(validLeafCerts), gun) + logrus.Debugf("found %d valid leaf certificates for %s: %s", len(validLeafCerts), gun, + prettyFormatCertIDs(validLeafCerts)) return validLeafCerts, nil } @@ -243,16 +264,12 @@ func parseAllCerts(signedRoot *data.SignedRoot) (map[string]*x509.Certificate, m return leafCerts, intCerts } -// certsToRemove returns all the certifificates from oldCerts that aren't present -// in newCerts -func certsToRemove(oldCerts, newCerts []*x509.Certificate) map[string]*x509.Certificate { +// certsToRemove returns all the certificates from oldCerts that aren't present +// in newCerts. Note that newCerts should never be empty, else this function will error. +// We expect newCerts to come from validateRootLeafCerts, which does not return empty sets. +func certsToRemove(oldCerts, newCerts []*x509.Certificate) (map[string]*x509.Certificate, error) { certsToRemove := make(map[string]*x509.Certificate) - // If no newCerts were provided - if len(newCerts) == 0 { - return certsToRemove - } - // Populate a map with all the IDs from newCert var newCertMap = make(map[string]struct{}) for _, cert := range newCerts { @@ -264,6 +281,14 @@ func certsToRemove(oldCerts, newCerts []*x509.Certificate) map[string]*x509.Cert newCertMap[certID] = struct{}{} } + // We don't want to "rotate" certificates to an empty set, nor keep old certificates if the + // new root does not trust them. newCerts should come from validRootLeafCerts, which refuses + // to return an empty set, and they should all be fingerprintable, so this should never happen + // - fail just to be sure. + if len(newCertMap) == 0 { + return nil, &ErrRootRotationFail{Reason: "internal error, got no certificates to rotate to"} + } + // Iterate over all the old certificates and check to see if we should remove them for _, cert := range oldCerts { certID, err := trustmanager.FingerprintCert(cert) @@ -276,5 +301,5 @@ func certsToRemove(oldCerts, newCerts []*x509.Certificate) map[string]*x509.Cert } } - return certsToRemove + return certsToRemove, nil } diff --git a/certs/certs_test.go b/certs/certs_test.go index 62817ddd9c..4ea992308c 100644 --- a/certs/certs_test.go +++ b/certs/certs_test.go @@ -55,7 +55,8 @@ func TestCertsToRemove(t *testing.T) { oldCerts := []*x509.Certificate{cert1} newCerts := []*x509.Certificate{cert2} - certificates := certsToRemove(oldCerts, newCerts) + certificates, err := certsToRemove(oldCerts, newCerts) + require.NoError(t, err) require.Len(t, certificates, 1) _, ok := certificates[cert1KeyID] require.True(t, ok) @@ -64,7 +65,8 @@ func TestCertsToRemove(t *testing.T) { oldCerts = []*x509.Certificate{cert1, cert2} newCerts = []*x509.Certificate{cert3} - certificates = certsToRemove(oldCerts, newCerts) + certificates, err = certsToRemove(oldCerts, newCerts) + require.NoError(t, err) require.Len(t, certificates, 2) _, ok = certificates[cert1KeyID] require.True(t, ok) @@ -77,7 +79,8 @@ func TestCertsToRemove(t *testing.T) { oldCerts = []*x509.Certificate{cert3} newCerts = []*x509.Certificate{cert2, cert1} - certificates = certsToRemove(oldCerts, newCerts) + certificates, err = certsToRemove(oldCerts, newCerts) + require.NoError(t, err) require.Len(t, certificates, 1) _, ok = certificates[cert3KeyID] require.True(t, ok) @@ -90,20 +93,15 @@ func TestCertsToRemove(t *testing.T) { oldCerts = []*x509.Certificate{cert1, cert2, cert3} newCerts = []*x509.Certificate{} - certificates = certsToRemove(oldCerts, newCerts) - require.Len(t, certificates, 0) - _, ok = certificates[cert1KeyID] - require.False(t, ok) - _, ok = certificates[cert2KeyID] - require.False(t, ok) - _, ok = certificates[cert3KeyID] - require.False(t, ok) + certificates, err = certsToRemove(oldCerts, newCerts) + require.Error(t, err) // Call CertsToRemove with three new certificates and no old oldCerts = []*x509.Certificate{} newCerts = []*x509.Certificate{cert1, cert2, cert3} - certificates = certsToRemove(oldCerts, newCerts) + certificates, err = certsToRemove(oldCerts, newCerts) + require.NoError(t, err) require.Len(t, certificates, 0) _, ok = certificates[cert1KeyID] require.False(t, ok) diff --git a/client/client.go b/client/client.go index a867ecc6c4..65e74845c6 100644 --- a/client/client.go +++ b/client/client.go @@ -160,7 +160,10 @@ func NewTarget(targetName string, targetPath string) (*Target, error) { } // Initialize creates a new repository by using rootKey as the root Key for the -// TUF repository. +// TUF repository. The server must be reachable (and is asked to generate a +// timestamp key and possibly other serverManagedRoles), but the created repository +// result is only stored on local disk, not published to the server. To do that, +// use r.Publish() eventually. func (r *NotaryRepository) Initialize(rootKeyID string, serverManagedRoles ...string) error { privKey, _, err := r.CryptoService.GetPrivateKey(rootKeyID) if err != nil { @@ -645,9 +648,10 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error { return remote.SetMultiMeta(updatedFiles) } -// bootstrapRepo loads the repository from the local file system. This attempts -// to load metadata for all roles. Since server snapshots are supported, -// if the snapshot metadata fails to load, that's ok. +// bootstrapRepo loads the repository from the local file system (i.e. +// a not yet published repo or a possibly obsolete local copy) into +// r.tufRepo. This attempts to load metadata for all roles. Since server +// snapshots are supported, if the snapshot metadata fails to load, that's ok. // This can also be unified with some cache reading tools from tuf/client. // This assumes that bootstrapRepo is only used by Publish() or RotateKey() func (r *NotaryRepository) bootstrapRepo() error { @@ -695,6 +699,8 @@ func (r *NotaryRepository) bootstrapRepo() error { return nil } +// saveMetadata saves contents of r.tufRepo onto the local disk, creating +// signatures as necessary, possibly prompting for passphrases. func (r *NotaryRepository) saveMetadata(ignoreSnapshot bool) error { logrus.Debugf("Saving changes to Trusted Collection.") @@ -776,6 +782,20 @@ func (r *NotaryRepository) Update(forWrite bool) error { // we should always attempt to contact the server to determine if the repository // is initialized or not. If set to true, we will always attempt to download // and return an error if the remote repository errors. +// +// Partially populates r.tufRepo with this root metadata (only; use +// tufclient.Client.Update to load the rest). +// +// As another side effect, r.CertManager's list of trusted certificates +// is updated with data from the loaded root.json. +// +// Fails if the remote server is reachable and does not know the repo +// (i.e. before the first r.Publish()), in which case the error is +// store.ErrMetaNotFound, or if the root metadata (from whichever source is used) +// is not trusted. +// +// Returns a tufclient.Client for the remote server, which may not be actually +// operational (if the URL is invalid but a root.json is cached). func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*tufclient.Client, error) { var ( rootJSON []byte diff --git a/cryptoservice/crypto_service_test.go b/cryptoservice/crypto_service_test.go index 57182eb203..6589fc60a5 100644 --- a/cryptoservice/crypto_service_test.go +++ b/cryptoservice/crypto_service_test.go @@ -97,7 +97,7 @@ func (c CryptoServiceTester) TestGetNonexistentKey(t *testing.T) { _, _, err := cryptoService.GetPrivateKey("boguskeyid") require.Error(t, err) // The underlying error has been correctly propagated. - _, ok := err.(*trustmanager.ErrKeyNotFound) + _, ok := err.(trustmanager.ErrKeyNotFound) require.True(t, ok) } diff --git a/signer/keydbstore/keydbstore.go b/signer/keydbstore/keydbstore.go index ac57263b3a..ebd0c49ae2 100644 --- a/signer/keydbstore/keydbstore.go +++ b/signer/keydbstore/keydbstore.go @@ -117,7 +117,7 @@ func (s *KeyDBStore) GetKey(name string) (data.PrivateKey, string, error) { // Retrieve the GORM private key from the database dbPrivateKey := GormPrivateKey{} if s.db.Where(&GormPrivateKey{KeyID: name}).First(&dbPrivateKey).RecordNotFound() { - return nil, "", trustmanager.ErrKeyNotFound{} + return nil, "", trustmanager.ErrKeyNotFound{KeyID: name} } // Get the passphrase to use for this key @@ -165,7 +165,7 @@ func (s *KeyDBStore) RemoveKey(keyID string) error { // Retrieve the GORM private key from the database dbPrivateKey := GormPrivateKey{} if s.db.Where(&GormPrivateKey{KeyID: keyID}).First(&dbPrivateKey).RecordNotFound() { - return trustmanager.ErrKeyNotFound{} + return trustmanager.ErrKeyNotFound{KeyID: keyID} } // Delete the key from the database @@ -179,7 +179,7 @@ func (s *KeyDBStore) RotateKeyPassphrase(name, newPassphraseAlias string) error // Retrieve the GORM private key from the database dbPrivateKey := GormPrivateKey{} if s.db.Where(&GormPrivateKey{KeyID: name}).First(&dbPrivateKey).RecordNotFound() { - return trustmanager.ErrKeyNotFound{} + return trustmanager.ErrKeyNotFound{KeyID: name} } // Get the current passphrase to use for this key diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 13fe4704f3..3a5be1cf62 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -361,7 +361,7 @@ func getKeyRole(s LimitedFileStore, keyID string) (string, bool, error) { } } - return "", false, &ErrKeyNotFound{KeyID: keyID} + return "", false, ErrKeyNotFound{KeyID: keyID} } // GetKey returns the PrivateKey given a KeyID diff --git a/trustmanager/keystore.go b/trustmanager/keystore.go index 4f1338e47b..237f570bdf 100644 --- a/trustmanager/keystore.go +++ b/trustmanager/keystore.go @@ -43,6 +43,8 @@ type KeyStore interface { // AddKey adds a key to the KeyStore, and if the key already exists, // succeeds. Otherwise, returns an error if it cannot add. AddKey(keyInfo KeyInfo, privKey data.PrivateKey) error + // Should fail with ErrKeyNotFound if the keystore is operating normally + // and knows that it does not store the requested key. GetKey(keyID string) (data.PrivateKey, string, error) GetKeyInfo(keyID string) (KeyInfo, error) ListKeys() map[string]KeyInfo diff --git a/trustmanager/yubikey/yubikeystore.go b/trustmanager/yubikey/yubikeystore.go index e1653aa63f..2e8300b396 100644 --- a/trustmanager/yubikey/yubikeystore.go +++ b/trustmanager/yubikey/yubikeystore.go @@ -103,6 +103,17 @@ func (err ErrBackupFailed) Error() string { return fmt.Sprintf("Failed to backup private key to: %s", err.err) } +// An error indicating that the HSM is not present (as opposed to failing), +// i.e. that we can confidently claim that the key is not stored in the HSM +// without notifying the user about a missing or failing HSM. +type errHSMNotPresent struct { + err string +} + +func (err errHSMNotPresent) Error() string { + return err.err +} + type yubiSlot struct { role string slotID []byte @@ -706,13 +717,16 @@ func (s *YubiKeyStore) GetKey(keyID string) (data.PrivateKey, string, error) { ctx, session, err := SetupHSMEnv(pkcs11Lib, s.libLoader) if err != nil { logrus.Debugf("Failed to initialize PKCS11 environment: %s", err.Error()) + if _, ok := err.(errHSMNotPresent); ok { + err = trustmanager.ErrKeyNotFound{KeyID: keyID} + } return nil, "", err } defer cleanup(ctx, session) key, ok := s.keys[keyID] if !ok { - return nil, "", errors.New("no matching keys found inside of yubikey") + return nil, "", trustmanager.ErrKeyNotFound{KeyID: keyID} } pubKey, alias, err := getECDSAKey(ctx, session, key.slotID) @@ -788,7 +802,7 @@ func SetupHSMEnv(libraryPath string, libLoader pkcs11LibLoader) ( IPKCS11Ctx, pkcs11.SessionHandle, error) { if libraryPath == "" { - return nil, 0, fmt.Errorf("no library found.") + return nil, 0, errHSMNotPresent{err: "no library found."} } p := libLoader(libraryPath) @@ -798,8 +812,7 @@ func SetupHSMEnv(libraryPath string, libLoader pkcs11LibLoader) ( if err := p.Initialize(); err != nil { defer finalizeAndDestroy(p) - return nil, 0, fmt.Errorf( - "found library %s, but initialize error %s", libraryPath, err.Error()) + return nil, 0, fmt.Errorf("found library %s, but initialize error %s", libraryPath, err.Error()) } slots, err := p.GetSlotList(true) diff --git a/tuf/signed/sign_test.go b/tuf/signed/sign_test.go index 3a8d172da1..b02338db55 100644 --- a/tuf/signed/sign_test.go +++ b/tuf/signed/sign_test.go @@ -14,24 +14,18 @@ import ( const ( testKeyPEM1 = "-----BEGIN PUBLIC KEY-----\nMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEAnKuXZeefa2LmgxaL5NsM\nzKOHNe+x/nL6ik+lDBCTV6OdcwAhHQS+PONGhrChIUVR6Vth3hUCrreLzPO73Oo5\nVSCuRJ53UronENl6lsa5mFKP8StYLvIDITNvkoT3j52BJIjyNUK9UKY9As2TNqDf\nBEPIRp28ev/NViwGOEkBu2UAbwCIdnDXm8JQErCZA0Ydm7PKGgjLbFsFGrVzqXHK\n6pdzJXlhr9yap3UpgQ/iO9JtoEYB2EXsnSrPc9JRjR30bNHHtnVql3fvinXrAEwq\n3xmN4p+R4VGzfdQN+8Kl/IPjqWB535twhFYEG/B7Ze8IwbygBjK3co/KnOPqMUrM\nBI8ztvPiogz+MvXb8WvarZ6TMTh8ifZI96r7zzqyzjR1hJulEy3IsMGvz8XS2J0X\n7sXoaqszEtXdq5ef5zKVxkiyIQZcbPgmpHLq4MgfdryuVVc/RPASoRIXG4lKaTJj\n1ANMFPxDQpHudCLxwCzjCb+sVa20HBRPTnzo8LSZkI6jAgMBAAE=\n-----END PUBLIC KEY-----" - testKeyID1 = "51324b59d4888faa91219ebbe5a3876bb4efb21f0602ddf363cd4c3996ded3d4" ) +// A CryptoService which does not contain any keys. type FailingCryptoService struct { - testKey data.PrivateKey -} - -func (mts *FailingCryptoService) Sign(keyIDs []string, _ []byte) ([]data.Signature, error) { - sigs := make([]data.Signature, 0, len(keyIDs)) - return sigs, nil } func (mts *FailingCryptoService) Create(_, _, _ string) (data.PublicKey, error) { - return mts.testKey, nil + return nil, nil } func (mts *FailingCryptoService) ListKeys(role string) []string { - return []string{mts.testKey.ID()} + return []string{} } func (mts *FailingCryptoService) AddKey(role, gun string, key data.PrivateKey) error { @@ -39,25 +33,14 @@ func (mts *FailingCryptoService) AddKey(role, gun string, key data.PrivateKey) e } func (mts *FailingCryptoService) ListAllKeys() map[string]string { - return map[string]string{ - mts.testKey.ID(): data.CanonicalRootRole, - mts.testKey.ID(): data.CanonicalTargetsRole, - mts.testKey.ID(): data.CanonicalSnapshotRole, - mts.testKey.ID(): data.CanonicalTimestampRole, - } + return map[string]string{} } func (mts *FailingCryptoService) GetKey(keyID string) data.PublicKey { - if keyID == "testID" { - return data.PublicKeyFromPrivate(mts.testKey) - } return nil } func (mts *FailingCryptoService) GetPrivateKey(keyID string) (data.PrivateKey, string, error) { - if mts.testKey != nil { - return mts.testKey, "testRole", nil - } return nil, "", trustmanager.ErrKeyNotFound{KeyID: keyID} } @@ -65,18 +48,11 @@ func (mts *FailingCryptoService) RemoveKey(keyID string) error { return nil } +// A CryptoService which only only allows using one key type MockCryptoService struct { testKey data.PrivateKey } -func (mts *MockCryptoService) Sign(keyIDs []string, _ []byte) ([]data.Signature, error) { - sigs := make([]data.Signature, 0, len(keyIDs)) - for _, keyID := range keyIDs { - sigs = append(sigs, data.Signature{KeyID: keyID}) - } - return sigs, nil -} - func (mts *MockCryptoService) Create(_, _, _ string) (data.PublicKey, error) { return mts.testKey, nil } @@ -86,7 +62,7 @@ func (mts *MockCryptoService) AddKey(role, gun string, key data.PrivateKey) erro } func (mts *MockCryptoService) GetKey(keyID string) data.PublicKey { - if keyID == "testID" { + if keyID == mts.testKey.ID() { return data.PublicKeyFromPrivate(mts.testKey) } return nil @@ -106,43 +82,16 @@ func (mts *MockCryptoService) ListAllKeys() map[string]string { } func (mts *MockCryptoService) GetPrivateKey(keyID string) (data.PrivateKey, string, error) { - return mts.testKey, "testRole", nil + if keyID == mts.testKey.ID() { + return mts.testKey, "testRole", nil + } + return nil, "", trustmanager.ErrKeyNotFound{KeyID: keyID} } func (mts *MockCryptoService) RemoveKey(keyID string) error { return nil } -var _ CryptoService = &MockCryptoService{} - -type StrictMockCryptoService struct { - MockCryptoService -} - -func (mts *StrictMockCryptoService) GetKey(keyID string) data.PublicKey { - if keyID == mts.testKey.ID() { - return data.PublicKeyFromPrivate(mts.testKey) - } - return nil -} - -func (mts *StrictMockCryptoService) ListKeys(role string) []string { - return []string{mts.testKey.ID()} -} - -func (mts *StrictMockCryptoService) ListAllKeys() map[string]string { - return map[string]string{ - mts.testKey.ID(): data.CanonicalRootRole, - mts.testKey.ID(): data.CanonicalTargetsRole, - mts.testKey.ID(): data.CanonicalSnapshotRole, - mts.testKey.ID(): data.CanonicalTimestampRole, - } -} - -func (mts *StrictMockCryptoService) AddKey(role, gun string, key data.PrivateKey) error { - return nil -} - // Test signing and ensure the expected signature is added func TestBasicSign(t *testing.T) { cs := NewEd25519() @@ -261,7 +210,7 @@ func TestSignWithX509(t *testing.T) { // test signing against a service that only recognizes a RSAKey (not // RSAx509 key) - mockCryptoService := &StrictMockCryptoService{MockCryptoService{privKey}} + mockCryptoService := &MockCryptoService{privKey} testData := data.Signed{ Signed: &json.RawMessage{}, }