From 0280a82ae09463a576a6463c79ed93126b0dd5af Mon Sep 17 00:00:00 2001 From: Ying Li Date: Sun, 8 Nov 2015 22:54:25 -0800 Subject: [PATCH] Do not back up a root key that is imported into Yubikey. Signed-off-by: Ying Li Signed-off-by: David Lawrence Signed-off-by: Ying Li (github: endophage) --- cmd/notary/integration_test.go | 44 +++++++++++++++---------------- cryptoservice/import_export.go | 4 +-- passphrase/passphrase.go | 11 +++++--- trustmanager/keyfilestore.go | 3 ++- trustmanager/yubikeystore.go | 29 +++++++++++++++----- trustmanager/yubikeystore_test.go | 39 +++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 34 deletions(-) diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index 328ce08e1b..ad3e1c509c 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -178,17 +178,18 @@ func GetKeys(t *testing.T, tempDir string) ([]string, []string) { // List keys, parses the output, and asserts something about the number of root // keys and number of signing keys, as well as returning them. -func assertNumKeys(t *testing.T, tempDir string, numRoot, numSigning int) ( - []string, []string) { +func assertNumKeys(t *testing.T, tempDir string, numRoot, numSigning int, + rootOnDisk bool) ([]string, []string) { root, signing := GetKeys(t, tempDir) assert.Len(t, root, numRoot) assert.Len(t, signing, numSigning) for _, rootKeyID := range root { - // it should always be present on disk _, err := os.Stat(filepath.Join( tempDir, "private", "root_keys", rootKeyID+"_root.key")) - assert.NoError(t, err) + // 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 + assert.Equal(t, rootOnDisk, !os.IsNotExist(err)) // this function is declared is in the build-tagged setup files verifyRootKeyOnHardware(t, rootKeyID) @@ -241,17 +242,17 @@ func TestClientKeyGenerationRotation(t *testing.T) { // -- tests -- // starts out with no keys - assertNumKeys(t, tempDir, 0, 0) + assertNumKeys(t, tempDir, 0, 0, true) // generate root key produces a single root key and no other keys _, err = runCommand(t, tempDir, "key", "generate", "ecdsa") assert.NoError(t, err) - assertNumKeys(t, tempDir, 1, 0) + assertNumKeys(t, tempDir, 1, 0, true) // initialize a repo, should have signing keys and no new root key _, err = runCommand(t, tempDir, "-s", server.URL, "init", "gun") assert.NoError(t, err) - origRoot, origSign := assertNumKeys(t, tempDir, 1, 2) + origRoot, origSign := assertNumKeys(t, tempDir, 1, 2, true) // publish using the original keys assertSuccessfullyPublish(t, tempDir, server.URL, "gun", target, tempfiles[0]) @@ -259,7 +260,7 @@ func TestClientKeyGenerationRotation(t *testing.T) { // rotate the signing keys _, err = runCommand(t, tempDir, "key", "rotate", "gun") assert.NoError(t, err) - root, sign := assertNumKeys(t, tempDir, 1, 4) + root, sign := assertNumKeys(t, tempDir, 1, 4, true) assert.Equal(t, origRoot[0], root[0]) // there should be the new keys and the old keys for _, origKey := range origSign { @@ -275,7 +276,7 @@ func TestClientKeyGenerationRotation(t *testing.T) { // publish the key rotation _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") assert.NoError(t, err) - root, sign = assertNumKeys(t, tempDir, 1, 2) + root, sign = assertNumKeys(t, tempDir, 1, 2, true) assert.Equal(t, origRoot[0], root[0]) // just do a cursory rotation check that the keys aren't equal anymore for _, origKey := range origSign { @@ -332,7 +333,7 @@ func TestClientKeyImportExportRootAndSigning(t *testing.T) { assertSuccessfullyPublish( t, dirs[0], server.URL, gun, target, tempfiles[0]) } - assertNumKeys(t, dirs[0], 1, 4) + assertNumKeys(t, dirs[0], 1, 4, true) // -- tests -- zipfile := tempfiles[0] + ".zip" @@ -344,7 +345,7 @@ func TestClientKeyImportExportRootAndSigning(t *testing.T) { _, err = runCommand(t, dirs[1], "key", "import", zipfile) assert.NoError(t, err) - assertNumKeys(t, dirs[1], 1, 4) // all keys should be there + assertNumKeys(t, dirs[1], 1, 4, true) // all keys should be there // can list and publish to both repos using imported keys for _, gun := range []string{"gun1", "gun2"} { @@ -365,16 +366,12 @@ func TestClientKeyImportExportRootAndSigning(t *testing.T) { // this function is declared is in the build-tagged setup files if rootOnHardware() { - // hardware root is still present, but two signing keys moved - we - // can't call assertNumKeys, because in this case it will ONLY be on - // hardware and not on disk - root, signing := GetKeys(t, dirs[2]) - assert.Len(t, root, 1) - verifyRootKeyOnHardware(t, root[0]) - assert.Len(t, signing, 2) + // hardware root is still present, and the key will ONLY be on hardware + // and not on disk + assertNumKeys(t, dirs[2], 1, 2, false) } else { // only 2 signing keys should be there, and no root key - assertNumKeys(t, dirs[2], 0, 2) + assertNumKeys(t, dirs[2], 0, 2, true) } } @@ -388,7 +385,7 @@ func exportRoot(t *testing.T, exportTo string) string { // generate root key produces a single root key and no other keys _, err = runCommand(t, tempDir, "key", "generate", "ecdsa") assert.NoError(t, err) - oldRoot, _ := assertNumKeys(t, tempDir, 1, 0) + oldRoot, _ := assertNumKeys(t, tempDir, 1, 0, true) // export does not require a password oldRetriever := retriever @@ -451,13 +448,16 @@ func TestClientKeyImportExportRootOnly(t *testing.T) { // import the key _, err = runCommand(t, tempDir, "key", "import-root", tempFile.Name()) assert.NoError(t, err) - newRoot, _ := assertNumKeys(t, tempDir, 1, 0) + + // if there is hardware available, root will only be on hardware, and not + // on disk + newRoot, _ := assertNumKeys(t, tempDir, 1, 0, !rootOnHardware()) assert.Equal(t, rootKeyID, newRoot[0]) // Just to make sure, init a repo and publish _, err = runCommand(t, tempDir, "-s", server.URL, "init", "gun") assert.NoError(t, err) - assertNumKeys(t, tempDir, 1, 2) + assertNumKeys(t, tempDir, 1, 2, !rootOnHardware()) assertSuccessfullyPublish( t, tempDir, server.URL, "gun", target, tempFile.Name()) } diff --git a/cryptoservice/import_export.go b/cryptoservice/import_export.go index 922779b789..7ee3ded5ad 100644 --- a/cryptoservice/import_export.go +++ b/cryptoservice/import_export.go @@ -112,8 +112,8 @@ func (cs *CryptoService) ImportRootKey(source io.Reader) error { for _, ks := range cs.keyStores { // don't redeclare err, we want the value carried out of the loop - if err = ks.ImportKey(pemBytes, "root"); err != nil { - continue + if err = ks.ImportKey(pemBytes, "root"); err == nil { + return nil //bail on the first keystore we import to } } diff --git a/passphrase/passphrase.go b/passphrase/passphrase.go index 3933ff1f71..fdaddce486 100644 --- a/passphrase/passphrase.go +++ b/passphrase/passphrase.go @@ -124,12 +124,17 @@ func PromptRetrieverWithInOut(in io.Reader, out io.Writer, aliasMap map[string]s } } + withID := fmt.Sprintf(" with ID %s", shortName) + if shortName == "" { + withID = "" + } + if createNew { - fmt.Fprintf(out, "Enter passphrase for new %s key with ID %s: ", displayAlias, shortName) + fmt.Fprintf(out, "Enter passphrase for new %s key%s: ", displayAlias, withID) } else if displayAlias == "yubikey" { fmt.Fprintf(out, "Enter the %s for the attached Yubikey: ", keyName) } else { - fmt.Fprintf(out, "Enter passphrase for %s key with ID %s: ", displayAlias, shortName) + fmt.Fprintf(out, "Enter passphrase for %s key%s: ", displayAlias, withID) } passphrase, err := stdin.ReadBytes('\n') @@ -157,7 +162,7 @@ func PromptRetrieverWithInOut(in io.Reader, out io.Writer, aliasMap map[string]s return "", false, ErrTooShort } - fmt.Fprintf(out, "Repeat passphrase for new %s key with ID %s: ", displayAlias, shortName) + fmt.Fprintf(out, "Repeat passphrase for new %s key%s: ", displayAlias, withID) confirmation, err := stdin.ReadBytes('\n') fmt.Fprintln(out) if err != nil { diff --git a/trustmanager/keyfilestore.go b/trustmanager/keyfilestore.go index 977e3e6983..18b8969b00 100644 --- a/trustmanager/keyfilestore.go +++ b/trustmanager/keyfilestore.go @@ -347,7 +347,8 @@ func importKey(s LimitedFileStore, passphraseRetriever passphrase.Retriever, cac return s.Add(alias, pemBytes) } - privKey, passphrase, err := GetPasswdDecryptBytes(passphraseRetriever, pemBytes, "imported", alias) + privKey, passphrase, err := GetPasswdDecryptBytes( + passphraseRetriever, pemBytes, "", "imported "+alias) if err != nil { return err diff --git a/trustmanager/yubikeystore.go b/trustmanager/yubikeystore.go index 1544556708..2abd36b45f 100644 --- a/trustmanager/yubikeystore.go +++ b/trustmanager/yubikeystore.go @@ -199,9 +199,11 @@ func addECDSAKey( return fmt.Errorf("error importing: %v", err) } - err = backupStore.AddKey(privKey.ID(), role, privKey) - if err != nil { - return ErrBackupFailed{err: err.Error()} + if backupStore != nil { + err = backupStore.AddKey(privKey.ID(), role, privKey) + if err != nil { + return ErrBackupFailed{err: err.Error()} + } } return nil @@ -551,6 +553,11 @@ func (s *YubiKeyStore) ListKeys() map[string]string { // AddKey puts a key inside the Yubikey, as well as writing it to the backup store func (s *YubiKeyStore) AddKey(keyID, role string, privKey data.PrivateKey) error { + return s.addKey(keyID, role, privKey, true) +} + +func (s *YubiKeyStore) addKey( + keyID, role string, privKey data.PrivateKey, backup bool) error { // We only allow adding root keys for now if role != data.CanonicalRootRole { return fmt.Errorf("yubikey only supports storing root keys, got %s for key: %s\n", role, keyID) @@ -574,7 +581,14 @@ func (s *YubiKeyStore) AddKey(keyID, role string, privKey data.PrivateKey) error return err } logrus.Debugf("Using yubikey slot %v", slot) - err = addECDSAKey(ctx, session, privKey, slot, s.passRetriever, role, s.backupStore) + + backupStore := s.backupStore + if !backup { + backupStore = nil + } + + err = addECDSAKey( + ctx, session, privKey, slot, s.passRetriever, role, backupStore) if err == nil { s.keys[privKey.ID()] = yubiSlot{ role: role, @@ -634,18 +648,21 @@ func (s *YubiKeyStore) RemoveKey(keyID string) error { return err } +// ExportKey doesn't work, because you can't export data from a Yubikey func (s *YubiKeyStore) ExportKey(keyID string) ([]byte, error) { logrus.Debugf("Attempting to export: %s key inside of YubiKeyStore", keyID) 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, keyID string) error { logrus.Debugf("Attempting to import: %s key inside of YubiKeyStore", keyID) - privKey, _, err := GetPasswdDecryptBytes(s.passRetriever, pemBytes, "imported", "root") + privKey, _, err := GetPasswdDecryptBytes( + s.passRetriever, pemBytes, "", "imported root") if err != nil { return err } - return s.AddKey(privKey.ID(), "root", privKey) + return s.addKey(privKey.ID(), "root", privKey, false) } func cleanup(ctx *pkcs11.Ctx, session pkcs11.SessionHandle) { diff --git a/trustmanager/yubikeystore_test.go b/trustmanager/yubikeystore_test.go index 6c1afe7e93..a2fe904c08 100644 --- a/trustmanager/yubikeystore_test.go +++ b/trustmanager/yubikeystore_test.go @@ -63,3 +63,42 @@ func TestAddKeyToNextEmptyYubikeySlot(t *testing.T) { // numSlots is not actually the max - some keys might have more, so do not // test that adding more keys will fail. } + +// ImportKey imports a key as root without adding it to the backup store +func TestImportKey(t *testing.T) { + if !YubikeyAccessible() { + t.Skip("Must have Yubikey access.") + } + clearAllKeys(t) + + ret := passphrase.ConstantRetriever("passphrase") + backup := NewKeyMemoryStore(ret) + store, err := NewYubiKeyStore(backup, ret) + assert.NoError(t, err) + SetYubikeyKeyMode(KeymodeNone) + defer func() { + SetYubikeyKeyMode(KeymodeTouch | KeymodePinOnce) + }() + + // generate key and import it + privKey, err := GenerateECDSAKey(rand.Reader) + assert.NoError(t, err) + + pemBytes, err := EncryptPrivateKey(privKey, "passphrase") + assert.NoError(t, err) + + err = store.ImportKey(pemBytes, privKey.ID()) + assert.NoError(t, err) + + // key is not in backup store + _, _, err = backup.GetKey(privKey.ID()) + assert.Error(t, err) + + // ensure key is in Yubikey - create a new store, to make sure we're not + // just using the keys cache + store, err = NewYubiKeyStore(NewKeyMemoryStore(ret), ret) + gottenKey, role, err := store.GetKey(privKey.ID()) + assert.NoError(t, err) + assert.Equal(t, data.CanonicalRootRole, role) + assert.Equal(t, privKey.Public(), gottenKey.Public()) +}