Fix bug where the yubikey store was not prioritized over the filestore

in a client repo.

Also, fix a test with exporting/importing all keys - because a key
that is imported into the yubikey is also backed up on disk, when exporting
all keys, it also gets exported.

Signed-off-by: Ying Li <ying.li@docker.com>

client/repo_pkcs11.go

@@ -26,7 +26,7 @@ func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper,
 	keyStores := []trustmanager.KeyStore{fileKeyStore}
 	yubiKeyStore, _ := yubikey.NewYubiKeyStore(fileKeyStore, retriever)
 	if yubiKeyStore != nil {
-		keyStores = append(keyStores, yubiKeyStore)
+		keyStores = []trustmanager.KeyStore{yubiKeyStore, fileKeyStore}
 	}
 
 	return repositoryFromKeystores(baseDir, gun, baseURL, rt, keyStores)

cmd/notary/integration_nonpkcs11_test.go

@@ -8,27 +8,18 @@ import (
 	"github.com/docker/notary/passphrase"
 )
 
+func init() {
+	fake := passphrase.ConstantRetriever("pass")
+	retriever = fake
+	getRetriever = func() passphrase.Retriever { return fake }
+}
+
 func rootOnHardware() bool {
 	return false
 }
 
-// Per-test set up that returns a cleanup function.  This set up changes the
-// passphrase retriever to always produce a constant passphrase
-func setUp(t *testing.T) func() {
-	oldRetriever := retriever
-
-	var fake = func(k, a string, c bool, n int) (string, bool, error) {
-		return testPassphrase, false, nil
-	}
-
-	retriever = fake
-	getRetriever = func() passphrase.Retriever { return fake }
-
-	return func() {
-		retriever = oldRetriever
-		getRetriever = getPassphraseRetriever
-	}
-}
+// Per-test set up that is a no-op
+func setUp(t *testing.T) {}
 
 // no-op
 func verifyRootKeyOnHardware(t *testing.T, rootKeyID string) {}

cmd/notary/integration_pkcs11_test.go

@@ -11,13 +11,8 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-var rootOnHardware = yubikey.YubikeyAccessible
-
-// Per-test set up that returns a cleanup function.  This set up:
-// - changes the passphrase retriever to always produce a constant passphrase
-// - disables touch on yubikeys
-// - deletes all keys on the yubikey
-func setUp(t *testing.T) func() {
+func init() {
+	yubikey.SetYubikeyKeyMode(yubikey.KeymodeNone)
 	oldRetriever := retriever
 
 	var fake = func(k, a string, c bool, n int) (string, bool, error) {
@@ -29,21 +24,27 @@ func setUp(t *testing.T) func() {
 
 	retriever = fake
 	getRetriever = func() passphrase.Retriever { return fake }
-	yubikey.SetYubikeyKeyMode(yubikey.KeymodeNone)
 
-	// //we're just removing keys here, so nil is fine
+	// best effort at removing keys here, so nil is fine
+	s, err := yubikey.NewYubiKeyStore(nil, retriever)
+	if err != nil {
+		for k := range s.ListKeys() {
+			s.RemoveKey(k)
+		}
+	}
+}
+
+var rootOnHardware = yubikey.YubikeyAccessible
+
+// Per-test set up deletes all keys on the yubikey
+func setUp(t *testing.T) {
+	//we're just removing keys here, so nil is fine
 	s, err := yubikey.NewYubiKeyStore(nil, retriever)
 	assert.NoError(t, err)
 	for k := range s.ListKeys() {
 		err := s.RemoveKey(k)
 		assert.NoError(t, err)
 	}
-
-	return func() {
-		retriever = oldRetriever
-		getRetriever = getPassphraseRetriever
-		yubikey.SetYubikeyKeyMode(yubikey.KeymodeTouch | yubikey.KeymodePinOnce)
-	}
 }
 
 // ensures that the root is actually on the yubikey - this makes sure the

cmd/notary/integration_test.go

@@ -75,8 +75,7 @@ func setupServer() *httptest.Server {
 // verifies the target, and then removes the target.
 func TestClientTufInteraction(t *testing.T) {
 	// -- setup --
-	cleanup := setUp(t)
-	defer cleanup()
+	setUp(t)
 
 	tempDir := tempDirWithConfig(t, "{}")
 	defer os.RemoveAll(tempDir)
@@ -255,8 +254,7 @@ func assertSuccessfullyPublish(
 // Tests root key generation and key rotation
 func TestClientKeyGenerationRotation(t *testing.T) {
 	// -- setup --
-	cleanup := setUp(t)
-	defer cleanup()
+	setUp(t)
 
 	tempDir := tempDirWithConfig(t, "{}")
 	defer os.RemoveAll(tempDir)
@@ -333,8 +331,7 @@ func TestClientKeyGenerationRotation(t *testing.T) {
 // able to publish successfully
 func TestClientKeyBackupAndRestore(t *testing.T) {
 	// -- setup --
-	cleanup := setUp(t)
-	defer cleanup()
+	setUp(t)
 
 	dirs := make([]string, 3)
 	for i := 0; i < 3; i++ {
@@ -380,7 +377,9 @@ func TestClientKeyBackupAndRestore(t *testing.T) {
 
 	_, err = runCommand(t, dirs[1], "key", "restore", zipfile)
 	assert.NoError(t, err)
-	assertNumKeys(t, dirs[1], 1, 4, !rootOnHardware()) // all keys should be there
+	// 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)
 
 	// can list and publish to both repos using restored keys
 	for _, gun := range []string{"gun1", "gun2"} {
@@ -438,8 +437,7 @@ func exportRoot(t *testing.T, exportTo string) string {
 // Tests import/export root key only
 func TestClientKeyImportExportRootOnly(t *testing.T) {
 	// -- setup --
-	cleanup := setUp(t)
-	defer cleanup()
+	setUp(t)
 
 	tempDir := tempDirWithConfig(t, "{}")
 	defer os.RemoveAll(tempDir)
@@ -513,8 +511,7 @@ func assertNumCerts(t *testing.T, tempDir string, expectedNum int) []string {
 // TestClientCertInteraction
 func TestClientCertInteraction(t *testing.T) {
 	// -- setup --
-	cleanup := setUp(t)
-	defer cleanup()
+	setUp(t)
 
 	tempDir := tempDirWithConfig(t, "{}")
 	defer os.RemoveAll(tempDir)
@@ -547,8 +544,7 @@ func TestClientCertInteraction(t *testing.T) {
 // Tests default root key generation
 func TestDefaultRootKeyGeneration(t *testing.T) {
 	// -- setup --
-	cleanup := setUp(t)
-	defer cleanup()
+	setUp(t)
 
 	tempDir := tempDirWithConfig(t, "{}")
 	defer os.RemoveAll(tempDir)