Merge pull request #292 from docker/fix-signer-sign

The NotarySigner cryptoservice now implements GetPrivateKey.

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,

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

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}
+}

signer/signer_trust_test.go

@@ -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))
-}