From 1e091a0f5623a2cad05e7eb5c79265e127761afd Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Sun, 15 Nov 2015 02:16:13 -0800 Subject: [PATCH 1/2] CryptoService.Sign is now dead code. Remove it and update tests Signed-off-by: David Lawrence (github: endophage) --- cryptoservice/crypto_service.go | 33 ---------------------------- cryptoservice/crypto_service_test.go | 26 ++++++++++++---------- signer/api/api.go | 15 ++++++++++--- signer/api/rpc_api.go | 14 ++++++++---- signer/client/signer_trust.go | 22 ------------------- tuf/signed/ed25519.go | 17 -------------- tuf/signed/interface.go | 9 -------- 7 files changed, 37 insertions(+), 99 deletions(-) diff --git a/cryptoservice/crypto_service.go b/cryptoservice/crypto_service.go index da14d75d0c..0e4f0e7be3 100644 --- a/cryptoservice/crypto_service.go +++ b/cryptoservice/crypto_service.go @@ -111,39 +111,6 @@ func (cs *CryptoService) RemoveKey(keyID string) (err error) { return // returns whatever the final values were } -// Sign returns the signatures for the payload with a set of keyIDs. It ignores -// errors to sign and expects the called to validate if the number of returned -// signatures is adequate. -func (cs *CryptoService) Sign(keyIDs []string, payload []byte) ([]data.Signature, error) { - signatures := make([]data.Signature, 0, len(keyIDs)) - for _, keyID := range keyIDs { - privKey, _, err := cs.GetPrivateKey(keyID) - if err != nil { - logrus.Debugf("error attempting to retrieve private key: %s, %v", keyID, err) - continue - } - - sigAlgo := privKey.SignatureAlgorithm() - sig, err := privKey.Sign(rand.Reader, payload, nil) - if err != nil { - logrus.Debugf("ignoring error attempting to %s sign with keyID: %s, %v", - privKey.Algorithm(), keyID, err) - continue - } - - logrus.Debugf("appending %s signature with Key ID: %s", privKey.Algorithm(), keyID) - - // Append signatures to result array - signatures = append(signatures, data.Signature{ - KeyID: keyID, - Method: sigAlgo, - Signature: sig[:], - }) - } - - return signatures, nil -} - // ListKeys returns a list of key IDs valid for the given role func (cs *CryptoService) ListKeys(role string) []string { var res []string diff --git a/cryptoservice/crypto_service_test.go b/cryptoservice/crypto_service_test.go index 41b9f45cc4..e1f73441b7 100644 --- a/cryptoservice/crypto_service_test.go +++ b/cryptoservice/crypto_service_test.go @@ -100,14 +100,17 @@ func (c CryptoServiceTester) TestSignWithKey(t *testing.T) { assert.NoError(t, err, c.errorMsg("error creating key")) // Test Sign - signatures, err := cryptoService.Sign([]string{tufKey.ID()}, content) + privKey, role, err := cryptoService.GetPrivateKey(tufKey.ID()) + assert.NoError(t, err, c.errorMsg("failed to get private key")) + assert.Equal(t, c.role, role) + + signature, err := privKey.Sign(rand.Reader, content, nil) assert.NoError(t, err, c.errorMsg("signing failed")) - assert.Len(t, signatures, 1, c.errorMsg("wrong number of signatures")) verifier, ok := signed.Verifiers[algoToSigType[c.keyAlgo]] assert.True(t, ok, c.errorMsg("Unknown verifier for algorithm")) - err = verifier.Verify(tufKey, signatures[0].Signature, content) + err = verifier.Verify(tufKey, signature, content) assert.NoError(t, err, c.errorMsg("verification failed for %s key type", c.keyAlgo)) } @@ -115,15 +118,13 @@ func (c CryptoServiceTester) TestSignWithKey(t *testing.T) { // asserts that signing, if there are no matching keys, produces no signatures func (c CryptoServiceTester) TestSignNoMatchingKeys(t *testing.T) { cryptoService := c.cryptoServiceFactory() - content := []byte("this is a secret") privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err, c.errorMsg("error creating key")) - // Test Sign with key that is not in the cryptoservice - signatures, err := cryptoService.Sign([]string{privKey.ID()}, content) - assert.NoError(t, err, c.errorMsg("signing failed")) - assert.Len(t, signatures, 0, c.errorMsg("wrong number of signatures")) + // Test Sign + _, _, err = cryptoService.GetPrivateKey(privKey.ID()) + assert.Error(t, err, c.errorMsg("Should not have found private key")) } // If there are multiple keystores, even if all of them have the same key, @@ -138,13 +139,16 @@ func (c CryptoServiceTester) TestSignWhenMultipleKeystores(t *testing.T) { assert.NoError(t, err, c.errorMsg("error creating key")) for _, store := range cryptoService.keyStores { - err := store.AddKey(privKey.ID(), "root", privKey) + err := store.AddKey(privKey.ID(), c.role, privKey) assert.NoError(t, err) } - signatures, err := cryptoService.Sign([]string{privKey.ID()}, content) + privKey, role, err := cryptoService.GetPrivateKey(privKey.ID()) + assert.NoError(t, err, c.errorMsg("failed to get private key")) + assert.Equal(t, c.role, role) + + _, err = privKey.Sign(rand.Reader, content, nil) assert.NoError(t, err, c.errorMsg("signing failed")) - assert.Len(t, signatures, 1, c.errorMsg("wrong number of signatures")) } // asserts that removing key that exists succeeds diff --git a/signer/api/api.go b/signer/api/api.go index 8a203f3673..9afce1670e 100644 --- a/signer/api/api.go +++ b/signer/api/api.go @@ -1,6 +1,7 @@ package api import ( + "crypto/rand" "encoding/json" "net/http" @@ -178,8 +179,15 @@ func Sign(cryptoServices signer.CryptoServiceIndex) http.Handler { return } - signatures, err := cryptoService.Sign([]string{sigRequest.KeyID.ID}, sigRequest.Content) - if err != nil || len(signatures) != 1 { + privKey, _, err := cryptoService.GetPrivateKey(tufKey.ID()) + if err != nil { + // We got an unexpected error + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(err.Error())) + return + } + sig, err := privKey.Sign(rand.Reader, sigRequest.Content, nil) + if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) return @@ -189,7 +197,8 @@ func Sign(cryptoServices signer.CryptoServiceIndex) http.Handler { KeyID: &pb.KeyID{ID: tufKey.ID()}, Algorithm: &pb.Algorithm{Algorithm: tufKey.Algorithm()}, }, - Content: signatures[0].Signature, + Algorithm: &pb.Algorithm{Algorithm: privKey.SignatureAlgorithm().String()}, + Content: sig, } json.NewEncoder(w).Encode(signature) diff --git a/signer/api/rpc_api.go b/signer/api/rpc_api.go index 2a6d817825..fa708c2616 100644 --- a/signer/api/rpc_api.go +++ b/signer/api/rpc_api.go @@ -1,6 +1,7 @@ package api import ( + "crypto/rand" "fmt" ctxu "github.com/docker/distribution/context" @@ -125,8 +126,13 @@ func (s *SignerServer) Sign(ctx context.Context, sr *pb.SignatureRequest) (*pb.S return nil, grpc.Errorf(codes.NotFound, "key %s not found", sr.KeyID.ID) } - signatures, err := service.Sign([]string{sr.KeyID.ID}, sr.Content) - if err != nil || len(signatures) != 1 { + privKey, _, err := service.GetPrivateKey(tufKey.ID()) + if err != nil { + logger.Errorf("Sign: key %s not found", sr.KeyID.ID) + return nil, grpc.Errorf(codes.NotFound, "key %s not found", sr.KeyID.ID) + } + sig, err := privKey.Sign(rand.Reader, sr.Content, nil) + if err != nil { logger.Errorf("Sign: signing failed for KeyID %s on hash %s", sr.KeyID.ID, sr.Content) return nil, grpc.Errorf(codes.Internal, "Signing failed for KeyID %s on hash %s", sr.KeyID.ID, sr.Content) } @@ -138,8 +144,8 @@ func (s *SignerServer) Sign(ctx context.Context, sr *pb.SignatureRequest) (*pb.S KeyID: &pb.KeyID{ID: tufKey.ID()}, Algorithm: &pb.Algorithm{Algorithm: tufKey.Algorithm()}, }, - Algorithm: &pb.Algorithm{Algorithm: signatures[0].Method.String()}, - Content: signatures[0].Signature, + Algorithm: &pb.Algorithm{Algorithm: privKey.SignatureAlgorithm().String()}, + Content: sig, } return signature, nil diff --git a/signer/client/signer_trust.go b/signer/client/signer_trust.go index aa01273ad4..99fc17aff2 100644 --- a/signer/client/signer_trust.go +++ b/signer/client/signer_trust.go @@ -127,28 +127,6 @@ func NewNotarySigner(hostname string, port string, tlsConfig *tls.Config) *Notar } } -// Sign signs a byte string with a number of KeyIDs -func (trust *NotarySigner) Sign(keyIDs []string, toSign []byte) ([]data.Signature, error) { - signatures := make([]data.Signature, 0, len(keyIDs)) - for _, ID := range keyIDs { - keyID := pb.KeyID{ID: ID} - sr := &pb.SignatureRequest{ - Content: toSign, - KeyID: &keyID, - } - sig, err := trust.sClient.Sign(context.Background(), sr) - if err != nil { - return nil, err - } - signatures = append(signatures, data.Signature{ - KeyID: sig.KeyInfo.KeyID.ID, - Method: data.SigAlgorithm(sig.Algorithm.Algorithm), - Signature: sig.Content, - }) - } - return signatures, nil -} - // Create creates a remote key and returns the PublicKey associated with the remote private key func (trust *NotarySigner) Create(role, algorithm string) (data.PublicKey, error) { publicKey, err := trust.kmClient.CreateKey(context.Background(), &pb.Algorithm{Algorithm: algorithm}) diff --git a/tuf/signed/ed25519.go b/tuf/signed/ed25519.go index 425b063bd5..6ee5a3c4fb 100644 --- a/tuf/signed/ed25519.go +++ b/tuf/signed/ed25519.go @@ -61,23 +61,6 @@ func (e *Ed25519) ListAllKeys() map[string]string { return keys } -// Sign generates an Ed25519 signature over the data -func (e *Ed25519) Sign(keyIDs []string, toSign []byte) ([]data.Signature, error) { - signatures := make([]data.Signature, 0, len(keyIDs)) - for _, keyID := range keyIDs { - priv := [ed25519.PrivateKeySize]byte{} - copy(priv[:], e.keys[keyID].privKey.Private()) - sig := ed25519.Sign(&priv, toSign) - signatures = append(signatures, data.Signature{ - KeyID: keyID, - Method: data.EDDSASignature, - Signature: sig[:], - }) - } - return signatures, nil - -} - // Create generates a new key and returns the public part func (e *Ed25519) Create(role, algorithm string) (data.PublicKey, error) { if algorithm != data.ED25519Key { diff --git a/tuf/signed/interface.go b/tuf/signed/interface.go index ecd680e861..0419646446 100644 --- a/tuf/signed/interface.go +++ b/tuf/signed/interface.go @@ -5,14 +5,6 @@ import ( "io" ) -// SigningService defines the necessary functions to determine -// if a user is able to sign with a key, and to perform signing. -type SigningService interface { - // Sign takes a slice of keyIDs and a piece of data to sign - // and returns a slice of signatures and an error - Sign(keyIDs []string, data []byte) ([]data.Signature, error) -} - // KeyService provides management of keys locally. It will never // accept or provide private keys. Communication between the KeyService // and a SigningService happen behind the Create function. @@ -47,7 +39,6 @@ type KeyService interface { // CryptoService defines a unified Signing and Key Service as this // will be most useful for most applications. type CryptoService interface { - SigningService KeyService } From ae7459b5f2e355748e5bb8cbcc751081c13e4b43 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 30 Nov 2015 16:38:14 -0800 Subject: [PATCH 2/2] updating commend and renaming test per comments Signed-off-by: David Lawrence (github: endophage) --- cryptoservice/crypto_service_test.go | 14 +++++--------- tuf/signed/interface.go | 4 ++-- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/cryptoservice/crypto_service_test.go b/cryptoservice/crypto_service_test.go index e1f73441b7..b46fc706b6 100644 --- a/cryptoservice/crypto_service_test.go +++ b/cryptoservice/crypto_service_test.go @@ -127,13 +127,11 @@ func (c CryptoServiceTester) TestSignNoMatchingKeys(t *testing.T) { assert.Error(t, err, c.errorMsg("Should not have found private key")) } -// If there are multiple keystores, even if all of them have the same key, -// only one signature is returned. -func (c CryptoServiceTester) TestSignWhenMultipleKeystores(t *testing.T) { +// Test GetPrivateKey succeeds when multiple keystores have the same key +func (c CryptoServiceTester) TestGetPrivateKeyMultipleKeystores(t *testing.T) { cryptoService := c.cryptoServiceFactory() cryptoService.keyStores = append(cryptoService.keyStores, trustmanager.NewKeyMemoryStore(passphraseRetriever)) - content := []byte("this is a secret") privKey, err := trustmanager.GenerateECDSAKey(rand.Reader) assert.NoError(t, err, c.errorMsg("error creating key")) @@ -143,12 +141,10 @@ func (c CryptoServiceTester) TestSignWhenMultipleKeystores(t *testing.T) { assert.NoError(t, err) } - privKey, role, err := cryptoService.GetPrivateKey(privKey.ID()) + foundKey, role, err := cryptoService.GetPrivateKey(privKey.ID()) assert.NoError(t, err, c.errorMsg("failed to get private key")) assert.Equal(t, c.role, role) - - _, err = privKey.Sign(rand.Reader, content, nil) - assert.NoError(t, err, c.errorMsg("signing failed")) + assert.Equal(t, privKey.ID(), foundKey.ID()) } // asserts that removing key that exists succeeds @@ -278,7 +274,7 @@ func testCryptoService(t *testing.T, gun string) { cst.TestGetNonexistentKey(t) cst.TestSignWithKey(t) cst.TestSignNoMatchingKeys(t) - cst.TestSignWhenMultipleKeystores(t) + cst.TestGetPrivateKeyMultipleKeystores(t) cst.TestRemoveCreatedKey(t) cst.TestRemoveFromMultipleKeystores(t) cst.TestListFromMultipleKeystores(t) diff --git a/tuf/signed/interface.go b/tuf/signed/interface.go index 0419646446..d1695c99bc 100644 --- a/tuf/signed/interface.go +++ b/tuf/signed/interface.go @@ -36,8 +36,8 @@ type KeyService interface { ImportRootKey(source io.Reader) error } -// CryptoService defines a unified Signing and Key Service as this -// will be most useful for most applications. +// CryptoService is deprecated and all instances of its use should be +// replaced with KeyService type CryptoService interface { KeyService }