From ef1d5caa1ae2bf49c787c13ce3e8e5e7a04ad0f0 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Sun, 8 Nov 2015 16:09:52 -0800 Subject: [PATCH 1/3] Fix an error message when there are insufficient signatures. Signed-off-by: Ying Li Signed-off-by: David Lawrence Signed-off-by: Ying Li (github: endophage) --- tuf/signed/errors.go | 3 +-- tuf/signed/sign.go | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tuf/signed/errors.go b/tuf/signed/errors.go index c042e60685..6cddcf2ba4 100644 --- a/tuf/signed/errors.go +++ b/tuf/signed/errors.go @@ -8,11 +8,10 @@ import ( // metadata type ErrInsufficientSignatures struct { Name string - Err error } func (e ErrInsufficientSignatures) Error() string { - return fmt.Sprintf("tuf: insufficient signatures for %s: %s", e.Name, e.Err) + return fmt.Sprintf("tuf: insufficient signatures: %s", e.Name) } // ErrExpired indicates a piece of metadata has expired diff --git a/tuf/signed/sign.go b/tuf/signed/sign.go index 99987a2fb5..48a6efd7f0 100644 --- a/tuf/signed/sign.go +++ b/tuf/signed/sign.go @@ -68,8 +68,9 @@ func Sign(service CryptoService, s *data.Signed, keys ...data.PublicKey) error { } if len(signatures) < 1 { return ErrInsufficientSignatures{ - Name: fmt.Sprintf("Cryptoservice failed to produce any signatures for keys with IDs: %v", keyIDs), - Err: nil, + Name: fmt.Sprintf( + "Cryptoservice failed to produce any signatures for keys with IDs: %v", + keyIDs), } } for _, sig := range s.Signatures { From 9a01cf091d8fde7519ac202bd55d9f1a8d21a2b8 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Sun, 8 Nov 2015 16:12:59 -0800 Subject: [PATCH 2/3] Add "notary lookup" to the integration tests. Signed-off-by: Ying Li Signed-off-by: David Lawrence Signed-off-by: Ying Li (github: endophage) --- cmd/notary/integration_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index dd6dec2d41..328ce08e1b 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -118,6 +118,11 @@ func TestClientTufInteraction(t *testing.T) { assert.NoError(t, err) assert.True(t, strings.Contains(string(output), target)) + // lookup target and repo - see target + output, err = runCommand(t, tempDir, "-s", server.URL, "lookup", "gun", target) + assert.NoError(t, err) + assert.True(t, strings.Contains(string(output), target)) + // verify repo - empty file output, err = runCommand(t, tempDir, "verify", "gun", target) assert.NoError(t, err) From 53114aabdcac2adba3d1cafcd204e07df8551e05 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Sun, 8 Nov 2015 16:36:35 -0800 Subject: [PATCH 3/3] Add a test to test adding multiple keys to a yubikey. If there are existing keys on the Yubikey, the YubiKeyStore should add a key to the next available slot. Signed-off-by: Ying Li Signed-off-by: David Lawrence Signed-off-by: Ying Li (github: endophage) --- passphrase/passphrase.go | 8 ++++ trustmanager/yubikeystore.go | 6 ++- trustmanager/yubikeystore_test.go | 65 +++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 trustmanager/yubikeystore_test.go diff --git a/passphrase/passphrase.go b/passphrase/passphrase.go index 777de63721..3933ff1f71 100644 --- a/passphrase/passphrase.go +++ b/passphrase/passphrase.go @@ -182,3 +182,11 @@ func PromptRetrieverWithInOut(in io.Reader, out io.Writer, aliasMap map[string]s return retPass, false, nil } } + +// ConstantRetriever returns a new Retriever which will return a constant string +// as a passphrase. +func ConstantRetriever(constantPassphrase string) Retriever { + return func(k, a string, c bool, n int) (string, bool, error) { + return constantPassphrase, false, nil + } +} diff --git a/trustmanager/yubikeystore.go b/trustmanager/yubikeystore.go index ee8ca17ade..1544556708 100644 --- a/trustmanager/yubikeystore.go +++ b/trustmanager/yubikeystore.go @@ -627,7 +627,11 @@ func (s *YubiKeyStore) RemoveKey(keyID string) error { if !ok { return errors.New("Key not present in yubikey") } - return yubiRemoveKey(ctx, session, key.slotID, s.passRetriever, keyID) + err = yubiRemoveKey(ctx, session, key.slotID, s.passRetriever, keyID) + if err == nil { + delete(s.keys, keyID) + } + return err } func (s *YubiKeyStore) ExportKey(keyID string) ([]byte, error) { diff --git a/trustmanager/yubikeystore_test.go b/trustmanager/yubikeystore_test.go new file mode 100644 index 0000000000..6c1afe7e93 --- /dev/null +++ b/trustmanager/yubikeystore_test.go @@ -0,0 +1,65 @@ +// +build pkcs11 + +package trustmanager + +import ( + "crypto/rand" + "testing" + + "github.com/docker/notary/passphrase" + "github.com/docker/notary/tuf/data" + "github.com/stretchr/testify/assert" +) + +func clearAllKeys(t *testing.T) { + // TODO(cyli): this is creating a new yubikey store because for some reason, + // removing and then adding with the same YubiKeyStore causes + // non-deterministic failures at least on Mac OS + ret := passphrase.ConstantRetriever("passphrase") + store, err := NewYubiKeyStore(NewKeyMemoryStore(ret), ret) + assert.NoError(t, err) + + for k := range store.ListKeys() { + err := store.RemoveKey(k) + assert.NoError(t, err) + } +} + +func TestAddKeyToNextEmptyYubikeySlot(t *testing.T) { + if !YubikeyAccessible() { + t.Skip("Must have Yubikey access.") + } + clearAllKeys(t) + + ret := passphrase.ConstantRetriever("passphrase") + store, err := NewYubiKeyStore(NewKeyMemoryStore(ret), ret) + assert.NoError(t, err) + SetYubikeyKeyMode(KeymodeNone) + defer func() { + SetYubikeyKeyMode(KeymodeTouch | KeymodePinOnce) + }() + + keys := make([]string, 0, numSlots) + + // create the maximum number of keys + for i := 0; i < numSlots; i++ { + privKey, err := GenerateECDSAKey(rand.Reader) + assert.NoError(t, err) + + err = store.AddKey(privKey.ID(), data.CanonicalRootRole, privKey) + assert.NoError(t, err) + + keys = append(keys, privKey.ID()) + } + + 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) + } + + // numSlots is not actually the max - some keys might have more, so do not + // test that adding more keys will fail. +}