pkg/crypto: replace old crypto with new crypto

cmd/dex-overlord/main.go

@@ -32,6 +32,8 @@ func main() {
 	keySecrets := pflag.NewBase64List(32)
 	fs.Var(keySecrets, "key-secrets", "A comma-separated list of base64 encoded 32 byte strings used as symmetric keys used to encrypt/decrypt signing key data in DB. The first key is considered the active key and used for encryption, while the others are used to decrypt.")
 
+	useOldFormat := fs.Bool("use-deprecated-secret-format", false, "In prior releases, the database used AES-CBC to encrypt keys. New deployments should use the default AES-GCM encryption.")
+
 	dbURL := fs.String("db-url", "", "DSN-formatted database connection string")
 
 	dbMigrate := fs.Bool("db-migrate", true, "perform database migrations when starting up overlord. This includes the initial DB objects creation.")
@@ -100,7 +102,7 @@ func main() {
 	userManager := user.NewManager(userRepo,
 		pwiRepo, db.TransactionFactory(dbc), user.ManagerOptions{})
 	adminAPI := admin.NewAdminAPI(userManager, userRepo, pwiRepo, *localConnectorID)
-	kRepo, err := db.NewPrivateKeySetRepo(dbc, keySecrets.BytesSlice()...)
+	kRepo, err := db.NewPrivateKeySetRepo(dbc, *useOldFormat, keySecrets.BytesSlice()...)
 	if err != nil {
 		log.Fatalf(err.Error())
 	}

cmd/dex-worker/main.go

@@ -58,6 +58,8 @@ func main() {
 	keySecrets := pflag.NewBase64List(32)
 	fs.Var(keySecrets, "key-secrets", "A comma-separated list of base64 encoded 32 byte strings used as symmetric keys used to encrypt/decrypt signing key data in DB. The first key is considered the active key and used for encryption, while the others are used to decrypt.")
 
+	useOldFormat := fs.Bool("use-deprecated-secret-format", false, "In prior releases, the database used AES-CBC to encrypt keys. New deployments should use the default AES-GCM encryption.")
+
 	dbMaxIdleConns := fs.Int("db-max-idle-conns", 0, "maximum number of connections in the idle connection pool")
 	dbMaxOpenConns := fs.Int("db-max-open-conns", 0, "maximum number of open connections to the database")
 
@@ -147,6 +149,7 @@ func main() {
 		scfg.StateConfig = &server.MultiServerConfig{
 			KeySecrets:     keySecrets.BytesSlice(),
 			DatabaseConfig: dbCfg,
+			UseOldFormat:   *useOldFormat,
 		}
 	}
 

db/key.go

@@ -89,7 +89,7 @@ type privateKeySetBlob struct {
 	Value []byte `db:"value"`
 }
 
-func NewPrivateKeySetRepo(dbm *gorp.DbMap, secrets ...[]byte) (*PrivateKeySetRepo, error) {
+func NewPrivateKeySetRepo(dbm *gorp.DbMap, useOldFormat bool, secrets ...[]byte) (*PrivateKeySetRepo, error) {
 	for i, secret := range secrets {
 		if len(secret) != 32 {
 			return nil, fmt.Errorf("key secret %d: expected 32-byte secret", i)
@@ -98,6 +98,7 @@ func NewPrivateKeySetRepo(dbm *gorp.DbMap, secrets ...[]byte) (*PrivateKeySetRep
 
 	r := &PrivateKeySetRepo{
 		dbMap:        dbm,
+		useOldFormat: useOldFormat,
 		secrets:      secrets,
 	}
 
@@ -106,6 +107,7 @@ func NewPrivateKeySetRepo(dbm *gorp.DbMap, secrets ...[]byte) (*PrivateKeySetRep
 
 type PrivateKeySetRepo struct {
 	dbMap        *gorp.DbMap
+	useOldFormat bool
 	secrets      [][]byte
 }
 
@@ -131,7 +133,14 @@ func (r *PrivateKeySetRepo) Set(ks key.KeySet) error {
 		return err
 	}
 
-	v, err := pcrypto.AESEncrypt(j, r.active())
+	var v []byte
+
+	if r.useOldFormat {
+		v, err = pcrypto.AESEncrypt(j, r.active())
+	} else {
+		v, err = pcrypto.Encrypt(j, r.active())
+	}
+
 	if err != nil {
 		return err
 	}
@@ -159,7 +168,13 @@ func (r *PrivateKeySetRepo) Get() (key.KeySet, error) {
 	var pks *key.PrivateKeySet
 	for _, secret := range r.secrets {
 		var j []byte
+
+		if r.useOldFormat {
 			j, err = pcrypto.AESDecrypt(b.Value, secret)
+		} else {
+			j, err = pcrypto.Decrypt(b.Value, secret)
+		}
+
 		if err != nil {
 			continue
 		}

db/key_test.go

@@ -5,7 +5,7 @@ import (
 )
 
 func TestNewPrivateKeySetRepoInvalidKey(t *testing.T) {
-	_, err := NewPrivateKeySetRepo(nil, []byte("sharks"))
+	_, err := NewPrivateKeySetRepo(nil, false, []byte("sharks"))
 	if err == nil {
 		t.Fatalf("Expected non-nil error")
 	}

functional/db_test.go

@@ -155,12 +155,12 @@ func TestDBPrivateKeySetRepoSetGet(t *testing.T) {
 	}
 
 	for i, tt := range tests {
-		setRepo, err := db.NewPrivateKeySetRepo(connect(t), tt.setSecrets...)
+		setRepo, err := db.NewPrivateKeySetRepo(connect(t), false, tt.setSecrets...)
 		if err != nil {
 			t.Fatalf(err.Error())
 		}
 
-		getRepo, err := db.NewPrivateKeySetRepo(connect(t), tt.getSecrets...)
+		getRepo, err := db.NewPrivateKeySetRepo(connect(t), false, tt.getSecrets...)
 		if err != nil {
 			t.Fatalf(err.Error())
 		}

pkg/crypto/aes.go → pkg/crypto/encrypt.go

@@ -35,11 +35,12 @@ func unpad(paddedtext []byte) ([]byte, error) {
 	return paddedtext[:length-(pad)], nil
 }
 
-// **DEPRECATED** AESEncrypt encrypts a payloaded with an AES cipher.
+// AESEncrypt encrypts a payload using AES-CBC and PKCS#7 padding.
 // The returned ciphertext has three notable properties:
 // 1. ciphertext is aligned to the standard AES block size
 // 2. ciphertext is padded using PKCS#7
 // 3. IV is prepended to the ciphertext
+// This function is DEPRECATED. Use Encrypt() instead.
 func AESEncrypt(plaintext, key []byte) ([]byte, error) {
 	plaintext, err := pad(plaintext, aes.BlockSize)
 	if err != nil {
@@ -63,11 +64,12 @@ func AESEncrypt(plaintext, key []byte) ([]byte, error) {
 	return ciphertext, nil
 }
 
-// **DEPRECATED** AESDecrypt decrypts an encrypted payload with an AES cipher.
+// AESDecrypt decrypts a payload encrypted using AES-CBC and PKCS#7 padding.
 // The decryption algorithm makes three assumptions:
 // 1. ciphertext is aligned to the standard AES block size
 // 2. ciphertext is padded using PKCS#7
 // 3. the IV is prepended to ciphertext
+// This function is DEPRECATED. Use Decrypt() instead.
 func AESDecrypt(ciphertext, key []byte) ([]byte, error) {
 	if len(ciphertext) < aes.BlockSize {
 		return nil, errors.New("ciphertext too short")
@@ -97,8 +99,9 @@ func AESDecrypt(ciphertext, key []byte) ([]byte, error) {
 	return unpad(plaintext)
 }
 
-// Takes plaintext and a key, returns ciphertext or error
-// Output takes the form nonce|ciphertext|tag where '|' indicates concatenation
+// Encrypt encrypts data using 256-bit AES-GCM.
+// This both hides the content of the data and provides a check that it hasn't been altered.
+// Output takes the form nonce|ciphertext|tag where '|' indicates concatenation.
 func Encrypt(plaintext, key []byte) (ciphertext []byte, err error) {
 	if len(key) != aesKeySize {
 		return nil, aes.KeySizeError(len(key))
@@ -122,8 +125,9 @@ func Encrypt(plaintext, key []byte) (ciphertext []byte, err error) {
 	return gcm.Seal(nonce, nonce, plaintext, nil), nil
 }
 
-// Takes ciphertext and a key, returns plaintext or error
-// Expects input form nonce|ciphertext|tag where '|' indicates concatenation
+// Decrypt decrypts data using 256-bit AES-GCM.
+// This both hides the content of the data and provides a check that it hasn't been altered.
+// Expects input form nonce|ciphertext|tag where '|' indicates concatenation.
 func Decrypt(ciphertext, key []byte) (plaintext []byte, err error) {
 	if len(key) != aesKeySize {
 		return nil, aes.KeySizeError(len(key))

server/config.go

@@ -49,6 +49,7 @@ type SingleServerConfig struct {
 type MultiServerConfig struct {
 	KeySecrets     [][]byte
 	DatabaseConfig db.Config
+	UseOldFormat   bool
 }
 
 func (cfg *ServerConfig) Server() (*Server, error) {
@@ -159,7 +160,7 @@ func (cfg *MultiServerConfig) Configure(srv *Server) error {
 		return fmt.Errorf("unable to initialize database connection: %v", err)
 	}
 
-	kRepo, err := db.NewPrivateKeySetRepo(dbc, cfg.KeySecrets...)
+	kRepo, err := db.NewPrivateKeySetRepo(dbc, cfg.UseOldFormat, cfg.KeySecrets...)
 	if err != nil {
 		return fmt.Errorf("unable to create PrivateKeySetRepo: %v", err)
 	}