diff --git a/cmd/notary/keys.go b/cmd/notary/keys.go index 3e44898152..102fb0fdc2 100644 --- a/cmd/notary/keys.go +++ b/cmd/notary/keys.go @@ -302,7 +302,7 @@ func (k *keyCommander) keysRestore(cmd *cobra.Command, args []string) error { if err != nil { return err } - ks, err := k.getKeyStores(config, true, true) + ks, err := k.getKeyStores(config, true, false) if err != nil { return err } diff --git a/cryptoservice/import_export.go b/cryptoservice/import_export.go index 9adcbcdba6..99445cdac0 100644 --- a/cryptoservice/import_export.go +++ b/cryptoservice/import_export.go @@ -164,7 +164,10 @@ func (cs *CryptoService) ImportKeysZip(zipReader zip.Reader, retriever passphras for keyName, pemBytes := range newKeys { // Get the key role information as well as its data.PrivateKey representation - _, keyInfo := trustmanager.KeyInfoFromPEM(pemBytes, keyName) + _, keyInfo, err := trustmanager.KeyInfoFromPEM(pemBytes, keyName) + if err != nil { + return err + } privKey, err := trustmanager.ParsePEMPrivateKey(pemBytes, "") if err != nil { privKey, _, err = trustmanager.GetPasswdDecryptBytes(retriever, pemBytes, "", "imported "+keyInfo.Role) diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index ca0732c0e6..13fe4704f3 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -70,7 +70,11 @@ func generateKeyInfoMap(s LimitedFileStore) map[string]KeyInfo { logrus.Error(err) continue } - keyID, keyInfo := KeyInfoFromPEM(d, keyPath) + keyID, keyInfo, err := KeyInfoFromPEM(d, keyPath) + if err != nil { + logrus.Error(err) + continue + } keyInfoMap[keyID] = keyInfo } return keyInfoMap @@ -184,12 +188,7 @@ func (s *KeyFileStore) RemoveKey(keyID string) error { return err } // Remove this key from our keyInfo map if we removed from our filesystem - if _, ok := s.keyInfoMap[keyID]; ok { - delete(s.keyInfoMap, keyID) - } else { - // This might be of the form GUN/ID - try to delete without the gun - delete(s.keyInfoMap, filepath.Base(keyID)) - } + delete(s.keyInfoMap, filepath.Base(keyID)) return nil } @@ -296,17 +295,18 @@ func (s *KeyMemoryStore) ExportKey(keyID string) ([]byte, error) { } // KeyInfoFromPEM attempts to get a keyID and KeyInfo from the filename and PEM bytes of a key -func KeyInfoFromPEM(pemBytes []byte, filename string) (string, KeyInfo) { +func KeyInfoFromPEM(pemBytes []byte, filename string) (string, KeyInfo, error) { keyID, role, gun := inferKeyInfoFromKeyPath(filename) if role == "" { block, _ := pem.Decode(pemBytes) - if block != nil { - if keyRole, ok := block.Headers["role"]; ok { - role = keyRole - } + if block == nil { + return "", KeyInfo{}, fmt.Errorf("could not decode PEM block for key %s", filename) + } + if keyRole, ok := block.Headers["role"]; ok { + role = keyRole } } - return keyID, KeyInfo{Gun: gun, Role: role} + return keyID, KeyInfo{Gun: gun, Role: role}, nil } func addKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, name, role string, privKey data.PrivateKey) error { diff --git a/trustmanager/yubikey/yubikeystore_test.go b/trustmanager/yubikey/yubikeystore_test.go index 8df9254d0e..6df6b4d291 100644 --- a/trustmanager/yubikey/yubikeystore_test.go +++ b/trustmanager/yubikey/yubikeystore_test.go @@ -116,6 +116,45 @@ func TestYubiAddKeysAndRetrieve(t *testing.T) { } } +// Test that we can successfully keys enough times to fill up all the slots in the Yubikey, even without a backup store +func TestYubiAddKeysWithoutBackup(t *testing.T) { + if !YubikeyAccessible() { + t.Skip("Must have Yubikey access.") + } + clearAllKeys(t) + + SetYubikeyKeyMode(KeymodeNone) + defer func() { + SetYubikeyKeyMode(KeymodeTouch | KeymodePinOnce) + }() + + // create 4 keys on the original store + store, err := NewYubiKeyStore(nil, ret) + assert.NoError(t, err) + keys := addMaxKeys(t, store) + + // create a new store, since we want to be sure the original store's cache + // is not masking any issues + cleanStore, err := NewYubiKeyStore(trustmanager.NewKeyMemoryStore(ret), ret) + assert.NoError(t, err) + + // All 4 keys should be in the original store, in the clean store (which + // makes sure the keys are actually on the Yubikey and not on the original + // store's cache) + for _, store := range []trustmanager.KeyStore{store, cleanStore} { + listedKeys := store.ListKeys() + assert.Len(t, listedKeys, numSlots) + for _, k := range keys { + r, ok := listedKeys[k] + assert.True(t, ok) + assert.Equal(t, data.CanonicalRootRole, r.Role) + + _, _, err := store.GetKey(k) + assert.NoError(t, err) + } + } +} + // We can't add a key if there are no more slots func TestYubiAddKeyFailureIfNoMoreSlots(t *testing.T) { if !YubikeyAccessible() {