Skip to content

Commit deb577b

Browse files
kylecarbskylecarbs
committed
Automatically delete rows when not encrypted
1 parent b9251fd commit deb577b

File tree

4 files changed

+36
-7
lines changed

4 files changed

+36
-7
lines changed

‎coderd/database/dbcrypt/dbcrypt.go

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@ package dbcrypt
33
import (
44
"context"
55
"database/sql"
6+
"encoding/base64"
7+
"runtime"
68
"strings"
79
"sync/atomic"
810

11+
"cdr.dev/slog"
912
"golang.org/x/xerrors"
1013

1114
"github.com/coder/coder/coderd/database"
@@ -22,6 +25,7 @@ type Options struct {
2225
// to encrypt/decrypt user link and git auth link tokens. If this is nil,
2326
// then no encryption/decryption will be performed.
2427
ExternalTokenCipher *atomic.Pointer[cryptorand.Cipher]
28+
Logger slog.Logger
2529
}
2630

2731
// New creates a database.Store wrapper that encrypts/decrypts values
@@ -128,19 +132,25 @@ func (db *dbCrypt) encryptFields(fields ...*string) error {
128132
if err != nil {
129133
return err
130134
}
131-
*field = MagicPrefix + string(encrypted)
135+
// Base64 is used to support UTF-8 encoding in PostgreSQL.
136+
*field = MagicPrefix + base64.StdEncoding.EncodeToString(encrypted)
132137
}
133138
return nil
134139
}
135140

136141
// decryptFields decrypts the given fields in place.
137142
// If the value fails to decrypt, sql.ErrNoRows will be returned.
138143
func (db *dbCrypt) decryptFields(deleteFn func() error, fields ...*string) error {
139-
delete := func() error {
144+
delete := func(reason string) error {
140145
err := deleteFn()
141146
if err != nil {
142147
return xerrors.Errorf("delete encrypted row: %w", err)
143148
}
149+
pc, _, _, ok := runtime.Caller(2)
150+
details := runtime.FuncForPC(pc)
151+
if ok && details != nil {
152+
db.Logger.Debug(context.Background(), "deleted row", slog.F("reason", reason), slog.F("caller", details.Name()))
153+
}
144154
return sql.ErrNoRows
145155
}
146156

@@ -154,7 +164,7 @@ func (db *dbCrypt) decryptFields(deleteFn func() error, fields ...*string) error
154164
if strings.HasPrefix(*field, MagicPrefix) {
155165
// If we have a magic prefix but encryption is disabled,
156166
// we should delete the row.
157-
return delete()
167+
return delete("encryption disabled")
158168
}
159169
}
160170
return nil
@@ -166,13 +176,19 @@ func (db *dbCrypt) decryptFields(deleteFn func() error, fields ...*string) error
166176
continue
167177
}
168178
if len(*field) < len(MagicPrefix) || !strings.HasPrefix(*field, MagicPrefix) {
179+
// We do not force encryption of unencrypted rows. This could be damaging
180+
// to the deployment, and admins can always manually purge data.
169181
continue
170182
}
171-
172-
decrypted, err := cipher.Decrypt([]byte((*field)[len(MagicPrefix):]))
183+
data, err := base64.StdEncoding.DecodeString((*field)[len(MagicPrefix):])
184+
if err != nil {
185+
// If it's not base64 with the prefix, we should delete the row.
186+
return delete("stored value was not base64 encoded")
187+
}
188+
decrypted, err := cipher.Decrypt(data)
173189
if err != nil {
174190
// If the encryption key changed, we should delete the row.
175-
return delete()
191+
return delete("encryption key changed")
176192
}
177193
*field = string(decrypted)
178194
}

‎coderd/database/dbcrypt/dbcrypt_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@ import (
44
"context"
55
"crypto/rand"
66
"database/sql"
7+
"encoding/base64"
78
"io"
89
"sync/atomic"
910
"testing"
1011

12+
"cdr.dev/slog"
13+
"cdr.dev/slog/sloggers/slogtest"
1114
"github.com/stretchr/testify/require"
1215

1316
"github.com/coder/coder/coderd/database"
@@ -172,7 +175,9 @@ func TestGitAuthLinks(t *testing.T) {
172175
func requireEncryptedEquals(t *testing.T, cipher *atomic.Pointer[cryptorand.Cipher], value, expected string) {
173176
t.Helper()
174177
c := (*cipher.Load())
175-
got, err := c.Decrypt([]byte(value[len(dbcrypt.MagicPrefix):]))
178+
data, err := base64.StdEncoding.DecodeString(value[len(dbcrypt.MagicPrefix):])
179+
require.NoError(t, err)
180+
got, err := c.Decrypt(data)
176181
require.NoError(t, err)
177182
require.Equal(t, expected, string(got))
178183
}
@@ -193,5 +198,6 @@ func setup(t *testing.T) (db, cryptodb database.Store, cipher *atomic.Pointer[cr
193198
cipher = &atomic.Pointer[cryptorand.Cipher]{}
194199
return rawDB, dbcrypt.New(rawDB, &dbcrypt.Options{
195200
ExternalTokenCipher: cipher,
201+
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
196202
}), cipher
197203
}

‎coderd/httpmw/apikey.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,12 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
229229
UserID: key.UserID,
230230
LoginType: key.LoginType,
231231
})
232+
if errors.Is(err, sql.ErrNoRows) {
233+
return optionalWrite(http.StatusUnauthorized, codersdk.Response{
234+
Message: SignedOutErrorMessage,
235+
Detail: "You must re-authenticate with the login provider.",
236+
})
237+
}
232238
if err != nil {
233239
return write(http.StatusInternalServerError, codersdk.Response{
234240
Message: "A database error occurred",

‎enterprise/coderd/coderd.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ func New(ctx context.Context, options *Options) (*API, error) {
5959
externalTokenCipher := &atomic.Pointer[cryptorand.Cipher]{}
6060
options.Database = dbcrypt.New(options.Database, &dbcrypt.Options{
6161
ExternalTokenCipher: externalTokenCipher,
62+
Logger: options.Logger.Named("dbcrypt"),
6263
})
6364

6465
api := &API{

0 commit comments

Comments
 (0)