From 8d6474b5fde88ffde54a1842d53a6c7ef25ae97e Mon Sep 17 00:00:00 2001 From: George Tankersley Date: Tue, 27 Oct 2015 13:58:54 -0700 Subject: [PATCH 1/2] pkg/crypto: add AES-GCM functions --- pkg/crypto/aes.go | 52 ++++++++++++++++++++++++++++++++++++++++-- pkg/crypto/aes_test.go | 35 ++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/pkg/crypto/aes.go b/pkg/crypto/aes.go index d5dad127..109cd201 100644 --- a/pkg/crypto/aes.go +++ b/pkg/crypto/aes.go @@ -7,6 +7,8 @@ import ( "errors" ) +const aesKeySize = 32 // force 256-bit AES + // pad uses the PKCS#7 padding scheme to align the a payload to a specific block size func pad(plaintext []byte, bsize int) ([]byte, error) { if bsize >= 256 { @@ -33,7 +35,7 @@ func unpad(paddedtext []byte) ([]byte, error) { return paddedtext[:length-(pad)], nil } -// AESEncrypt encrypts a payloaded with an AES cipher. +// **DEPRECATED** AESEncrypt encrypts a payloaded with an AES cipher. // The returned ciphertext has three notable properties: // 1. ciphertext is aligned to the standard AES block size // 2. ciphertext is padded using PKCS#7 @@ -61,7 +63,7 @@ func AESEncrypt(plaintext, key []byte) ([]byte, error) { return ciphertext, nil } -// AESDecrypt decrypts an encrypted payload with an AES cipher. +// **DEPRECATED** AESDecrypt decrypts an encrypted payload with an AES cipher. // The decryption algorithm makes three assumptions: // 1. ciphertext is aligned to the standard AES block size // 2. ciphertext is padded using PKCS#7 @@ -94,3 +96,49 @@ 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 +func Encrypt(plaintext, key []byte) (ciphertext []byte, err error) { + if len(key) != aesKeySize { + return nil, aes.KeySizeError(len(key)) + } + + aes, err := aes.NewCipher(key) + if err != nil { + return nil, err + } + + gcm, err := cipher.NewGCM(aes) + if err != nil { + return nil, err + } + + nonce, err := RandBytes(gcm.NonceSize()) + if err != nil { + return nil, err + } + + 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 +func Decrypt(ciphertext, key []byte) (plaintext []byte, err error) { + if len(key) != aesKeySize { + return nil, aes.KeySizeError(len(key)) + } + + aes, err := aes.NewCipher(key) + if err != nil { + return nil, err + } + + gcm, err := cipher.NewGCM(aes) + if err != nil { + return nil, err + } + + return gcm.Open(nil, ciphertext[:gcm.NonceSize()], + ciphertext[gcm.NonceSize():], nil) +} diff --git a/pkg/crypto/aes_test.go b/pkg/crypto/aes_test.go index eafb5b8b..5cb46c5a 100644 --- a/pkg/crypto/aes_test.go +++ b/pkg/crypto/aes_test.go @@ -1,6 +1,7 @@ package crypto import ( + "bytes" "reflect" "testing" ) @@ -91,3 +92,37 @@ func TestAESDecryptWrongKey(t *testing.T) { t.Fatalf("Data decrypted with different key matches original payload") } } + +func TestEncryptDecryptGCM(t *testing.T) { + gcmTests := []struct { + plaintext []byte + key []byte + }{ + { + plaintext: []byte("Hello, world!"), + key: append([]byte("shark"), make([]byte, 27)...), + }, + } + + for _, tt := range gcmTests { + ciphertext, err := Encrypt(tt.plaintext, tt.key) + if err != nil { + t.Fatal(err) + } + + plaintext, err := Decrypt(ciphertext, tt.key) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(plaintext, tt.plaintext) { + t.Errorf("plaintexts don't match") + } + + ciphertext[0] ^= 0xff + plaintext, err = Decrypt(ciphertext, tt.key) + if err == nil { + t.Errorf("gcmOpen should not have worked, but did") + } + } +} From 07a4d4441e20ba004ea66c4d97b097b1b7c55e1a Mon Sep 17 00:00:00 2001 From: George Tankersley Date: Wed, 28 Oct 2015 16:49:25 -0700 Subject: [PATCH 2/2] pkg/crypto: replace old crypto with new crypto --- cmd/dex-overlord/main.go | 4 ++- cmd/dex-worker/main.go | 3 +++ db/key.go | 29 ++++++++++++++++----- db/key_test.go | 2 +- functional/db_test.go | 4 +-- pkg/crypto/{aes.go => encrypt.go} | 16 +++++++----- pkg/crypto/{aes_test.go => encrypt_test.go} | 0 server/config.go | 3 ++- 8 files changed, 43 insertions(+), 18 deletions(-) rename pkg/crypto/{aes.go => encrypt.go} (85%) rename pkg/crypto/{aes_test.go => encrypt_test.go} (100%) diff --git a/cmd/dex-overlord/main.go b/cmd/dex-overlord/main.go index 523f7bfe..8a3a9936 100644 --- a/cmd/dex-overlord/main.go +++ b/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()) } diff --git a/cmd/dex-worker/main.go b/cmd/dex-worker/main.go index 1ebbc10e..59f9bd9f 100644 --- a/cmd/dex-worker/main.go +++ b/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, } } diff --git a/db/key.go b/db/key.go index 16759136..3ac45034 100644 --- a/db/key.go +++ b/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) @@ -97,16 +97,18 @@ func NewPrivateKeySetRepo(dbm *gorp.DbMap, secrets ...[]byte) (*PrivateKeySetRep } r := &PrivateKeySetRepo{ - dbMap: dbm, - secrets: secrets, + dbMap: dbm, + useOldFormat: useOldFormat, + secrets: secrets, } return r, nil } type PrivateKeySetRepo struct { - dbMap *gorp.DbMap - secrets [][]byte + dbMap *gorp.DbMap + useOldFormat bool + secrets [][]byte } func (r *PrivateKeySetRepo) Set(ks key.KeySet) error { @@ -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 - j, err = pcrypto.AESDecrypt(b.Value, secret) + + if r.useOldFormat { + j, err = pcrypto.AESDecrypt(b.Value, secret) + } else { + j, err = pcrypto.Decrypt(b.Value, secret) + } + if err != nil { continue } diff --git a/db/key_test.go b/db/key_test.go index fe2c2570..99449b46 100644 --- a/db/key_test.go +++ b/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") } diff --git a/functional/db_test.go b/functional/db_test.go index 1b99a8c3..5c125146 100644 --- a/functional/db_test.go +++ b/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()) } diff --git a/pkg/crypto/aes.go b/pkg/crypto/encrypt.go similarity index 85% rename from pkg/crypto/aes.go rename to pkg/crypto/encrypt.go index 109cd201..0372a781 100644 --- a/pkg/crypto/aes.go +++ b/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)) diff --git a/pkg/crypto/aes_test.go b/pkg/crypto/encrypt_test.go similarity index 100% rename from pkg/crypto/aes_test.go rename to pkg/crypto/encrypt_test.go diff --git a/server/config.go b/server/config.go index fe7c41f7..08eb318c 100644 --- a/server/config.go +++ b/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) }