From 204a4f1534ede87cbebe82161e82e63f82101573 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Sun, 15 Nov 2015 01:08:51 -0800 Subject: [PATCH] The NotarySigner cryptoservice now implements GetPrivateKey. Previously, because it's a CryptoService wrapper around a remote signer service, it returned nil all the time. Now, because signing is done via private key more than CryptoService, it has to return a PrivateKey. The key doesn't have private bytes, but can be used for signing. Signed-off-by: Ying Li --- cmd/notary-server/main.go | 4 +- signer/{ => client}/signer_trust.go | 86 ++++++++++++- signer/client/signer_trust_test.go | 189 ++++++++++++++++++++++++++++ signer/signer_trust_test.go | 98 --------------- 4 files changed, 275 insertions(+), 102 deletions(-) rename signer/{ => client}/signer_trust.go (65%) create mode 100644 signer/client/signer_trust_test.go delete mode 100644 signer/signer_trust_test.go diff --git a/cmd/notary-server/main.go b/cmd/notary-server/main.go index 2ba9a66480..729e1b1945 100644 --- a/cmd/notary-server/main.go +++ b/cmd/notary-server/main.go @@ -17,6 +17,7 @@ import ( "github.com/docker/distribution/health" _ "github.com/docker/distribution/registry/auth/htpasswd" _ "github.com/docker/distribution/registry/auth/token" + "github.com/docker/notary/signer/client" "github.com/docker/notary/tuf/signed" _ "github.com/go-sql-driver/mysql" "golang.org/x/net/context" @@ -24,7 +25,6 @@ import ( bugsnag_hook "github.com/Sirupsen/logrus/hooks/bugsnag" "github.com/docker/notary/server" "github.com/docker/notary/server/storage" - "github.com/docker/notary/signer" "github.com/docker/notary/utils" "github.com/docker/notary/version" "github.com/spf13/viper" @@ -163,7 +163,7 @@ func main() { if err != nil { logrus.Fatal(err.Error()) } - notarySigner := signer.NewNotarySigner( + notarySigner := client.NewNotarySigner( mainViper.GetString("trust_service.hostname"), mainViper.GetString("trust_service.port"), clientTLS, diff --git a/signer/signer_trust.go b/signer/client/signer_trust.go similarity index 65% rename from signer/signer_trust.go rename to signer/client/signer_trust.go index 53ae075415..aa01273ad4 100644 --- a/signer/signer_trust.go +++ b/signer/client/signer_trust.go @@ -1,7 +1,11 @@ -package signer +// A CryptoService client wrapper around a remote wrapper service. + +package client import ( + "crypto" "crypto/tls" + "crypto/x509" "errors" "fmt" "io" @@ -22,6 +26,80 @@ type checkableConnectionState interface { State() grpc.ConnectivityState } +// RemotePrivateKey is a key that is on a remote service, so no private +// key bytes are available +type RemotePrivateKey struct { + data.PublicKey + sClient pb.SignerClient +} + +// RemoteSigner wraps a RemotePrivateKey and implements the crypto.Signer +// interface +type RemoteSigner struct { + RemotePrivateKey +} + +// Public method of a crypto.Signer needs to return a crypto public key. +func (rs *RemoteSigner) Public() crypto.PublicKey { + publicKey, err := x509.ParsePKIXPublicKey(rs.RemotePrivateKey.Public()) + if err != nil { + return nil + } + + return publicKey +} + +// NewRemotePrivateKey returns RemotePrivateKey, a data.PrivateKey that is only +// good for signing. (You can't get the private bytes out for instance.) +func NewRemotePrivateKey(pubKey data.PublicKey, sClient pb.SignerClient) *RemotePrivateKey { + return &RemotePrivateKey{ + PublicKey: pubKey, + sClient: sClient, + } +} + +// Private returns nil bytes +func (pk *RemotePrivateKey) Private() []byte { + return nil +} + +// Sign calls a remote service to sign a message. +func (pk *RemotePrivateKey) Sign(rand io.Reader, msg []byte, + opts crypto.SignerOpts) ([]byte, error) { + + keyID := pb.KeyID{ID: pk.ID()} + sr := &pb.SignatureRequest{ + Content: msg, + KeyID: &keyID, + } + sig, err := pk.sClient.Sign(context.Background(), sr) + if err != nil { + return nil, err + } + return sig.Content, nil +} + +// SignatureAlgorithm returns the signing algorithm based on the type of +// PublicKey algorithm. +func (pk *RemotePrivateKey) SignatureAlgorithm() data.SigAlgorithm { + switch pk.PublicKey.Algorithm() { + case data.ECDSAKey, data.ECDSAx509Key: + return data.ECDSASignature + case data.RSAKey, data.RSAx509Key: + return data.RSAPSSSignature + case data.ED25519Key: + return data.EDDSASignature + default: // unknown + return "" + } +} + +// CryptoSigner returns a crypto.Signer tha wraps the RemotePrivateKey. Needed +// for implementing the interface. +func (pk *RemotePrivateKey) CryptoSigner() crypto.Signer { + return &RemoteSigner{RemotePrivateKey: *pk} +} + // NotarySigner implements a RPC based Trust service that calls the Notary-signer Service type NotarySigner struct { kmClient pb.KeyManagementClient @@ -98,7 +176,11 @@ func (trust *NotarySigner) GetKey(keyid string) data.PublicKey { // GetPrivateKey errors in all cases func (trust *NotarySigner) GetPrivateKey(keyid string) (data.PrivateKey, string, error) { - return nil, "", errors.New("Private key access not permitted.") + pubKey := trust.GetKey(keyid) + if pubKey == nil { + return nil, "", nil + } + return NewRemotePrivateKey(pubKey, trust.sClient), "", nil } // ListKeys not supported for NotarySigner diff --git a/signer/client/signer_trust_test.go b/signer/client/signer_trust_test.go new file mode 100644 index 0000000000..2fcc739d07 --- /dev/null +++ b/signer/client/signer_trust_test.go @@ -0,0 +1,189 @@ +package client + +import ( + "crypto/rand" + "errors" + "strings" + "testing" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + + "github.com/docker/notary/cryptoservice" + "github.com/docker/notary/passphrase" + pb "github.com/docker/notary/proto" + "github.com/docker/notary/signer" + "github.com/docker/notary/signer/api" + "github.com/docker/notary/trustmanager" + "github.com/docker/notary/tuf/data" + "github.com/docker/notary/tuf/signed" + "github.com/stretchr/testify/assert" + "golang.org/x/net/context" +) + +type rpcHealthCheck func( + context.Context, *pb.Void, ...grpc.CallOption) (*pb.HealthStatus, error) + +type StubKeyManagementClient struct { + pb.KeyManagementClient + healthCheck rpcHealthCheck +} + +func (c StubKeyManagementClient) CheckHealth(x context.Context, + v *pb.Void, o ...grpc.CallOption) (*pb.HealthStatus, error) { + return c.healthCheck(x, v, o...) +} + +type StubGRPCConnection struct { + fakeConnStatus grpc.ConnectivityState +} + +func (c StubGRPCConnection) State() grpc.ConnectivityState { + return c.fakeConnStatus +} + +func stubHealthFunction(t *testing.T, status map[string]string, err error) rpcHealthCheck { + return func(ctx context.Context, v *pb.Void, o ...grpc.CallOption) (*pb.HealthStatus, error) { + _, withDeadline := ctx.Deadline() + assert.True(t, withDeadline) + + return &pb.HealthStatus{Status: status}, err + } +} + +func makeSigner(kmFunc rpcHealthCheck, conn StubGRPCConnection) NotarySigner { + return NotarySigner{ + StubKeyManagementClient{ + pb.NewKeyManagementClient(nil), + kmFunc, + }, + pb.NewSignerClient(nil), + conn, + } +} + +// CheckHealth does not succeed if the KM server is unhealthy +func TestHealthCheckKMUnhealthy(t *testing.T) { + signer := makeSigner( + stubHealthFunction(t, map[string]string{"health": "not good"}, nil), + StubGRPCConnection{}) + assert.Error(t, signer.CheckHealth(1*time.Second)) +} + +// CheckHealth does not succeed if the health check to the KM server errors +func TestHealthCheckKMError(t *testing.T) { + signer := makeSigner( + stubHealthFunction(t, nil, errors.New("Something's wrong")), + StubGRPCConnection{}) + assert.Error(t, signer.CheckHealth(1*time.Second)) +} + +// CheckHealth does not succeed if the health check to the KM server times out +func TestHealthCheckKMTimeout(t *testing.T) { + signer := makeSigner( + stubHealthFunction(t, nil, grpc.Errorf(codes.DeadlineExceeded, "")), + StubGRPCConnection{}) + err := signer.CheckHealth(1 * time.Second) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "Timed out")) +} + +// CheckHealth succeeds if KM is healthy and reachable. +func TestHealthCheckKMHealthy(t *testing.T) { + signer := makeSigner( + stubHealthFunction(t, make(map[string]string), nil), + StubGRPCConnection{}) + assert.NoError(t, signer.CheckHealth(1*time.Second)) +} + +// CheckHealth fails immediately if not connected to the server. +func TestHealthCheckConnectionDied(t *testing.T) { + signer := makeSigner( + stubHealthFunction(t, make(map[string]string), nil), + StubGRPCConnection{grpc.Connecting}) + assert.Error(t, signer.CheckHealth(1*time.Second)) +} + +var ret = passphrase.ConstantRetriever("pass") + +func TestGetPrivateKeyIfNoKey(t *testing.T) { + signer := setUpSigner(t, trustmanager.NewKeyMemoryStore(ret)) + privKey, _, err := signer.GetPrivateKey("bogus key ID") + assert.NoError(t, err) + assert.Nil(t, privKey) +} + +func TestGetPrivateKeyAndSignWithExistingKey(t *testing.T) { + key, err := trustmanager.GenerateECDSAKey(rand.Reader) + assert.NoError(t, err, "could not generate key") + + store := trustmanager.NewKeyMemoryStore(ret) + + err = store.AddKey(key.ID(), "timestamp", key) + assert.NoError(t, err, "could not add key to store") + + signer := setUpSigner(t, store) + + privKey, _, err := signer.GetPrivateKey(key.ID()) + assert.NoError(t, err) + assert.NotNil(t, privKey) + + msg := []byte("message!") + sig, err := privKey.Sign(rand.Reader, msg, nil) + assert.NoError(t, err) + + err = signed.Verifiers[data.ECDSASignature].Verify( + data.PublicKeyFromPrivate(key), sig, msg) + assert.NoError(t, err) +} + +type StubClientFromServers struct { + api.KeyManagementServer + api.SignerServer +} + +func (c *StubClientFromServers) CreateKey(ctx context.Context, + algorithm *pb.Algorithm, _ ...grpc.CallOption) (*pb.PublicKey, error) { + return c.KeyManagementServer.CreateKey(ctx, algorithm) +} + +func (c *StubClientFromServers) DeleteKey(ctx context.Context, keyID *pb.KeyID, + _ ...grpc.CallOption) (*pb.Void, error) { + return c.KeyManagementServer.DeleteKey(ctx, keyID) +} + +func (c *StubClientFromServers) GetKeyInfo(ctx context.Context, keyID *pb.KeyID, + _ ...grpc.CallOption) (*pb.PublicKey, error) { + return c.KeyManagementServer.GetKeyInfo(ctx, keyID) +} + +func (c *StubClientFromServers) Sign(ctx context.Context, + sr *pb.SignatureRequest, _ ...grpc.CallOption) (*pb.Signature, error) { + return c.SignerServer.Sign(ctx, sr) +} + +func (c *StubClientFromServers) CheckHealth(ctx context.Context, v *pb.Void, + _ ...grpc.CallOption) (*pb.HealthStatus, error) { + return c.KeyManagementServer.CheckHealth(ctx, v) +} + +func setUpSigner(t *testing.T, store trustmanager.KeyStore) NotarySigner { + cryptoService := cryptoservice.NewCryptoService("", store) + cryptoServices := signer.CryptoServiceIndex{ + data.ED25519Key: cryptoService, + data.RSAKey: cryptoService, + data.ECDSAKey: cryptoService, + } + + fakeHealth := func() map[string]string { return map[string]string{} } + + client := StubClientFromServers{ + KeyManagementServer: api.KeyManagementServer{CryptoServices: cryptoServices, + HealthChecker: fakeHealth}, + SignerServer: api.SignerServer{CryptoServices: cryptoServices, + HealthChecker: fakeHealth}, + } + + return NotarySigner{kmClient: &client, sClient: &client} +} diff --git a/signer/signer_trust_test.go b/signer/signer_trust_test.go deleted file mode 100644 index 4b5ccef40d..0000000000 --- a/signer/signer_trust_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package signer - -import ( - "errors" - "strings" - "testing" - "time" - - "google.golang.org/grpc" - "google.golang.org/grpc/codes" - - pb "github.com/docker/notary/proto" - "github.com/stretchr/testify/assert" - "golang.org/x/net/context" -) - -type rpcHealthCheck func( - context.Context, *pb.Void, ...grpc.CallOption) (*pb.HealthStatus, error) - -type StubKeyManagementClient struct { - pb.KeyManagementClient - healthCheck rpcHealthCheck -} - -func (c StubKeyManagementClient) CheckHealth(x context.Context, - v *pb.Void, o ...grpc.CallOption) (*pb.HealthStatus, error) { - return c.healthCheck(x, v, o...) -} - -type StubGRPCConnection struct { - fakeConnStatus grpc.ConnectivityState -} - -func (c StubGRPCConnection) State() grpc.ConnectivityState { - return c.fakeConnStatus -} - -func stubHealthFunction(t *testing.T, status map[string]string, err error) rpcHealthCheck { - return func(ctx context.Context, v *pb.Void, o ...grpc.CallOption) (*pb.HealthStatus, error) { - _, withDeadline := ctx.Deadline() - assert.True(t, withDeadline) - - return &pb.HealthStatus{Status: status}, err - } -} - -func makeSigner(kmFunc rpcHealthCheck, conn StubGRPCConnection) NotarySigner { - return NotarySigner{ - StubKeyManagementClient{ - pb.NewKeyManagementClient(nil), - kmFunc, - }, - pb.NewSignerClient(nil), - conn, - } -} - -// CheckHealth does not succeed if the KM server is unhealthy -func TestHealthCheckKMUnhealthy(t *testing.T) { - signer := makeSigner( - stubHealthFunction(t, map[string]string{"health": "not good"}, nil), - StubGRPCConnection{}) - assert.Error(t, signer.CheckHealth(1*time.Second)) -} - -// CheckHealth does not succeed if the health check to the KM server errors -func TestHealthCheckKMError(t *testing.T) { - signer := makeSigner( - stubHealthFunction(t, nil, errors.New("Something's wrong")), - StubGRPCConnection{}) - assert.Error(t, signer.CheckHealth(1*time.Second)) -} - -// CheckHealth does not succeed if the health check to the KM server times out -func TestHealthCheckKMTimeout(t *testing.T) { - signer := makeSigner( - stubHealthFunction(t, nil, grpc.Errorf(codes.DeadlineExceeded, "")), - StubGRPCConnection{}) - err := signer.CheckHealth(1 * time.Second) - assert.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "Timed out")) -} - -// CheckHealth succeeds if KM is healthy and reachable. -func TestHealthCheckKMHealthy(t *testing.T) { - signer := makeSigner( - stubHealthFunction(t, make(map[string]string), nil), - StubGRPCConnection{}) - assert.NoError(t, signer.CheckHealth(1*time.Second)) -} - -// CheckHealth fails immediately if not connected to the server. -func TestHealthCheckConnectionDied(t *testing.T) { - signer := makeSigner( - stubHealthFunction(t, make(map[string]string), nil), - StubGRPCConnection{grpc.Connecting}) - assert.Error(t, signer.CheckHealth(1*time.Second)) -}