feat: encrypt oidc and git auth tokens in the database

coder · kylecarbs · May 30, 2023 · Jun 11, 2023 · Jun 12, 2023 · Jun 12, 2023
commit 6651fe1bd2735e406fe9fc2c97c516374fa54e1d
diff --git a/coderd/authorize.go b/coderd/authorize.go
@@ -75,7 +75,7 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
 		}
 		// Log information for debugging. This will be very helpful
 		// in the early days
-		logger.Warn(r.Context(), "unauthorized",
+		logger.Debug(r.Context(), "unauthorized",
 			slog.F("roles", roles.Actor.SafeRoleNames()),
 			slog.F("actor_id", roles.Actor.ID),
 			slog.F("actor_name", roles.ActorName),

diff --git a/coderd/database/dbcrypt/dbcrypt.go b/coderd/database/dbcrypt/dbcrypt.go
@@ -0,0 +1,164 @@
+package dbcrypt
+
+import (
+	"context"
+	"database/sql"
+	"strings"
+	"sync/atomic"
+
+	"golang.org/x/xerrors"
+
+	"github.com/coder/coder/coderd/database"
+	"github.com/coder/coder/cryptorand"
+)
+
+// MagicPrefix is prepended to all encrypted values in the database.
+// This is used to determine if a value is encrypted or not.
+// If it is encrypted but a key is not provided, an error is returned.
+const MagicPrefix = "dbcrypt-"
+
+// ErrInvalidCipher is returned when an invalid cipher is provided
+// for the encrypted data.
+var ErrInvalidCipher = xerrors.New("an invalid encryption cipher was provided for the encrypted data")
+
+type Options struct {
+	// ExternalTokenCipher is an optional cipher that is used
+	// to encrypt/decrypt user link and git auth link tokens. If this is nil,
+	// then no encryption/decryption will be performed.
+	ExternalTokenCipher *atomic.Pointer[cryptorand.Cipher]
+}
+
+// New creates a database.Store wrapper that encrypts/decrypts values
+// stored at rest in the database.
+func New(db database.Store, options *Options) database.Store {
+	return &dbCrypt{
+		Options: options,
+		Store:   db,
+	}
+}
+
+type dbCrypt struct {
+	*Options
+	database.Store
+}
+
+func (db *dbCrypt) InTx(function func(database.Store) error, txOpts *sql.TxOptions) error {
+	return db.Store.InTx(func(s database.Store) error {
+		return function(&dbCrypt{
+			Options: db.Options,
+			Store:   s,
+		})
+	}, txOpts)
+}
+
+func (db *dbCrypt) GetUserLinkByLinkedID(ctx context.Context, linkedID string) (database.UserLink, error) {
+	link, err := db.Store.GetUserLinkByLinkedID(ctx, linkedID)
+	if err != nil {
+		return database.UserLink{}, err
+	}
+	return link, db.decryptFields(&link.OAuthAccessToken, &link.OAuthRefreshToken)
+}
+
+func (db *dbCrypt) GetUserLinkByUserIDLoginType(ctx context.Context, params database.GetUserLinkByUserIDLoginTypeParams) (database.UserLink, error) {
+	link, err := db.Store.GetUserLinkByUserIDLoginType(ctx, params)
+	if err != nil {
+		return database.UserLink{}, err
+	}
+	return link, db.decryptFields(&link.OAuthAccessToken, &link.OAuthRefreshToken)
+}
+
+func (db *dbCrypt) InsertUserLink(ctx context.Context, params database.InsertUserLinkParams) (database.UserLink, error) {
+	err := db.encryptFields(&params.OAuthAccessToken, &params.OAuthRefreshToken)
+	if err != nil {
+		return database.UserLink{}, err
+	}
+	return db.Store.InsertUserLink(ctx, params)
+}
+
+func (db *dbCrypt) UpdateUserLink(ctx context.Context, params database.UpdateUserLinkParams) (database.UserLink, error) {
+	err := db.encryptFields(&params.OAuthAccessToken, &params.OAuthRefreshToken)
+	if err != nil {
+		return database.UserLink{}, err
+	}
+	return db.Store.UpdateUserLink(ctx, params)
+}
+
+func (db *dbCrypt) InsertGitAuthLink(ctx context.Context, params database.InsertGitAuthLinkParams) (database.GitAuthLink, error) {
+	err := db.encryptFields(&params.OAuthAccessToken, &params.OAuthRefreshToken)
+	if err != nil {
+		return database.GitAuthLink{}, err
+	}
+	return db.Store.InsertGitAuthLink(ctx, params)
+}
+
+func (db *dbCrypt) GetGitAuthLink(ctx context.Context, params database.GetGitAuthLinkParams) (database.GitAuthLink, error) {
+	link, err := db.Store.GetGitAuthLink(ctx, params)
+	if err != nil {
+		return database.GitAuthLink{}, err
+	}
+	return link, db.decryptFields(&link.OAuthAccessToken, &link.OAuthRefreshToken)
+}
+
+func (db *dbCrypt) UpdateGitAuthLink(ctx context.Context, params database.UpdateGitAuthLinkParams) (database.GitAuthLink, error) {
+	err := db.encryptFields(&params.OAuthAccessToken, &params.OAuthRefreshToken)
+	if err != nil {
+		return database.GitAuthLink{}, err
+	}
+	return db.Store.UpdateGitAuthLink(ctx, params)
+}
+
+func (db *dbCrypt) encryptFields(fields ...*string) error {
+	cipherPtr := db.ExternalTokenCipher.Load()
+	// If no cipher is loaded, then we don't need to encrypt or decrypt anything!
+	if cipherPtr == nil {
+		return nil
+	}
+	cipher := *cipherPtr
+	for _, field := range fields {
+		if field == nil {
+			continue
+		}
+
+		encrypted, err := cipher.Encrypt([]byte(*field))
+		if err != nil {
+			return err
+		}
+		*field = MagicPrefix + string(encrypted)
+	}
+	return nil
+}
+
+// decryptFields decrypts the given fields in place.
+// If the value fails to decrypt, sql.ErrNoRows will be returned.
+func (db *dbCrypt) decryptFields(fields ...*string) error {
+	cipherPtr := db.ExternalTokenCipher.Load()
+	// If no cipher is loaded, then we don't need to encrypt or decrypt anything!
+	if cipherPtr == nil {
+		for _, field := range fields {
+			if field == nil {
+				continue
+			}
+			if strings.HasPrefix(*field, MagicPrefix) {
+				return ErrInvalidCipher
+			}
+		}
+		return nil
+	}
+
+	cipher := *cipherPtr
+	for _, field := range fields {
+		if field == nil {
+			continue
+		}
+		if len(*field) < len(MagicPrefix) || !strings.HasPrefix(*field, MagicPrefix) {
+			continue
+		}
+
+		decrypted, err := cipher.Decrypt([]byte((*field)[len(MagicPrefix):]))
+		if err != nil {
+			return xerrors.Errorf("%w: %s", ErrInvalidCipher, err)
+		}
+		*field = string(decrypted)
+	}
+	return nil
+}
diff --git a/coderd/database/dbcrypt/dbcrypt_test.go b/coderd/database/dbcrypt/dbcrypt_test.go
@@ -0,0 +1,196 @@
+package dbcrypt_test
+
+import (
+	"context"
+	"crypto/rand"
+	"io"
+	"sync/atomic"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/coderd/database"
+	"github.com/coder/coder/coderd/database/dbcrypt"
+	"github.com/coder/coder/coderd/database/dbfake"
+	"github.com/coder/coder/coderd/database/dbgen"
+	"github.com/coder/coder/cryptorand"
+)
+
+func TestUserLinks(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	t.Run("InsertUserLink", func(t *testing.T) {
+		t.Parallel()
+		db, crypt, cipher := setup(t)
+		initCipher(t, cipher)
+		link := dbgen.UserLink(t, crypt, database.UserLink{
+			OAuthAccessToken:  "access",
+			OAuthRefreshToken: "refresh",
+		})
+		link, err := db.GetUserLinkByLinkedID(ctx, link.LinkedID)
+		require.NoError(t, err)
+		requireEncryptedEquals(t, cipher, link.OAuthAccessToken, "access")
+		requireEncryptedEquals(t, cipher, link.OAuthRefreshToken, "refresh")
+	})
+
+	t.Run("UpdateUserLink", func(t *testing.T) {
+		t.Parallel()
+		db, crypt, cipher := setup(t)
+		initCipher(t, cipher)
+		link := dbgen.UserLink(t, crypt, database.UserLink{})
+		_, err := crypt.UpdateUserLink(ctx, database.UpdateUserLinkParams{
+			OAuthAccessToken:  "access",
+			OAuthRefreshToken: "refresh",
+			UserID:            link.UserID,
+			LoginType:         link.LoginType,
+		})
+		require.NoError(t, err)
+		link, err = db.GetUserLinkByLinkedID(ctx, link.LinkedID)
+		require.NoError(t, err)
+		requireEncryptedEquals(t, cipher, link.OAuthAccessToken, "access")
+		requireEncryptedEquals(t, cipher, link.OAuthRefreshToken, "refresh")
+	})
+
+	t.Run("GetUserLinkByLinkedID", func(t *testing.T) {
+		t.Parallel()
+		db, crypt, cipher := setup(t)
+		initCipher(t, cipher)
+		link := dbgen.UserLink(t, crypt, database.UserLink{
+			OAuthAccessToken:  "access",
+			OAuthRefreshToken: "refresh",
+		})
+		link, err := db.GetUserLinkByLinkedID(ctx, link.LinkedID)
+		require.NoError(t, err)
+		requireEncryptedEquals(t, cipher, link.OAuthAccessToken, "access")
+		requireEncryptedEquals(t, cipher, link.OAuthRefreshToken, "refresh")
+
+		// Reset the key and empty values should be returned!
+		initCipher(t, cipher)
+
+		link, err = crypt.GetUserLinkByLinkedID(ctx, link.LinkedID)
+		require.ErrorIs(t, err, dbcrypt.ErrInvalidCipher)
+	})
+
+	t.Run("GetUserLinkByUserIDLoginType", func(t *testing.T) {
+		t.Parallel()
+		db, crypt, cipher := setup(t)
+		initCipher(t, cipher)
+		link := dbgen.UserLink(t, crypt, database.UserLink{
+			OAuthAccessToken:  "access",
+			OAuthRefreshToken: "refresh",
+		})
+		link, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{
+			UserID:    link.UserID,
+			LoginType: link.LoginType,
+		})
+		require.NoError(t, err)
+		requireEncryptedEquals(t, cipher, link.OAuthAccessToken, "access")
+		requireEncryptedEquals(t, cipher, link.OAuthRefreshToken, "refresh")
+
+		// Reset the key and empty values should be returned!
+		initCipher(t, cipher)
+
+		link, err = crypt.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{
+			UserID:    link.UserID,
+			LoginType: link.LoginType,
+		})
+		require.ErrorIs(t, err, dbcrypt.ErrInvalidCipher)
+	})
+}
+
+func TestGitAuthLinks(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	t.Run("InsertGitAuthLink", func(t *testing.T) {
+		t.Parallel()
+		db, crypt, cipher := setup(t)
+		initCipher(t, cipher)
+		link := dbgen.GitAuthLink(t, crypt, database.GitAuthLink{
+			OAuthAccessToken:  "access",
+			OAuthRefreshToken: "refresh",
+		})
+		link, err := db.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{
+			ProviderID: link.ProviderID,
+			UserID:     link.UserID,
+		})
+		require.NoError(t, err)
+		requireEncryptedEquals(t, cipher, link.OAuthAccessToken, "access")
+		requireEncryptedEquals(t, cipher, link.OAuthRefreshToken, "refresh")
+	})
+
+	t.Run("UpdateGitAuthLink", func(t *testing.T) {
+		t.Parallel()
+		db, crypt, cipher := setup(t)
+		initCipher(t, cipher)
+		link := dbgen.GitAuthLink(t, crypt, database.GitAuthLink{})
+		_, err := crypt.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{
+			ProviderID:        link.ProviderID,
+			UserID:            link.UserID,
+			OAuthAccessToken:  "access",
+			OAuthRefreshToken: "refresh",
+		})
+		require.NoError(t, err)
+		link, err = db.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{
+			ProviderID: link.ProviderID,
+			UserID:     link.UserID,
+		})
+		require.NoError(t, err)
+		requireEncryptedEquals(t, cipher, link.OAuthAccessToken, "access")
+		requireEncryptedEquals(t, cipher, link.OAuthRefreshToken, "refresh")
+	})
+
+	t.Run("GetGitAuthLink", func(t *testing.T) {
+		t.Parallel()
+		db, crypt, cipher := setup(t)
+		initCipher(t, cipher)
+		link := dbgen.GitAuthLink(t, crypt, database.GitAuthLink{
+			OAuthAccessToken:  "access",
+			OAuthRefreshToken: "refresh",
+		})
+		link, err := db.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{
+			UserID:     link.UserID,
+			ProviderID: link.ProviderID,
+		})
+		require.NoError(t, err)
+		requireEncryptedEquals(t, cipher, link.OAuthAccessToken, "access")
+		requireEncryptedEquals(t, cipher, link.OAuthRefreshToken, "refresh")
+
+		// Reset the key and empty values should be returned!
+		initCipher(t, cipher)
+
+		link, err = crypt.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{
+			UserID:     link.UserID,
+			ProviderID: link.ProviderID,
+		})
+		require.ErrorIs(t, err, dbcrypt.ErrInvalidCipher)
+	})
+}
+
+func requireEncryptedEquals(t *testing.T, cipher *atomic.Pointer[cryptorand.Cipher], value, expected string) {
+	t.Helper()
+	c := (*cipher.Load())
+	got, err := c.Decrypt([]byte(value[len(dbcrypt.MagicPrefix):]))
+	require.NoError(t, err)
+	require.Equal(t, expected, string(got))
+}
+
+func initCipher(t *testing.T, cipher *atomic.Pointer[cryptorand.Cipher]) {
+	t.Helper()
+	key := make([]byte, 32) // AES-256 key size is 32 bytes
+	_, err := io.ReadFull(rand.Reader, key)
+	require.NoError(t, err)
+	c, err := cryptorand.CipherAES256(key)
+	require.NoError(t, err)
+	cipher.Store(&c)
+}
+
+func setup(t *testing.T) (db, cryptodb database.Store, cipher *atomic.Pointer[cryptorand.Cipher]) {
+	t.Helper()
+	rawDB := dbfake.New()
+	cipher = &atomic.Pointer[cryptorand.Cipher]{}
+	return rawDB, dbcrypt.New(rawDB, &dbcrypt.Options{
+		ExternalTokenCipher: cipher,
+	}), cipher
+}
diff --git a/coderd/database/dbgen/generator.go b/coderd/database/dbgen/generator.go
@@ -385,7 +385,7 @@ func UserLink(t testing.TB, db database.Store, orig database.UserLink) database.
 		LoginType:         takeFirst(orig.LoginType, database.LoginTypeGithub),
 		LinkedID:          takeFirst(orig.LinkedID),
 		OAuthAccessToken:  takeFirst(orig.OAuthAccessToken, uuid.NewString()),
-		OAuthRefreshToken: takeFirst(orig.OAuthAccessToken, uuid.NewString()),
+		OAuthRefreshToken: takeFirst(orig.OAuthRefreshToken, uuid.NewString()),
 		OAuthExpiry:       takeFirst(orig.OAuthExpiry, database.Now().Add(time.Hour*24)),
 	})
 
@@ -398,7 +398,7 @@ func GitAuthLink(t testing.TB, db database.Store, orig database.GitAuthLink) dat
 		ProviderID:        takeFirst(orig.ProviderID, uuid.New().String()),
 		UserID:            takeFirst(orig.UserID, uuid.New()),
 		OAuthAccessToken:  takeFirst(orig.OAuthAccessToken, uuid.NewString()),
-		OAuthRefreshToken: takeFirst(orig.OAuthAccessToken, uuid.NewString()),
+		OAuthRefreshToken: takeFirst(orig.OAuthRefreshToken, uuid.NewString()),
 		OAuthExpiry:       takeFirst(orig.OAuthExpiry, database.Now().Add(time.Hour*24)),
 		CreatedAt:         takeFirst(orig.CreatedAt, database.Now()),
 		UpdatedAt:         takeFirst(orig.UpdatedAt, database.Now()),