diff --git a/tuf/signed/errors.go b/tuf/signed/errors.go index 79617a1eb4..07f6193394 100644 --- a/tuf/signed/errors.go +++ b/tuf/signed/errors.go @@ -8,17 +8,17 @@ import ( // ErrInsufficientSignatures - can not create enough signatures on a piece of // metadata type ErrInsufficientSignatures struct { - FoundKeys int - NeededKeys int - KeyIDs []string + FoundKeys int + NeededKeys int + MissingKeyIDs []string } func (e ErrInsufficientSignatures) Error() string { - candidates := strings.Join(e.KeyIDs, ", ") + candidates := strings.Join(e.MissingKeyIDs, ", ") if e.FoundKeys == 0 { return fmt.Sprintf("signing keys not available, need %d keys out of: %s", e.NeededKeys, candidates) } - return fmt.Sprintf("not enough signing keys: got %d of %d needed keys out of: %s", + return fmt.Sprintf("not enough signing keys: got %d of %d needed keys, other candidates: %s", e.FoundKeys, e.NeededKeys, candidates) } diff --git a/tuf/signed/sign.go b/tuf/signed/sign.go index c3b680209c..2ffa26dab3 100644 --- a/tuf/signed/sign.go +++ b/tuf/signed/sign.go @@ -15,6 +15,7 @@ import ( "crypto/rand" "github.com/Sirupsen/logrus" + "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/utils" ) @@ -32,21 +33,24 @@ func Sign(service CryptoService, s *data.Signed, primaryKeys []data.PublicKey, signatures := make([]data.Signature, 0, len(s.Signatures)+1) signingKeyIDs := make(map[string]struct{}) tufIDs := make(map[string]data.PublicKey) - ids := make([]string, 0, len(primaryKeys)) privKeys := make(map[string]data.PrivateKey) // Get all the private key objects related to the public keys + missingKeyIDs := []string{} for _, key := range primaryKeys { canonicalID, err := utils.CanonicalKeyID(key) - ids = append(ids, canonicalID) tufIDs[key.ID()] = key if err != nil { - continue + return err } k, _, err := service.GetPrivateKey(canonicalID) if err != nil { - continue + if _, ok := err.(trustmanager.ErrKeyNotFound); ok { + missingKeyIDs = append(missingKeyIDs, canonicalID) + continue + } + return err } privKeys[key.ID()] = k } @@ -54,7 +58,7 @@ func Sign(service CryptoService, s *data.Signed, primaryKeys []data.PublicKey, // Check to ensure we have enough signing keys if len(privKeys) < minSignatures { return ErrInsufficientSignatures{FoundKeys: len(privKeys), - NeededKeys: minSignatures, KeyIDs: ids} + NeededKeys: minSignatures, MissingKeyIDs: missingKeyIDs} } for _, key := range optionalKeys { @@ -78,7 +82,7 @@ func Sign(service CryptoService, s *data.Signed, primaryKeys []data.PublicKey, sig, err := pk.Sign(rand.Reader, *s.Signed, nil) if err != nil { logrus.Debugf("Failed to sign with key: %s. Reason: %v", keyID, err) - continue + return err } signingKeyIDs[keyID] = struct{}{} signatures = append(signatures, data.Signature{ @@ -88,12 +92,6 @@ func Sign(service CryptoService, s *data.Signed, primaryKeys []data.PublicKey, }) } - // Check we produced enough signatures - if len(signatures) < minSignatures { - return ErrInsufficientSignatures{FoundKeys: len(signatures), - NeededKeys: minSignatures, KeyIDs: ids} - } - for _, sig := range s.Signatures { if _, ok := signingKeyIDs[sig.KeyID]; ok { // key is in the set of key IDs for which a signature has been created diff --git a/tuf/signed/sign_test.go b/tuf/signed/sign_test.go index bb840e4064..1483a8c76e 100644 --- a/tuf/signed/sign_test.go +++ b/tuf/signed/sign_test.go @@ -1,8 +1,10 @@ package signed import ( + "crypto" "crypto/rand" "encoding/pem" + "io" "testing" "github.com/docker/go/canonical/json" @@ -16,6 +18,22 @@ const ( testKeyPEM1 = "-----BEGIN PUBLIC KEY-----\nMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEAnKuXZeefa2LmgxaL5NsM\nzKOHNe+x/nL6ik+lDBCTV6OdcwAhHQS+PONGhrChIUVR6Vth3hUCrreLzPO73Oo5\nVSCuRJ53UronENl6lsa5mFKP8StYLvIDITNvkoT3j52BJIjyNUK9UKY9As2TNqDf\nBEPIRp28ev/NViwGOEkBu2UAbwCIdnDXm8JQErCZA0Ydm7PKGgjLbFsFGrVzqXHK\n6pdzJXlhr9yap3UpgQ/iO9JtoEYB2EXsnSrPc9JRjR30bNHHtnVql3fvinXrAEwq\n3xmN4p+R4VGzfdQN+8Kl/IPjqWB535twhFYEG/B7Ze8IwbygBjK3co/KnOPqMUrM\nBI8ztvPiogz+MvXb8WvarZ6TMTh8ifZI96r7zzqyzjR1hJulEy3IsMGvz8XS2J0X\n7sXoaqszEtXdq5ef5zKVxkiyIQZcbPgmpHLq4MgfdryuVVc/RPASoRIXG4lKaTJj\n1ANMFPxDQpHudCLxwCzjCb+sVa20HBRPTnzo8LSZkI6jAgMBAAE=\n-----END PUBLIC KEY-----" ) +type FailingPrivateKeyErr struct { +} + +func (err FailingPrivateKeyErr) Error() string { + return "FailingPrivateKey.Sign failed" +} + +// A data.PrivateKey which fails signing with a recognizable error. +type FailingPrivateKey struct { + data.PrivateKey +} + +func (fpk FailingPrivateKey) Sign(rand io.Reader, msg []byte, opts crypto.SignerOpts) (signature []byte, err error) { + return nil, FailingPrivateKeyErr{} +} + // A CryptoService which does not contain any keys. type FailingCryptoService struct { } @@ -337,3 +355,19 @@ func TestSignOptionalKeys(t *testing.T) { require.IsType(t, ErrInsufficientSignatures{}, err) } } + +func TestSignFailingKeys(t *testing.T) { + privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) + require.NoError(t, err) + cs := &MockCryptoService{FailingPrivateKey{privKey}} + + testData := data.Signed{Signed: &json.RawMessage{}} + err = Sign(cs, &testData, []data.PublicKey{privKey}, 1, nil) + require.Error(t, err) + require.IsType(t, FailingPrivateKeyErr{}, err) + + testData = data.Signed{Signed: &json.RawMessage{}} + err = Sign(cs, &testData, []data.PublicKey{}, 0, []data.PublicKey{privKey}) + require.Error(t, err) + require.IsType(t, FailingPrivateKeyErr{}, err) +}