diff --git a/client/client_test.go b/client/client_test.go index f5fc9b3f0b..39bf752814 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -18,6 +18,7 @@ import ( "github.com/docker/notary/certs" "github.com/docker/notary/client/changelist" "github.com/docker/notary/cryptoservice" + "github.com/docker/notary/passphrase" "github.com/docker/notary/server" "github.com/docker/notary/server/storage" "github.com/docker/notary/trustmanager" @@ -238,6 +239,77 @@ func testInitRepo(t *testing.T, rootType string) { assertRepoHasExpectedMetadata(t, repo) } +// TestInitRepoAttemptsExceeded tests error handling when passphrase.Retriever +// (or rather the user) insists on an incorrect password. +func TestInitRepoAttemptsExceeded(t *testing.T) { + testInitRepoAttemptsExceeded(t, data.ECDSAKey) + if !testing.Short() { + testInitRepoAttemptsExceeded(t, data.RSAKey) + } +} + +func testInitRepoAttemptsExceeded(t *testing.T, rootType string) { + gun := "docker.com/notary" + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + defer os.RemoveAll(tempBaseDir) + + ts, _, _ := simpleTestServer(t) + defer ts.Close() + + retriever := passphrase.ConstantRetriever("password") + repo, err := NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport, retriever) + assert.NoError(t, err, "error creating repo: %s", err) + rootPubKey, err := repo.CryptoService.Create("root", rootType) + assert.NoError(t, err, "error generating root key: %s", err) + + retriever = passphrase.ConstantRetriever("incorrect password") + // repo.CryptoService’s FileKeyStore caches the unlocked private key, so to test + // private key unlocking we need a new repo instance. + repo, err = NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport, retriever) + assert.NoError(t, err, "error creating repo: %s", err) + err = repo.Initialize(rootPubKey.ID()) + assert.EqualError(t, err, trustmanager.ErrAttemptsExceeded{}.Error()) +} + +// TestInitRepoPasswordInvalid tests error handling when passphrase.Retriever +// (or rather the user) fails to provide a correct password. +func TestInitRepoPasswordInvalid(t *testing.T) { + testInitRepoPasswordInvalid(t, data.ECDSAKey) + if !testing.Short() { + testInitRepoPasswordInvalid(t, data.RSAKey) + } +} + +func giveUpPassphraseRetriever(_, _ string, _ bool, _ int) (string, bool, error) { + return "", true, nil +} + +func testInitRepoPasswordInvalid(t *testing.T, rootType string) { + gun := "docker.com/notary" + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("", "notary-test-") + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + defer os.RemoveAll(tempBaseDir) + + ts, _, _ := simpleTestServer(t) + defer ts.Close() + + retriever := passphrase.ConstantRetriever("password") + repo, err := NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport, retriever) + assert.NoError(t, err, "error creating repo: %s", err) + rootPubKey, err := repo.CryptoService.Create("root", rootType) + assert.NoError(t, err, "error generating root key: %s", err) + + // repo.CryptoService’s FileKeyStore caches the unlocked private key, so to test + // private key unlocking we need a new repo instance. + repo, err = NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport, giveUpPassphraseRetriever) + assert.NoError(t, err, "error creating repo: %s", err) + err = repo.Initialize(rootPubKey.ID()) + assert.EqualError(t, err, trustmanager.ErrPasswordInvalid{}.Error()) +} + // TestAddTarget adds a target to the repo and confirms that the changelist // is updated correctly. // We test this with both an RSA and ECDSA root key diff --git a/cryptoservice/crypto_service.go b/cryptoservice/crypto_service.go index 0e4f0e7be3..d473564da4 100644 --- a/cryptoservice/crypto_service.go +++ b/cryptoservice/crypto_service.go @@ -82,10 +82,15 @@ func (cs *CryptoService) GetPrivateKey(keyID string) (k data.PrivateKey, role st for _, ks := range cs.keyStores { for _, keyPath := range keyPaths { k, role, err = ks.GetKey(keyPath) - if err != nil { + if err == nil { + return + } + switch err.(type) { + case trustmanager.ErrPasswordInvalid, trustmanager.ErrAttemptsExceeded: + return + default: continue } - return } } return // returns whatever the final values were diff --git a/cryptoservice/crypto_service_test.go b/cryptoservice/crypto_service_test.go index 6909b231ea..3bd1c9f4e7 100644 --- a/cryptoservice/crypto_service_test.go +++ b/cryptoservice/crypto_service_test.go @@ -3,12 +3,15 @@ package cryptoservice import ( "crypto/rand" "fmt" + "io/ioutil" + "os" "path/filepath" "runtime" "testing" "github.com/stretchr/testify/assert" + "github.com/docker/notary/passphrase" "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" @@ -92,7 +95,10 @@ func (c CryptoServiceTester) TestGetNonexistentKey(t *testing.T) { c.errorMsg("non-nil result for bogus keyid")) _, _, err := cryptoService.GetPrivateKey("boguskeyid") - assert.NotNil(t, err) + assert.Error(t, err) + // The underlying error has been correctly propagated. + _, ok := err.(*trustmanager.ErrKeyNotFound) + assert.True(t, ok) } // asserts that signing with a created key creates a valid signature @@ -151,6 +157,61 @@ func (c CryptoServiceTester) TestGetPrivateKeyMultipleKeystores(t *testing.T) { assert.Equal(t, privKey.ID(), foundKey.ID()) } +func giveUpPassphraseRetriever(_, _ string, _ bool, _ int) (string, bool, error) { + return "", true, nil +} + +// Test that ErrPasswordInvalid is correctly propagated +func (c CryptoServiceTester) TestGetPrivateKeyPasswordInvalid(t *testing.T) { + tempBaseDir, err := ioutil.TempDir("", "cs-test-") + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + defer os.RemoveAll(tempBaseDir) + + // Do not use c.cryptoServiceFactory(), we need a KeyFileStore. + retriever := passphrase.ConstantRetriever("password") + store, err := trustmanager.NewKeyFileStore(tempBaseDir, retriever) + assert.NoError(t, err) + cryptoService := NewCryptoService(c.gun, store) + pubKey, err := cryptoService.Create(c.role, c.keyAlgo) + assert.NoError(t, err, "error generating key: %s", err) + + // cryptoService's FileKeyStore caches the unlocked private key, so to test + // private key unlocking we need a new instance. + store, err = trustmanager.NewKeyFileStore(tempBaseDir, giveUpPassphraseRetriever) + assert.NoError(t, err) + cryptoService = NewCryptoService(c.gun, store) + + _, _, err = cryptoService.GetPrivateKey(pubKey.ID()) + assert.EqualError(t, err, trustmanager.ErrPasswordInvalid{}.Error()) +} + +// Test that ErrAtttemptsExceeded is correctly propagated +func (c CryptoServiceTester) TestGetPrivateKeyAttemptsExceeded(t *testing.T) { + tempBaseDir, err := ioutil.TempDir("", "cs-test-") + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + defer os.RemoveAll(tempBaseDir) + + // Do not use c.cryptoServiceFactory(), we need a KeyFileStore. + retriever := passphrase.ConstantRetriever("password") + store, err := trustmanager.NewKeyFileStore(tempBaseDir, retriever) + assert.NoError(t, err) + cryptoService := NewCryptoService(c.gun, store) + pubKey, err := cryptoService.Create(c.role, c.keyAlgo) + assert.NoError(t, err, "error generating key: %s", err) + + // trustmanager.KeyFileStore and trustmanager.KeyMemoryStore both cache the unlocked + // private key, so to test private key unlocking we need a new instance using the + // same underlying storage; this also makes trustmanager.KeyMemoryStore (and + // c.cryptoServiceFactory()) unsuitable. + retriever = passphrase.ConstantRetriever("incorrect password") + store, err = trustmanager.NewKeyFileStore(tempBaseDir, retriever) + assert.NoError(t, err) + cryptoService = NewCryptoService(c.gun, store) + + _, _, err = cryptoService.GetPrivateKey(pubKey.ID()) + assert.EqualError(t, err, trustmanager.ErrAttemptsExceeded{}.Error()) +} + // asserts that removing key that exists succeeds func (c CryptoServiceTester) TestRemoveCreatedKey(t *testing.T) { cryptoService := c.cryptoServiceFactory() @@ -278,6 +339,8 @@ func testCryptoService(t *testing.T, gun string) { cst.TestRemoveCreatedKey(t) cst.TestRemoveFromMultipleKeystores(t) cst.TestListFromMultipleKeystores(t) + cst.TestGetPrivateKeyPasswordInvalid(t) + cst.TestGetPrivateKeyAttemptsExceeded(t) } } }