Wire up pending key method to use on signer create, do not create

extra keys until we used a pending one

Signed-off-by: Riyaz Faizullabhoy <[email protected]>

cmd/notary-signer/config.go

@@ -67,7 +67,7 @@ func parseSignerConfig(configFilePath string) (signer.Config, error) {
 	}
 
 	// setup the cryptoservices
-	cryptoServices, markFunc, err := setUpCryptoservices(config, []string{notary.MySQLBackend, notary.MemoryBackend, notary.RethinkDBBackend})
+	cryptoServices, markFunc, pendingKeyFunc, err := setUpCryptoservices(config, []string{notary.MySQLBackend, notary.MemoryBackend, notary.RethinkDBBackend})
 	if err != nil {
 		return signer.Config{}, err
 	}
@@ -77,6 +77,7 @@ func parseSignerConfig(configFilePath string) (signer.Config, error) {
 		TLSConfig:      tlsConfig,
 		CryptoServices: cryptoServices,
 		MarkKeyActive:  markFunc,
+		PendingKeyFunc: pendingKeyFunc,
 	}, nil
 }
 
@@ -97,19 +98,21 @@ func passphraseRetriever(keyName, alias string, createNew bool, attempts int) (p
 }
 
 type markActive func(string) error
+type pendingKeyFunc func(string, string) (data.PublicKey, error)
 
 // Reads the configuration file for storage setup, and sets up the cryptoservice
 // mapping
 func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string) (
-	signer.CryptoServiceIndex, markActive, error) {
+	signer.CryptoServiceIndex, markActive, pendingKeyFunc, error) {
 	backend := configuration.GetString("storage.backend")
 
 	if !tufutils.StrSliceContains(allowedBackends, backend) {
-		return nil, nil, fmt.Errorf("%s is not an allowed backend, must be one of: %s", backend, allowedBackends)
+		return nil, nil, nil, fmt.Errorf("%s is not an allowed backend, must be one of: %s", backend, allowedBackends)
 	}
 
 	var keyStore trustmanager.KeyStore
 	var markFunc = func(string) error { return nil }
+	var pendingKeyFunc = func(string, string) (data.PublicKey, error) { return nil, fmt.Errorf("no pending key") }
 	switch backend {
 	case notary.MemoryBackend:
 		keyStore = trustmanager.NewKeyMemoryStore(
@@ -118,11 +121,11 @@ func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string) (
 		var sess *gorethink.Session
 		storeConfig, err := utils.ParseRethinkDBStorage(configuration)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 		defaultAlias, err := getDefaultAlias(configuration)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 		tlsOpts := tlsconfig.Options{
 			CAFile:   storeConfig.CA,
@@ -135,11 +138,12 @@ func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string) (
 			sess, err = rethinkdb.UserConnection(tlsOpts, storeConfig.Source, storeConfig.Username, storeConfig.Password)
 		}
 		if err != nil {
-			return nil, nil, fmt.Errorf("Error starting %s driver: %s", backend, err.Error())
+			return nil, nil, nil, fmt.Errorf("Error starting %s driver: %s", backend, err.Error())
 		}
 		s := keydbstore.NewRethinkDBKeyStore(storeConfig.DBName, storeConfig.Username, storeConfig.Password, passphraseRetriever, defaultAlias, sess)
 		health.RegisterPeriodicFunc("DB operational", time.Minute, s.CheckHealth)
 		markFunc = s.MarkActive
+		pendingKeyFunc = s.GetPendingKey
 
 		if doBootstrap {
 			keyStore = s
@@ -149,22 +153,23 @@ func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string) (
 	case notary.MySQLBackend, notary.SQLiteBackend:
 		storeConfig, err := utils.ParseSQLStorage(configuration)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 		defaultAlias, err := getDefaultAlias(configuration)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 		dbStore, err := keydbstore.NewSQLKeyDBStore(
 			passphraseRetriever, defaultAlias, storeConfig.Backend, storeConfig.Source)
 		if err != nil {
-			return nil, nil, fmt.Errorf("failed to create a new keydbstore: %v", err)
+			return nil, nil, nil, fmt.Errorf("failed to create a new keydbstore: %v", err)
 		}
 
 		health.RegisterPeriodicFunc(
 			"DB operational", time.Minute, dbStore.HealthCheck)
 		keyStore = keydbstore.NewCachedKeyStore(dbStore)
 		markFunc = dbStore.MarkActive
+		pendingKeyFunc = dbStore.GetPendingKey
 	}
 
 	if doBootstrap {
@@ -179,7 +184,7 @@ func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string) (
 	cryptoServices := make(signer.CryptoServiceIndex)
 	cryptoServices[data.ED25519Key] = cryptoService
 	cryptoServices[data.ECDSAKey] = cryptoService
-	return cryptoServices, markFunc, nil
+	return cryptoServices, markFunc, pendingKeyFunc, nil
 }
 
 func getDefaultAlias(configuration *viper.Viper) (string, error) {
@@ -203,6 +208,7 @@ func setupGRPCServer(signerConfig signer.Config) (*grpc.Server, net.Listener, er
 	kms := &api.KeyManagementServer{
 		CryptoServices: signerConfig.CryptoServices,
 		HealthChecker:  health.CheckStatus,
+		PendingKeyFunc: signerConfig.PendingKeyFunc,
 	}
 	ss := &api.SignerServer{
 		CryptoServices: signerConfig.CryptoServices,

cmd/notary-signer/main_test.go

@@ -87,7 +87,7 @@ func TestSetupCryptoServicesDBStoreNoDefaultAlias(t *testing.T) {
 	tmpFile.Close()
 	defer os.Remove(tmpFile.Name())
 
-	_, _, err = setUpCryptoservices(
+	_, _, _, err = setUpCryptoservices(
 		configure(fmt.Sprintf(
 			`{"storage": {"backend": "%s", "db_url": "%s"}}`,
 			notary.SQLiteBackend, tmpFile.Name())),
@@ -98,7 +98,7 @@ func TestSetupCryptoServicesDBStoreNoDefaultAlias(t *testing.T) {
 
 // If a default alias is not provided to a rethinkdb backend, an error is returned.
 func TestSetupCryptoServicesRethinkDBStoreNoDefaultAlias(t *testing.T) {
-	_, _, err := setUpCryptoservices(
+	_, _, _, err := setUpCryptoservices(
 		configure(fmt.Sprintf(
 			`{"storage": {
 				"backend": "%s",
@@ -119,7 +119,7 @@ func TestSetupCryptoServicesRethinkDBStoreNoDefaultAlias(t *testing.T) {
 
 func TestSetupCryptoServicesRethinkDBStoreConnectionFails(t *testing.T) {
 	// We don't have a rethink instance up, so the Connection() call will fail
-	_, _, err := setUpCryptoservices(
+	_, _, _, err := setUpCryptoservices(
 		configure(fmt.Sprintf(
 			`{"storage": {
 				"backend": "%s",
@@ -160,7 +160,7 @@ func TestSetupCryptoServicesDBStoreSuccess(t *testing.T) {
 	db.Model(&gormKey).Count(&count)
 	require.Equal(t, 0, count)
 
-	cryptoServices, markFunc, err := setUpCryptoservices(
+	cryptoServices, markFunc, _, err := setUpCryptoservices(
 		configure(fmt.Sprintf(
 			`{"storage": {"backend": "%s", "db_url": "%s"},
 			"default_alias": "timestamp"}`,
@@ -194,7 +194,7 @@ func TestSetupCryptoServicesDBStoreSuccess(t *testing.T) {
 func TestSetupCryptoServicesMemoryStore(t *testing.T) {
 	config := configure(fmt.Sprintf(`{"storage": {"backend": "%s"}}`,
 		notary.MemoryBackend))
-	cryptoServices, markFunc, err := setUpCryptoservices(config,
+	cryptoServices, markFunc, _, err := setUpCryptoservices(config,
 		[]string{notary.SQLiteBackend, notary.MemoryBackend})
 	require.NoError(t, err)
 	require.Len(t, cryptoServices, 2)
@@ -220,7 +220,7 @@ func TestSetupCryptoServicesMemoryStore(t *testing.T) {
 func TestSetupCryptoServicesInvalidStore(t *testing.T) {
 	config := configure(fmt.Sprintf(`{"storage": {"backend": "%s"}}`,
 		"invalid_backend"))
-	_, _, err := setUpCryptoservices(config,
+	_, _, _, err := setUpCryptoservices(config,
 		[]string{notary.SQLiteBackend, notary.MemoryBackend, notary.RethinkDBBackend})
 	require.Error(t, err)
 	require.Equal(t, err.Error(), fmt.Sprintf("%s is not an allowed backend, must be one of: %s", "invalid_backend", []string{notary.SQLiteBackend, notary.MemoryBackend, notary.RethinkDBBackend}))

server/handlers/validation_test.go

@@ -670,7 +670,7 @@ func TestValidateSnapshotMissingNoSnapshotKey(t *testing.T) {
 	require.IsType(t, validation.ErrBadHierarchy{}, err)
 }
 
-// TODO(riyazdf): translate these tests
+// TODO(riyazdf): Need to update these tests with the new behavior
 //func TestValidateSnapshotGenerateNoPrev(t *testing.T) {
 //	gun := "docker.com/notary"
 //	repo, cs, err := testutils.EmptyRepo(gun)

server/timestamp/timestamp.go

@@ -57,7 +57,7 @@ func GetOrCreateTimestampKey(gun string, store storage.MetaStore, crypto signed.
 	return nil, err
 }
 
-// RotateTimestampKey TODO(riyazdf)
+// RotateTimestampKey attempts to rotate a timestamp key in the signer, but might be rate-limited by the signer
 func RotateTimestampKey(gun string, store storage.MetaStore, crypto signed.CryptoService, createAlgorithm string) (data.PublicKey, error) {
 	// Always attempt to create a new key, but this might be rate-limited
 	key, err := crypto.Create(data.CanonicalTimestampRole, gun, createAlgorithm)

server/timestamp/timestamp_test.go

@@ -45,8 +45,8 @@ func TestGetTimestampKey(t *testing.T) {
 
 	require.Nil(t, err, "Expected nil error")
 
-	// NOTE(riyazdf): this is one place (and on rotate) that we have to rate-limit
-	// trying to get the same key again should return a different value until we've published metadata for the repo
+	// Note that this cryptoservice does not perform any rate-limiting, unlike the notary-signer,
+	// so we get a different key until we've published valid TUF metadata in the store
 	require.NotEqual(t, k, k2, "Received same key when attempting to recreate.")
 	require.NotNil(t, k2, "Key should not be nil")
 }

signer/api/rpc_api.go

@@ -8,6 +8,7 @@ import (
 	ctxu "github.com/docker/distribution/context"
 	"github.com/docker/notary/signer"
 	"github.com/docker/notary/trustmanager"
+	"github.com/docker/notary/tuf/data"
 	"golang.org/x/net/context"
 
 	"google.golang.org/grpc"
@@ -20,6 +21,7 @@ import (
 type KeyManagementServer struct {
 	CryptoServices signer.CryptoServiceIndex
 	HealthChecker  func() map[string]string
+	PendingKeyFunc func(string, string) (data.PublicKey, error)
 }
 
 //SignerServer implements the SignerServer grpc interface
@@ -40,12 +42,20 @@ func (s *KeyManagementServer) CreateKey(ctx context.Context, req *pb.CreateKeyRe
 		return nil, fmt.Errorf("algorithm %s not supported for create key", req.Algorithm)
 	}
 
-	tufKey, err := service.Create(req.Role, req.Gun, req.Algorithm)
-	if err != nil {
-		logger.Error("CreateKey: failed to create key: ", err)
-		return nil, grpc.Errorf(codes.Internal, "Key creation failed")
+	var tufKey data.PublicKey
+	var err error
+
+	if tufKey, err = s.PendingKeyFunc(req.Gun, req.Role); err == nil {
+		logger.Debugf("CreateKey: found pending key for role %s GUN %s that will be used", req.Role, req.Gun)
+	} else {
+		tufKey, err = service.Create(req.Role, req.Gun, req.Algorithm)
+		if err != nil {
+			logger.Error("CreateKey: failed to create key: ", err)
+			return nil, grpc.Errorf(codes.Internal, "Key creation failed")
+		}
+		logger.Info("CreateKey: Created KeyID ", tufKey.ID())
 	}
-	logger.Info("CreateKey: Created KeyID ", tufKey.ID())
+
 	return &pb.PublicKey{
 		KeyInfo: &pb.KeyInfo{
 			KeyID:     &pb.KeyID{ID: tufKey.ID()},

signer/keydbstore/rethink_keydbstore.go

@@ -264,6 +264,23 @@ func (rdb RethinkDBKeyStore) MarkActive(keyID string) error {
 	return err
 }
 
+// GetPendingKey gets the public key component of a key that was created but never used for signing a given gun and role
+func (rdb RethinkDBKeyStore) GetPendingKey(gun, role string) (data.PublicKey, error) {
+	dbPrivateKey := RDBPrivateKey{}
+	res, err := gorethink.DB(rdb.dbName).Table(dbPrivateKey.TableName()).Filter(gorethink.Row.Field("gun").Eq(gun)).Filter(gorethink.Row.Field("role").Eq(role)).Filter(gorethink.Row.Field("last_used").Eq(time.Time{})).Run(rdb.sess)
+	if err != nil {
+		return nil, err
+	}
+	defer res.Close()
+
+	err = res.One(&dbPrivateKey)
+	if err != nil {
+		return nil, trustmanager.ErrKeyNotFound{}
+	}
+
+	return data.NewPublicKey(dbPrivateKey.Algorithm, dbPrivateKey.Public), nil
+}
+
 // Bootstrap sets up the database and tables, also creating the notary signer user with appropriate db permission
 func (rdb RethinkDBKeyStore) Bootstrap() error {
 	if err := rethinkdb.SetupDB(rdb.sess, rdb.dbName, []rethinkdb.Table{

signer/keydbstore/sql_keydbstore.go

@@ -197,6 +197,17 @@ func (s *SQLKeyDBStore) MarkActive(keyID string) error {
 	return s.db.Model(GormPrivateKey{}).Where("key_id = ?", keyID).Updates(GormPrivateKey{LastUsed: s.nowFunc()}).Error
 }
 
+// GetPendingKey gets the public key component of a key that was created but never used for signing a given gun and role
+func (s *SQLKeyDBStore) GetPendingKey(gun, role string) (data.PublicKey, error) {
+	// Retrieve the GORM private key from the database
+	dbPrivateKey := GormPrivateKey{}
+	if s.db.Where(&GormPrivateKey{Gun: gun, Role: role, LastUsed: time.Time{}}).First(&dbPrivateKey).RecordNotFound() {
+		return nil, trustmanager.ErrKeyNotFound{}
+	}
+	// Just return the public key component if we found one
+	return data.NewPublicKey(dbPrivateKey.Algorithm, []byte(dbPrivateKey.Public)), nil
+}
+
 // HealthCheck verifies that DB exists and is query-able
 func (s *SQLKeyDBStore) HealthCheck() error {
 	dbPrivateKey := GormPrivateKey{}

signer/rpc_and_client_test.go

@@ -162,6 +162,7 @@ func setUpSignerServer(t *testing.T, store trustmanager.KeyStore, markActive fun
 	}
 
 	fakeHealth := func() map[string]string { return nil }
+	fakePendingCheck := func(string, string) (data.PublicKey, error) { return nil, fmt.Errorf("none pending") }
 	if markActive == nil {
 		markActive = func(string) error { return nil }
 	}
@@ -171,6 +172,7 @@ func setUpSignerServer(t *testing.T, store trustmanager.KeyStore, markActive fun
 	pb.RegisterKeyManagementServer(grpcServer, &api.KeyManagementServer{
 		CryptoServices: cryptoServices,
 		HealthChecker:  fakeHealth,
+		PendingKeyFunc: fakePendingCheck,
 	})
 	pb.RegisterSignerServer(grpcServer, &api.SignerServer{
 		CryptoServices: cryptoServices,

signer/signer.go

@@ -4,6 +4,7 @@ import (
 	"crypto/tls"
 
 	pb "github.com/docker/notary/proto"
+	"github.com/docker/notary/tuf/data"
 	"github.com/docker/notary/tuf/signed"
 )
 
@@ -42,4 +43,5 @@ type Config struct {
 	TLSConfig      *tls.Config
 	CryptoServices CryptoServiceIndex
 	MarkKeyActive  func(string) error
+	PendingKeyFunc func(string, string) (data.PublicKey, error)
 }