From 7bd550a39a588461e71cbfe643b4bbddbd5817a6 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Thu, 18 Feb 2016 09:36:39 -0800 Subject: [PATCH] import refactor Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/integration_test.go | 4 +- cryptoservice/import_export.go | 37 +------- cryptoservice/import_export_test.go | 20 ++-- signer/client/signer_trust.go | 6 -- trustmanager/keyfilestore.go | 107 ++++++++++++---------- trustmanager/keyfilestore_test.go | 4 +- trustmanager/keystore.go | 2 +- trustmanager/yubikey/yubikeystore.go | 12 +-- trustmanager/yubikey/yubikeystore_test.go | 16 ++-- tuf/signed/ed25519.go | 22 ----- tuf/signed/interface.go | 5 - tuf/signed/sign_test.go | 13 --- 12 files changed, 98 insertions(+), 150 deletions(-) diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index 6356d8196f..a9ac72b9b4 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -803,7 +803,7 @@ func assertNumKeys(t *testing.T, tempDir string, numRoot, numSigning int, _, err := os.Stat(filepath.Join( tempDir, "private", "root_keys", rootKeyID+".key")) // os.IsExist checks to see if the error is because a file already - // exist, and hence doesn't actually the right funciton to use here + // exists, and hence it isn't actually the right function to use here assert.Equal(t, rootOnDisk, !os.IsNotExist(err)) // this function is declared is in the build-tagged setup files @@ -945,7 +945,7 @@ func TestClientKeyBackupAndRestore(t *testing.T) { assert.NoError(t, err) // all keys should be there, including root because the root key was backed up to disk, // and export just backs up all the keys on disk - assertNumKeys(t, dirs[1], 1, 4, true) + assertNumKeys(t, dirs[1], 1, 4, !rootOnHardware()) // can list and publish to both repos using restored keys for _, gun := range []string{"gun1", "gun2"} { diff --git a/cryptoservice/import_export.go b/cryptoservice/import_export.go index b0d8b702b4..714d8f4ef2 100644 --- a/cryptoservice/import_export.go +++ b/cryptoservice/import_export.go @@ -11,7 +11,6 @@ import ( "path/filepath" "strings" - "github.com/docker/notary" "github.com/docker/notary/passphrase" "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" @@ -102,49 +101,22 @@ func (cs *CryptoService) ExportKeyReencrypt(dest io.Writer, keyID string, newPas return nil } -// ImportRootKey imports a root in PEM format key from an io.Reader -// It prompts for the key's passphrase to verify the data and to determine -// the key ID. -func (cs *CryptoService) ImportRootKey(source io.Reader) error { - pemBytes, err := ioutil.ReadAll(source) - if err != nil { - return err - } - return cs.ImportRoleKey(pemBytes, data.CanonicalRootRole, nil) -} - // ImportRoleKey imports a private key in PEM format key from a byte array // It prompts for the key's passphrase to verify the data and to determine // the key ID. func (cs *CryptoService) ImportRoleKey(pemBytes []byte, role string, newPassphraseRetriever passphrase.Retriever) error { - var alias string var err error + keyGun := cs.gun if role == data.CanonicalRootRole { - alias = role + keyGun = "" if err = checkRootKeyIsEncrypted(pemBytes); err != nil { return err } - } else { - // Parse the private key to get the key ID so that we can import it to the correct location - privKey, err := trustmanager.ParsePEMPrivateKey(pemBytes, "") - if err != nil { - privKey, _, err = trustmanager.GetPasswdDecryptBytes(newPassphraseRetriever, pemBytes, role, string(role)) - if err != nil { - return err - } - } - // Since we're importing a non-root role, we need to pass the path as an alias - alias = filepath.Join(notary.NonRootKeysSubdir, cs.gun, privKey.ID()) - // We also need to ensure that the role is properly set in the PEM headers - pemBytes, err = trustmanager.KeyToPEM(privKey, role) - if err != nil { - return err - } } for _, ks := range cs.keyStores { // don't redeclare err, we want the value carried out of the loop - if err = ks.ImportKey(pemBytes, alias); err == nil { + if err = ks.ImportKey(pemBytes, role, keyGun); err == nil { return nil //bail on the first keystore we import to } } @@ -215,11 +187,12 @@ func (cs *CryptoService) ImportKeysZip(zipReader zip.Reader) error { } for keyName, pemBytes := range newKeys { + _, keyInfo := trustmanager.KeyInfoFromPEM(pemBytes, keyName) // try to import the key to all key stores. As long as one of them // succeeds, consider it a success var tmpErr error for _, ks := range cs.keyStores { - if err := ks.ImportKey(pemBytes, keyName); err != nil { + if err := ks.ImportKey(pemBytes, keyInfo.Role, keyInfo.Gun); err != nil { tmpErr = err } else { tmpErr = nil diff --git a/cryptoservice/import_export_test.go b/cryptoservice/import_export_test.go index 43412f76db..2ba53e1efc 100644 --- a/cryptoservice/import_export_test.go +++ b/cryptoservice/import_export_test.go @@ -300,9 +300,11 @@ func TestImportExportRootKey(t *testing.T) { keyReader, err := os.Open(tempKeyFilePath) assert.NoError(t, err, "could not open key file") - err = cs2.ImportRootKey(keyReader) - assert.NoError(t, err) + pemImportBytes, err := ioutil.ReadAll(keyReader) keyReader.Close() + assert.NoError(t, err) + err = cs2.ImportRoleKey(pemImportBytes, data.CanonicalRootRole, nil) + assert.NoError(t, err) // Look for repo's root key in repo2 // There should be a file named after the key ID of the root key we @@ -320,11 +322,15 @@ func TestImportExportRootKey(t *testing.T) { decryptedPEMBytes, err := trustmanager.KeyToPEM(privKey, "root") assert.NoError(t, err, "could not convert key to PEM") - err = cs2.ImportRootKey(bytes.NewReader(decryptedPEMBytes)) + pemImportBytes, err = ioutil.ReadAll(bytes.NewReader(decryptedPEMBytes)) + assert.NoError(t, err) + err = cs2.ImportRoleKey(pemImportBytes, data.CanonicalRootRole, nil) assert.EqualError(t, err, ErrRootKeyNotEncrypted.Error()) // Try to import garbage and make sure it doesn't succeed - err = cs2.ImportRootKey(strings.NewReader("this is not PEM")) + pemImportBytes, err = ioutil.ReadAll(strings.NewReader("this is not PEM")) + assert.NoError(t, err) + err = cs2.ImportRoleKey(pemImportBytes, data.CanonicalRootRole, nil) assert.EqualError(t, err, ErrNoValidPrivateKey.Error()) // Should be able to unlock the root key with the old password @@ -368,9 +374,11 @@ func TestImportExportRootKeyReencrypt(t *testing.T) { keyReader, err := os.Open(tempKeyFilePath) assert.NoError(t, err, "could not open key file") - err = cs2.ImportRootKey(keyReader) - assert.NoError(t, err) + pemImportBytes, err := ioutil.ReadAll(keyReader) keyReader.Close() + assert.NoError(t, err) + err = cs2.ImportRoleKey(pemImportBytes, data.CanonicalRootRole, nil) + assert.NoError(t, err) // Look for repo's root key in repo2 // There should be a file named after the key ID of the root key we diff --git a/signer/client/signer_trust.go b/signer/client/signer_trust.go index 91d1706a0b..5afb49e590 100644 --- a/signer/client/signer_trust.go +++ b/signer/client/signer_trust.go @@ -202,9 +202,3 @@ func (trust *NotarySigner) CheckHealth(timeout time.Duration) error { return err } - -// ImportRootKey satisfies the CryptoService interface. It should not be implemented -// for a NotarySigner. -func (trust *NotarySigner) ImportRootKey(r io.Reader) error { - return errors.New("Importing a root key to NotarySigner is not supported") -} diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 344b4273c0..1b790054ca 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -70,12 +70,37 @@ func generateKeyInfoMap(s LimitedFileStore) map[string]KeyInfo { logrus.Error(err) continue } - keyID, keyInfo := getImportedKeyInfo(d, keyPath) + keyID, keyInfo := KeyInfoFromPEM(d, keyPath) keyInfoMap[keyID] = keyInfo } return keyInfoMap } +// Attempts to infer the keyID, role, and GUN from the specified key path. +// Note that non-root roles can only be inferred if this is a legacy style filename: KEYID_ROLE.key +func inferKeyInfoFromKeyPath(keyPath string) (string, string, string) { + var keyID, role, gun string + keyID = filepath.Base(keyPath) + underscoreIndex := strings.LastIndex(keyID, "_") + + // This is the legacy KEYID_ROLE filename + // The keyID is the first part of the keyname + // The keyRole is the second part of the keyname + // in a key named abcde_root, abcde is the keyID and root is the KeyAlias + if underscoreIndex != -1 { + role = keyID[underscoreIndex+1:] + keyID = keyID[:underscoreIndex] + } + + if filepath.HasPrefix(keyPath, notary.RootKeysSubdir+"/") { + return keyID, data.CanonicalRootRole, "" + } + + keyPath = strings.TrimPrefix(keyPath, notary.NonRootKeysSubdir+"/") + gun = getGunFromFullID(keyPath) + return keyID, role, gun +} + func getGunFromFullID(fullKeyID string) string { keyGun := filepath.Dir(fullKeyID) // If the gun is empty, Dir will return . @@ -182,14 +207,16 @@ func (s *KeyFileStore) ExportKey(name string) ([]byte, error) { } // ImportKey imports the private key in the encrypted bytes into the keystore -// with the given key ID and alias. -func (s *KeyFileStore) ImportKey(pemBytes []byte, alias string) error { - err := importKey(s, s.Retriever, s.cachedKeys, alias, pemBytes) +// with the given key ID, role, and gun. +func (s *KeyFileStore) ImportKey(pemBytes []byte, role, gun string) error { + if data.IsDelegation(role) { + gun = "" + } + keyID, err := importKey(s, s.Retriever, s.cachedKeys, role, gun, pemBytes) if err != nil { return err } - keyID, keyInfo := getImportedKeyInfo(pemBytes, alias) - s.keyInfoMap[keyID] = keyInfo + s.keyInfoMap[keyID] = KeyInfo{Role: role, Gun: gun} return nil } @@ -291,45 +318,28 @@ func (s *KeyMemoryStore) ExportKey(name string) ([]byte, error) { // ImportKey imports the private key in the encrypted bytes into the keystore // with the given key ID and alias. -func (s *KeyMemoryStore) ImportKey(pemBytes []byte, alias string) error { - err := importKey(s, s.Retriever, s.cachedKeys, alias, pemBytes) +func (s *KeyMemoryStore) ImportKey(pemBytes []byte, role, gun string) error { + if data.IsDelegation(role) { + gun = "" + } + keyID, err := importKey(s, s.Retriever, s.cachedKeys, role, gun, pemBytes) if err != nil { return err } - keyID, keyInfo := getImportedKeyInfo(pemBytes, alias) - s.keyInfoMap[keyID] = keyInfo + s.keyInfoMap[keyID] = KeyInfo{Role: role, Gun: gun} return nil } -func getImportedKeyInfo(pemBytes []byte, filename string) (string, KeyInfo) { - var keyID, gun, role string - // Try to read the role and gun from the filepath and PEM headers - if filepath.HasPrefix(filename, notary.RootKeysSubdir+"/") { - role = data.CanonicalRootRole - gun = "" - filename = strings.TrimPrefix(filename, notary.RootKeysSubdir+"/") - } else { - filename = strings.TrimPrefix(filename, notary.NonRootKeysSubdir+"/") - gun = getGunFromFullID(filename) - } - keyIDFull := filepath.Base(filename) - underscoreIndex := strings.LastIndex(keyIDFull, "_") - if underscoreIndex == -1 { +// 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) { + keyID, role, gun := inferKeyInfoFromKeyPath(filename) + if role == "" { block, _ := pem.Decode(pemBytes) if block != nil { if keyRole, ok := block.Headers["role"]; ok { role = keyRole } } - keyID = filepath.Base(keyIDFull) - } else { - // This is the legacy KEYID_ROLE filename - // The keyID is the first part of the keyname - // The keyRole is the second part of the keyname - // in a key named abcde_root, abcde is the keyID and root is the KeyAlias - legacyID := keyIDFull - keyID = legacyID[:underscoreIndex] - role = legacyID[underscoreIndex+1:] } return keyID, KeyInfo{Gun: gun, Role: role} } @@ -521,20 +531,23 @@ func encryptAndAddKey(s LimitedFileStore, passwd string, cachedKeys map[string]* return s.Add(filepath.Join(getSubdir(role), name), pemPrivKey) } -func importKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, alias string, pemBytes []byte) error { - - if alias != data.CanonicalRootRole { - return s.Add(alias, pemBytes) - } - - privKey, passphrase, err := GetPasswdDecryptBytes( - passphraseRetriever, pemBytes, "", "imported "+alias) - +func importKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cachedKeys map[string]*cachedKey, role, gun string, pemBytes []byte) (string, error) { + // See if the key is encrypted. If its encrypted we'll fail to parse the private key on this first check + privKey, err := ParsePEMPrivateKey(pemBytes, "") if err != nil { - return err + var password string + privKey, password, err = GetPasswdDecryptBytes(passphraseRetriever, pemBytes, "", "imported "+role) + if err != nil { + return "", err + } + if role == data.CanonicalRootRole { + return privKey.ID(), encryptAndAddKey(s, password, cachedKeys, privKey.ID(), role, privKey) + } } - - var name string - name = privKey.ID() - return encryptAndAddKey(s, passphrase, cachedKeys, name, alias, privKey) + pemBytesWithRole, err := KeyToPEM(privKey, role) + if err != nil { + return privKey.ID(), err + } + s.Add(filepath.Join(getSubdir(role), gun, privKey.ID()), pemBytesWithRole) + return privKey.ID(), nil } diff --git a/trustmanager/keyfilestore_test.go b/trustmanager/keyfilestore_test.go index 4fdb8e3a06..f44130489a 100644 --- a/trustmanager/keyfilestore_test.go +++ b/trustmanager/keyfilestore_test.go @@ -771,10 +771,10 @@ func assertExportKeySuccess( func assertImportKeySuccess( t *testing.T, s KeyStore, expectedKey data.PrivateKey) { - pemBytes, err := EncryptPrivateKey(expectedKey, "root", cannedPassphrase) + pemBytes, err := EncryptPrivateKey(expectedKey, data.CanonicalRootRole, cannedPassphrase) assert.NoError(t, err) - err = s.ImportKey(pemBytes, "root") + err = s.ImportKey(pemBytes, data.CanonicalRootRole, "") assert.NoError(t, err) reimportedKey, reimportedAlias, err := s.GetKey(expectedKey.ID()) diff --git a/trustmanager/keystore.go b/trustmanager/keystore.go index b663c53abb..a6f98ecd34 100644 --- a/trustmanager/keystore.go +++ b/trustmanager/keystore.go @@ -48,7 +48,7 @@ type KeyStore interface { ListKeys() map[string]KeyInfo RemoveKey(name string) error ExportKey(name string) ([]byte, error) - ImportKey(pemBytes []byte, alias string) error + ImportKey(pemBytes []byte, role, gun string) error Name() string } diff --git a/trustmanager/yubikey/yubikeystore.go b/trustmanager/yubikey/yubikeystore.go index d4924efb78..f9f1372d22 100644 --- a/trustmanager/yubikey/yubikeystore.go +++ b/trustmanager/yubikey/yubikeystore.go @@ -762,19 +762,19 @@ func (s *YubiKeyStore) ExportKey(keyID string) ([]byte, error) { return nil, errors.New("Keys cannot be exported from a Yubikey.") } -// ImportKey imports a root key into a Yubikey -func (s *YubiKeyStore) ImportKey(pemBytes []byte, keyPath string) error { - logrus.Debugf("Attempting to import: %s key inside of YubiKeyStore", keyPath) - if keyPath != data.CanonicalRootRole { +// ImportKey imports only root keys into a Yubikey, ignoring the specified gun +func (s *YubiKeyStore) ImportKey(pemBytes []byte, role, gun string) error { + logrus.Debugf("Attempting to import: %s key inside of YubiKeyStore", role) + if role != data.CanonicalRootRole { return fmt.Errorf("yubikey only supports storing root keys") } privKey, _, err := trustmanager.GetPasswdDecryptBytes( s.passRetriever, pemBytes, "", "imported root") if err != nil { - logrus.Debugf("Failed to get and retrieve a key from: %s", keyPath) + logrus.Debugf("Failed to get and retrieve a key from: %s", role) return err } - _, err = s.addKey(privKey.ID(), "root", privKey) + _, err = s.addKey(privKey.ID(), role, privKey) return err } diff --git a/trustmanager/yubikey/yubikeystore_test.go b/trustmanager/yubikey/yubikeystore_test.go index 81801aa30d..d7d4a46465 100644 --- a/trustmanager/yubikey/yubikeystore_test.go +++ b/trustmanager/yubikey/yubikeystore_test.go @@ -339,10 +339,10 @@ func TestYubiImportNewKey(t *testing.T) { privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err) - pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "root", "passphrase") + pemBytes, err := trustmanager.EncryptPrivateKey(privKey, data.CanonicalRootRole, "passphrase") assert.NoError(t, err) - err = store.ImportKey(pemBytes, "root") + err = store.ImportKey(pemBytes, data.CanonicalRootRole, "") assert.NoError(t, err) // key is not in backup store @@ -388,9 +388,9 @@ func TestYubiImportExistingKey(t *testing.T) { assert.NotNil(t, k) // import the key, which should have already been added to the yubikey - pemBytes, err := trustmanager.EncryptPrivateKey(key, "root", "passphrase") + pemBytes, err := trustmanager.EncryptPrivateKey(key, data.CanonicalRootRole, "passphrase") assert.NoError(t, err) - err = newStore.ImportKey(pemBytes, "root") + err = newStore.ImportKey(pemBytes, data.CanonicalRootRole, "") assert.NoError(t, err) // key is not in backup store @@ -418,10 +418,10 @@ func TestYubiImportNonRootKey(t *testing.T) { privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err) - pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "root", "passphrase") + pemBytes, err := trustmanager.EncryptPrivateKey(privKey, data.CanonicalRootRole, "passphrase") assert.NoError(t, err) - err = store.ImportKey(pemBytes, privKey.ID()) + err = store.ImportKey(pemBytes, data.CanonicalTargetsRole, "") assert.Error(t, err) // key is not in backup store @@ -690,10 +690,10 @@ func TestYubiImportKeyCleansUpOnError(t *testing.T) { privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err) - pemBytes, err := trustmanager.EncryptPrivateKey(privKey, "root", "passphrase") + pemBytes, err := trustmanager.EncryptPrivateKey(privKey, data.CanonicalRootRole, "passphrase") assert.NoError(t, err) - var _importkey = func() error { return store.ImportKey(pemBytes, "root") } + var _importkey = func() error { return store.ImportKey(pemBytes, data.CanonicalRootRole, "") } testYubiFunctionCleansUpOnLoginError(t, store, _importkey) // all the PKCS11 functions ImportKey depends on that aren't the login/logout diff --git a/tuf/signed/ed25519.go b/tuf/signed/ed25519.go index bdfff2515c..dbc0a342cd 100644 --- a/tuf/signed/ed25519.go +++ b/tuf/signed/ed25519.go @@ -3,10 +3,7 @@ package signed import ( "crypto/rand" "errors" - "io" - "io/ioutil" - "github.com/agl/ed25519" "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" ) @@ -108,22 +105,3 @@ func (e *Ed25519) GetPrivateKey(keyID string) (data.PrivateKey, string, error) { } return nil, "", trustmanager.ErrKeyNotFound{KeyID: keyID} } - -// ImportRootKey adds an Ed25519 key to the store as a root key -func (e *Ed25519) ImportRootKey(r io.Reader) error { - raw, err := ioutil.ReadAll(r) - if err != nil { - return err - } - dataSize := ed25519.PublicKeySize + ed25519.PrivateKeySize - if len(raw) < dataSize || len(raw) > dataSize { - return errors.New("Wrong length of data for Ed25519 Key Import") - } - public := data.NewED25519PublicKey(raw[:ed25519.PublicKeySize]) - private, err := data.NewED25519PrivateKey(*public, raw[ed25519.PublicKeySize:]) - e.keys[private.ID()] = edCryptoKey{ - role: "root", - privKey: private, - } - return nil -} diff --git a/tuf/signed/interface.go b/tuf/signed/interface.go index 545a1c4d1c..3f1f5675e8 100644 --- a/tuf/signed/interface.go +++ b/tuf/signed/interface.go @@ -2,7 +2,6 @@ package signed import ( "github.com/docker/notary/tuf/data" - "io" ) // KeyService provides management of keys locally. It will never @@ -33,10 +32,6 @@ type KeyService interface { // ListAllKeys returns a map of all available signing key IDs to role ListAllKeys() map[string]string - - // ImportRootKey imports a root key to the highest priority keystore associated with - // the cryptoservice - ImportRootKey(source io.Reader) error } // CryptoService is deprecated and all instances of its use should be diff --git a/tuf/signed/sign_test.go b/tuf/signed/sign_test.go index ed9ce83fba..147859f061 100644 --- a/tuf/signed/sign_test.go +++ b/tuf/signed/sign_test.go @@ -3,7 +3,6 @@ package signed import ( "crypto/rand" "encoding/pem" - "io" "testing" "github.com/docker/notary/cryptoservice" @@ -60,10 +59,6 @@ func (mts *FailingCryptoService) RemoveKey(keyID string) error { return nil } -func (mts *FailingCryptoService) ImportRootKey(r io.Reader) error { - return nil -} - type MockCryptoService struct { testKey data.PrivateKey } @@ -104,10 +99,6 @@ func (mts *MockCryptoService) RemoveKey(keyID string) error { return nil } -func (mts *MockCryptoService) ImportRootKey(r io.Reader) error { - return nil -} - var _ CryptoService = &MockCryptoService{} type StrictMockCryptoService struct { @@ -138,10 +129,6 @@ func (mts *StrictMockCryptoService) AddKey(key data.PrivateKey, role string) err return nil } -func (mts *StrictMockCryptoService) ImportRootKey(r io.Reader) error { - return nil -} - // Test signing and ensure the expected signature is added func TestBasicSign(t *testing.T) { cs := NewEd25519()