Addressed comments, changed to PBES2, added key rotation

Signed-off-by: Diogo Monica <diogo@docker.com>

cmd/notary-signer/main.go

@@ -107,7 +107,7 @@ func main() {
 		log.Fatalf("failed to open the database: %s, %v", dbURL, err)
 	}
 
-	keyStore, err := trustmanager.NewKeyDBStore(passphraseRetriever, _DBType, dbSQL)
+	keyStore, err := trustmanager.NewKeyDBStore(passphraseRetriever, "", _DBType, dbSQL)
 	if err != nil {
 		log.Fatalf("failed to create a new keydbstore: %v", err)
 	}

notarymysql/initial.sql

@@ -24,10 +24,12 @@ CREATE TABLE `private_keys` (
 	`updated_at` datetime NOT NULL,
 	`deleted_at` datetime DEFAULT NULL,
 	`key_id`  varchar(255) NOT NULL,
-	`encryption`  varchar(255) NOT NULL,
+	`encryption_alg`  varchar(255) NOT NULL,
+	`keywrap_alg`  varchar(255) NOT NULL,
 	`algorithm`  varchar(50) NOT NULL,
+	`passphrase_alias`  varchar(50) NOT NULL,
 	`public`  blob NOT NULL,
-	`private`  longblob NOT NULL,
+	`private`  blob NOT NULL,
 	PRIMARY KEY (`id`),
 	UNIQUE (`key_id`),
 	UNIQUE (`key_id`,`encryption`)

trustmanager/keydbstore.go

@@ -6,28 +6,36 @@ import (
 	"sync"
 
 	"github.com/docker/notary/pkg/passphrase"
+	jose "github.com/dvsekhvalnov/jose2go"
 	"github.com/endophage/gotuf/data"
 	"github.com/jinzhu/gorm"
-	gojose "github.com/square/go-jose"
+)
+
+// Constants
+const (
+	EncryptionAlg = jose.A256GCM
+	KeywrapAlg    = jose.PBES2_HS256_A128KW
 )
 
 // KeyDBStore persists and manages private keys on a SQL database
 type KeyDBStore struct {
 	sync.Mutex
-	db         gorm.DB
-	passphrase string
-	encrypter  gojose.Encrypter
-	cachedKeys map[string]data.PrivateKey
+	db               gorm.DB
+	defaultPassAlias string
+	retriever        passphrase.Retriever
+	cachedKeys       map[string]data.PrivateKey
 }
 
 // GormPrivateKey represents a PrivateKey in the database
 type GormPrivateKey struct {
 	gorm.Model
-	KeyID      string `sql:"not null;unique"`
-	Encryption string `sql:"not null"`
-	Algorithm  string `sql:"not null"`
-	Public     []byte `sql:"not null"`
-	Private    string `sql:"not null"`
+	KeyID           string `sql:"not null;unique;index:key_id_idx"`
+	EncryptionAlg   string `sql:"not null"`
+	KeywrapAlg      string `sql:"not null"`
+	Algorithm       string `sql:"not null"`
+	PassphraseAlias string `sql:"not null"`
+	Public          string `sql:"not null"`
+	Private         string `sql:"not null"`
 }
 
 // TableName sets a specific table name for our GormPrivateKey
@@ -36,47 +44,40 @@ func (g GormPrivateKey) TableName() string {
 }
 
 // NewKeyDBStore returns a new KeyDBStore backed by a SQL database
-func NewKeyDBStore(passphraseRetriever passphrase.Retriever, dbType string, dbSQL *sql.DB) (*KeyDBStore, error) {
+func NewKeyDBStore(passphraseRetriever passphrase.Retriever, defaultPassAlias, dbType string, dbSQL *sql.DB) (*KeyDBStore, error) {
 	cachedKeys := make(map[string]data.PrivateKey)
 
-	// Retreive the passphrase that will be used to encrypt the keys
-	passphrase, _, err := passphraseRetriever("", "", false, 0)
-	if err != nil {
-		return nil, err
-	}
-
-	// Setup our encrypted object
-	encrypter, err := gojose.NewEncrypter(gojose.A256GCMKW, gojose.A256GCM, []byte(passphrase))
-	if err != nil {
-		return nil, err
-	}
-
 	// Open a connection to our database
 	db, _ := gorm.Open(dbType, dbSQL)
 
 	return &KeyDBStore{db: db,
-		passphrase: passphrase,
-		encrypter:  encrypter,
-		cachedKeys: cachedKeys}, nil
+		defaultPassAlias: defaultPassAlias,
+		retriever:        passphraseRetriever,
+		cachedKeys:       cachedKeys}, nil
 }
 
 // AddKey stores the contents of a private key. Both name and alias are ignored,
 // we always use Key IDs as name, and don't support aliases
 func (s *KeyDBStore) AddKey(name, alias string, privKey data.PrivateKey) error {
-	encryptedKey, err := s.encrypter.Encrypt(privKey.Private())
+
+	passphrase, _, err := s.retriever(privKey.ID(), s.defaultPassAlias, false, 1)
 	if err != nil {
 		return err
 	}
 
-	// Encrypt the private key material
-	encryptedPrivKeyStr := encryptedKey.FullSerialize()
+	encryptedKey, err := jose.Encrypt(string(privKey.Private()), KeywrapAlg, EncryptionAlg, passphrase)
+	if err != nil {
+		return err
+	}
 
 	gormPrivKey := GormPrivateKey{
-		KeyID:      privKey.ID(),
-		Encryption: string(gojose.PBES2_HS512_A256KW),
-		Algorithm:  privKey.Algorithm().String(),
-		Public:     privKey.Public(),
-		Private:    encryptedPrivKeyStr}
+		KeyID:           privKey.ID(),
+		EncryptionAlg:   EncryptionAlg,
+		KeywrapAlg:      KeywrapAlg,
+		PassphraseAlias: s.defaultPassAlias,
+		Algorithm:       privKey.Algorithm().String(),
+		Public:          string(privKey.Public()),
+		Private:         encryptedKey}
 
 	// Add encrypted private key to the database
 	s.db.Create(&gormPrivKey)
@@ -109,18 +110,20 @@ func (s *KeyDBStore) GetKey(name string) (data.PrivateKey, string, error) {
 		return nil, "", ErrKeyNotFound{}
 	}
 
-	// Decrypt private bytes from the gorm key
-	encryptedPrivKeyJWE, err := gojose.ParseEncrypted(dbPrivateKey.Private)
+	// Get the passphrase to use for this key
+	passphrase, _, err := s.retriever(dbPrivateKey.KeyID, dbPrivateKey.PassphraseAlias, false, 1)
 	if err != nil {
 		return nil, "", err
 	}
-	decryptedPrivKeyBytes, err := encryptedPrivKeyJWE.Decrypt([]byte(s.passphrase))
+
+	// Decrypt private bytes from the gorm key
+	decryptedPrivKey, _, err := jose.Decode(dbPrivateKey.Private, passphrase)
 	if err != nil {
 		return nil, "", err
 	}
 
 	// Create a new PrivateKey with unencrypted bytes
-	privKey := data.NewPrivateKey(data.KeyAlgorithm(dbPrivateKey.Algorithm), dbPrivateKey.Public, decryptedPrivKeyBytes)
+	privKey := data.NewPrivateKey(data.KeyAlgorithm(dbPrivateKey.Algorithm), []byte(dbPrivateKey.Public), []byte(decryptedPrivKey))
 
 	// Add the key to cache
 	s.cachedKeys[privKey.ID()] = privKey
@@ -151,3 +154,50 @@ func (s *KeyDBStore) RemoveKey(name string) error {
 
 	return nil
 }
+
+// RotateKeyPassphrase rotates the key-encryption-key
+func (s *KeyDBStore) RotateKeyPassphrase(name, newPassphraseAlias string) error {
+	// Retrieve the GORM private key from the database
+	dbPrivateKey := GormPrivateKey{}
+	if s.db.Where(&GormPrivateKey{KeyID: name}).First(&dbPrivateKey).RecordNotFound() {
+		return ErrKeyNotFound{}
+	}
+
+	// Get the current passphrase to use for this key
+	passphrase, _, err := s.retriever(dbPrivateKey.KeyID, dbPrivateKey.PassphraseAlias, false, 1)
+	if err != nil {
+		return err
+	}
+
+	fmt.Println("Got old passphrase: ", passphrase)
+
+	// Decrypt private bytes from the gorm key
+	decryptedPrivKey, _, err := jose.Decode(dbPrivateKey.Private, passphrase)
+	if err != nil {
+		return err
+	}
+
+	// Get the new passphrase to use for this key
+	newPassphrase, _, err := s.retriever(dbPrivateKey.KeyID, newPassphraseAlias, false, 1)
+	if err != nil {
+		return err
+	}
+
+	fmt.Println("new passphrase: ", newPassphrase)
+
+	// Re-encrypt the private bytes with the new passphrase
+	newEncryptedKey, err := jose.Encrypt(decryptedPrivKey, KeywrapAlg, EncryptionAlg, newPassphrase)
+	if err != nil {
+		return err
+	}
+	fmt.Println("encrypted key: ", newEncryptedKey)
+
+	// Update the database object
+	dbPrivateKey.Private = newEncryptedKey
+	dbPrivateKey.PassphraseAlias = newPassphraseAlias
+	s.db.Save(dbPrivateKey)
+
+	fmt.Printf("DB Private key: %v", dbPrivateKey)
+
+	return nil
+}

trustmanager/keydbstore_test.go

@@ -3,6 +3,7 @@ package trustmanager
 import (
 	"crypto/rand"
 	"database/sql"
+	"errors"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -12,7 +13,17 @@ import (
 )
 
 var retriever = func(string, string, bool, int) (string, bool, error) {
-	return "abcgdhfjdhfjhfgdhejnfhdfgshdjfbv", false, nil
+	return "passphrase-1", false, nil
+}
+
+var anotherRetriever = func(keyName, alias string, createNew bool, attempts int) (string, bool, error) {
+	switch alias {
+	case "alias-1":
+		return "passphrase-1", false, nil
+	case "alias-2":
+		return "passphrase-2", false, nil
+	}
+	return "", false, errors.New("password alias no found")
 }
 
 func TestCreateRead(t *testing.T) {
@@ -27,7 +38,7 @@ func TestCreateRead(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Create a new KeyDB store
-	dbStore, err := NewKeyDBStore(retriever, "sqlite3", db)
+	dbStore, err := NewKeyDBStore(retriever, "", "sqlite3", db)
 	assert.NoError(t, err)
 
 	// Ensure that the private_key table exists
@@ -69,7 +80,7 @@ func TestDoubleCreate(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Create a new KeyDB store
-	dbStore, err := NewKeyDBStore(retriever, "sqlite3", db)
+	dbStore, err := NewKeyDBStore(retriever, "", "sqlite3", db)
 	assert.NoError(t, err)
 
 	// Ensure that the private_key table exists
@@ -100,7 +111,7 @@ func TestCreateDelete(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Create a new KeyDB store
-	dbStore, err := NewKeyDBStore(retriever, "sqlite3", db)
+	dbStore, err := NewKeyDBStore(retriever, "", "sqlite3", db)
 	assert.NoError(t, err)
 
 	// Ensure that the private_key table exists
@@ -118,3 +129,34 @@ func TestCreateDelete(t *testing.T) {
 	_, _, err = dbStore.GetKey(testKey.ID())
 	assert.Error(t, err, "signing key not found:")
 }
+
+func TestKeyRotation(t *testing.T) {
+	tempBaseDir, err := ioutil.TempDir("", "notary-test-")
+	defer os.RemoveAll(tempBaseDir)
+
+	testKey, err := GenerateECDSAKey(rand.Reader)
+	assert.NoError(t, err)
+
+	// We are using SQLite for the tests
+	db, err := sql.Open("sqlite3", tempBaseDir+"test_db")
+	assert.NoError(t, err)
+
+	// Create a new KeyDB store
+	dbStore, err := NewKeyDBStore(anotherRetriever, "alias-1", "sqlite3", db)
+	assert.NoError(t, err)
+
+	// Ensure that the private_key table exists
+	dbStore.db.CreateTable(&GormPrivateKey{})
+
+	// Test writing new key in database/cache
+	err = dbStore.AddKey("", "", testKey)
+	assert.NoError(t, err)
+
+	// Try rotating the key to alias-2
+	err = dbStore.RotateKeyPassphrase(testKey.ID(), "alias-2")
+	assert.NoError(t, err)
+
+	// Try rotating the key to alias-3
+	err = dbStore.RotateKeyPassphrase(testKey.ID(), "alias-3")
+	assert.Error(t, err, "password alias no found")
+}

trustmanager/keystore.go

@@ -50,9 +50,3 @@ type cachedKey struct {
 	alias string
 	key   data.PrivateKey
 }
-
-// PassphraseRetriever is a callback function that should retrieve a passphrase
-// for a given named key. If it should be treated as new passphrase (e.g. with
-// confirmation), createNew will be true. Attempts is passed in so that implementers
-// decide how many chances to give to a human, for example.
-type PassphraseRetriever func(keyId, alias string, createNew bool, attempts int) (passphrase string, giveup bool, err error)