Automatically delete rows when not encrypted

coder · kylecarbs · May 30, 2023 · Jun 11, 2023 · Jun 12, 2023 · Jun 12, 2023
commit deb577b6bab23bac01103303d3eeef7860caa23a
diff --git a/coderd/database/dbcrypt/dbcrypt.go b/coderd/database/dbcrypt/dbcrypt.go
@@ -3,9 +3,12 @@ package dbcrypt
 import (
 	"context"
 	"database/sql"
+	"encoding/base64"
+	"runtime"
 	"strings"
 	"sync/atomic"
 
+	"cdr.dev/slog"
 	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/coderd/database"
@@ -22,6 +25,7 @@ type Options struct {
 	// 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]
+	Logger              slog.Logger
 }
 
 // New creates a database.Store wrapper that encrypts/decrypts values
@@ -128,19 +132,25 @@ func (db *dbCrypt) encryptFields(fields ...*string) error {
 		if err != nil {
 			return err
 		}
-		*field = MagicPrefix + string(encrypted)
+		// Base64 is used to support UTF-8 encoding in PostgreSQL.
+		*field = MagicPrefix + base64.StdEncoding.EncodeToString(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(deleteFn func() error, fields ...*string) error {
-	delete := func() error {
+	delete := func(reason string) error {
 		err := deleteFn()
 		if err != nil {
 			return xerrors.Errorf("delete encrypted row: %w", err)
 		}
+		pc, _, _, ok := runtime.Caller(2)
+		details := runtime.FuncForPC(pc)
+		if ok && details != nil {
+			db.Logger.Debug(context.Background(), "deleted row", slog.F("reason", reason), slog.F("caller", details.Name()))
+		}
 		return sql.ErrNoRows
 	}
 
@@ -154,7 +164,7 @@ func (db *dbCrypt) decryptFields(deleteFn func() error, fields ...*string) error
 			if strings.HasPrefix(*field, MagicPrefix) {
 				// If we have a magic prefix but encryption is disabled,
 				// we should delete the row.
-				return delete()
+				return delete("encryption disabled")
 			}
 		}
 		return nil
@@ -166,13 +176,19 @@ func (db *dbCrypt) decryptFields(deleteFn func() error, fields ...*string) error
 			continue
 		}
 		if len(*field) < len(MagicPrefix) || !strings.HasPrefix(*field, MagicPrefix) {
+			// We do not force encryption of unencrypted rows. This could be damaging
+			// to the deployment, and admins can always manually purge data.
 			continue
 		}
-
-		decrypted, err := cipher.Decrypt([]byte((*field)[len(MagicPrefix):]))
+		data, err := base64.StdEncoding.DecodeString((*field)[len(MagicPrefix):])
+		if err != nil {
+			// If it's not base64 with the prefix, we should delete the row.
+			return delete("stored value was not base64 encoded")
+		}
+		decrypted, err := cipher.Decrypt(data)
 		if err != nil {
 			// If the encryption key changed, we should delete the row.
-			return delete()
+			return delete("encryption key changed")
 		}
 		*field = string(decrypted)
 	}

diff --git a/coderd/database/dbcrypt/dbcrypt_test.go b/coderd/database/dbcrypt/dbcrypt_test.go
@@ -4,10 +4,13 @@ import (
 	"context"
 	"crypto/rand"
 	"database/sql"
+	"encoding/base64"
 	"io"
 	"sync/atomic"
 	"testing"
 
+	"cdr.dev/slog"
+	"cdr.dev/slog/sloggers/slogtest"
 	"github.com/stretchr/testify/require"
 
 	"github.com/coder/coder/coderd/database"
@@ -172,7 +175,9 @@ func TestGitAuthLinks(t *testing.T) {
 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):]))
+	data, err := base64.StdEncoding.DecodeString(value[len(dbcrypt.MagicPrefix):])
+	require.NoError(t, err)
+	got, err := c.Decrypt(data)
 	require.NoError(t, err)
 	require.Equal(t, expected, string(got))
 }
@@ -193,5 +198,6 @@ func setup(t *testing.T) (db, cryptodb database.Store, cipher *atomic.Pointer[cr
 	cipher = &atomic.Pointer[cryptorand.Cipher]{}
 	return rawDB, dbcrypt.New(rawDB, &dbcrypt.Options{
 		ExternalTokenCipher: cipher,
+		Logger:              slogtest.Make(t, nil).Leveled(slog.LevelDebug),
 	}), cipher
 }
diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
@@ -229,6 +229,12 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
 			UserID:    key.UserID,
 			LoginType: key.LoginType,
 		})
+		if errors.Is(err, sql.ErrNoRows) {
-		if errors.Is(err, sql.ErrNoRows) {
+		if xerrors.Is(err, sql.ErrNoRows) {
-		if errors.Is(err, sql.ErrNoRows) {
+		if xerrors.Is(err, sql.ErrNoRows) {
+			return optionalWrite(http.StatusUnauthorized, codersdk.Response{
+				Message: SignedOutErrorMessage,
+				Detail:  "You must re-authenticate with the login provider.",
+			})
+		}
 		if err != nil {
 			return write(http.StatusInternalServerError, codersdk.Response{
 				Message: "A database error occurred",

diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go
@@ -59,6 +59,7 @@ func New(ctx context.Context, options *Options) (*API, error) {
 	externalTokenCipher := &atomic.Pointer[cryptorand.Cipher]{}
 	options.Database = dbcrypt.New(options.Database, &dbcrypt.Options{
 		ExternalTokenCipher: externalTokenCipher,
+		Logger:              options.Logger.Named("dbcrypt"),
 	})
 
 	api := &API{