Add RethinkDB signer tests, and elaborate in the key passphrase rotat…

…ion tests. Fix the following in the RethinkDB implementation: - health check needs to ensure the user has access to the db and table - store the public and private bits as byte slices, because the otherwise rethink can't deal with the binary data - fix the rethink queries for rotating a key passphrase - use the same code as GetKey Signed-off-by: Ying Li <[email protected]>
notaryproject · Jul 21, 2016 · 84fd090 · 84fd090
1 parent 81dc94f
commit 84fd090
Show file tree

Hide file tree

Showing 6 changed files with 251 additions and 76 deletions.
diff --git a/server/storage/rethink_realdb_test.go b/server/storage/rethink_realdb_test.go
@@ -30,7 +30,7 @@ func rethinkSessionSetup(t *testing.T) (*gorethink.Session, string) {
 
 func rethinkDBSetup(t *testing.T) (RethinkDB, func()) {
 	session, _ := rethinkSessionSetup(t)
-	dbName := "testdb"
+	dbName := "servertestdb"
 	var cleanup = func() { gorethink.DBDrop(dbName).Exec(session) }
 
 	cleanup()
@@ -41,32 +41,36 @@ func rethinkDBSetup(t *testing.T) (RethinkDB, func()) {
 	return NewRethinkDBStorage(dbName, "", "", session), cleanup
 }
 
-func TestBootstrapSetsUsernamePassword(t *testing.T) {
+func TestRethinkBootstrapSetsUsernamePassword(t *testing.T) {
 	adminSession, source := rethinkSessionSetup(t)
-	dbname, username, password := "testdb", "testuser", "testpassword"
-	otherDB, otherUser, otherPass := "otherdb", "otheruser", "otherpassword"
+	dbname, username, password := "servertestdb", "testuser", "testpassword"
+	otherDB, otherUser, otherPass := "otherservertestdb", "otheruser", "otherpassword"
 
 	// create a separate user with access to a different DB
 	require.NoError(t, rethinkdb.SetupDB(adminSession, otherDB, nil))
+	defer gorethink.DBDrop(otherDB).Exec(adminSession)
 	require.NoError(t, rethinkdb.CreateAndGrantDBUser(adminSession, otherDB, otherUser, otherPass))
 
 	// Bootstrap
 	s := NewRethinkDBStorage(dbname, username, password, adminSession)
 	require.NoError(t, s.Bootstrap())
+	defer gorethink.DBDrop(dbname).Exec(adminSession)
 
 	// A user with an invalid password cannot connect to rethink DB at all
 	_, err := rethinkdb.UserConnection(tlsOpts, source, username, "wrongpass")
 	require.Error(t, err)
 
 	// the other user cannot access rethink
 	userSession, err := rethinkdb.UserConnection(tlsOpts, source, otherUser, otherPass)
+	require.NoError(t, err)
 	s = NewRethinkDBStorage(dbname, otherUser, otherPass, userSession)
 	_, _, err = s.GetCurrent("gun", data.CanonicalRootRole)
 	require.Error(t, err)
 	require.IsType(t, gorethink.RQLRuntimeError{}, err)
 
 	// our user can access the DB though
 	userSession, err = rethinkdb.UserConnection(tlsOpts, source, username, password)
+	require.NoError(t, err)
 	s = NewRethinkDBStorage(dbname, username, password, userSession)
 	_, _, err = s.GetCurrent("gun", data.CanonicalRootRole)
 	require.Error(t, err)

diff --git a/signer/keydbstore/keydbstore_test.go b/signer/keydbstore/keydbstore_test.go
@@ -3,7 +3,6 @@ package keydbstore
 import (
 	"crypto/rand"
 	"errors"
-	"fmt"
 	"testing"
 
 	"github.com/docker/notary/trustmanager"
@@ -13,16 +12,18 @@ import (
 )
 
 func constRetriever(string, string, bool, int) (string, bool, error) {
-	return "passphrase_1", false, nil
+	return "constantPass", false, nil
 }
 
 var validAliases = []string{"validAlias1", "validAlias2"}
+var validAliasesAndPasswds = map[string]string{
+	"validAlias1": "passphrase_1",
+	"validAlias2": "passphrase_2",
+}
 
 func multiAliasRetriever(_, alias string, _ bool, _ int) (string, bool, error) {
-	for i, validAlias := range validAliases {
-		if alias == validAlias {
-			return fmt.Sprintf("passphrase_%d", i), false, nil
-		}
+	if passwd, ok := validAliasesAndPasswds[alias]; ok {
+		return passwd, false, nil
 	}
 	return "", false, errors.New("password alias no found")
 }
@@ -33,26 +34,29 @@ type keyRotator interface {
 }
 
 // A key can only be added to the DB once
-func testKeyCanOnlyBeAddedOnce(t *testing.T, dbStore trustmanager.KeyStore) {
-	testKey, err := utils.GenerateECDSAKey(rand.Reader)
-	require.NoError(t, err)
+func testKeyCanOnlyBeAddedOnce(t *testing.T, dbStore trustmanager.KeyStore) []data.PrivateKey {
+	expectedKeys := make([]data.PrivateKey, 2)
+	for i := 0; i < len(expectedKeys); i++ {
+		testKey, err := utils.GenerateECDSAKey(rand.Reader)
+		require.NoError(t, err)
+		expectedKeys[i] = testKey
+	}
 
 	// Test writing new key in database alone, not cache
-	err = dbStore.AddKey(trustmanager.KeyInfo{Role: data.CanonicalTimestampRole, Gun: "gun/ignored"}, testKey)
+	err := dbStore.AddKey(trustmanager.KeyInfo{Role: data.CanonicalTimestampRole, Gun: "gun/ignored"}, expectedKeys[0])
 	require.NoError(t, err)
 	// Currently we ignore roles
-	requireGetKeySuccess(t, dbStore, "", testKey)
+	requireGetKeySuccess(t, dbStore, "", expectedKeys[0])
 
 	// Test writing the same key in the database. Should fail.
-	err = dbStore.AddKey(trustmanager.KeyInfo{Role: data.CanonicalTimestampRole, Gun: "gun/ignored"}, testKey)
+	err = dbStore.AddKey(trustmanager.KeyInfo{Role: data.CanonicalTimestampRole, Gun: "gun/ignored"}, expectedKeys[0])
 	require.Error(t, err, "failed to add private key to database:")
 
-	anotherTestKey, err := utils.GenerateECDSAKey(rand.Reader)
-	require.NoError(t, err)
-
 	// Test writing new key succeeds
-	err = dbStore.AddKey(trustmanager.KeyInfo{Role: data.CanonicalTimestampRole, Gun: "gun/ignored"}, anotherTestKey)
+	err = dbStore.AddKey(trustmanager.KeyInfo{Role: data.CanonicalTimestampRole, Gun: "gun/ignored"}, expectedKeys[1])
 	require.NoError(t, err)
+
+	return expectedKeys
 }
 
 // a key can be deleted
@@ -78,7 +82,7 @@ func testCreateDelete(t *testing.T, dbStore trustmanager.KeyStore) {
 }
 
 // key rotation is successful provided the other alias is valid
-func testKeyRotation(t *testing.T, dbStore keyRotator, newValidAlias string) {
+func testKeyRotation(t *testing.T, dbStore keyRotator, newValidAlias string) data.PrivateKey {
 	testKey, err := utils.GenerateECDSAKey(rand.Reader)
 	require.NoError(t, err)
 
@@ -93,4 +97,6 @@ func testKeyRotation(t *testing.T, dbStore keyRotator, newValidAlias string) {
 	// Try rotating the key to an invalid alias
 	err = dbStore.RotateKeyPassphrase(testKey.ID(), "invalidAlias")
 	require.Error(t, err, "there should be no password for invalidAlias so rotation should fail")
+
+	return testKey
 }
diff --git a/signer/keydbstore/rethink_keydbstore.go b/signer/keydbstore/rethink_keydbstore.go
@@ -9,7 +9,6 @@ import (
 	"github.com/docker/notary/storage/rethinkdb"
 	"github.com/docker/notary/trustmanager"
 	"github.com/docker/notary/tuf/data"
-	"github.com/docker/notary/tuf/utils"
 	jose "github.com/dvsekhvalnov/jose2go"
 	"gopkg.in/dancannon/gorethink.v2"
 )
@@ -32,8 +31,14 @@ type RDBPrivateKey struct {
 	KeywrapAlg      string `gorethink:"keywrap_alg"`
 	Algorithm       string `gorethink:"algorithm"`
 	PassphraseAlias string `gorethink:"passphrase_alias"`
-	Public          string `gorethink:"public"`
-	Private         string `gorethink:"private"`
+
+	// gorethink specifically supports binary types, and says to pass it in as
+	// a byteslice.  Currently our encryption method for the private key bytes
+	// produces a base64-encoded string, but for future compatibility in case
+	// we change how we encrypt, use a byteslace for the encrypted private key
+	// too
+	Public  []byte `gorethink:"public"`
+	Private []byte `gorethink:"private"`
 }
 
 // gorethink can't handle an UnmarshalJSON function (see https://github.com/dancannon/gorethink/issues/201),
@@ -48,8 +53,8 @@ func rdbPrivateKeyFromJSON(data []byte) (interface{}, error) {
 		KeywrapAlg      string    `json:"keywrap_alg"`
 		Algorithm       string    `json:"algorithm"`
 		PassphraseAlias string    `json:"passphrase_alias"`
-		Public          string    `json:"public"`
-		Private         string    `json:"private"`
+		Public          []byte    `json:"public"`
+		Private         []byte    `json:"private"`
 	}{}
 	if err := json.Unmarshal(data, &a); err != nil {
 		return RDBPrivateKey{}, err
@@ -74,7 +79,7 @@ func rdbPrivateKeyFromJSON(data []byte) (interface{}, error) {
 // PrivateKeysRethinkTable is the table definition for notary signer's key information
 var PrivateKeysRethinkTable = rethinkdb.Table{
 	Name:             RDBPrivateKey{}.TableName(),
-	PrimaryKey:       RDBPrivateKey{}.KeyID,
+	PrimaryKey:       "key_id",
 	JSONUnmarshaller: rdbPrivateKeyFromJSON,
 }
 
@@ -124,8 +129,9 @@ func (rdb *RethinkDBKeyStore) AddKey(keyInfo trustmanager.KeyInfo, privKey data.
 		KeywrapAlg:      KeywrapAlg,
 		PassphraseAlias: rdb.defaultPassAlias,
 		Algorithm:       privKey.Algorithm(),
-		Public:          string(privKey.Public()),
-		Private:         encryptedKey}
+		Public:          privKey.Public(),
+		Private:         []byte(encryptedKey),
+	}
 
 	// Add encrypted private key to the database
 	_, err = gorethink.DB(rdb.dbName).Table(rethinkPrivKey.TableName()).Insert(rethinkPrivKey).RunWrite(rdb.sess)
@@ -136,13 +142,13 @@ func (rdb *RethinkDBKeyStore) AddKey(keyInfo trustmanager.KeyInfo, privKey data.
 	return nil
 }
 
-// GetKey returns the PrivateKey given a KeyID
-func (rdb *RethinkDBKeyStore) GetKey(name string) (data.PrivateKey, string, error) {
+// getKeyBytes returns the RDBPrivateKey given a KeyID, as well as the decrypted private bytes
+func (rdb *RethinkDBKeyStore) getKey(keyID string) (*RDBPrivateKey, string, error) {
 	// Retrieve the RethinkDB private key from the database
 	dbPrivateKey := RDBPrivateKey{}
-	res, err := gorethink.DB(rdb.dbName).Table(dbPrivateKey.TableName()).Filter(gorethink.Row.Field("key_id").Eq(name)).Run(rdb.sess)
+	res, err := gorethink.DB(rdb.dbName).Table(dbPrivateKey.TableName()).Filter(gorethink.Row.Field("key_id").Eq(keyID)).Run(rdb.sess)
 	if err != nil {
-		return nil, "", trustmanager.ErrKeyNotFound{}
+		return nil, "", err
 	}
 	defer res.Close()
 
@@ -158,12 +164,23 @@ func (rdb *RethinkDBKeyStore) GetKey(name string) (data.PrivateKey, string, erro
 	}
 
 	// Decrypt private bytes from the gorm key
-	decryptedPrivKey, _, err := jose.Decode(dbPrivateKey.Private, passphrase)
+	decryptedPrivKey, _, err := jose.Decode(string(dbPrivateKey.Private), passphrase)
 	if err != nil {
 		return nil, "", err
 	}
 
-	pubKey := data.NewPublicKey(dbPrivateKey.Algorithm, []byte(dbPrivateKey.Public))
+	return &dbPrivateKey, decryptedPrivKey, nil
+}
+
+// GetKey returns the PrivateKey given a KeyID
+func (rdb *RethinkDBKeyStore) GetKey(keyID string) (data.PrivateKey, string, error) {
+	dbPrivateKey, decryptedPrivKey, err := rdb.getKey(keyID)
+	if err != nil {
+		return nil, "", err
+	}
+
+	pubKey := data.NewPublicKey(dbPrivateKey.Algorithm, dbPrivateKey.Public)
+
 	// Create a new PrivateKey with unencrypted bytes
 	privKey, err := data.NewPrivateKey(pubKey, []byte(decryptedPrivKey))
 	if err != nil {
@@ -196,28 +213,8 @@ func (rdb RethinkDBKeyStore) RemoveKey(keyID string) error {
 }
 
 // RotateKeyPassphrase rotates the key-encryption-key
-func (rdb RethinkDBKeyStore) RotateKeyPassphrase(name, newPassphraseAlias string) error {
-	// Retrieve the RethinkDB private key from the database
-	dbPrivateKey := RDBPrivateKey{KeyID: name}
-	res, err := gorethink.DB(rdb.dbName).Table(dbPrivateKey.TableName()).Get(dbPrivateKey).Run(rdb.sess)
-	if err != nil {
-		return trustmanager.ErrKeyNotFound{}
-	}
-	defer res.Close()
-
-	err = res.One(&dbPrivateKey)
-	if err != nil {
-		return trustmanager.ErrKeyNotFound{}
-	}
-
-	// Get the current passphrase to use for this key
-	passphrase, _, err := rdb.retriever(dbPrivateKey.KeyID, dbPrivateKey.PassphraseAlias, false, 1)
-	if err != nil {
-		return err
-	}
-
-	// Decrypt private bytes from the rethinkDB key
-	decryptedPrivKey, _, err := jose.Decode(dbPrivateKey.Private, passphrase)
+func (rdb RethinkDBKeyStore) RotateKeyPassphrase(keyID, newPassphraseAlias string) error {
+	dbPrivateKey, decryptedPrivKey, err := rdb.getKey(keyID)
 	if err != nil {
 		return err
 	}
@@ -235,9 +232,9 @@ func (rdb RethinkDBKeyStore) RotateKeyPassphrase(name, newPassphraseAlias string
 	}
 
 	// Update the database object
-	dbPrivateKey.Private = newEncryptedKey
+	dbPrivateKey.Private = []byte(newEncryptedKey)
 	dbPrivateKey.PassphraseAlias = newPassphraseAlias
-	if _, err := gorethink.DB(rdb.dbName).Table(dbPrivateKey.TableName()).Get(RDBPrivateKey{KeyID: name}).Update(dbPrivateKey).RunWrite(rdb.sess); err != nil {
+	if _, err := gorethink.DB(rdb.dbName).Table(dbPrivateKey.TableName()).Get(keyID).Update(dbPrivateKey).RunWrite(rdb.sess); err != nil {
 		return err
 	}
 
@@ -256,17 +253,10 @@ func (rdb RethinkDBKeyStore) Bootstrap() error {
 
 // CheckHealth verifies that DB exists and is query-able
 func (rdb RethinkDBKeyStore) CheckHealth() error {
-	var tables []string
-	dbPrivateKey := RDBPrivateKey{}
-	res, err := gorethink.DB(rdb.dbName).TableList().Run(rdb.sess)
+	res, err := gorethink.DB(rdb.dbName).Table(PrivateKeysRethinkTable.Name).Info().Run(rdb.sess)
 	if err != nil {
-		return err
+		return fmt.Errorf("%s is unavailable, or missing one or more tables, or permissions are incorrectly set", rdb.dbName)
 	}
 	defer res.Close()
-	err = res.All(&tables)
-	if err != nil || !utils.StrSliceContains(tables, dbPrivateKey.TableName()) {
-		return fmt.Errorf(
-			"Cannot access table: %s", dbPrivateKey.TableName())
-	}
 	return nil
 }
diff --git a/signer/keydbstore/rethink_keydbstore_test.go b/signer/keydbstore/rethink_keydbstore_test.go
@@ -13,13 +13,19 @@ func TestRDBPrivateKeyJSONUnmarshalling(t *testing.T) {
 	created := time.Now().AddDate(-1, -1, -1)
 	updated := time.Now().AddDate(0, -5, 0)
 	deleted := time.Time{}
+	publicKey := []byte("Hello world public")
+	privateKey := []byte("Hello world private")
 
 	createdMarshalled, err := json.Marshal(created)
 	require.NoError(t, err)
 	updatedMarshalled, err := json.Marshal(updated)
 	require.NoError(t, err)
 	deletedMarshalled, err := json.Marshal(deleted)
 	require.NoError(t, err)
+	publicMarshalled, err := json.Marshal(publicKey)
+	require.NoError(t, err)
+	privateMarshalled, err := json.Marshal(privateKey)
+	require.NoError(t, err)
 
 	jsonBytes := []byte(fmt.Sprintf(`
 	{
@@ -31,11 +37,10 @@ func TestRDBPrivateKeyJSONUnmarshalling(t *testing.T) {
 		"keywrap_alg": "PBES2-HS256+A128KW",
 		"algorithm": "ecdsa",
 		"passphrase_alias": "timestamp_1",
-		"public": "Hello world public",
-		"private": "Hello world private"
+		"public": %s,
+		"private": %s
 	}
-	`, createdMarshalled, updatedMarshalled, deletedMarshalled))
-	fmt.Println(string(jsonBytes))
+	`, createdMarshalled, updatedMarshalled, deletedMarshalled, publicMarshalled, privateMarshalled))
 
 	unmarshalledAnon, err := PrivateKeysRethinkTable.JSONUnmarshaller(jsonBytes)
 	require.NoError(t, err)
@@ -56,8 +61,8 @@ func TestRDBPrivateKeyJSONUnmarshalling(t *testing.T) {
 		KeywrapAlg:      "PBES2-HS256+A128KW",
 		Algorithm:       "ecdsa",
 		PassphraseAlias: "timestamp_1",
-		Public:          "Hello world public",
-		Private:         "Hello world private",
+		Public:          publicKey,
+		Private:         privateKey,
 	}
 	require.Equal(t, expected, unmarshalled)
 }