From fb953e44d04ae1266d90557e91766b87dacf751e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 4 Sep 2023 20:43:23 +0000 Subject: [PATCH 01/34] feat(coderd): add dbcrypt package - Adds package enterprise/dbcrypt to implement database encryption/decryption - Adds table dbcrypt_keys and associated queries - Adds columns oauth_access_token_key_id and oauth_refresh_token_key_id to tables git_auth_links and user_links NOTE: This is part 1 of a 2-part PR. This PR focuses mainly on the dbcrypt and database packages. A separate PR will add the required plumbing to integrate this into enterprise/coderd properly. Co-authored-by: Kyle Carberry --- coderd/database/dbauthz/dbauthz.go | 35 ++ coderd/database/dbfake/dbfake.go | 151 +++++- coderd/database/dbgen/dbgen.go | 30 +- coderd/database/dbmetrics/dbmetrics.go | 35 ++ coderd/database/dbmock/dbmock.go | 73 +++ coderd/database/dump.sql | 60 ++- .../000154_dbcrypt_key_ids.down.sql | 43 ++ .../migrations/000154_dbcrypt_key_ids.up.sql | 30 ++ coderd/database/migrations/migrate_test.go | 1 + coderd/database/models.go | 24 + coderd/database/querier.go | 5 + coderd/database/queries.sql.go | 257 +++++++-- coderd/database/queries/dbcrypt.sql | 18 + coderd/database/queries/gitauth.sql | 15 +- coderd/database/queries/user_links.sql | 15 +- coderd/database/sqlc.yaml | 3 + coderd/database/unique_constraint.go | 2 + enterprise/dbcrypt/cipher.go | 98 ++++ enterprise/dbcrypt/cipher_internal_test.go | 91 ++++ enterprise/dbcrypt/dbcrypt.go | 374 ++++++++++++++ enterprise/dbcrypt/dbcrypt_internal_test.go | 489 ++++++++++++++++++ enterprise/dbcrypt/doc.go | 34 ++ 22 files changed, 1811 insertions(+), 72 deletions(-) create mode 100644 coderd/database/migrations/000154_dbcrypt_key_ids.down.sql create mode 100644 coderd/database/migrations/000154_dbcrypt_key_ids.up.sql create mode 100644 coderd/database/queries/dbcrypt.sql create mode 100644 enterprise/dbcrypt/cipher.go create mode 100644 enterprise/dbcrypt/cipher_internal_test.go create mode 100644 enterprise/dbcrypt/dbcrypt.go create mode 100644 enterprise/dbcrypt/dbcrypt_internal_test.go create mode 100644 enterprise/dbcrypt/doc.go diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index e4b802092f03d..8ddd779d795e9 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -838,6 +838,13 @@ func (q *querier) GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUI return q.db.GetAuthorizationUserRoles(ctx, userID) } +func (q *querier) GetDBCryptKeys(ctx context.Context) ([]database.DBCryptKey, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.GetDBCryptKeys(ctx) +} + func (q *querier) GetDERPMeshKey(ctx context.Context) (string, error) { if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { return "", err @@ -914,6 +921,13 @@ func (q *querier) GetGitAuthLink(ctx context.Context, arg database.GetGitAuthLin return fetch(q.log, q.auth, q.db.GetGitAuthLink)(ctx, arg) } +func (q *querier) GetGitAuthLinksByUserID(ctx context.Context, userID uuid.UUID) ([]database.GitAuthLink, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.GetGitAuthLinksByUserID(ctx, userID) +} + func (q *querier) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (database.GitSSHKey, error) { return fetch(q.log, q.auth, q.db.GetGitSSHKey)(ctx, userID) } @@ -1482,6 +1496,13 @@ func (q *querier) GetUserLinkByUserIDLoginType(ctx context.Context, arg database return q.db.GetUserLinkByUserIDLoginType(ctx, arg) } +func (q *querier) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]database.UserLink, error) { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.GetUserLinksByUserID(ctx, userID) +} + func (q *querier) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]database.GetUsersRow, error) { // This does the filtering in SQL. prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceUser.Type) @@ -1845,6 +1866,13 @@ func (q *querier) InsertAuditLog(ctx context.Context, arg database.InsertAuditLo return insert(q.log, q.auth, rbac.ResourceAuditLog, q.db.InsertAuditLog)(ctx, arg) } +func (q *querier) InsertDBCryptKey(ctx context.Context, arg database.InsertDBCryptKeyParams) error { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return err + } + return q.db.InsertDBCryptKey(ctx, arg) +} + func (q *querier) InsertDERPMeshKey(ctx context.Context, value string) error { if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { return err @@ -2144,6 +2172,13 @@ func (q *querier) RegisterWorkspaceProxy(ctx context.Context, arg database.Regis return updateWithReturn(q.log, q.auth, fetch, q.db.RegisterWorkspaceProxy)(ctx, arg) } +func (q *querier) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return err + } + return q.db.RevokeDBCryptKey(ctx, activeKeyDigest) +} + func (q *querier) TryAcquireLock(ctx context.Context, id int64) (bool, error) { return q.db.TryAcquireLock(ctx, id) } diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 9ba29ddf6d682..0c5c377960a7b 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -31,6 +31,11 @@ import ( var validProxyByHostnameRegex = regexp.MustCompile(`^[a-zA-Z0-9._-]+$`) +var errForeignKeyConstraint = &pq.Error{ + Code: "23503", + Message: "update or delete on table violates foreign key constraint", +} + var errDuplicateKey = &pq.Error{ Code: "23505", Message: "duplicate key value violates unique constraint", @@ -45,6 +50,7 @@ func New() database.Store { organizationMembers: make([]database.OrganizationMember, 0), organizations: make([]database.Organization, 0), users: make([]database.User, 0), + dbcryptKeys: make([]database.DBCryptKey, 0), gitAuthLinks: make([]database.GitAuthLink, 0), groups: make([]database.Group, 0), groupMembers: make([]database.GroupMember, 0), @@ -117,6 +123,7 @@ type data struct { // New tables workspaceAgentStats []database.WorkspaceAgentStat auditLogs []database.AuditLog + dbcryptKeys []database.DBCryptKey files []database.File gitAuthLinks []database.GitAuthLink gitSSHKey []database.GitSSHKey @@ -665,6 +672,39 @@ func (q *FakeQuerier) isEveryoneGroup(id uuid.UUID) bool { return false } +func (q *FakeQuerier) insertDBCryptKeyNoLock(_ context.Context, arg database.InsertDBCryptKeyParams) error { + err := validateDatabaseType(arg) + if err != nil { + return err + } + + for _, key := range q.dbcryptKeys { + if key.Number == arg.Number { + return errDuplicateKey + } + } + + q.dbcryptKeys = append(q.dbcryptKeys, database.DBCryptKey{ + Number: arg.Number, + ActiveKeyDigest: sql.NullString{String: arg.ActiveKeyDigest, Valid: true}, + Test: arg.Test, + }) + return nil +} + +func (q *FakeQuerier) GetActiveDBCryptKeys(_ context.Context) ([]database.DBCryptKey, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + ks := make([]database.DBCryptKey, 0, len(q.dbcryptKeys)) + for _, k := range q.dbcryptKeys { + if !k.ActiveKeyDigest.Valid { + continue + } + ks = append([]database.DBCryptKey{}, k) + } + return ks, nil +} + func (*FakeQuerier) AcquireLock(_ context.Context, _ int64) error { return xerrors.New("AcquireLock must only be called within a transaction") } @@ -1151,6 +1191,14 @@ func (q *FakeQuerier) GetAuthorizationUserRoles(_ context.Context, userID uuid.U }, nil } +func (q *FakeQuerier) GetDBCryptKeys(_ context.Context) ([]database.DBCryptKey, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + ks := make([]database.DBCryptKey, 0) + ks = append(ks, q.dbcryptKeys...) + return ks, nil +} + func (q *FakeQuerier) GetDERPMeshKey(_ context.Context) (string, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -1393,6 +1441,18 @@ func (q *FakeQuerier) GetGitAuthLink(_ context.Context, arg database.GetGitAuthL return database.GitAuthLink{}, sql.ErrNoRows } +func (q *FakeQuerier) GetGitAuthLinksByUserID(_ context.Context, userID uuid.UUID) ([]database.GitAuthLink, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + gals := make([]database.GitAuthLink, 0) + for _, gal := range q.gitAuthLinks { + if gal.UserID == userID { + gals = append(gals, gal) + } + } + return gals, nil +} + func (q *FakeQuerier) GetGitSSHKey(_ context.Context, userID uuid.UUID) (database.GitSSHKey, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -2833,6 +2893,18 @@ func (q *FakeQuerier) GetUserLinkByUserIDLoginType(_ context.Context, params dat return database.UserLink{}, sql.ErrNoRows } +func (q *FakeQuerier) GetUserLinksByUserID(_ context.Context, userID uuid.UUID) ([]database.UserLink, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + uls := make([]database.UserLink, 0) + for _, ul := range q.userLinks { + if ul.UserID == userID { + uls = append(uls, ul) + } + } + return uls, nil +} + func (q *FakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams) ([]database.GetUsersRow, error) { if err := validateDatabaseType(params); err != nil { return nil, err @@ -3846,6 +3918,11 @@ func (q *FakeQuerier) InsertAuditLog(_ context.Context, arg database.InsertAudit return alog, nil } +func (q *FakeQuerier) InsertDBCryptKey(ctx context.Context, arg database.InsertDBCryptKeyParams) error { + // This only ever gets called inside a transaction, so we need to not lock. + return q.insertDBCryptKeyNoLock(ctx, arg) +} + func (q *FakeQuerier) InsertDERPMeshKey(_ context.Context, id string) error { q.mutex.Lock() defer q.mutex.Unlock() @@ -3892,13 +3969,15 @@ func (q *FakeQuerier) InsertGitAuthLink(_ context.Context, arg database.InsertGi defer q.mutex.Unlock() // nolint:gosimple gitAuthLink := database.GitAuthLink{ - ProviderID: arg.ProviderID, - UserID: arg.UserID, - CreatedAt: arg.CreatedAt, - UpdatedAt: arg.UpdatedAt, - OAuthAccessToken: arg.OAuthAccessToken, - OAuthRefreshToken: arg.OAuthRefreshToken, - OAuthExpiry: arg.OAuthExpiry, + ProviderID: arg.ProviderID, + UserID: arg.UserID, + CreatedAt: arg.CreatedAt, + UpdatedAt: arg.UpdatedAt, + OAuthAccessToken: arg.OAuthAccessToken, + OAuthAccessTokenKeyID: arg.OAuthAccessTokenKeyID, + OAuthRefreshToken: arg.OAuthRefreshToken, + OAuthRefreshTokenKeyID: arg.OAuthRefreshTokenKeyID, + OAuthExpiry: arg.OAuthExpiry, } q.gitAuthLinks = append(q.gitAuthLinks, gitAuthLink) return gitAuthLink, nil @@ -4362,12 +4441,14 @@ func (q *FakeQuerier) InsertUserLink(_ context.Context, args database.InsertUser //nolint:gosimple link := database.UserLink{ - UserID: args.UserID, - LoginType: args.LoginType, - LinkedID: args.LinkedID, - OAuthAccessToken: args.OAuthAccessToken, - OAuthRefreshToken: args.OAuthRefreshToken, - OAuthExpiry: args.OAuthExpiry, + UserID: args.UserID, + LoginType: args.LoginType, + LinkedID: args.LinkedID, + OAuthAccessToken: args.OAuthAccessToken, + OAuthAccessTokenKeyID: args.OAuthAccessTokenKeyID, + OAuthRefreshToken: args.OAuthRefreshToken, + OAuthRefreshTokenKeyID: args.OAuthRefreshTokenKeyID, + OAuthExpiry: args.OAuthExpiry, } q.userLinks = append(q.userLinks, link) @@ -4793,6 +4874,46 @@ func (q *FakeQuerier) RegisterWorkspaceProxy(_ context.Context, arg database.Reg return database.WorkspaceProxy{}, sql.ErrNoRows } +func (q *FakeQuerier) RevokeDBCryptKey(_ context.Context, activeKeyDigest string) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + for i := range q.dbcryptKeys { + key := q.dbcryptKeys[i] + + // Is the key already revoked? + if !key.ActiveKeyDigest.Valid { + continue + } + + if key.ActiveKeyDigest.String != activeKeyDigest { + continue + } + + // Check for foreign key constraints. + for _, ul := range q.userLinks { + if (ul.OAuthAccessTokenKeyID.Valid && ul.OAuthAccessTokenKeyID.String == activeKeyDigest) || + (ul.OAuthRefreshTokenKeyID.Valid && ul.OAuthRefreshTokenKeyID.String == activeKeyDigest) { + return errForeignKeyConstraint + } + } + for _, gal := range q.gitAuthLinks { + if (gal.OAuthAccessTokenKeyID.Valid && gal.OAuthAccessTokenKeyID.String == activeKeyDigest) || + (gal.OAuthRefreshTokenKeyID.Valid && gal.OAuthRefreshTokenKeyID.String == activeKeyDigest) { + return errForeignKeyConstraint + } + } + + // Revoke the key. + q.dbcryptKeys[i].RevokedAt = sql.NullTime{Time: dbtime.Now(), Valid: true} + q.dbcryptKeys[i].RevokedKeyDigest = sql.NullString{String: key.ActiveKeyDigest.String, Valid: true} + q.dbcryptKeys[i].ActiveKeyDigest = sql.NullString{} + return nil + } + + return sql.ErrNoRows +} + func (*FakeQuerier) TryAcquireLock(_ context.Context, _ int64) (bool, error) { return false, xerrors.New("TryAcquireLock must only be called within a transaction") } @@ -4834,7 +4955,9 @@ func (q *FakeQuerier) UpdateGitAuthLink(_ context.Context, arg database.UpdateGi } gitAuthLink.UpdatedAt = arg.UpdatedAt gitAuthLink.OAuthAccessToken = arg.OAuthAccessToken + gitAuthLink.OAuthAccessTokenKeyID = arg.OAuthAccessTokenKeyID gitAuthLink.OAuthRefreshToken = arg.OAuthRefreshToken + gitAuthLink.OAuthRefreshTokenKeyID = arg.OAuthRefreshTokenKeyID gitAuthLink.OAuthExpiry = arg.OAuthExpiry q.gitAuthLinks[index] = gitAuthLink @@ -5306,7 +5429,9 @@ func (q *FakeQuerier) UpdateUserLink(_ context.Context, params database.UpdateUs for i, link := range q.userLinks { if link.UserID == params.UserID && link.LoginType == params.LoginType { link.OAuthAccessToken = params.OAuthAccessToken + link.OAuthAccessTokenKeyID = params.OAuthAccessTokenKeyID link.OAuthRefreshToken = params.OAuthRefreshToken + link.OAuthRefreshTokenKeyID = params.OAuthRefreshTokenKeyID link.OAuthExpiry = params.OAuthExpiry q.userLinks[i] = link diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 12036274eb811..f68a8cebadcc8 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -470,12 +470,14 @@ func File(t testing.TB, db database.Store, orig database.File) database.File { func UserLink(t testing.TB, db database.Store, orig database.UserLink) database.UserLink { link, err := db.InsertUserLink(genCtx, database.InsertUserLinkParams{ - UserID: takeFirst(orig.UserID, uuid.New()), - LoginType: takeFirst(orig.LoginType, database.LoginTypeGithub), - LinkedID: takeFirst(orig.LinkedID), - OAuthAccessToken: takeFirst(orig.OAuthAccessToken, uuid.NewString()), - OAuthRefreshToken: takeFirst(orig.OAuthAccessToken, uuid.NewString()), - OAuthExpiry: takeFirst(orig.OAuthExpiry, dbtime.Now().Add(time.Hour*24)), + UserID: takeFirst(orig.UserID, uuid.New()), + LoginType: takeFirst(orig.LoginType, database.LoginTypeGithub), + LinkedID: takeFirst(orig.LinkedID), + OAuthAccessToken: takeFirst(orig.OAuthAccessToken, uuid.NewString()), + OAuthAccessTokenKeyID: takeFirst(orig.OAuthAccessTokenKeyID, sql.NullString{}), + OAuthRefreshToken: takeFirst(orig.OAuthRefreshToken, uuid.NewString()), + OAuthRefreshTokenKeyID: takeFirst(orig.OAuthRefreshTokenKeyID, sql.NullString{}), + OAuthExpiry: takeFirst(orig.OAuthExpiry, dbtime.Now().Add(time.Hour*24)), }) require.NoError(t, err, "insert link") @@ -484,13 +486,15 @@ func UserLink(t testing.TB, db database.Store, orig database.UserLink) database. func GitAuthLink(t testing.TB, db database.Store, orig database.GitAuthLink) database.GitAuthLink { link, err := db.InsertGitAuthLink(genCtx, database.InsertGitAuthLinkParams{ - 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()), - OAuthExpiry: takeFirst(orig.OAuthExpiry, dbtime.Now().Add(time.Hour*24)), - CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()), - UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()), + ProviderID: takeFirst(orig.ProviderID, uuid.New().String()), + UserID: takeFirst(orig.UserID, uuid.New()), + OAuthAccessToken: takeFirst(orig.OAuthAccessToken, uuid.NewString()), + OAuthAccessTokenKeyID: takeFirst(orig.OAuthAccessTokenKeyID, sql.NullString{}), + OAuthRefreshToken: takeFirst(orig.OAuthRefreshToken, uuid.NewString()), + OAuthRefreshTokenKeyID: takeFirst(orig.OAuthRefreshTokenKeyID, sql.NullString{}), + OAuthExpiry: takeFirst(orig.OAuthExpiry, dbtime.Now().Add(time.Hour*24)), + CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()), + UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()), }) require.NoError(t, err, "insert git auth link") diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 8526eb4da1078..0a02896200f60 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -279,6 +279,13 @@ func (m metricsStore) GetAuthorizationUserRoles(ctx context.Context, userID uuid return row, err } +func (m metricsStore) GetDBCryptKeys(ctx context.Context) ([]database.DBCryptKey, error) { + start := time.Now() + r0, r1 := m.s.GetDBCryptKeys(ctx) + m.queryLatencies.WithLabelValues("GetDBCryptKeys").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetDERPMeshKey(ctx context.Context) (string, error) { start := time.Now() key, err := m.s.GetDERPMeshKey(ctx) @@ -349,6 +356,13 @@ func (m metricsStore) GetGitAuthLink(ctx context.Context, arg database.GetGitAut return link, err } +func (m metricsStore) GetGitAuthLinksByUserID(ctx context.Context, userID uuid.UUID) ([]database.GitAuthLink, error) { + start := time.Now() + r0, r1 := m.s.GetGitAuthLinksByUserID(ctx, userID) + m.queryLatencies.WithLabelValues("GetGitAuthLinksByUserID").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (database.GitSSHKey, error) { start := time.Now() key, err := m.s.GetGitSSHKey(ctx, userID) @@ -774,6 +788,13 @@ func (m metricsStore) GetUserLinkByUserIDLoginType(ctx context.Context, arg data return link, err } +func (m metricsStore) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]database.UserLink, error) { + start := time.Now() + r0, r1 := m.s.GetUserLinksByUserID(ctx, userID) + m.queryLatencies.WithLabelValues("GetUserLinksByUserID").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]database.GetUsersRow, error) { start := time.Now() users, err := m.s.GetUsers(ctx, arg) @@ -1068,6 +1089,13 @@ func (m metricsStore) InsertAuditLog(ctx context.Context, arg database.InsertAud return log, err } +func (m metricsStore) InsertDBCryptKey(ctx context.Context, arg database.InsertDBCryptKeyParams) error { + start := time.Now() + r0 := m.s.InsertDBCryptKey(ctx, arg) + m.queryLatencies.WithLabelValues("InsertDBCryptKey").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) InsertDERPMeshKey(ctx context.Context, value string) error { start := time.Now() err := m.s.InsertDERPMeshKey(ctx, value) @@ -1320,6 +1348,13 @@ func (m metricsStore) RegisterWorkspaceProxy(ctx context.Context, arg database.R return proxy, err } +func (m metricsStore) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error { + start := time.Now() + r0 := m.s.RevokeDBCryptKey(ctx, activeKeyDigest) + m.queryLatencies.WithLabelValues("RevokeDBCryptKey").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) TryAcquireLock(ctx context.Context, pgTryAdvisoryXactLock int64) (bool, error) { start := time.Now() ok, err := m.s.TryAcquireLock(ctx, pgTryAdvisoryXactLock) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index b0ae7955a458d..be1f994d81161 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -506,6 +506,21 @@ func (mr *MockStoreMockRecorder) GetAuthorizedWorkspaces(arg0, arg1, arg2 interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAuthorizedWorkspaces", reflect.TypeOf((*MockStore)(nil).GetAuthorizedWorkspaces), arg0, arg1, arg2) } +// GetDBCryptKeys mocks base method. +func (m *MockStore) GetDBCryptKeys(arg0 context.Context) ([]database.DBCryptKey, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetDBCryptKeys", arg0) + ret0, _ := ret[0].([]database.DBCryptKey) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetDBCryptKeys indicates an expected call of GetDBCryptKeys. +func (mr *MockStoreMockRecorder) GetDBCryptKeys(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDBCryptKeys", reflect.TypeOf((*MockStore)(nil).GetDBCryptKeys), arg0) +} + // GetDERPMeshKey mocks base method. func (m *MockStore) GetDERPMeshKey(arg0 context.Context) (string, error) { m.ctrl.T.Helper() @@ -656,6 +671,21 @@ func (mr *MockStoreMockRecorder) GetGitAuthLink(arg0, arg1 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGitAuthLink", reflect.TypeOf((*MockStore)(nil).GetGitAuthLink), arg0, arg1) } +// GetGitAuthLinksByUserID mocks base method. +func (m *MockStore) GetGitAuthLinksByUserID(arg0 context.Context, arg1 uuid.UUID) ([]database.GitAuthLink, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGitAuthLinksByUserID", arg0, arg1) + ret0, _ := ret[0].([]database.GitAuthLink) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGitAuthLinksByUserID indicates an expected call of GetGitAuthLinksByUserID. +func (mr *MockStoreMockRecorder) GetGitAuthLinksByUserID(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGitAuthLinksByUserID", reflect.TypeOf((*MockStore)(nil).GetGitAuthLinksByUserID), arg0, arg1) +} + // GetGitSSHKey mocks base method. func (m *MockStore) GetGitSSHKey(arg0 context.Context, arg1 uuid.UUID) (database.GitSSHKey, error) { m.ctrl.T.Helper() @@ -1601,6 +1631,21 @@ func (mr *MockStoreMockRecorder) GetUserLinkByUserIDLoginType(arg0, arg1 interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserLinkByUserIDLoginType", reflect.TypeOf((*MockStore)(nil).GetUserLinkByUserIDLoginType), arg0, arg1) } +// GetUserLinksByUserID mocks base method. +func (m *MockStore) GetUserLinksByUserID(arg0 context.Context, arg1 uuid.UUID) ([]database.UserLink, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserLinksByUserID", arg0, arg1) + ret0, _ := ret[0].([]database.UserLink) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserLinksByUserID indicates an expected call of GetUserLinksByUserID. +func (mr *MockStoreMockRecorder) GetUserLinksByUserID(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserLinksByUserID", reflect.TypeOf((*MockStore)(nil).GetUserLinksByUserID), arg0, arg1) +} + // GetUsers mocks base method. func (m *MockStore) GetUsers(arg0 context.Context, arg1 database.GetUsersParams) ([]database.GetUsersRow, error) { m.ctrl.T.Helper() @@ -2245,6 +2290,20 @@ func (mr *MockStoreMockRecorder) InsertAuditLog(arg0, arg1 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertAuditLog", reflect.TypeOf((*MockStore)(nil).InsertAuditLog), arg0, arg1) } +// InsertDBCryptKey mocks base method. +func (m *MockStore) InsertDBCryptKey(arg0 context.Context, arg1 database.InsertDBCryptKeyParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InsertDBCryptKey", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// InsertDBCryptKey indicates an expected call of InsertDBCryptKey. +func (mr *MockStoreMockRecorder) InsertDBCryptKey(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertDBCryptKey", reflect.TypeOf((*MockStore)(nil).InsertDBCryptKey), arg0, arg1) +} + // InsertDERPMeshKey mocks base method. func (m *MockStore) InsertDERPMeshKey(arg0 context.Context, arg1 string) error { m.ctrl.T.Helper() @@ -2789,6 +2848,20 @@ func (mr *MockStoreMockRecorder) RegisterWorkspaceProxy(arg0, arg1 interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterWorkspaceProxy", reflect.TypeOf((*MockStore)(nil).RegisterWorkspaceProxy), arg0, arg1) } +// RevokeDBCryptKey mocks base method. +func (m *MockStore) RevokeDBCryptKey(arg0 context.Context, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RevokeDBCryptKey", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// RevokeDBCryptKey indicates an expected call of RevokeDBCryptKey. +func (mr *MockStoreMockRecorder) RevokeDBCryptKey(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevokeDBCryptKey", reflect.TypeOf((*MockStore)(nil).RevokeDBCryptKey), arg0, arg1) +} + // TryAcquireLock mocks base method. func (m *MockStore) TryAcquireLock(arg0 context.Context, arg1 int64) (bool, error) { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 48e4e1a8862c1..3ee0ac7e19894 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -275,6 +275,29 @@ CREATE TABLE audit_logs ( resource_icon text NOT NULL ); +CREATE TABLE dbcrypt_keys ( + number integer NOT NULL, + active_key_digest text, + revoked_key_digest text, + created_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP, + revoked_at timestamp with time zone, + test text NOT NULL +); + +COMMENT ON TABLE dbcrypt_keys IS 'A table used to store the keys used to encrypt the database.'; + +COMMENT ON COLUMN dbcrypt_keys.number IS 'An integer used to identify the key.'; + +COMMENT ON COLUMN dbcrypt_keys.active_key_digest IS 'If the key is active, the digest of the active key.'; + +COMMENT ON COLUMN dbcrypt_keys.revoked_key_digest IS 'If the key has been revoked, the digest of the revoked key.'; + +COMMENT ON COLUMN dbcrypt_keys.created_at IS 'The time at which the key was created.'; + +COMMENT ON COLUMN dbcrypt_keys.revoked_at IS 'The time at which the key was revoked.'; + +COMMENT ON COLUMN dbcrypt_keys.test IS 'A column used to test the encryption.'; + CREATE TABLE files ( hash character varying(64) NOT NULL, created_at timestamp with time zone NOT NULL, @@ -291,9 +314,15 @@ CREATE TABLE git_auth_links ( updated_at timestamp with time zone NOT NULL, oauth_access_token text NOT NULL, oauth_refresh_token text NOT NULL, - oauth_expiry timestamp with time zone NOT NULL + oauth_expiry timestamp with time zone NOT NULL, + oauth_access_token_key_id text, + oauth_refresh_token_key_id text ); +COMMENT ON COLUMN git_auth_links.oauth_access_token_key_id IS 'The ID of the key used to encrypt the OAuth access token. If this is NULL, the access token is not encrypted'; + +COMMENT ON COLUMN git_auth_links.oauth_refresh_token_key_id IS 'The ID of the key used to encrypt the OAuth refresh token. If this is NULL, the refresh token is not encrypted'; + CREATE TABLE gitsshkeys ( user_id uuid NOT NULL, created_at timestamp with time zone NOT NULL, @@ -701,9 +730,15 @@ CREATE TABLE user_links ( linked_id text DEFAULT ''::text NOT NULL, oauth_access_token text DEFAULT ''::text NOT NULL, oauth_refresh_token text DEFAULT ''::text NOT NULL, - oauth_expiry timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL + oauth_expiry timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL, + oauth_access_token_key_id text, + oauth_refresh_token_key_id text ); +COMMENT ON COLUMN user_links.oauth_access_token_key_id IS 'The ID of the key used to encrypt the OAuth access token. If this is NULL, the access token is not encrypted'; + +COMMENT ON COLUMN user_links.oauth_refresh_token_key_id IS 'The ID of the key used to encrypt the OAuth refresh token. If this is NULL, the refresh token is not encrypted'; + CREATE TABLE workspace_agent_logs ( agent_id uuid NOT NULL, created_at timestamp with time zone NOT NULL, @@ -1037,6 +1072,15 @@ ALTER TABLE ONLY api_keys ALTER TABLE ONLY audit_logs ADD CONSTRAINT audit_logs_pkey PRIMARY KEY (id); +ALTER TABLE ONLY dbcrypt_keys + ADD CONSTRAINT dbcrypt_keys_active_key_digest_key UNIQUE (active_key_digest); + +ALTER TABLE ONLY dbcrypt_keys + ADD CONSTRAINT dbcrypt_keys_pkey PRIMARY KEY (number); + +ALTER TABLE ONLY dbcrypt_keys + ADD CONSTRAINT dbcrypt_keys_revoked_key_digest_key UNIQUE (revoked_key_digest); + ALTER TABLE ONLY files ADD CONSTRAINT files_hash_created_by_key UNIQUE (hash, created_by); @@ -1249,6 +1293,12 @@ CREATE TRIGGER trigger_update_users AFTER INSERT OR UPDATE ON users FOR EACH ROW ALTER TABLE ONLY api_keys ADD CONSTRAINT api_keys_user_id_uuid_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY git_auth_links + ADD CONSTRAINT git_auth_links_oauth_access_token_key_id_fkey FOREIGN KEY (oauth_access_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); + +ALTER TABLE ONLY git_auth_links + ADD CONSTRAINT git_auth_links_oauth_refresh_token_key_id_fkey FOREIGN KEY (oauth_refresh_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); + ALTER TABLE ONLY gitsshkeys ADD CONSTRAINT gitsshkeys_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); @@ -1303,6 +1353,12 @@ ALTER TABLE ONLY templates ALTER TABLE ONLY templates ADD CONSTRAINT templates_organization_id_fkey FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; +ALTER TABLE ONLY user_links + ADD CONSTRAINT user_links_oauth_access_token_key_id_fkey FOREIGN KEY (oauth_access_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); + +ALTER TABLE ONLY user_links + ADD CONSTRAINT user_links_oauth_refresh_token_key_id_fkey FOREIGN KEY (oauth_refresh_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); + ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; diff --git a/coderd/database/migrations/000154_dbcrypt_key_ids.down.sql b/coderd/database/migrations/000154_dbcrypt_key_ids.down.sql new file mode 100644 index 0000000000000..7dea0a1909227 --- /dev/null +++ b/coderd/database/migrations/000154_dbcrypt_key_ids.down.sql @@ -0,0 +1,43 @@ +BEGIN; + +-- Before dropping this table, we need to check if there exist any +-- foreign key references to it. We do this by checking the following: +-- user_links.oauth_access_token_key_id +-- user_links.oauth_refresh_token_key_id +-- git_auth_links.oauth_access_token_key_id +-- git_auth_links.oauth_refresh_token_key_id +DO $$ +BEGIN +IF EXISTS ( + SELECT * + FROM user_links + WHERE oauth_access_token_key_id IS NOT NULL + OR oauth_refresh_token_key_id IS NOT NULL + ) THEN RAISE EXCEPTION 'Cannot drop dbcrypt_keys table as there are still foreign key references to it from user_links.'; +END IF; + +IF EXISTS ( + SELECT * + FROM git_auth_links + WHERE oauth_access_token_key_id IS NOT NULL + OR oauth_refresh_token_key_id IS NOT NULL + ) THEN RAISE EXCEPTION 'Cannot drop dbcrypt_keys table as there are still foreign key references to it from git_auth_links.'; +END IF; + +END +$$; + + +-- Drop the columns first. +ALTER TABLE git_auth_links + DROP COLUMN IF EXISTS oauth_access_token_key_id, + DROP COLUMN IF EXISTS oauth_refresh_token_key_id; + +ALTER TABLE user_links + DROP COLUMN IF EXISTS oauth_access_token_key_id, + DROP COLUMN IF EXISTS oauth_refresh_token_key_id; + +-- Finally, drop the table. +DROP TABLE IF EXISTS dbcrypt_keys; + +COMMIT; diff --git a/coderd/database/migrations/000154_dbcrypt_key_ids.up.sql b/coderd/database/migrations/000154_dbcrypt_key_ids.up.sql new file mode 100644 index 0000000000000..804c41b82b67f --- /dev/null +++ b/coderd/database/migrations/000154_dbcrypt_key_ids.up.sql @@ -0,0 +1,30 @@ +CREATE TABLE IF NOT EXISTS dbcrypt_keys ( + number int NOT NULL PRIMARY KEY, + active_key_digest text UNIQUE, + revoked_key_digest text UNIQUE, + created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, + revoked_at TIMESTAMP WITH TIME ZONE DEFAULT NULL, + test TEXT NOT NULL +); + +COMMENT ON TABLE dbcrypt_keys IS 'A table used to store the keys used to encrypt the database.'; +COMMENT ON COLUMN dbcrypt_keys.number IS 'An integer used to identify the key.'; +COMMENT ON COLUMN dbcrypt_keys.active_key_digest IS 'If the key is active, the digest of the active key.'; +COMMENT ON COLUMN dbcrypt_keys.revoked_key_digest IS 'If the key has been revoked, the digest of the revoked key.'; +COMMENT ON COLUMN dbcrypt_keys.created_at IS 'The time at which the key was created.'; +COMMENT ON COLUMN dbcrypt_keys.revoked_at IS 'The time at which the key was revoked.'; +COMMENT ON COLUMN dbcrypt_keys.test IS 'A column used to test the encryption.'; + +ALTER TABLE git_auth_links +ADD COLUMN IF NOT EXISTS oauth_access_token_key_id text REFERENCES dbcrypt_keys(active_key_digest), +ADD COLUMN IF NOT EXISTS oauth_refresh_token_key_id text REFERENCES dbcrypt_keys(active_key_digest); + +COMMENT ON COLUMN git_auth_links.oauth_access_token_key_id IS 'The ID of the key used to encrypt the OAuth access token. If this is NULL, the access token is not encrypted'; +COMMENT ON COLUMN git_auth_links.oauth_refresh_token_key_id IS 'The ID of the key used to encrypt the OAuth refresh token. If this is NULL, the refresh token is not encrypted'; + +ALTER TABLE user_links +ADD COLUMN IF NOT EXISTS oauth_access_token_key_id text REFERENCES dbcrypt_keys(active_key_digest), +ADD COLUMN IF NOT EXISTS oauth_refresh_token_key_id text REFERENCES dbcrypt_keys(active_key_digest); + +COMMENT ON COLUMN user_links.oauth_access_token_key_id IS 'The ID of the key used to encrypt the OAuth access token. If this is NULL, the access token is not encrypted'; +COMMENT ON COLUMN user_links.oauth_refresh_token_key_id IS 'The ID of the key used to encrypt the OAuth refresh token. If this is NULL, the refresh token is not encrypted'; diff --git a/coderd/database/migrations/migrate_test.go b/coderd/database/migrations/migrate_test.go index a138e58bac05f..b512811f2ab18 100644 --- a/coderd/database/migrations/migrate_test.go +++ b/coderd/database/migrations/migrate_test.go @@ -266,6 +266,7 @@ func TestMigrateUpWithFixtures(t *testing.T) { "template_version_parameters", "workspace_build_parameters", "template_version_variables", + "dbcrypt_keys", // having zero rows is a valid state for this table } s := &tableStats{s: make(map[string]int)} diff --git a/coderd/database/models.go b/coderd/database/models.go index cd9ba8f990d2d..4d1852a54114e 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1591,6 +1591,22 @@ type AuditLog struct { ResourceIcon string `db:"resource_icon" json:"resource_icon"` } +// A table used to store the keys used to encrypt the database. +type DBCryptKey struct { + // An integer used to identify the key. + Number int32 `db:"number" json:"number"` + // If the key is active, the digest of the active key. + ActiveKeyDigest sql.NullString `db:"active_key_digest" json:"active_key_digest"` + // If the key has been revoked, the digest of the revoked key. + RevokedKeyDigest sql.NullString `db:"revoked_key_digest" json:"revoked_key_digest"` + // The time at which the key was created. + CreatedAt sql.NullTime `db:"created_at" json:"created_at"` + // The time at which the key was revoked. + RevokedAt sql.NullTime `db:"revoked_at" json:"revoked_at"` + // A column used to test the encryption. + Test string `db:"test" json:"test"` +} + type File struct { Hash string `db:"hash" json:"hash"` CreatedAt time.Time `db:"created_at" json:"created_at"` @@ -1608,6 +1624,10 @@ type GitAuthLink struct { OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` + // The ID of the key used to encrypt the OAuth access token. If this is NULL, the access token is not encrypted + OAuthAccessTokenKeyID sql.NullString `db:"oauth_access_token_key_id" json:"oauth_access_token_key_id"` + // The ID of the key used to encrypt the OAuth refresh token. If this is NULL, the refresh token is not encrypted + OAuthRefreshTokenKeyID sql.NullString `db:"oauth_refresh_token_key_id" json:"oauth_refresh_token_key_id"` } type GitSSHKey struct { @@ -1949,6 +1969,10 @@ type UserLink struct { OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` + // The ID of the key used to encrypt the OAuth access token. If this is NULL, the access token is not encrypted + OAuthAccessTokenKeyID sql.NullString `db:"oauth_access_token_key_id" json:"oauth_access_token_key_id"` + // The ID of the key used to encrypt the OAuth refresh token. If this is NULL, the refresh token is not encrypted + OAuthRefreshTokenKeyID sql.NullString `db:"oauth_refresh_token_key_id" json:"oauth_refresh_token_key_id"` } // Visible fields of users are allowed to be joined with other tables for including context of other resources. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 520266bd1d25c..cdf4d184544bb 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -58,6 +58,7 @@ type sqlcQuerier interface { // This function returns roles for authorization purposes. Implied member roles // are included. GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUID) (GetAuthorizationUserRolesRow, error) + GetDBCryptKeys(ctx context.Context) ([]DBCryptKey, error) GetDERPMeshKey(ctx context.Context) (string, error) GetDefaultProxyConfig(ctx context.Context) (GetDefaultProxyConfigRow, error) GetDeploymentDAUs(ctx context.Context, tzOffset int32) ([]GetDeploymentDAUsRow, error) @@ -69,6 +70,7 @@ type sqlcQuerier interface { // Get all templates that use a file. GetFileTemplates(ctx context.Context, fileID uuid.UUID) ([]GetFileTemplatesRow, error) GetGitAuthLink(ctx context.Context, arg GetGitAuthLinkParams) (GitAuthLink, error) + GetGitAuthLinksByUserID(ctx context.Context, userID uuid.UUID) ([]GitAuthLink, error) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (GitSSHKey, error) GetGroupByID(ctx context.Context, id uuid.UUID) (Group, error) GetGroupByOrgAndName(ctx context.Context, arg GetGroupByOrgAndNameParams) (Group, error) @@ -150,6 +152,7 @@ type sqlcQuerier interface { GetUserLatencyInsights(ctx context.Context, arg GetUserLatencyInsightsParams) ([]GetUserLatencyInsightsRow, error) GetUserLinkByLinkedID(ctx context.Context, linkedID string) (UserLink, error) GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) + GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) // This shouldn't check for deleted, because it's frequently used @@ -206,6 +209,7 @@ type sqlcQuerier interface { // every member of the org. InsertAllUsersGroup(ctx context.Context, organizationID uuid.UUID) (Group, error) InsertAuditLog(ctx context.Context, arg InsertAuditLogParams) (AuditLog, error) + InsertDBCryptKey(ctx context.Context, arg InsertDBCryptKeyParams) error InsertDERPMeshKey(ctx context.Context, value string) error InsertDeploymentID(ctx context.Context, value string) error InsertFile(ctx context.Context, arg InsertFileParams) (File, error) @@ -247,6 +251,7 @@ type sqlcQuerier interface { InsertWorkspaceResource(ctx context.Context, arg InsertWorkspaceResourceParams) (WorkspaceResource, error) InsertWorkspaceResourceMetadata(ctx context.Context, arg InsertWorkspaceResourceMetadataParams) ([]WorkspaceResourceMetadatum, error) RegisterWorkspaceProxy(ctx context.Context, arg RegisterWorkspaceProxyParams) (WorkspaceProxy, error) + RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error // Non blocking lock. Returns true if the lock was acquired, false otherwise. // // This must be called from within a transaction. The lock will be automatically diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index dbbb5d4085e93..700cfeb1da89f 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -636,6 +636,74 @@ func (q *sqlQuerier) InsertAuditLog(ctx context.Context, arg InsertAuditLogParam return i, err } +const getDBCryptKeys = `-- name: GetDBCryptKeys :many +SELECT number, active_key_digest, revoked_key_digest, created_at, revoked_at, test FROM dbcrypt_keys ORDER BY number ASC +` + +func (q *sqlQuerier) GetDBCryptKeys(ctx context.Context) ([]DBCryptKey, error) { + rows, err := q.db.QueryContext(ctx, getDBCryptKeys) + if err != nil { + return nil, err + } + defer rows.Close() + var items []DBCryptKey + for rows.Next() { + var i DBCryptKey + if err := rows.Scan( + &i.Number, + &i.ActiveKeyDigest, + &i.RevokedKeyDigest, + &i.CreatedAt, + &i.RevokedAt, + &i.Test, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const insertDBCryptKey = `-- name: InsertDBCryptKey :exec +INSERT INTO dbcrypt_keys + (number, active_key_digest, created_at, test) +VALUES ($1::int, $2::text, CURRENT_TIMESTAMP, $3::text) +` + +type InsertDBCryptKeyParams struct { + Number int32 `db:"number" json:"number"` + ActiveKeyDigest string `db:"active_key_digest" json:"active_key_digest"` + Test string `db:"test" json:"test"` +} + +func (q *sqlQuerier) InsertDBCryptKey(ctx context.Context, arg InsertDBCryptKeyParams) error { + _, err := q.db.ExecContext(ctx, insertDBCryptKey, arg.Number, arg.ActiveKeyDigest, arg.Test) + return err +} + +const revokeDBCryptKey = `-- name: RevokeDBCryptKey :exec +UPDATE dbcrypt_keys +SET + revoked_key_digest = active_key_digest, + active_key_digest = revoked_key_digest, + revoked_at = CURRENT_TIMESTAMP +WHERE + active_key_digest = $1::text +AND + revoked_key_digest IS NULL +` + +func (q *sqlQuerier) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error { + _, err := q.db.ExecContext(ctx, revokeDBCryptKey, activeKeyDigest) + return err +} + const getFileByHashAndCreator = `-- name: GetFileByHashAndCreator :one SELECT hash, created_at, created_by, mimetype, data, id @@ -800,7 +868,7 @@ func (q *sqlQuerier) InsertFile(ctx context.Context, arg InsertFileParams) (File } const getGitAuthLink = `-- name: GetGitAuthLink :one -SELECT provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry FROM git_auth_links WHERE provider_id = $1 AND user_id = $2 +SELECT provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id FROM git_auth_links WHERE provider_id = $1 AND user_id = $2 ` type GetGitAuthLinkParams struct { @@ -819,10 +887,49 @@ func (q *sqlQuerier) GetGitAuthLink(ctx context.Context, arg GetGitAuthLinkParam &i.OAuthAccessToken, &i.OAuthRefreshToken, &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, ) return i, err } +const getGitAuthLinksByUserID = `-- name: GetGitAuthLinksByUserID :many +SELECT provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id FROM git_auth_links WHERE user_id = $1 +` + +func (q *sqlQuerier) GetGitAuthLinksByUserID(ctx context.Context, userID uuid.UUID) ([]GitAuthLink, error) { + rows, err := q.db.QueryContext(ctx, getGitAuthLinksByUserID, userID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GitAuthLink + for rows.Next() { + var i GitAuthLink + if err := rows.Scan( + &i.ProviderID, + &i.UserID, + &i.CreatedAt, + &i.UpdatedAt, + &i.OAuthAccessToken, + &i.OAuthRefreshToken, + &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const insertGitAuthLink = `-- name: InsertGitAuthLink :one INSERT INTO git_auth_links ( provider_id, @@ -830,7 +937,9 @@ INSERT INTO git_auth_links ( created_at, updated_at, oauth_access_token, + oauth_access_token_key_id, oauth_refresh_token, + oauth_refresh_token_key_id, oauth_expiry ) VALUES ( $1, @@ -839,18 +948,22 @@ INSERT INTO git_auth_links ( $4, $5, $6, - $7 -) RETURNING provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry + $7, + $8, + $9 +) RETURNING provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id ` type InsertGitAuthLinkParams struct { - ProviderID string `db:"provider_id" json:"provider_id"` - UserID uuid.UUID `db:"user_id" json:"user_id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` - OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` - OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` + ProviderID string `db:"provider_id" json:"provider_id"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` + OAuthAccessTokenKeyID sql.NullString `db:"oauth_access_token_key_id" json:"oauth_access_token_key_id"` + OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` + OAuthRefreshTokenKeyID sql.NullString `db:"oauth_refresh_token_key_id" json:"oauth_refresh_token_key_id"` + OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` } func (q *sqlQuerier) InsertGitAuthLink(ctx context.Context, arg InsertGitAuthLinkParams) (GitAuthLink, error) { @@ -860,7 +973,9 @@ func (q *sqlQuerier) InsertGitAuthLink(ctx context.Context, arg InsertGitAuthLin arg.CreatedAt, arg.UpdatedAt, arg.OAuthAccessToken, + arg.OAuthAccessTokenKeyID, arg.OAuthRefreshToken, + arg.OAuthRefreshTokenKeyID, arg.OAuthExpiry, ) var i GitAuthLink @@ -872,6 +987,8 @@ func (q *sqlQuerier) InsertGitAuthLink(ctx context.Context, arg InsertGitAuthLin &i.OAuthAccessToken, &i.OAuthRefreshToken, &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, ) return i, err } @@ -880,18 +997,22 @@ const updateGitAuthLink = `-- name: UpdateGitAuthLink :one UPDATE git_auth_links SET updated_at = $3, oauth_access_token = $4, - oauth_refresh_token = $5, - oauth_expiry = $6 -WHERE provider_id = $1 AND user_id = $2 RETURNING provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry + oauth_access_token_key_id = $5, + oauth_refresh_token = $6, + oauth_refresh_token_key_id = $7, + oauth_expiry = $8 +WHERE provider_id = $1 AND user_id = $2 RETURNING provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id ` type UpdateGitAuthLinkParams struct { - ProviderID string `db:"provider_id" json:"provider_id"` - UserID uuid.UUID `db:"user_id" json:"user_id"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` - OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` - OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` + ProviderID string `db:"provider_id" json:"provider_id"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` + OAuthAccessTokenKeyID sql.NullString `db:"oauth_access_token_key_id" json:"oauth_access_token_key_id"` + OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` + OAuthRefreshTokenKeyID sql.NullString `db:"oauth_refresh_token_key_id" json:"oauth_refresh_token_key_id"` + OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` } func (q *sqlQuerier) UpdateGitAuthLink(ctx context.Context, arg UpdateGitAuthLinkParams) (GitAuthLink, error) { @@ -900,7 +1021,9 @@ func (q *sqlQuerier) UpdateGitAuthLink(ctx context.Context, arg UpdateGitAuthLin arg.UserID, arg.UpdatedAt, arg.OAuthAccessToken, + arg.OAuthAccessTokenKeyID, arg.OAuthRefreshToken, + arg.OAuthRefreshTokenKeyID, arg.OAuthExpiry, ) var i GitAuthLink @@ -912,6 +1035,8 @@ func (q *sqlQuerier) UpdateGitAuthLink(ctx context.Context, arg UpdateGitAuthLin &i.OAuthAccessToken, &i.OAuthRefreshToken, &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, ) return i, err } @@ -5450,7 +5575,7 @@ func (q *sqlQuerier) InsertTemplateVersionVariable(ctx context.Context, arg Inse const getUserLinkByLinkedID = `-- name: GetUserLinkByLinkedID :one SELECT - user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry + user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id FROM user_links WHERE @@ -5467,13 +5592,15 @@ func (q *sqlQuerier) GetUserLinkByLinkedID(ctx context.Context, linkedID string) &i.OAuthAccessToken, &i.OAuthRefreshToken, &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, ) return i, err } const getUserLinkByUserIDLoginType = `-- name: GetUserLinkByUserIDLoginType :one SELECT - user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry + user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id FROM user_links WHERE @@ -5495,10 +5622,48 @@ func (q *sqlQuerier) GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUs &i.OAuthAccessToken, &i.OAuthRefreshToken, &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, ) return i, err } +const getUserLinksByUserID = `-- name: GetUserLinksByUserID :many +SELECT user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id FROM user_links WHERE user_id = $1 +` + +func (q *sqlQuerier) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) { + rows, err := q.db.QueryContext(ctx, getUserLinksByUserID, userID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []UserLink + for rows.Next() { + var i UserLink + if err := rows.Scan( + &i.UserID, + &i.LoginType, + &i.LinkedID, + &i.OAuthAccessToken, + &i.OAuthRefreshToken, + &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const insertUserLink = `-- name: InsertUserLink :one INSERT INTO user_links ( @@ -5506,20 +5671,24 @@ INSERT INTO login_type, linked_id, oauth_access_token, + oauth_access_token_key_id, oauth_refresh_token, + oauth_refresh_token_key_id, oauth_expiry ) VALUES - ( $1, $2, $3, $4, $5, $6 ) RETURNING user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry + ( $1, $2, $3, $4, $5, $6, $7, $8 ) RETURNING user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id ` type InsertUserLinkParams struct { - UserID uuid.UUID `db:"user_id" json:"user_id"` - LoginType LoginType `db:"login_type" json:"login_type"` - LinkedID string `db:"linked_id" json:"linked_id"` - OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` - OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` - OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + LoginType LoginType `db:"login_type" json:"login_type"` + LinkedID string `db:"linked_id" json:"linked_id"` + OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` + OAuthAccessTokenKeyID sql.NullString `db:"oauth_access_token_key_id" json:"oauth_access_token_key_id"` + OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` + OAuthRefreshTokenKeyID sql.NullString `db:"oauth_refresh_token_key_id" json:"oauth_refresh_token_key_id"` + OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` } func (q *sqlQuerier) InsertUserLink(ctx context.Context, arg InsertUserLinkParams) (UserLink, error) { @@ -5528,7 +5697,9 @@ func (q *sqlQuerier) InsertUserLink(ctx context.Context, arg InsertUserLinkParam arg.LoginType, arg.LinkedID, arg.OAuthAccessToken, + arg.OAuthAccessTokenKeyID, arg.OAuthRefreshToken, + arg.OAuthRefreshTokenKeyID, arg.OAuthExpiry, ) var i UserLink @@ -5539,6 +5710,8 @@ func (q *sqlQuerier) InsertUserLink(ctx context.Context, arg InsertUserLinkParam &i.OAuthAccessToken, &i.OAuthRefreshToken, &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, ) return i, err } @@ -5548,24 +5721,30 @@ UPDATE user_links SET oauth_access_token = $1, - oauth_refresh_token = $2, - oauth_expiry = $3 + oauth_access_token_key_id = $2, + oauth_refresh_token = $3, + oauth_refresh_token_key_id = $4, + oauth_expiry = $5 WHERE - user_id = $4 AND login_type = $5 RETURNING user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry + user_id = $6 AND login_type = $7 RETURNING user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id ` type UpdateUserLinkParams struct { - OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` - OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` - OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` - UserID uuid.UUID `db:"user_id" json:"user_id"` - LoginType LoginType `db:"login_type" json:"login_type"` + OAuthAccessToken string `db:"oauth_access_token" json:"oauth_access_token"` + OAuthAccessTokenKeyID sql.NullString `db:"oauth_access_token_key_id" json:"oauth_access_token_key_id"` + OAuthRefreshToken string `db:"oauth_refresh_token" json:"oauth_refresh_token"` + OAuthRefreshTokenKeyID sql.NullString `db:"oauth_refresh_token_key_id" json:"oauth_refresh_token_key_id"` + OAuthExpiry time.Time `db:"oauth_expiry" json:"oauth_expiry"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + LoginType LoginType `db:"login_type" json:"login_type"` } func (q *sqlQuerier) UpdateUserLink(ctx context.Context, arg UpdateUserLinkParams) (UserLink, error) { row := q.db.QueryRowContext(ctx, updateUserLink, arg.OAuthAccessToken, + arg.OAuthAccessTokenKeyID, arg.OAuthRefreshToken, + arg.OAuthRefreshTokenKeyID, arg.OAuthExpiry, arg.UserID, arg.LoginType, @@ -5578,6 +5757,8 @@ func (q *sqlQuerier) UpdateUserLink(ctx context.Context, arg UpdateUserLinkParam &i.OAuthAccessToken, &i.OAuthRefreshToken, &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, ) return i, err } @@ -5588,7 +5769,7 @@ UPDATE SET linked_id = $1 WHERE - user_id = $2 AND login_type = $3 RETURNING user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry + user_id = $2 AND login_type = $3 RETURNING user_id, login_type, linked_id, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id ` type UpdateUserLinkedIDParams struct { @@ -5607,6 +5788,8 @@ func (q *sqlQuerier) UpdateUserLinkedID(ctx context.Context, arg UpdateUserLinke &i.OAuthAccessToken, &i.OAuthRefreshToken, &i.OAuthExpiry, + &i.OAuthAccessTokenKeyID, + &i.OAuthRefreshTokenKeyID, ) return i, err } diff --git a/coderd/database/queries/dbcrypt.sql b/coderd/database/queries/dbcrypt.sql new file mode 100644 index 0000000000000..ef1021609d5a7 --- /dev/null +++ b/coderd/database/queries/dbcrypt.sql @@ -0,0 +1,18 @@ +-- name: GetDBCryptKeys :many +SELECT * FROM dbcrypt_keys ORDER BY number ASC; + +-- name: RevokeDBCryptKey :exec +UPDATE dbcrypt_keys +SET + revoked_key_digest = active_key_digest, + active_key_digest = revoked_key_digest, + revoked_at = CURRENT_TIMESTAMP +WHERE + active_key_digest = @active_key_digest::text +AND + revoked_key_digest IS NULL; + +-- name: InsertDBCryptKey :exec +INSERT INTO dbcrypt_keys + (number, active_key_digest, created_at, test) +VALUES (@number::int, @active_key_digest::text, CURRENT_TIMESTAMP, @test::text); diff --git a/coderd/database/queries/gitauth.sql b/coderd/database/queries/gitauth.sql index a35de98a08908..ad1c92cb68d0b 100644 --- a/coderd/database/queries/gitauth.sql +++ b/coderd/database/queries/gitauth.sql @@ -1,6 +1,9 @@ -- name: GetGitAuthLink :one SELECT * FROM git_auth_links WHERE provider_id = $1 AND user_id = $2; +-- name: GetGitAuthLinksByUserID :many +SELECT * FROM git_auth_links WHERE user_id = $1; + -- name: InsertGitAuthLink :one INSERT INTO git_auth_links ( provider_id, @@ -8,7 +11,9 @@ INSERT INTO git_auth_links ( created_at, updated_at, oauth_access_token, + oauth_access_token_key_id, oauth_refresh_token, + oauth_refresh_token_key_id, oauth_expiry ) VALUES ( $1, @@ -17,13 +22,17 @@ INSERT INTO git_auth_links ( $4, $5, $6, - $7 + $7, + $8, + $9 ) RETURNING *; -- name: UpdateGitAuthLink :one UPDATE git_auth_links SET updated_at = $3, oauth_access_token = $4, - oauth_refresh_token = $5, - oauth_expiry = $6 + oauth_access_token_key_id = $5, + oauth_refresh_token = $6, + oauth_refresh_token_key_id = $7, + oauth_expiry = $8 WHERE provider_id = $1 AND user_id = $2 RETURNING *; diff --git a/coderd/database/queries/user_links.sql b/coderd/database/queries/user_links.sql index 2390cb9782b30..5db3324c676a2 100644 --- a/coderd/database/queries/user_links.sql +++ b/coderd/database/queries/user_links.sql @@ -14,6 +14,9 @@ FROM WHERE user_id = $1 AND login_type = $2; +-- name: GetUserLinksByUserID :many +SELECT * FROM user_links WHERE user_id = $1; + -- name: InsertUserLink :one INSERT INTO user_links ( @@ -21,11 +24,13 @@ INSERT INTO login_type, linked_id, oauth_access_token, + oauth_access_token_key_id, oauth_refresh_token, + oauth_refresh_token_key_id, oauth_expiry ) VALUES - ( $1, $2, $3, $4, $5, $6 ) RETURNING *; + ( $1, $2, $3, $4, $5, $6, $7, $8 ) RETURNING *; -- name: UpdateUserLinkedID :one UPDATE @@ -40,7 +45,9 @@ UPDATE user_links SET oauth_access_token = $1, - oauth_refresh_token = $2, - oauth_expiry = $3 + oauth_access_token_key_id = $2, + oauth_refresh_token = $3, + oauth_refresh_token_key_id = $4, + oauth_expiry = $5 WHERE - user_id = $4 AND login_type = $5 RETURNING *; + user_id = $6 AND login_type = $7 RETURNING *; diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index 73c59c257de31..1bdc972927f6f 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -40,6 +40,7 @@ overrides: api_key_scope_application_connect: APIKeyScopeApplicationConnect avatar_url: AvatarURL created_by_avatar_url: CreatedByAvatarURL + dbcrypt_key: DBCryptKey session_count_vscode: SessionCountVSCode session_count_jetbrains: SessionCountJetBrains session_count_reconnecting_pty: SessionCountReconnectingPTY @@ -47,9 +48,11 @@ overrides: connection_median_latency_ms: ConnectionMedianLatencyMS login_type_oidc: LoginTypeOIDC oauth_access_token: OAuthAccessToken + oauth_access_token_key_id: OAuthAccessTokenKeyID oauth_expiry: OAuthExpiry oauth_id_token: OAuthIDToken oauth_refresh_token: OAuthRefreshToken + oauth_refresh_token_key_id: OAuthRefreshTokenKeyID parameter_type_system_hcl: ParameterTypeSystemHCL userstatus: UserStatus gitsshkey: GitSSHKey diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index 294b4b12d51af..ea0a9a64d3137 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -6,6 +6,8 @@ type UniqueConstraint string // UniqueConstraint enums. const ( + UniqueDbcryptKeysActiveKeyDigestKey UniqueConstraint = "dbcrypt_keys_active_key_digest_key" // ALTER TABLE ONLY dbcrypt_keys ADD CONSTRAINT dbcrypt_keys_active_key_digest_key UNIQUE (active_key_digest); + UniqueDbcryptKeysRevokedKeyDigestKey UniqueConstraint = "dbcrypt_keys_revoked_key_digest_key" // ALTER TABLE ONLY dbcrypt_keys ADD CONSTRAINT dbcrypt_keys_revoked_key_digest_key UNIQUE (revoked_key_digest); UniqueFilesHashCreatedByKey UniqueConstraint = "files_hash_created_by_key" // ALTER TABLE ONLY files ADD CONSTRAINT files_hash_created_by_key UNIQUE (hash, created_by); UniqueGitAuthLinksProviderIDUserIDKey UniqueConstraint = "git_auth_links_provider_id_user_id_key" // ALTER TABLE ONLY git_auth_links ADD CONSTRAINT git_auth_links_provider_id_user_id_key UNIQUE (provider_id, user_id); UniqueGroupMembersUserIDGroupIDKey UniqueConstraint = "group_members_user_id_group_id_key" // ALTER TABLE ONLY group_members ADD CONSTRAINT group_members_user_id_group_id_key UNIQUE (user_id, group_id); diff --git a/enterprise/dbcrypt/cipher.go b/enterprise/dbcrypt/cipher.go new file mode 100644 index 0000000000000..fc6f25ee90955 --- /dev/null +++ b/enterprise/dbcrypt/cipher.go @@ -0,0 +1,98 @@ +package dbcrypt + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "crypto/sha256" + "fmt" + "io" + + "golang.org/x/xerrors" +) + +// cipherAES256GCM is the name of the AES-256 cipher. +// This is used to identify the cipher used to encrypt a value. +// It is added to the digest to ensure that if, in the future, +// we add a new cipher type, and a key is re-used, we don't +// accidentally decrypt the wrong values. +// When adding a new cipher type, add a new constant here +// and ensure to add the cipher name to the digest of the new +// cipher type. +const ( + cipherAES256GCM = "aes256gcm" +) + +type Cipher interface { + Encrypt([]byte) ([]byte, error) + Decrypt([]byte) ([]byte, error) + HexDigest() string +} + +// NewCiphers is a convenience function for creating multiple ciphers. +// It currently only supports AES-256-GCM. +func NewCiphers(keys ...[]byte) ([]Cipher, error) { + var cs []Cipher + for _, key := range keys { + c, err := cipherAES256(key) + if err != nil { + return nil, err + } + cs = append(cs, c) + } + return cs, nil +} + +// cipherAES256 returns a new AES-256 cipher. +func cipherAES256(key []byte) (*aes256, error) { + if len(key) != 32 { + return nil, xerrors.Errorf("key must be 32 bytes") + } + block, err := aes.NewCipher(key) + if err != nil { + return nil, err + } + aead, err := cipher.NewGCM(block) + if err != nil { + return nil, err + } + // We add the cipher name to the digest to ensure that if, in the future, + // we add a new cipher type, and a key is re-used, we don't accidentally + // decrypt the wrong values. + toDigest := []byte(cipherAES256GCM) + toDigest = append(toDigest, key...) + digest := fmt.Sprintf("%x", sha256.Sum256(toDigest))[:7] + return &aes256{aead: aead, digest: digest}, nil +} + +type aes256 struct { + aead cipher.AEAD + // digest is the first 7 bytes of the hex-encoded SHA-256 digest of aead. + digest string +} + +func (a *aes256) Encrypt(plaintext []byte) ([]byte, error) { + nonce := make([]byte, a.aead.NonceSize()) + _, err := io.ReadFull(rand.Reader, nonce) + if err != nil { + return nil, err + } + dst := make([]byte, len(nonce)) + copy(dst, nonce) + return a.aead.Seal(dst, nonce, plaintext, nil), nil +} + +func (a *aes256) Decrypt(ciphertext []byte) ([]byte, error) { + if len(ciphertext) < a.aead.NonceSize() { + return nil, xerrors.Errorf("ciphertext too short") + } + decrypted, err := a.aead.Open(nil, ciphertext[:a.aead.NonceSize()], ciphertext[a.aead.NonceSize():], nil) + if err != nil { + return nil, &DecryptFailedError{Inner: err} + } + return decrypted, nil +} + +func (a *aes256) HexDigest() string { + return a.digest +} diff --git a/enterprise/dbcrypt/cipher_internal_test.go b/enterprise/dbcrypt/cipher_internal_test.go new file mode 100644 index 0000000000000..b6740de17eec6 --- /dev/null +++ b/enterprise/dbcrypt/cipher_internal_test.go @@ -0,0 +1,91 @@ +package dbcrypt + +import ( + "bytes" + "encoding/base64" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCipherAES256(t *testing.T) { + t.Parallel() + + t.Run("ValidInput", func(t *testing.T) { + t.Parallel() + key := bytes.Repeat([]byte{'a'}, 32) + cipher, err := cipherAES256(key) + require.NoError(t, err) + + output, err := cipher.Encrypt([]byte("hello world")) + require.NoError(t, err) + + response, err := cipher.Decrypt(output) + require.NoError(t, err) + require.Equal(t, "hello world", string(response)) + }) + + t.Run("InvalidInput", func(t *testing.T) { + t.Parallel() + key := bytes.Repeat([]byte{'a'}, 32) + cipher, err := cipherAES256(key) + require.NoError(t, err) + _, err = cipher.Decrypt(bytes.Repeat([]byte{'a'}, 100)) + var decryptErr *DecryptFailedError + require.ErrorAs(t, err, &decryptErr) + }) + + t.Run("InvalidKeySize", func(t *testing.T) { + t.Parallel() + + _, err := cipherAES256(bytes.Repeat([]byte{'a'}, 31)) + require.ErrorContains(t, err, "key must be 32 bytes") + }) + + t.Run("TestNonce", func(t *testing.T) { + t.Parallel() + key := bytes.Repeat([]byte{'a'}, 32) + cipher, err := cipherAES256(key) + require.NoError(t, err) + require.Equal(t, "864f702", cipher.HexDigest()) + + encrypted1, err := cipher.Encrypt([]byte("hello world")) + require.NoError(t, err) + encrypted2, err := cipher.Encrypt([]byte("hello world")) + require.NoError(t, err) + require.NotEqual(t, encrypted1, encrypted2, "nonce should be different for each encryption") + + munged := make([]byte, len(encrypted1)) + copy(munged, encrypted1) + munged[0] = munged[0] ^ 0xff + _, err = cipher.Decrypt(munged) + var decryptErr *DecryptFailedError + require.ErrorAs(t, err, &decryptErr, "munging the first byte of the encrypted data should cause decryption to fail") + }) +} + +// This test ensures backwards compatibility. If it breaks, something is very wrong. +func TestCiphersBackwardCompatibility(t *testing.T) { + t.Parallel() + var ( + msg = "hello world" + key = bytes.Repeat([]byte{'a'}, 32) + //nolint: gosec // The below is the base64-encoded result of encrypting the above message with the above key. + encoded = `YhAz+lE2fFeeiVPH9voKN7UV1xSDrgcnC0LmNXmaAk1Yg0kPFO3x` + ) + + cipher, err := cipherAES256(key) + require.NoError(t, err) + + // This is the code that was used to generate the above. + // Note that the output of this code will change every time it is run. + // encrypted, err := cipher.Encrypt([]byte(msg)) + // require.NoError(t, err) + // t.Logf("encoded: %q", base64.StdEncoding.EncodeToString(encrypted)) + + decoded, err := base64.StdEncoding.DecodeString(encoded) + require.NoError(t, err, "the encoded string should be valid base64") + decrypted, err := cipher.Decrypt(decoded) + require.NoError(t, err, "decryption should succeed") + require.Equal(t, msg, string(decrypted), "decrypted message should match original message") +} diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go new file mode 100644 index 0000000000000..9bd42a518ab82 --- /dev/null +++ b/enterprise/dbcrypt/dbcrypt.go @@ -0,0 +1,374 @@ +package dbcrypt + +import ( + "context" + "database/sql" + "encoding/base64" + + "github.com/lib/pq" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + + "github.com/google/uuid" + "golang.org/x/xerrors" +) + +// testValue is the value that is stored in dbcrypt_keys.test. +// This is used to determine if the key is valid. +const testValue = "coder" + +var ( + ErrNotEnabled = xerrors.New("encryption is not enabled") + b64encode = base64.StdEncoding.EncodeToString + b64decode = base64.StdEncoding.DecodeString +) + +// DecryptFailedError is returned when decryption fails. +type DecryptFailedError struct { + Inner error +} + +func (e *DecryptFailedError) Error() string { + return xerrors.Errorf("decrypt failed: %w", e.Inner).Error() +} + +// New creates a database.Store wrapper that encrypts/decrypts values +// stored at rest in the database. +func New(ctx context.Context, db database.Store, ciphers ...Cipher) (database.Store, error) { + if len(ciphers) == 0 { + return nil, xerrors.Errorf("no ciphers configured") + } + cm := make(map[string]Cipher) + for _, c := range ciphers { + cm[c.HexDigest()] = c + } + dbc := &dbCrypt{ + primaryCipherDigest: ciphers[0].HexDigest(), + ciphers: cm, + Store: db, + } + // nolint: gocritic // This is allowed. + if err := dbc.ensureEncrypted(dbauthz.AsSystemRestricted(ctx)); err != nil { + return nil, xerrors.Errorf("ensure encrypted database fields: %w", err) + } + return dbc, nil +} + +type dbCrypt struct { + // primaryCipherDigest is the digest of the primary cipher used for encrypting data. + primaryCipherDigest string + // ciphers is a map of cipher digests to ciphers. + ciphers map[string]Cipher + 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{ + primaryCipherDigest: db.primaryCipherDigest, + ciphers: db.ciphers, + Store: s, + }) + }, txOpts) +} + +func (db *dbCrypt) GetDBCryptKeys(ctx context.Context) ([]database.DBCryptKey, error) { + ks, err := db.Store.GetDBCryptKeys(ctx) + if err != nil { + return nil, err + } + // Decrypt the test field to ensure that the key is valid. + for i := range ks { + if !ks[i].ActiveKeyDigest.Valid { + // Key has been revoked. We can't decrypt the test field, but + // we need to return it so that the caller knows that the key + // has been revoked. + continue + } + if err := db.decryptField(&ks[i].Test, ks[i].ActiveKeyDigest); err != nil { + return nil, err + } + } + return ks, nil +} + +// This does not need any special handling as it does not touch any encrypted fields. +// Explicitly defining this here to avoid confusion. +func (db *dbCrypt) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error { + return db.Store.RevokeDBCryptKey(ctx, activeKeyDigest) +} + +func (db *dbCrypt) InsertDBCryptKey(ctx context.Context, arg database.InsertDBCryptKeyParams) error { + // It's nicer to be able to pass a *sql.NullString to encryptField, but we need to pass a string here. + var digest sql.NullString + err := db.encryptField(&arg.Test, &digest) + if err != nil { + return err + } + arg.ActiveKeyDigest = digest.String + return db.Store.InsertDBCryptKey(ctx, arg) +} + +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 + } + if err := db.decryptField(&link.OAuthAccessToken, link.OAuthAccessTokenKeyID); err != nil { + return database.UserLink{}, err + } + if err := db.decryptField(&link.OAuthRefreshToken, link.OAuthRefreshTokenKeyID); err != nil { + return database.UserLink{}, err + } + return link, nil +} + +func (db *dbCrypt) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]database.UserLink, error) { + links, err := db.Store.GetUserLinksByUserID(ctx, userID) + if err != nil { + return nil, err + } + for idx := range links { + if err := db.decryptField(&links[idx].OAuthAccessToken, links[idx].OAuthAccessTokenKeyID); err != nil { + return nil, err + } + if err := db.decryptField(&links[idx].OAuthRefreshToken, links[idx].OAuthRefreshTokenKeyID); err != nil { + return nil, err + } + } + return links, nil +} + +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 + } + if err := db.decryptField(&link.OAuthAccessToken, link.OAuthAccessTokenKeyID); err != nil { + return database.UserLink{}, err + } + if err := db.decryptField(&link.OAuthRefreshToken, link.OAuthRefreshTokenKeyID); err != nil { + return database.UserLink{}, err + } + return link, nil +} + +func (db *dbCrypt) InsertUserLink(ctx context.Context, params database.InsertUserLinkParams) (database.UserLink, error) { + if err := db.encryptField(¶ms.OAuthAccessToken, ¶ms.OAuthAccessTokenKeyID); err != nil { + return database.UserLink{}, err + } + if err := db.encryptField(¶ms.OAuthRefreshToken, ¶ms.OAuthRefreshTokenKeyID); err != nil { + return database.UserLink{}, err + } + link, err := db.Store.InsertUserLink(ctx, params) + if err != nil { + return database.UserLink{}, err + } + if err := db.decryptField(&link.OAuthAccessToken, link.OAuthAccessTokenKeyID); err != nil { + return database.UserLink{}, err + } + if err := db.decryptField(&link.OAuthRefreshToken, link.OAuthRefreshTokenKeyID); err != nil { + return database.UserLink{}, err + } + return link, nil +} + +func (db *dbCrypt) UpdateUserLink(ctx context.Context, params database.UpdateUserLinkParams) (database.UserLink, error) { + if err := db.encryptField(¶ms.OAuthAccessToken, ¶ms.OAuthAccessTokenKeyID); err != nil { + return database.UserLink{}, err + } + if err := db.encryptField(¶ms.OAuthRefreshToken, ¶ms.OAuthRefreshTokenKeyID); err != nil { + return database.UserLink{}, err + } + link, err := db.Store.UpdateUserLink(ctx, params) + if err != nil { + return database.UserLink{}, err + } + if err := db.decryptField(&link.OAuthAccessToken, link.OAuthAccessTokenKeyID); err != nil { + return database.UserLink{}, err + } + if err := db.decryptField(&link.OAuthRefreshToken, link.OAuthRefreshTokenKeyID); err != nil { + return database.UserLink{}, err + } + return link, nil +} + +func (db *dbCrypt) InsertGitAuthLink(ctx context.Context, params database.InsertGitAuthLinkParams) (database.GitAuthLink, error) { + if err := db.encryptField(¶ms.OAuthAccessToken, ¶ms.OAuthAccessTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + if err := db.encryptField(¶ms.OAuthRefreshToken, ¶ms.OAuthRefreshTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + link, err := db.Store.InsertGitAuthLink(ctx, params) + if err != nil { + return database.GitAuthLink{}, err + } + if err := db.decryptField(&link.OAuthAccessToken, link.OAuthAccessTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + if err := db.decryptField(&link.OAuthRefreshToken, link.OAuthRefreshTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + return link, nil +} + +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 + } + if err := db.decryptField(&link.OAuthAccessToken, link.OAuthAccessTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + if err := db.decryptField(&link.OAuthRefreshToken, link.OAuthRefreshTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + return link, nil +} + +func (db *dbCrypt) GetGitAuthLinksByUserID(ctx context.Context, userID uuid.UUID) ([]database.GitAuthLink, error) { + links, err := db.Store.GetGitAuthLinksByUserID(ctx, userID) + if err != nil { + return nil, err + } + for idx := range links { + if err := db.decryptField(&links[idx].OAuthAccessToken, links[idx].OAuthAccessTokenKeyID); err != nil { + return nil, err + } + if err := db.decryptField(&links[idx].OAuthRefreshToken, links[idx].OAuthRefreshTokenKeyID); err != nil { + return nil, err + } + } + return links, nil +} + +func (db *dbCrypt) UpdateGitAuthLink(ctx context.Context, params database.UpdateGitAuthLinkParams) (database.GitAuthLink, error) { + if err := db.encryptField(¶ms.OAuthAccessToken, ¶ms.OAuthAccessTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + if err := db.encryptField(¶ms.OAuthRefreshToken, ¶ms.OAuthRefreshTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + link, err := db.Store.UpdateGitAuthLink(ctx, params) + if err != nil { + return database.GitAuthLink{}, err + } + if err := db.decryptField(&link.OAuthAccessToken, link.OAuthAccessTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + if err := db.decryptField(&link.OAuthRefreshToken, link.OAuthRefreshTokenKeyID); err != nil { + return database.GitAuthLink{}, err + } + return link, nil +} + +func (db *dbCrypt) encryptField(field *string, digest *sql.NullString) error { + // If no cipher is loaded, then we can't encrypt anything! + if db.ciphers == nil || db.primaryCipherDigest == "" { + return ErrNotEnabled + } + + if field == nil { + return nil + } + + encrypted, err := db.ciphers[db.primaryCipherDigest].Encrypt([]byte(*field)) + if err != nil { + return err + } + // Base64 is used to support UTF-8 encoding in PostgreSQL. + *field = b64encode(encrypted) + *digest = sql.NullString{String: db.primaryCipherDigest, Valid: true} + return nil +} + +// decryptFields decrypts the given field using the key with the given digest. +// If the value fails to decrypt, sql.ErrNoRows will be returned. +func (db *dbCrypt) decryptField(field *string, digest sql.NullString) error { + if db.ciphers == nil { + return ErrNotEnabled + } + + if !digest.Valid || digest.String == "" { + // This field is not encrypted. + return nil + } + + if field == nil || len(*field) == 0 { + // We've been asked to decrypt a field that is empty. + // There is a digest present, so it should have been encrypted, + // which would have produced a non-empty value. + // If we return sql.ErrNoRows, then the caller will assume that + // the value is not present in the database, which is not true. + // Return a DecryptFailedError to indicate that there is a value + // present, but it could not be decrypted. + // Unfortunately the only real way to fix this is to mark this + // field as not encrypted in the database. We do not want to + // silently do this, as it would mask a real problem. + return &DecryptFailedError{ + Inner: xerrors.Errorf("unexpected empty encrypted field with digest %q", digest.String), + } + } + + key, ok := db.ciphers[digest.String] + if !ok { + return &DecryptFailedError{ + Inner: xerrors.Errorf("no cipher with digest %q", digest.String), + } + } + + data, err := b64decode(*field) + if err != nil { + // If it's not valid base64, we should complain loudly. + return &DecryptFailedError{ + Inner: xerrors.Errorf("malformed encrypted field %q: %w", *field, err), + } + } + decrypted, err := key.Decrypt(data) + if err != nil { + return &DecryptFailedError{Inner: err} + } + *field = string(decrypted) + return nil +} + +func (db *dbCrypt) ensureEncrypted(ctx context.Context) error { + return db.InTx(func(s database.Store) error { + // Attempt to read the encrypted test fields of the currently active keys. + ks, err := s.GetDBCryptKeys(ctx) + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + return err + } + + var highestNumber int32 + for _, k := range ks { + if k.ActiveKeyDigest.Valid && k.ActiveKeyDigest.String == db.primaryCipherDigest { + // This is our currently active key. We don't need to do anything further. + return nil + } + if k.Number > highestNumber { + highestNumber = k.Number + } + } + + // If we get here, then we have a new key that we need to insert. + // If this conflicts with another transaction, we do not need to retry as + // the other transaction will have inserted the key for us. + if err := db.InsertDBCryptKey(ctx, database.InsertDBCryptKeyParams{ + Number: highestNumber + 1, + ActiveKeyDigest: db.primaryCipherDigest, + Test: testValue, + }); err != nil { + var pqErr *pq.Error + if xerrors.As(err, &pqErr) && pqErr.Code == "23505" { + // Unique constraint violation -> another transaction has inserted the key for us. + return nil + } + return err + } + + return nil + }, &sql.TxOptions{Isolation: sql.LevelRepeatableRead}) +} diff --git a/enterprise/dbcrypt/dbcrypt_internal_test.go b/enterprise/dbcrypt/dbcrypt_internal_test.go new file mode 100644 index 0000000000000..817833e7a91c4 --- /dev/null +++ b/enterprise/dbcrypt/dbcrypt_internal_test.go @@ -0,0 +1,489 @@ +package dbcrypt + +import ( + "context" + "crypto/rand" + "database/sql" + "encoding/base64" + "io" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" +) + +func TestUserLinks(t *testing.T) { + t.Parallel() + ctx := context.Background() + + t.Run("InsertUserLink", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, crypt, database.User{}) + link := dbgen.UserLink(t, crypt, database.UserLink{ + UserID: user.ID, + OAuthAccessToken: "access", + OAuthRefreshToken: "refresh", + }) + require.Equal(t, "access", link.OAuthAccessToken) + require.Equal(t, "refresh", link.OAuthRefreshToken) + require.Equal(t, ciphers[0].HexDigest(), link.OAuthAccessTokenKeyID.String) + require.Equal(t, ciphers[0].HexDigest(), link.OAuthRefreshTokenKeyID.String) + + rawLink, err := db.GetUserLinkByLinkedID(ctx, link.LinkedID) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], rawLink.OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], rawLink.OAuthRefreshToken, "refresh") + }) + + t.Run("UpdateUserLink", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, crypt, database.User{}) + link := dbgen.UserLink(t, crypt, database.UserLink{ + UserID: user.ID, + }) + + updated, err := crypt.UpdateUserLink(ctx, database.UpdateUserLinkParams{ + OAuthAccessToken: "access", + OAuthRefreshToken: "refresh", + UserID: link.UserID, + LoginType: link.LoginType, + }) + require.NoError(t, err) + require.Equal(t, "access", updated.OAuthAccessToken) + require.Equal(t, "refresh", updated.OAuthRefreshToken) + require.Equal(t, ciphers[0].HexDigest(), link.OAuthAccessTokenKeyID.String) + require.Equal(t, ciphers[0].HexDigest(), link.OAuthRefreshTokenKeyID.String) + + rawLink, err := db.GetUserLinkByLinkedID(ctx, link.LinkedID) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], rawLink.OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], rawLink.OAuthRefreshToken, "refresh") + }) + + t.Run("GetUserLinkByLinkedID", func(t *testing.T) { + t.Parallel() + t.Run("OK", func(t *testing.T) { + db, crypt, ciphers := setup(t) + user := dbgen.User(t, crypt, database.User{}) + link := dbgen.UserLink(t, crypt, database.UserLink{ + UserID: user.ID, + OAuthAccessToken: "access", + OAuthRefreshToken: "refresh", + }) + + link, err := crypt.GetUserLinkByLinkedID(ctx, link.LinkedID) + require.NoError(t, err) + require.Equal(t, "access", link.OAuthAccessToken) + require.Equal(t, "refresh", link.OAuthRefreshToken) + require.Equal(t, ciphers[0].HexDigest(), link.OAuthAccessTokenKeyID.String) + require.Equal(t, ciphers[0].HexDigest(), link.OAuthRefreshTokenKeyID.String) + + rawLink, err := db.GetUserLinkByLinkedID(ctx, link.LinkedID) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], rawLink.OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], rawLink.OAuthRefreshToken, "refresh") + }) + + t.Run("DecryptErr", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + link := dbgen.UserLink(t, db, database.UserLink{ + UserID: user.ID, + OAuthAccessToken: fakeBase64RandomData(t, 32), + OAuthRefreshToken: fakeBase64RandomData(t, 32), + OAuthAccessTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + OAuthRefreshTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + }) + + _, err := crypt.GetUserLinkByLinkedID(ctx, link.LinkedID) + require.Error(t, err, "expected an error") + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr, "expected a decrypt error") + }) + }) + + t.Run("GetUserLinksByUserID", func(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, crypt, database.User{}) + link := dbgen.UserLink(t, crypt, database.UserLink{ + UserID: user.ID, + OAuthAccessToken: "access", + OAuthRefreshToken: "refresh", + }) + links, err := crypt.GetUserLinksByUserID(ctx, link.UserID) + require.NoError(t, err) + require.Len(t, links, 1) + require.Equal(t, "access", links[0].OAuthAccessToken) + require.Equal(t, "refresh", links[0].OAuthRefreshToken) + require.Equal(t, ciphers[0].HexDigest(), links[0].OAuthAccessTokenKeyID.String) + require.Equal(t, ciphers[0].HexDigest(), links[0].OAuthRefreshTokenKeyID.String) + + rawLinks, err := db.GetUserLinksByUserID(ctx, link.UserID) + require.NoError(t, err) + require.Len(t, rawLinks, 1) + requireEncryptedEquals(t, ciphers[0], rawLinks[0].OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], rawLinks[0].OAuthRefreshToken, "refresh") + }) + + t.Run("Empty", func(t *testing.T) { + t.Parallel() + _, crypt, _ := setup(t) + user := dbgen.User(t, crypt, database.User{}) + links, err := crypt.GetUserLinksByUserID(ctx, user.ID) + require.NoError(t, err) + require.Empty(t, links) + }) + + t.Run("DecryptErr", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + _ = dbgen.UserLink(t, db, database.UserLink{ + UserID: user.ID, + OAuthAccessToken: fakeBase64RandomData(t, 32), + OAuthRefreshToken: fakeBase64RandomData(t, 32), + OAuthAccessTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + OAuthRefreshTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + }) + _, err := crypt.GetUserLinksByUserID(ctx, user.ID) + require.Error(t, err, "expected an error") + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr, "expected a decrypt error") + }) + }) + + t.Run("GetUserLinkByUserIDLoginType", func(t *testing.T) { + t.Parallel() + t.Run("OK", func(t *testing.T) { + db, crypt, ciphers := setup(t) + user := dbgen.User(t, crypt, database.User{}) + link := dbgen.UserLink(t, crypt, database.UserLink{ + UserID: user.ID, + OAuthAccessToken: "access", + OAuthRefreshToken: "refresh", + }) + + link, err := crypt.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: link.UserID, + LoginType: link.LoginType, + }) + require.NoError(t, err) + require.Equal(t, "access", link.OAuthAccessToken) + require.Equal(t, "refresh", link.OAuthRefreshToken) + require.Equal(t, ciphers[0].HexDigest(), link.OAuthAccessTokenKeyID.String) + require.Equal(t, ciphers[0].HexDigest(), link.OAuthRefreshTokenKeyID.String) + + rawLink, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: link.UserID, + LoginType: link.LoginType, + }) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], rawLink.OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], rawLink.OAuthRefreshToken, "refresh") + }) + + t.Run("DecryptErr", func(t *testing.T) { + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + link := dbgen.UserLink(t, db, database.UserLink{ + UserID: user.ID, + OAuthAccessToken: fakeBase64RandomData(t, 32), + OAuthRefreshToken: fakeBase64RandomData(t, 32), + OAuthAccessTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + OAuthRefreshTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + }) + + _, err := crypt.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: link.UserID, + LoginType: link.LoginType, + }) + require.Error(t, err, "expected an error") + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr, "expected a decrypt error") + }) + }) +} + +func TestGitAuthLinks(t *testing.T) { + t.Parallel() + ctx := context.Background() + + t.Run("InsertGitAuthLink", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + link := dbgen.GitAuthLink(t, crypt, database.GitAuthLink{ + OAuthAccessToken: "access", + OAuthRefreshToken: "refresh", + }) + require.Equal(t, "access", link.OAuthAccessToken) + require.Equal(t, "refresh", link.OAuthRefreshToken) + + link, err := db.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ + ProviderID: link.ProviderID, + UserID: link.UserID, + }) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], link.OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], link.OAuthRefreshToken, "refresh") + }) + + t.Run("UpdateGitAuthLink", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + link := dbgen.GitAuthLink(t, crypt, database.GitAuthLink{}) + updated, err := crypt.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ + ProviderID: link.ProviderID, + UserID: link.UserID, + OAuthAccessToken: "access", + OAuthRefreshToken: "refresh", + }) + require.NoError(t, err) + require.Equal(t, "access", updated.OAuthAccessToken) + require.Equal(t, "refresh", updated.OAuthRefreshToken) + + link, err = db.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ + ProviderID: link.ProviderID, + UserID: link.UserID, + }) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], link.OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], link.OAuthRefreshToken, "refresh") + }) + + t.Run("GetGitAuthLink", func(t *testing.T) { + t.Run("OK", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + 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, ciphers[0], link.OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], link.OAuthRefreshToken, "refresh") + }) + t.Run("DecryptErr", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + link := dbgen.GitAuthLink(t, db, database.GitAuthLink{ + OAuthAccessToken: fakeBase64RandomData(t, 32), + OAuthRefreshToken: fakeBase64RandomData(t, 32), + OAuthAccessTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + OAuthRefreshTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + }) + + _, err := crypt.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ + UserID: link.UserID, + ProviderID: link.ProviderID, + }) + require.Error(t, err, "expected an error") + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr, "expected a decrypt error") + }) + }) + + t.Run("GetGitAuthLinksByUserID", func(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, crypt, database.User{}) + link := dbgen.GitAuthLink(t, crypt, database.GitAuthLink{ + UserID: user.ID, + OAuthAccessToken: "access", + OAuthRefreshToken: "refresh", + }) + links, err := crypt.GetGitAuthLinksByUserID(ctx, link.UserID) + require.NoError(t, err) + require.Len(t, links, 1) + require.Equal(t, "access", links[0].OAuthAccessToken) + require.Equal(t, "refresh", links[0].OAuthRefreshToken) + require.Equal(t, ciphers[0].HexDigest(), links[0].OAuthAccessTokenKeyID.String) + require.Equal(t, ciphers[0].HexDigest(), links[0].OAuthRefreshTokenKeyID.String) + + rawLinks, err := db.GetGitAuthLinksByUserID(ctx, link.UserID) + require.NoError(t, err) + require.Len(t, rawLinks, 1) + requireEncryptedEquals(t, ciphers[0], rawLinks[0].OAuthAccessToken, "access") + requireEncryptedEquals(t, ciphers[0], rawLinks[0].OAuthRefreshToken, "refresh") + }) + + t.Run("DecryptErr", func(t *testing.T) { + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + link := dbgen.GitAuthLink(t, db, database.GitAuthLink{ + UserID: user.ID, + OAuthAccessToken: fakeBase64RandomData(t, 32), + OAuthRefreshToken: fakeBase64RandomData(t, 32), + OAuthAccessTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + OAuthRefreshTokenKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + }) + _, err := crypt.GetGitAuthLinksByUserID(ctx, link.UserID) + require.Error(t, err, "expected an error") + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr, "expected a decrypt error") + }) + }) +} + +func TestNew(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + // Given: a cipher is loaded + cipher := initCipher(t) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + rawDB, _ := dbtestutil.NewDB(t) + + // Before: no keys should be present + keys, err := rawDB.GetDBCryptKeys(ctx) + require.NoError(t, err, "no error should be returned") + require.Empty(t, keys, "no keys should be present") + + // When: we init the crypt db + _, err = New(ctx, rawDB, cipher) + require.NoError(t, err) + + // Then: a new key is inserted + keys, err = rawDB.GetDBCryptKeys(ctx) + require.NoError(t, err) + require.Len(t, keys, 1, "one key should be present") + require.Equal(t, cipher.HexDigest(), keys[0].ActiveKeyDigest.String, "key digest mismatch") + require.Empty(t, keys[0].RevokedKeyDigest.String, "key should not be revoked") + requireEncryptedEquals(t, cipher, keys[0].Test, "coder") + }) + + t.Run("MissingKey", func(t *testing.T) { + t.Parallel() + + // Given: there exist two valid encryption keys + cipher1 := initCipher(t) + cipher2 := initCipher(t) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + rawDB, _ := dbtestutil.NewDB(t) + + // Given: key 1 is already present in the database + err := rawDB.InsertDBCryptKey(ctx, database.InsertDBCryptKeyParams{ + Number: 1, + ActiveKeyDigest: cipher1.HexDigest(), + Test: fakeBase64RandomData(t, 32), + }) + require.NoError(t, err, "no error should be returned") + keys, err := rawDB.GetDBCryptKeys(ctx) + require.NoError(t, err, "no error should be returned") + require.Len(t, keys, 1, "one key should be present") + + // When: we init the crypt db with key 2 + _, err = New(ctx, rawDB, cipher2) + + // Then: we error because the key is not revoked and we don't know how to decrypt it + require.Error(t, err) + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr, "expected a decrypt error") + + // When: the existing key is marked as having been revoked + err = rawDB.RevokeDBCryptKey(ctx, cipher1.HexDigest()) + require.NoError(t, err, "no error should be returned") + + // And: we init the crypt db with key 2 + _, err = New(ctx, rawDB, cipher2) + + // Then: we succeed + require.NoError(t, err) + + // And: key 2 should now be the active key + keys, err = rawDB.GetDBCryptKeys(ctx) + require.NoError(t, err) + require.Len(t, keys, 2, "two keys should be present") + require.EqualValues(t, keys[0].Number, 1, "key number mismatch") + require.Empty(t, keys[0].ActiveKeyDigest.String, "key should not be active") + require.Equal(t, cipher1.HexDigest(), keys[0].RevokedKeyDigest.String, "key should be revoked") + + require.EqualValues(t, keys[1].Number, 2, "key number mismatch") + require.Equal(t, cipher2.HexDigest(), keys[1].ActiveKeyDigest.String, "key digest mismatch") + require.Empty(t, keys[1].RevokedKeyDigest.String, "key should not be revoked") + requireEncryptedEquals(t, cipher2, keys[1].Test, "coder") + }) + + t.Run("NoCipher", func(t *testing.T) { + t.Parallel() + // Given: no cipher is loaded + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + rawDB, _ := dbtestutil.NewDB(t) + + keys, err := rawDB.GetDBCryptKeys(ctx) + require.NoError(t, err, "no error should be returned") + require.Empty(t, keys, "no keys should be present") + + // When: we init the crypt db with no ciphers + cs := make([]Cipher, 0) + _, err = New(ctx, rawDB, cs...) + + // Then: an error is returned + require.ErrorContains(t, err, "no ciphers configured") + + // Assert invariant: no keys are inserted + keys, err = rawDB.GetDBCryptKeys(ctx) + require.NoError(t, err, "no error should be returned") + require.Empty(t, keys, "no keys should be present") + }) +} + +func requireEncryptedEquals(t *testing.T, c Cipher, value, expected string) { + t.Helper() + data, err := base64.StdEncoding.DecodeString(value) + require.NoError(t, err, "invalid base64") + got, err := c.Decrypt(data) + require.NoError(t, err, "failed to decrypt data") + require.Equal(t, expected, string(got), "decrypted data does not match") +} + +func initCipher(t *testing.T) *aes256 { + 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 := cipherAES256(key) + require.NoError(t, err) + return c +} + +func setup(t *testing.T) (db, cryptodb database.Store, cs []Cipher) { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + rawDB, _ := dbtestutil.NewDB(t) + + cs = append(cs, initCipher(t)) + cryptDB, err := New(ctx, rawDB, cs...) + require.NoError(t, err) + + return rawDB, cryptDB, cs +} + +func fakeBase64RandomData(t *testing.T, n int) string { + t.Helper() + b := make([]byte, n) + _, err := io.ReadFull(rand.Reader, b) + require.NoError(t, err) + return base64.StdEncoding.EncodeToString(b) +} diff --git a/enterprise/dbcrypt/doc.go b/enterprise/dbcrypt/doc.go new file mode 100644 index 0000000000000..39429f059a200 --- /dev/null +++ b/enterprise/dbcrypt/doc.go @@ -0,0 +1,34 @@ +// Package dbcrypt provides a database.Store wrapper that encrypts/decrypts +// values stored at rest in the database. +// +// Encryption is done using Ciphers, which is an abstraction over a set of +// encryption keys. Each key has a unique identifier, which is used to +// uniquely identify the key whilst maintaining secrecy. +// +// Currently, AES-256-GCM is the only implemented cipher mode. +// The Cipher is currently used to encrypt/decrypt the following fields: +// - database.UserLink.OAuthAccessToken +// - database.UserLink.OAuthRefreshToken +// - database.GitAuthLink.OAuthAccessToken +// - database.GitAuthLink.OAuthRefreshToken +// - database.DBCryptSentinelValue +// +// Multiple ciphers can be provided to support key rotation. The primary cipher +// is used to encrypt and decrypt all data. Secondary ciphers are only used +// for decryption and, as a general rule, should only be active when rotating +// keys. +// +// Encryption keys are stored in the database in the table `dbcrypt_keys`. +// The table has the following schema: +// - number: the key number. This is used to avoid conflicts when rotating keys. +// - created_at: the time the key was created. +// - active_key_digest: the SHA256 digest of the active key. If null, the key has been revoked. +// - revoked_key_digest: the SHA256 digest of the revoked key. If null, the key has not been revoked. +// - revoked_at: the time the key was revoked. If null, the key has not been revoked. +// - test: the encrypted value of the string "coder". This is used to ensure that the key is valid. +// +// Encrypted fields are stored in the database as a base64-encoded string. +// Each encrypted column MUST have a corresponding _key_id column that is a foreign key +// reference to `dbcrypt_keys.active_key_digest`. This ensures that a key cannot be +// revoked until all rows that use that key have been migrated to a new key. +package dbcrypt From 3b8140bb0ca967ebee5ab3f6c457810d03e066ca Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 4 Sep 2023 21:02:28 +0000 Subject: [PATCH 02/34] feat(coderd): plumb through dbcrypt package This builds upon a previous PR. It is recommended to read that first. - Adds a command dbcrypt-rotate to re-enncrypt encrypted data - Plumbs through dbcrypt in enterprise/coderd (including unit tests) - Enables database encryption in develop.sh by default - Adds documentation in admin/encryption.md --- cli/server.go | 4 +- cli/server_createadminuser.go | 2 +- cli/testdata/coder_server_--help.golden | 8 + coderd/apidoc/docs.go | 6 + coderd/apidoc/swagger.json | 6 + coderd/deployment_test.go | 2 + coderd/httpmw/apikey.go | 6 + codersdk/deployment.go | 17 +- codersdk/deployment_test.go | 3 + docs/admin/encryption.md | 129 +++++++++++++ docs/api/general.md | 1 + docs/api/schemas.md | 3 + docs/cli.md | 1 + docs/cli/dbcrypt-rotate.md | 31 +++ docs/cli/server.md | 9 + docs/images/icons/lock.svg | 3 + docs/manifest.json | 12 ++ enterprise/cli/dbcrypt_rotate.go | 152 +++++++++++++++ enterprise/cli/dbcrypt_rotate_slim.go | 20 ++ enterprise/cli/dbcrypt_rotate_test.go | 179 ++++++++++++++++++ enterprise/cli/root.go | 1 + enterprise/cli/server.go | 18 ++ enterprise/cli/testdata/coder_--help.golden | 1 + .../coder_dbcrypt-rotate_--help.golden | 24 +++ .../cli/testdata/coder_server_--help.golden | 8 + enterprise/coderd/coderd.go | 34 +++- enterprise/coderd/coderd_test.go | 26 +-- .../coderd/coderdenttest/coderdenttest.go | 13 +- enterprise/coderd/licenses.go | 4 +- scripts/develop.sh | 3 +- site/src/api/typesGenerated.ts | 4 + 31 files changed, 700 insertions(+), 30 deletions(-) create mode 100644 docs/admin/encryption.md create mode 100644 docs/cli/dbcrypt-rotate.md create mode 100644 docs/images/icons/lock.svg create mode 100644 enterprise/cli/dbcrypt_rotate.go create mode 100644 enterprise/cli/dbcrypt_rotate_slim.go create mode 100644 enterprise/cli/dbcrypt_rotate_test.go create mode 100644 enterprise/cli/testdata/coder_dbcrypt-rotate_--help.golden diff --git a/cli/server.go b/cli/server.go index 683d8aec58fb9..63fca59ec3fec 100644 --- a/cli/server.go +++ b/cli/server.go @@ -691,7 +691,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. options.Database = dbfake.New() options.Pubsub = pubsub.NewInMemory() } else { - sqlDB, err := connectToPostgres(ctx, logger, sqlDriver, vals.PostgresURL.String()) + sqlDB, err := ConnectToPostgres(ctx, logger, sqlDriver, vals.PostgresURL.String()) if err != nil { return xerrors.Errorf("connect to postgres: %w", err) } @@ -1953,7 +1953,7 @@ func BuildLogger(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (slog. }, nil } -func connectToPostgres(ctx context.Context, logger slog.Logger, driver string, dbURL string) (*sql.DB, error) { +func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, dbURL string) (*sql.DB, error) { logger.Debug(ctx, "connecting to postgresql") // Try to connect for 30 seconds. diff --git a/cli/server_createadminuser.go b/cli/server_createadminuser.go index a9f4bfb00b906..fa82e4fbcd051 100644 --- a/cli/server_createadminuser.go +++ b/cli/server_createadminuser.go @@ -63,7 +63,7 @@ func (r *RootCmd) newCreateAdminUserCommand() *clibase.Cmd { newUserDBURL = url } - sqlDB, err := connectToPostgres(ctx, logger, "postgres", newUserDBURL) + sqlDB, err := ConnectToPostgres(ctx, logger, "postgres", newUserDBURL) if err != nil { return xerrors.Errorf("connect to postgres: %w", err) } diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index d3a5d74bcddbe..d8629351985bb 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -458,6 +458,14 @@ These options are only available in the Enterprise Edition. An HTTP URL that is accessible by other replicas to relay DERP traffic. Required for high availability. + --external-token-encryption-keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS + Encrypt OIDC and Git authentication tokens with AES-256-GCM in the + database. The value must be a comma-separated list of base64-encoded + keys. Each key, when base64-decoded, must be exactly 32 bytes in + length. The first key will be used to encrypt new values. Subsequent + keys will be used as a fallback when decrypting. During normal + operation it is recommended to only set one key. + --scim-auth-header string, $CODER_SCIM_AUTH_HEADER Enables SCIM and sets the authentication header for the built-in SCIM server. New users are automatically created with OIDC authentication. diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index bc382c82221e3..2c45b383cc90b 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7956,6 +7956,12 @@ const docTemplate = `{ "type": "string" } }, + "external_token_encryption_keys": { + "type": "array", + "items": { + "type": "string" + } + }, "git_auth": { "$ref": "#/definitions/clibase.Struct-array_codersdk_GitAuthConfig" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 50c68490d8415..21fbd82b348ca 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -7112,6 +7112,12 @@ "type": "string" } }, + "external_token_encryption_keys": { + "type": "array", + "items": { + "type": "string" + } + }, "git_auth": { "$ref": "#/definitions/clibase.Struct-array_codersdk_GitAuthConfig" }, diff --git a/coderd/deployment_test.go b/coderd/deployment_test.go index 617947e6eb607..66e3990e25ff3 100644 --- a/coderd/deployment_test.go +++ b/coderd/deployment_test.go @@ -26,6 +26,7 @@ func TestDeploymentValues(t *testing.T) { cfg.OIDC.EmailField.Set("some_random_field_you_never_expected") cfg.PostgresURL.Set(hi) cfg.SCIMAPIKey.Set(hi) + cfg.ExternalTokenEncryptionKeys.Set("the_random_key_we_never_expected,an_other_key_we_never_unexpected") client := coderdtest.New(t, &coderdtest.Options{ DeploymentValues: cfg, @@ -44,6 +45,7 @@ func TestDeploymentValues(t *testing.T) { require.Empty(t, scrubbed.Values.OIDC.ClientSecret.Value()) require.Empty(t, scrubbed.Values.PostgresURL.Value()) require.Empty(t, scrubbed.Values.SCIMAPIKey.Value()) + require.Empty(t, scrubbed.Values.ExternalTokenEncryptionKeys.Value()) } func TestDeploymentStats(t *testing.T) { diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index 5fbb68f410cda..f2a7a0d3ececf 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -248,6 +248,12 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon UserID: key.UserID, LoginType: key.LoginType, }) + if errors.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/codersdk/deployment.go b/codersdk/deployment.go index 70fa7bbfc483a..27f52f1c340a2 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -46,8 +46,9 @@ const ( FeatureExternalProvisionerDaemons FeatureName = "external_provisioner_daemons" FeatureAppearance FeatureName = "appearance" FeatureAdvancedTemplateScheduling FeatureName = "advanced_template_scheduling" - FeatureTemplateAutostopRequirement FeatureName = "template_autostop_requirement" FeatureWorkspaceProxy FeatureName = "workspace_proxy" + FeatureExternalTokenEncryption FeatureName = "external_token_encryption" + FeatureTemplateAutostopRequirement FeatureName = "template_autostop_requirement" FeatureWorkspaceBatchActions FeatureName = "workspace_batch_actions" ) @@ -65,6 +66,8 @@ var FeatureNames = []FeatureName{ FeatureAdvancedTemplateScheduling, FeatureWorkspaceProxy, FeatureUserRoleManagement, + FeatureExternalTokenEncryption, + FeatureTemplateAutostopRequirement, FeatureWorkspaceBatchActions, } @@ -154,6 +157,7 @@ type DeploymentValues struct { AgentFallbackTroubleshootingURL clibase.URL `json:"agent_fallback_troubleshooting_url,omitempty" typescript:",notnull"` BrowserOnly clibase.Bool `json:"browser_only,omitempty" typescript:",notnull"` SCIMAPIKey clibase.String `json:"scim_api_key,omitempty" typescript:",notnull"` + ExternalTokenEncryptionKeys clibase.StringArray `json:"external_token_encryption_keys,omitempty" typescript:",notnull"` Provisioner ProvisionerConfig `json:"provisioner,omitempty" typescript:",notnull"` RateLimit RateLimitConfig `json:"rate_limit,omitempty" typescript:",notnull"` Experiments clibase.StringArray `json:"experiments,omitempty" typescript:",notnull"` @@ -1605,7 +1609,14 @@ when required by your organization's security policy.`, Annotations: clibase.Annotations{}.Mark(annotationEnterpriseKey, "true").Mark(annotationSecretKey, "true"), Value: &c.SCIMAPIKey, }, - + { + Name: "External Token Encryption Keys", + Description: "Encrypt OIDC and Git authentication tokens with AES-256-GCM in the database. The value must be a comma-separated list of base64-encoded keys. Each key, when base64-decoded, must be exactly 32 bytes in length. The first key will be used to encrypt new values. Subsequent keys will be used as a fallback when decrypting. During normal operation it is recommended to only set one key.", + Flag: "external-token-encryption-keys", + Env: "CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS", + Annotations: clibase.Annotations{}.Mark(annotationEnterpriseKey, "true").Mark(annotationSecretKey, "true"), + Value: &c.ExternalTokenEncryptionKeys, + }, { Name: "Disable Path Apps", Description: "Disable workspace apps that are not served from subdomains. Path-based apps can make requests to the Coder API and pose a security risk when the workspace serves malicious JavaScript. This is recommended for security purposes if a --wildcard-access-url is configured.", @@ -1783,7 +1794,7 @@ func (c *DeploymentValues) WithoutSecrets() (*DeploymentValues, error) { // This only works with string values for now. switch v := opt.Value.(type) { - case *clibase.String: + case *clibase.String, *clibase.StringArray: err := v.Set("") if err != nil { panic(err) diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 408aa4fd21ae5..287e34c741226 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -57,6 +57,9 @@ func TestDeploymentValues_HighlyConfigurable(t *testing.T) { "SCIM API Key": { yaml: true, }, + "External Token Encryption Keys": { + yaml: true, + }, // These complex objects should be configured through YAML. "Support Links": { flag: true, diff --git a/docs/admin/encryption.md b/docs/admin/encryption.md new file mode 100644 index 0000000000000..15a15ebb00b67 --- /dev/null +++ b/docs/admin/encryption.md @@ -0,0 +1,129 @@ +# Database Encryption + +By default, Coder stores external user tokens in plaintext in the database. +Database Encryption allows Coder administrators to encrypt these tokens at-rest, +preventing attackers with database access from using them to impersonate users. + +## How it works + +Coder allows administrators to specify up to two +[external token encryption keys](../cli/server.md#external-token-encryption-keys). +If configured, Coder will use these keys to encrypt external user tokens before +storing them in the database. The encryption algorithm used is AES-256-GCM with +a 32-byte key length. + +Coder will use the first key provided for both encryption and decryption. If a +second key is provided, Coder will use it for decryption only. This allows +administrators to rotate encryption keys without invalidating existing tokens. + +The following database fields are currently encrypted: + +- `user_links.oauth_access_token` +- `user_links.oauth_refresh_token` +- `git_auth_links.oauth_access_token` +- `git_auth_links.oauth_refresh_token` + +Additional database fields may be encrypted in the future. + +> Implementation notes: each encrypted database column `$C` has a corresponding +> `$C_key_id` column. This column is used to determine which encryption key was +> used to encrypt the data. This allows Coder to rotate encryption keys without +> invalidating existing tokens, and provides referential integrity for encrypted +> data. +> +> The `$C_key_id` column stores the first 7 bytes of the SHA-256 hash of the +> encryption key used to encrypt the data. +> +> Encryption keys in use are stored in `dbcrypt_keys`. This table stores a +> record of all encryption keys that have been used to encrypt data. Active keys +> have a null `revoked_key_id` column, and revoked keys have a non-null +> `revoked_key_id` column. A key cannot be revoked until all rows referring to +> it have been re-encrypted with a different key. + +## Enabling encryption + +1. Ensure you have a valid backup of your database. **Do not skip this step.** + If you are using the built-in PostgreSQL database, you can run + [`coder server postgres-builtin-url`](../cli/server_postgres-builtin-url.md) + to get the connection URL. + +1. Generate a 32-byte random key and base64-encode it. For example: + +```shell +dd if=/dev/urandom bs=32 count=1 | base64 +``` + +1. Store this key in a secure location (for example, a Kubernetes secret): + +```shell +kubectl create secret generic coder-external-token-encryption-keys --from-literal=keys= +``` + +1. In your Coder configuration set `CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS` to a + comma-separated list of base64-encoded keys. For example, in your Helm + `values.yaml`: + +```yaml +coder: + env: + [...] + - name: CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS + valueFrom: + secretKeyRef: + name: coder-external-token-encryption-keys + key: keys +``` + +## Rotating keys + +We recommend only having one active encryption key at a time normally. However, +if you need to rotate keys, you can perform the following procedure: + +1. Ensure you have a valid backup of your database. **Do not skip this step.** + +1. Generate a new encryption key following the same procedure as above. + +1. Add the above key to the list of + [external token encryption keys](../cli/server.md#external-token-encryption-keys). + **The new key must appear first in the list**. For example, in the Kubernetes + secret created above: + +```yaml +apiVersion: v1 +kind: Secret +type: Opaque +metadata: + name: coder-external-token-encryption-keys + namespace: coder-namespace +data: + keys: ,,,... +``` + +1. After updating the configuration, restart the Coder server. The server will + now encrypt all new data with the new key, but will be able to decrypt tokens + encrypted with the old key(s). + +1. To re-encrypt all encrypted database fields with the new key, run + [`coder dbcrypt-rotate`](../cli/dbcrypt-rotate.md). This command will + re-encrypt all tokens with the first key in the list of external token + encryption keys. We recommend performing this action during a maintenance + window. + + > Note: this command requires direct access to the database. If you are using + > the built-in PostgreSQL database, you can run + > [`coder server postgres-builtin-url`](../cli/server_postgres-builtin-url.md) + > to get the connection URL. + +1. Once the above command completes successfully, remove the old encryption key + from Coder's configuration and restart Coder once more. You can now safely + delete the old key from your secret store. + +## Disabling encryption + +Disabling encryption is currently not supported. + +## Troubleshooting + +- If Coder detects that the data stored in the database was not encrypted with + any known keys, it will refuse to start. If you are seeing this behaviour, + ensure that the encryption keys provided are correct. diff --git a/docs/api/general.md b/docs/api/general.md index b50771e16d2ad..ded8e5df4d319 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -212,6 +212,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ }, "enable_terraform_debug_mode": true, "experiments": ["string"], + "external_token_encryption_keys": ["string"], "git_auth": { "value": [ { diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 31d23faad36f6..4cf60a773f81d 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -2036,6 +2036,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in }, "enable_terraform_debug_mode": true, "experiments": ["string"], + "external_token_encryption_keys": ["string"], "git_auth": { "value": [ { @@ -2400,6 +2401,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in }, "enable_terraform_debug_mode": true, "experiments": ["string"], + "external_token_encryption_keys": ["string"], "git_auth": { "value": [ { @@ -2613,6 +2615,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `docs_url` | [clibase.URL](#clibaseurl) | false | | | | `enable_terraform_debug_mode` | boolean | false | | | | `experiments` | array of string | false | | | +| `external_token_encryption_keys` | array of string | false | | | | `git_auth` | [clibase.Struct-array_codersdk_GitAuthConfig](#clibasestruct-array_codersdk_gitauthconfig) | false | | | | `http_address` | string | false | | Http address is a string because it may be set to zero to disable. | | `in_memory_database` | boolean | false | | | diff --git a/docs/cli.md b/docs/cli.md index c9ffdc7c46421..40e5a9430f6fb 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -27,6 +27,7 @@ Coder — A tool for provisioning self-hosted development environments with Terr | ------------------------------------------------------ | ----------------------------------------------------------------------------------------------------- | | [config-ssh](./cli/config-ssh.md) | Add an SSH Host entry for your workspaces "ssh coder.workspace" | | [create](./cli/create.md) | Create a workspace | +| [dbcrypt-rotate](./cli/dbcrypt-rotate.md) | Rotate database encryption keys | | [delete](./cli/delete.md) | Delete a workspace | | [dotfiles](./cli/dotfiles.md) | Personalize your workspace by applying a canonical dotfiles repository | | [features](./cli/features.md) | List Enterprise features | diff --git a/docs/cli/dbcrypt-rotate.md b/docs/cli/dbcrypt-rotate.md new file mode 100644 index 0000000000000..c3d5a34c9b98f --- /dev/null +++ b/docs/cli/dbcrypt-rotate.md @@ -0,0 +1,31 @@ + + +# dbcrypt-rotate + +Rotate database encryption keys + +## Usage + +```console +coder dbcrypt-rotate [flags] --postgres-url --external-token-encryption-keys , +``` + +## Options + +### --external-token-encryption-keys + +| | | +| ----------- | -------------------------------------------------- | +| Type | string-array | +| Environment | $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS | + +Encrypt OIDC and Git authentication tokens with AES-256-GCM in the database. The value must be a comma-separated list of base64-encoded keys. Each key, when base64-decoded, must be exactly 32 bytes in length. The first key will be used to encrypt new values. Subsequent keys will be used as a fallback when decrypting. During normal operation it is recommended to only set one key. + +### --postgres-url + +| | | +| ----------- | ------------------------------------- | +| Type | string | +| Environment | $CODER_PG_CONNECTION_URL | + +URL of a PostgreSQL database. If empty, PostgreSQL binaries will be downloaded from Maven (https://repo1.maven.org/maven2) and store all data in the config root. Access the built-in database with "coder server postgres-builtin-url". diff --git a/docs/cli/server.md b/docs/cli/server.md index 49ba37d7a4236..2e56bfa959ad6 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -273,6 +273,15 @@ Expose the swagger endpoint via /swagger. Enable one or more experiments. These are not ready for production. Separate multiple experiments with commas, or enter '\*' to opt-in to all available experiments. +### --external-token-encryption-keys + +| | | +| ----------- | -------------------------------------------------- | +| Type | string-array | +| Environment | $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS | + +Encrypt OIDC and Git authentication tokens with AES-256-GCM in the database. The value must be a comma-separated list of base64-encoded keys. Each key, when base64-decoded, must be exactly 32 bytes in length. The first key will be used to encrypt new values. Subsequent keys will be used as a fallback when decrypting. During normal operation it is recommended to only set one key. + ### --provisioner-force-cancel-interval | | | diff --git a/docs/images/icons/lock.svg b/docs/images/icons/lock.svg new file mode 100644 index 0000000000000..620af5152163c --- /dev/null +++ b/docs/images/icons/lock.svg @@ -0,0 +1,3 @@ + + + diff --git a/docs/manifest.json b/docs/manifest.json index 2ad15b2a13402..cbd37d12f1e50 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -390,6 +390,13 @@ "description": "Learn what usage telemetry Coder collects", "path": "./admin/telemetry.md", "icon_path": "./images/icons/science.svg" + }, + { + "title": "Database Encryption", + "description": "Learn how to encrypt sensitive data at rest in Coder", + "path": "./admin/database-encryption.md", + "icon_path": "./images/icons/lock.svg", + "state": "enterprise" } ] }, @@ -540,6 +547,11 @@ "description": "Create a workspace", "path": "cli/create.md" }, + { + "title": "dbcrypt-rotate", + "description": "Rotate database encryption keys", + "path": "cli/dbcrypt-rotate.md" + }, { "title": "delete", "description": "Delete a workspace", diff --git a/enterprise/cli/dbcrypt_rotate.go b/enterprise/cli/dbcrypt_rotate.go new file mode 100644 index 0000000000000..f145a65f73d32 --- /dev/null +++ b/enterprise/cli/dbcrypt_rotate.go @@ -0,0 +1,152 @@ +//go:build !slim + +package cli + +import ( + "bytes" + "context" + "database/sql" + "encoding/base64" + + "cdr.dev/slog" + "cdr.dev/slog/sloggers/sloghuman" + "github.com/coder/coder/v2/cli" + "github.com/coder/coder/v2/cli/clibase" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/dbcrypt" + + "golang.org/x/xerrors" +) + +func (*RootCmd) dbcryptRotate() *clibase.Cmd { + var ( + vals = new(codersdk.DeploymentValues) + opts = vals.Options() + ) + cmd := &clibase.Cmd{ + Use: "dbcrypt-rotate --postgres-url --external-token-encryption-keys ,", + Short: "Rotate database encryption keys", + Options: clibase.OptionSet{ + *opts.ByName("Postgres Connection URL"), + *opts.ByName("External Token Encryption Keys"), + }, + Middleware: clibase.Chain( + clibase.RequireNArgs(0), + ), + Handler: func(inv *clibase.Invocation) error { + ctx, cancel := context.WithCancel(inv.Context()) + defer cancel() + logger := slog.Make(sloghuman.Sink(inv.Stdout)) + if ok, _ := inv.ParsedFlags().GetBool("verbose"); ok { + logger = logger.Leveled(slog.LevelDebug) + } + + if vals.PostgresURL == "" { + return xerrors.Errorf("no database configured") + } + + switch len(vals.ExternalTokenEncryptionKeys) { + case 0: + return xerrors.Errorf("no external token encryption keys provided") + case 1: + logger.Info(ctx, "only one key provided, data will be re-encrypted with the same key") + } + + keys := make([][]byte, 0, len(vals.ExternalTokenEncryptionKeys)) + var newKey []byte + for idx, ek := range vals.ExternalTokenEncryptionKeys { + dk, err := base64.StdEncoding.DecodeString(ek) + if err != nil { + return xerrors.Errorf("key must be base64-encoded") + } + if idx == 0 { + newKey = dk + } else if bytes.Equal(dk, newKey) { + return xerrors.Errorf("old key at index %d is the same as the new key", idx) + } + keys = append(keys, dk) + } + + ciphers, err := dbcrypt.NewCiphers(keys...) + if err != nil { + return xerrors.Errorf("create ciphers: %w", err) + } + + sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", vals.PostgresURL.Value()) + if err != nil { + return xerrors.Errorf("connect to postgres: %w", err) + } + defer func() { + _ = sqlDB.Close() + }() + logger.Info(ctx, "connected to postgres") + + db := database.New(sqlDB) + + cryptDB, err := dbcrypt.New(ctx, db, ciphers...) + if err != nil { + return xerrors.Errorf("create cryptdb: %w", err) + } + + users, err := cryptDB.GetUsers(ctx, database.GetUsersParams{}) + if err != nil { + return xerrors.Errorf("get users: %w", err) + } + logger.Info(ctx, "encrypting user tokens", slog.F("count", len(users))) + for idx, usr := range users { + err := cryptDB.InTx(func(tx database.Store) error { + userLinks, err := tx.GetUserLinksByUserID(ctx, usr.ID) + if err != nil { + return xerrors.Errorf("get user links for user: %w", err) + } + for _, userLink := range userLinks { + if _, err := tx.UpdateUserLink(ctx, database.UpdateUserLinkParams{ + OAuthAccessToken: userLink.OAuthAccessToken, + OAuthRefreshToken: userLink.OAuthRefreshToken, + OAuthExpiry: userLink.OAuthExpiry, + UserID: usr.ID, + LoginType: usr.LoginType, + }); err != nil { + return xerrors.Errorf("update user link: %w", err) + } + } + gitAuthLinks, err := tx.GetGitAuthLinksByUserID(ctx, usr.ID) + if err != nil { + return xerrors.Errorf("get git auth links for user: %w", err) + } + for _, gitAuthLink := range gitAuthLinks { + if _, err := tx.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ + ProviderID: gitAuthLink.ProviderID, + UserID: usr.ID, + UpdatedAt: gitAuthLink.UpdatedAt, + OAuthAccessToken: gitAuthLink.OAuthAccessToken, + OAuthRefreshToken: gitAuthLink.OAuthRefreshToken, + OAuthExpiry: gitAuthLink.OAuthExpiry, + }); err != nil { + return xerrors.Errorf("update git auth link: %w", err) + } + } + return nil + }, &sql.TxOptions{ + Isolation: sql.LevelRepeatableRead, + }) + if err != nil { + return xerrors.Errorf("update user links: %w", err) + } + logger.Debug(ctx, "encrypted user tokens", slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + } + logger.Info(ctx, "operation completed successfully") + + // Revoke old keys + for _, c := range ciphers[1:] { + if err := db.RevokeDBCryptKey(ctx, c.HexDigest()); err != nil { + return xerrors.Errorf("revoke key: %w", err) + } + logger.Info(ctx, "revoked unused key", slog.F("digest", c.HexDigest())) + } + return nil + }, + } + return cmd +} diff --git a/enterprise/cli/dbcrypt_rotate_slim.go b/enterprise/cli/dbcrypt_rotate_slim.go new file mode 100644 index 0000000000000..63c5bb9205ee0 --- /dev/null +++ b/enterprise/cli/dbcrypt_rotate_slim.go @@ -0,0 +1,20 @@ +//go:build slim + +package cli + +import ( + "github.com/coder/coder/v2/cli/clibase" + "golang.org/x/xerrors" +) + +func (*RootCmd) dbcryptRotate() *clibase.Cmd { + return &clibase.Cmd{ + Use: "dbcrypt-rotate --postgres-url --external-token-encryption-keys ,", + Short: "Rotate database encryption keys", + Options: clibase.OptionSet{}, + Hidden: true, + Handler: func(inv *clibase.Invocation) error { + return xerrors.Errorf("slim build does not support `coder dbcrypt-rotate`") + }, + } +} diff --git a/enterprise/cli/dbcrypt_rotate_test.go b/enterprise/cli/dbcrypt_rotate_test.go new file mode 100644 index 0000000000000..6faaf2f8775c1 --- /dev/null +++ b/enterprise/cli/dbcrypt_rotate_test.go @@ -0,0 +1,179 @@ +package cli_test + +import ( + "context" + "database/sql" + "encoding/base64" + "fmt" + "testing" + + "github.com/lib/pq" + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/database/postgres" + "github.com/coder/coder/v2/cryptorand" + "github.com/coder/coder/v2/enterprise/dbcrypt" + "github.com/coder/coder/v2/pty/ptytest" +) + +// nolint: paralleltest // use of t.Setenv +func TestDBCryptRotate(t *testing.T) { + if !dbtestutil.WillUsePostgres() { + t.Skip("this test requires a postgres instance") + } + // + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + // + // Setup a postgres database. + connectionURL, closePg, err := postgres.Open() + require.NoError(t, err) + t.Cleanup(closePg) + // + sqlDB, err := sql.Open("postgres", connectionURL) + require.NoError(t, err) + t.Cleanup(func() { + _ = sqlDB.Close() + }) + db := database.New(sqlDB) + + // Populate the database with some unencrypted data. + users := genData(t, db, 10) + + // Setup an initial cipher + keyA := mustString(t, 32) + cipherA, err := dbcrypt.NewCiphers([]byte(keyA)) + require.NoError(t, err) + + // Encrypt all the data with the initial cipher. + inv, _ := newCLI(t, "dbcrypt-rotate", + "--postgres-url", connectionURL, + "--external-token-encryption-keys", base64.StdEncoding.EncodeToString([]byte(keyA)), + ) + pty := ptytest.New(t) + inv.Stdout = pty.Output() + err = inv.Run() + require.NoError(t, err) + + // Validate that all existing data has been encrypted with cipher A. + requireDataDecryptsWithCipher(ctx, t, db, cipherA[0], users) + + // Create an encrypted database + cryptdb, err := dbcrypt.New(ctx, db, cipherA...) + require.NoError(t, err) + + // Populate the database with some encrypted data using cipher A. + users = append(users, genData(t, cryptdb, 10)...) + + // Re-encrypt all existing data with a new cipher. + keyB := mustString(t, 32) + cipherBA, err := dbcrypt.NewCiphers([]byte(keyB), []byte(keyA)) + require.NoError(t, err) + externalTokensArg := fmt.Sprintf( + "%s,%s", + base64.StdEncoding.EncodeToString([]byte(keyB)), + base64.StdEncoding.EncodeToString([]byte(keyA)), + ) + + inv, _ = newCLI(t, "dbcrypt-rotate", + "--postgres-url", connectionURL, + "--external-token-encryption-keys", externalTokensArg, + ) + pty = ptytest.New(t) + inv.Stdout = pty.Output() + err = inv.Run() + require.NoError(t, err) + + // Validate that all data has been re-encrypted with cipher B. + requireDataDecryptsWithCipher(ctx, t, db, cipherBA[0], users) + + // Assert that we can revoke the old key. + err = db.RevokeDBCryptKey(ctx, cipherA[0].HexDigest()) + require.NoError(t, err, "failed to revoke old key") + + // Assert that the key has been revoked in the database. + keys, err := db.GetDBCryptKeys(ctx) + oldKey := keys[0] // ORDER BY number ASC; + newKey := keys[1] + require.NoError(t, err, "failed to get db crypt keys") + require.Len(t, keys, 2, "expected exactly 2 keys") + require.Equal(t, cipherBA[0].HexDigest(), newKey.ActiveKeyDigest.String, "expected the new key to be the active key") + require.Empty(t, newKey.RevokedKeyDigest.String, "expected the new key to not be revoked") + require.Equal(t, cipherBA[1].HexDigest(), oldKey.RevokedKeyDigest.String, "expected the old key to be revoked") + require.Empty(t, oldKey.ActiveKeyDigest.String, "expected the old key to not be active") + + // Revoking the new key should fail. + err = db.RevokeDBCryptKey(ctx, cipherBA[0].HexDigest()) + require.Error(t, err, "expected to fail to revoke the new key") + var pgErr *pq.Error + require.True(t, xerrors.As(err, &pgErr), "expected a pg error") + require.EqualValues(t, "23503", pgErr.Code, "expected a foreign key constraint violation error") +} + +func genData(t *testing.T, db database.Store, n int) []database.User { + t.Helper() + var users []database.User + for i := 0; i < n; i++ { + usr := dbgen.User(t, db, database.User{ + LoginType: database.LoginTypeOIDC, + }) + _ = dbgen.UserLink(t, db, database.UserLink{ + UserID: usr.ID, + LoginType: usr.LoginType, + OAuthAccessToken: mustString(t, 16), + OAuthRefreshToken: mustString(t, 16), + }) + _ = dbgen.GitAuthLink(t, db, database.GitAuthLink{ + UserID: usr.ID, + ProviderID: "fake", + OAuthAccessToken: mustString(t, 16), + OAuthRefreshToken: mustString(t, 16), + }) + users = append(users, usr) + } + return users +} + +func mustString(t *testing.T, n int) string { + t.Helper() + s, err := cryptorand.String(n) + require.NoError(t, err) + return s +} + +func requireDecryptWithCipher(t *testing.T, c dbcrypt.Cipher, s string) { + t.Helper() + decodedVal, err := base64.StdEncoding.DecodeString(s) + require.NoError(t, err, "failed to decode base64 string") + _, err = c.Decrypt(decodedVal) + require.NoError(t, err, "failed to decrypt value") +} + +func requireDataDecryptsWithCipher(ctx context.Context, t *testing.T, db database.Store, c dbcrypt.Cipher, users []database.User) { + t.Helper() + for _, usr := range users { + ul, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ + UserID: usr.ID, + LoginType: usr.LoginType, + }) + require.NoError(t, err, "failed to get user link for user %s", usr.ID) + requireDecryptWithCipher(t, c, ul.OAuthAccessToken) + requireDecryptWithCipher(t, c, ul.OAuthRefreshToken) + require.Equal(t, c.HexDigest(), ul.OAuthAccessTokenKeyID.String) + require.Equal(t, c.HexDigest(), ul.OAuthRefreshTokenKeyID.String) + // + gal, err := db.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ + UserID: usr.ID, + ProviderID: "fake", + }) + require.NoError(t, err, "failed to get git auth link for user %s", usr.ID) + requireDecryptWithCipher(t, c, gal.OAuthAccessToken) + requireDecryptWithCipher(t, c, gal.OAuthRefreshToken) + require.Equal(t, c.HexDigest(), gal.OAuthAccessTokenKeyID.String) + require.Equal(t, c.HexDigest(), gal.OAuthRefreshTokenKeyID.String) + } +} diff --git a/enterprise/cli/root.go b/enterprise/cli/root.go index 360582a3e5193..6f7887518c31b 100644 --- a/enterprise/cli/root.go +++ b/enterprise/cli/root.go @@ -17,6 +17,7 @@ func (r *RootCmd) enterpriseOnly() []*clibase.Cmd { r.licenses(), r.groups(), r.provisionerDaemons(), + r.dbcryptRotate(), } } diff --git a/enterprise/cli/server.go b/enterprise/cli/server.go index 021b66f2f7a60..0cd5d8e1ffc97 100644 --- a/enterprise/cli/server.go +++ b/enterprise/cli/server.go @@ -5,6 +5,7 @@ package cli import ( "context" "database/sql" + "encoding/base64" "errors" "io" "net/url" @@ -19,6 +20,7 @@ import ( "github.com/coder/coder/v2/enterprise/audit/backends" "github.com/coder/coder/v2/enterprise/coderd" "github.com/coder/coder/v2/enterprise/coderd/dormancy" + "github.com/coder/coder/v2/enterprise/dbcrypt" "github.com/coder/coder/v2/enterprise/trialer" "github.com/coder/coder/v2/tailnet" @@ -74,6 +76,22 @@ func (r *RootCmd) Server(_ func()) *clibase.Cmd { CheckInactiveUsersCancelFunc: dormancy.CheckInactiveUsers(ctx, options.Logger, options.Database), } + if encKeys := options.DeploymentValues.ExternalTokenEncryptionKeys.Value(); len(encKeys) != 0 { + keys := make([][]byte, 0, len(encKeys)) + for idx, ek := range encKeys { + dk, err := base64.StdEncoding.DecodeString(ek) + if err != nil { + return nil, nil, xerrors.Errorf("decode external-token-encryption-key %d: %w", idx, err) + } + keys = append(keys, dk) + } + cs, err := dbcrypt.NewCiphers(keys...) + if err != nil { + return nil, nil, xerrors.Errorf("initialize encryption: %w", err) + } + o.ExternalTokenEncryption = cs + } + api, err := coderd.New(ctx, o) if err != nil { return nil, nil, err diff --git a/enterprise/cli/testdata/coder_--help.golden b/enterprise/cli/testdata/coder_--help.golden index ae24592079a69..de2b9c5f9e7d9 100644 --- a/enterprise/cli/testdata/coder_--help.golden +++ b/enterprise/cli/testdata/coder_--help.golden @@ -10,6 +10,7 @@ Coder v0.0.0-devel — A tool for provisioning self-hosted development environme  $ coder templates init  Subcommands + dbcrypt-rotate Rotate database encryption keys features List Enterprise features groups Manage groups licenses Add, delete, and list licenses diff --git a/enterprise/cli/testdata/coder_dbcrypt-rotate_--help.golden b/enterprise/cli/testdata/coder_dbcrypt-rotate_--help.golden new file mode 100644 index 0000000000000..bf3dae88c08bc --- /dev/null +++ b/enterprise/cli/testdata/coder_dbcrypt-rotate_--help.golden @@ -0,0 +1,24 @@ +Usage: coder dbcrypt-rotate [flags] --postgres-url --external-token-encryption-keys , + +Rotate database encryption keys + +Options + --postgres-url string, $CODER_PG_CONNECTION_URL + URL of a PostgreSQL database. If empty, PostgreSQL binaries will be + downloaded from Maven (https://repo1.maven.org/maven2) and store all + data in the config root. Access the built-in database with "coder + server postgres-builtin-url". + +Enterprise Options +These options are only available in the Enterprise Edition. + + --external-token-encryption-keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS + Encrypt OIDC and Git authentication tokens with AES-256-GCM in the + database. The value must be a comma-separated list of base64-encoded + keys. Each key, when base64-decoded, must be exactly 32 bytes in + length. The first key will be used to encrypt new values. Subsequent + keys will be used as a fallback when decrypting. During normal + operation it is recommended to only set one key. + +--- +Run `coder --help` for a list of global options. diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index d3a5d74bcddbe..d8629351985bb 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -458,6 +458,14 @@ These options are only available in the Enterprise Edition. An HTTP URL that is accessible by other replicas to relay DERP traffic. Required for high availability. + --external-token-encryption-keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS + Encrypt OIDC and Git authentication tokens with AES-256-GCM in the + database. The value must be a comma-separated list of base64-encoded + keys. Each key, when base64-decoded, must be exactly 32 bytes in + length. The first key will be used to encrypt new values. Subsequent + keys will be used as a fallback when decrypting. During normal + operation it is recommended to only set one key. + --scim-auth-header string, $CODER_SCIM_AUTH_HEADER Enables SCIM and sets the authentication header for the built-in SCIM server. New users are automatically created with OIDC authentication. diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 1bc0815cd5248..2e525ddf13dd8 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -33,6 +33,7 @@ import ( "github.com/coder/coder/v2/enterprise/coderd/license" "github.com/coder/coder/v2/enterprise/coderd/proxyhealth" "github.com/coder/coder/v2/enterprise/coderd/schedule" + "github.com/coder/coder/v2/enterprise/dbcrypt" "github.com/coder/coder/v2/enterprise/derpmesh" "github.com/coder/coder/v2/enterprise/replicasync" "github.com/coder/coder/v2/enterprise/tailnet" @@ -47,8 +48,8 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { if options.EntitlementsUpdateInterval == 0 { options.EntitlementsUpdateInterval = 10 * time.Minute } - if options.Keys == nil { - options.Keys = Keys + if options.LicenseKeys == nil { + options.LicenseKeys = Keys } if options.Options == nil { options.Options = &coderd.Options{} @@ -61,10 +62,26 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { } ctx, cancelFunc := context.WithCancel(ctx) - api := &API{ - ctx: ctx, - cancel: cancelFunc, + if options.ExternalTokenEncryption != nil { + cryptDB, err := dbcrypt.New(ctx, options.Database, options.ExternalTokenEncryption...) + if err != nil { + cancelFunc() + // If we fail to initialize the database, it's likely that the + // database is encrypted with an unknown external token encryption key. + // This is a fatal error. + var derr *dbcrypt.DecryptFailedError + if xerrors.As(err, &derr) { + panic(`Coder has shut down to prevent data corruption: your configured database is encrypted with an unknown external token encryption key. Please check your configuration and try again.`) + } + return nil, xerrors.Errorf("init dbcrypt: %w", err) + } + options.Database = cryptDB + } + + api := &API{ + ctx: ctx, + cancel: cancelFunc, AGPL: coderd.New(options.Options), Options: options, provisionerDaemonAuth: &provisionerDaemonAuth{ @@ -364,6 +381,8 @@ type Options struct { BrowserOnly bool SCIMAPIKey []byte + ExternalTokenEncryption []dbcrypt.Cipher + // Used for high availability. ReplicaSyncUpdateInterval time.Duration DERPServerRelayAddress string @@ -374,7 +393,7 @@ type Options struct { EntitlementsUpdateInterval time.Duration ProxyHealthInterval time.Duration - Keys map[string]ed25519.PublicKey + LicenseKeys map[string]ed25519.PublicKey // optional pre-shared key for authentication of external provisioner daemons ProvisionerDaemonPSK string @@ -429,13 +448,14 @@ func (api *API) updateEntitlements(ctx context.Context) error { entitlements, err := license.Entitlements( ctx, api.Database, - api.Logger, len(api.replicaManager.AllPrimary()), len(api.GitAuthConfigs), api.Keys, map[codersdk.FeatureName]bool{ + api.Logger, len(api.replicaManager.AllPrimary()), len(api.GitAuthConfigs), api.LicenseKeys, map[codersdk.FeatureName]bool{ codersdk.FeatureAuditLog: api.AuditLogging, codersdk.FeatureBrowserOnly: api.BrowserOnly, codersdk.FeatureSCIM: len(api.SCIMAPIKey) != 0, codersdk.FeatureHighAvailability: api.DERPServerRelayAddress != "", codersdk.FeatureMultipleGitAuth: len(api.GitAuthConfigs) > 1, codersdk.FeatureTemplateRBAC: api.RBAC, + codersdk.FeatureExternalTokenEncryption: api.ExternalTokenEncryption != nil, codersdk.FeatureExternalProvisionerDaemons: true, codersdk.FeatureAdvancedTemplateScheduling: true, // FeatureTemplateAutostopRequirement depends on diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index d34d146a7079f..9fcb748bf9ca0 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -48,25 +48,27 @@ func TestEntitlements(t *testing.T) { AuditLogging: true, DontAddLicense: true, }) + // Enable all features + features := make(license.Features) + for _, feature := range codersdk.FeatureNames { + features[feature] = 1 + } + features[codersdk.FeatureUserLimit] = 100 coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ - Features: license.Features{ - codersdk.FeatureUserLimit: 100, - codersdk.FeatureAuditLog: 1, - codersdk.FeatureTemplateRBAC: 1, - codersdk.FeatureExternalProvisionerDaemons: 1, - codersdk.FeatureAdvancedTemplateScheduling: 1, - codersdk.FeatureWorkspaceProxy: 1, - codersdk.FeatureUserRoleManagement: 1, - }, - GraceAt: time.Now().Add(59 * 24 * time.Hour), + Features: features, + GraceAt: time.Now().Add(59 * 24 * time.Hour), }) res, err := client.Entitlements(context.Background()) require.NoError(t, err) assert.True(t, res.HasLicense) ul := res.Features[codersdk.FeatureUserLimit] assert.Equal(t, codersdk.EntitlementEntitled, ul.Entitlement) - assert.Equal(t, int64(100), *ul.Limit) - assert.Equal(t, int64(1), *ul.Actual) + if assert.NotNil(t, ul.Limit) { + assert.Equal(t, int64(100), *ul.Limit) + } + if assert.NotNil(t, ul.Actual) { + assert.Equal(t, int64(1), *ul.Actual) + } assert.True(t, ul.Enabled) al := res.Features[codersdk.FeatureAuditLog] assert.Equal(t, codersdk.EntitlementEntitled, al.Entitlement) diff --git a/enterprise/coderd/coderdenttest/coderdenttest.go b/enterprise/coderd/coderdenttest/coderdenttest.go index 81e43c4fd5755..38b92e35b1346 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest.go +++ b/enterprise/coderd/coderdenttest/coderdenttest.go @@ -21,10 +21,12 @@ import ( "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd" "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/enterprise/dbcrypt" ) const ( - testKeyID = "enterprise-test" + testKeyID = "enterprise-test" + testEncryptionKey = "coder-coder-coder-coder-coder-1!" // nolint:gosec ) var ( @@ -56,6 +58,7 @@ type Options struct { DontAddLicense bool DontAddFirstUser bool ReplicaSyncUpdateInterval time.Duration + ExternalTokenEncryption []dbcrypt.Cipher ProvisionerDaemonPSK string } @@ -82,6 +85,11 @@ func NewWithAPI(t *testing.T, options *Options) ( err := oop.DeploymentValues.UserQuietHoursSchedule.DefaultSchedule.Set("0 0 * * *") require.NoError(t, err) } + if options.ExternalTokenEncryption == nil { + c, err := dbcrypt.NewCiphers([]byte(testEncryptionKey)) + require.NoError(t, err) + options.ExternalTokenEncryption = c + } coderAPI, err := coderd.New(context.Background(), &coderd.Options{ RBAC: true, AuditLogging: options.AuditLogging, @@ -92,10 +100,11 @@ func NewWithAPI(t *testing.T, options *Options) ( ReplicaSyncUpdateInterval: options.ReplicaSyncUpdateInterval, Options: oop, EntitlementsUpdateInterval: options.EntitlementsUpdateInterval, - Keys: Keys, + LicenseKeys: Keys, ProxyHealthInterval: options.ProxyHealthInterval, DefaultQuietHoursSchedule: oop.DeploymentValues.UserQuietHoursSchedule.DefaultSchedule.Value(), ProvisionerDaemonPSK: options.ProvisionerDaemonPSK, + ExternalTokenEncryption: options.ExternalTokenEncryption, }) require.NoError(t, err) setHandler(coderAPI.AGPL.RootHandler) diff --git a/enterprise/coderd/licenses.go b/enterprise/coderd/licenses.go index 9796d21937b35..b7c7b5af6e4f0 100644 --- a/enterprise/coderd/licenses.go +++ b/enterprise/coderd/licenses.go @@ -84,7 +84,7 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) { return } - rawClaims, err := license.ParseRaw(addLicense.License, api.Keys) + rawClaims, err := license.ParseRaw(addLicense.License, api.LicenseKeys) if err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid license", @@ -102,7 +102,7 @@ func (api *API) postLicense(rw http.ResponseWriter, r *http.Request) { } expTime := time.Unix(int64(exp), 0) - claims, err := license.ParseClaims(addLicense.License, api.Keys) + claims, err := license.ParseClaims(addLicense.License, api.LicenseKeys) if err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid license", diff --git a/scripts/develop.sh b/scripts/develop.sh index 39f81c2951bc4..327f2192ce2c4 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -15,6 +15,7 @@ set -euo pipefail CODER_DEV_ACCESS_URL="${CODER_DEV_ACCESS_URL:-http://127.0.0.1:3000}" DEFAULT_PASSWORD="SomeSecurePassword!" +EXTERNAL_TOKEN_ENCRYPTION_KEYS="Y29kZXItY29kZXItY29kZXItY29kZXItY29kZXItMSE=" password="${CODER_DEV_ADMIN_PASSWORD:-${DEFAULT_PASSWORD}}" use_proxy=0 @@ -136,7 +137,7 @@ fatal() { trap 'fatal "Script encountered an error"' ERR cdroot - start_cmd API "" "${CODER_DEV_SHIM}" server --http-address 0.0.0.0:3000 --swagger-enable --access-url "${CODER_DEV_ACCESS_URL}" --dangerous-allow-cors-requests=true "$@" + start_cmd API "" "${CODER_DEV_SHIM}" server --http-address 0.0.0.0:3000 --swagger-enable --access-url "${CODER_DEV_ACCESS_URL}" --dangerous-allow-cors-requests=true --external-token-encryption-keys="${EXTERNAL_TOKEN_ENCRYPTION_KEYS}" "$@" echo '== Waiting for Coder to become ready' # Start the timeout in the background so interrupting this script diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 41239d8bd984f..b18e1c49d0137 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -379,6 +379,8 @@ export interface DeploymentValues { readonly agent_fallback_troubleshooting_url?: string readonly browser_only?: boolean readonly scim_api_key?: string + // This is likely an enum in an external package ("github.com/coder/coder/v2/cli/clibase.StringArray") + readonly external_token_encryption_keys?: string[] readonly provisioner?: ProvisionerConfig readonly rate_limit?: RateLimitConfig // This is likely an enum in an external package ("github.com/coder/coder/v2/cli/clibase.StringArray") @@ -1639,6 +1641,7 @@ export type FeatureName = | "audit_log" | "browser_only" | "external_provisioner_daemons" + | "external_token_encryption" | "high_availability" | "multiple_git_auth" | "scim" @@ -1654,6 +1657,7 @@ export const FeatureNames: FeatureName[] = [ "audit_log", "browser_only", "external_provisioner_daemons", + "external_token_encryption", "high_availability", "multiple_git_auth", "scim", From 55b93e7d60509326208bd48aa0138229be53e89e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 10:01:50 +0100 Subject: [PATCH 03/34] fix indentation --- coderd/database/queries/gitauth.sql | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/coderd/database/queries/gitauth.sql b/coderd/database/queries/gitauth.sql index ad1c92cb68d0b..b2ce97dae1404 100644 --- a/coderd/database/queries/gitauth.sql +++ b/coderd/database/queries/gitauth.sql @@ -11,9 +11,9 @@ INSERT INTO git_auth_links ( created_at, updated_at, oauth_access_token, - oauth_access_token_key_id, + oauth_access_token_key_id, oauth_refresh_token, - oauth_refresh_token_key_id, + oauth_refresh_token_key_id, oauth_expiry ) VALUES ( $1, @@ -23,16 +23,16 @@ INSERT INTO git_auth_links ( $5, $6, $7, - $8, - $9 + $8, + $9 ) RETURNING *; -- name: UpdateGitAuthLink :one UPDATE git_auth_links SET updated_at = $3, oauth_access_token = $4, - oauth_access_token_key_id = $5, + oauth_access_token_key_id = $5, oauth_refresh_token = $6, - oauth_refresh_token_key_id = $7, + oauth_refresh_token_key_id = $7, oauth_expiry = $8 WHERE provider_id = $1 AND user_id = $2 RETURNING *; From f340cbab8cec717bbf81d104d82176507adee7b9 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 10:11:13 +0100 Subject: [PATCH 04/34] fixup! fix indentation --- coderd/database/queries.sql.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 700cfeb1da89f..4d9bc72a37157 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -937,9 +937,9 @@ INSERT INTO git_auth_links ( created_at, updated_at, oauth_access_token, - oauth_access_token_key_id, + oauth_access_token_key_id, oauth_refresh_token, - oauth_refresh_token_key_id, + oauth_refresh_token_key_id, oauth_expiry ) VALUES ( $1, @@ -949,8 +949,8 @@ INSERT INTO git_auth_links ( $5, $6, $7, - $8, - $9 + $8, + $9 ) RETURNING provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id ` @@ -997,9 +997,9 @@ const updateGitAuthLink = `-- name: UpdateGitAuthLink :one UPDATE git_auth_links SET updated_at = $3, oauth_access_token = $4, - oauth_access_token_key_id = $5, + oauth_access_token_key_id = $5, oauth_refresh_token = $6, - oauth_refresh_token_key_id = $7, + oauth_refresh_token_key_id = $7, oauth_expiry = $8 WHERE provider_id = $1 AND user_id = $2 RETURNING provider_id, user_id, created_at, updated_at, oauth_access_token, oauth_refresh_token, oauth_expiry, oauth_access_token_key_id, oauth_refresh_token_key_id ` From feae634540d75285bb514d4fbf4b6086b232366e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 14:10:17 +0000 Subject: [PATCH 05/34] check for primary key revocation on startup --- enterprise/dbcrypt/dbcrypt.go | 14 ++++++++++-- enterprise/dbcrypt/dbcrypt_internal_test.go | 24 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go index 9bd42a518ab82..441f0f2a03ba7 100644 --- a/enterprise/dbcrypt/dbcrypt.go +++ b/enterprise/dbcrypt/dbcrypt.go @@ -343,16 +343,26 @@ func (db *dbCrypt) ensureEncrypted(ctx context.Context) error { } var highestNumber int32 + var activeCipherFound bool for _, k := range ks { + // If our primary key has been revoked, then we can't do anything. + if k.RevokedKeyDigest.Valid && k.RevokedKeyDigest.String == db.primaryCipherDigest { + return xerrors.Errorf("primary encryption key %q has been revoked", db.primaryCipherDigest) + } + if k.ActiveKeyDigest.Valid && k.ActiveKeyDigest.String == db.primaryCipherDigest { - // This is our currently active key. We don't need to do anything further. - return nil + activeCipherFound = true } + if k.Number > highestNumber { highestNumber = k.Number } } + if activeCipherFound { + return nil + } + // If we get here, then we have a new key that we need to insert. // If this conflicts with another transaction, we do not need to retry as // the other transaction will have inserted the key for us. diff --git a/enterprise/dbcrypt/dbcrypt_internal_test.go b/enterprise/dbcrypt/dbcrypt_internal_test.go index 817833e7a91c4..7be23486694b8 100644 --- a/enterprise/dbcrypt/dbcrypt_internal_test.go +++ b/enterprise/dbcrypt/dbcrypt_internal_test.go @@ -446,6 +446,30 @@ func TestNew(t *testing.T) { require.NoError(t, err, "no error should be returned") require.Empty(t, keys, "no keys should be present") }) + + t.Run("PrimaryRevoked", func(t *testing.T) { + t.Parallel() + // Given: a cipher is loaded + cipher := initCipher(t) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + rawDB, _ := dbtestutil.NewDB(t) + + // And: the cipher is revoked before we init the crypt db + err := rawDB.InsertDBCryptKey(ctx, database.InsertDBCryptKeyParams{ + Number: 1, + ActiveKeyDigest: cipher.HexDigest(), + Test: fakeBase64RandomData(t, 32), + }) + require.NoError(t, err, "no error should be returned") + err = rawDB.RevokeDBCryptKey(ctx, cipher.HexDigest()) + require.NoError(t, err, "no error should be returned") + + // Then: when we init the crypt db, we error because the key is revoked + _, err = New(ctx, rawDB, cipher) + require.Error(t, err) + require.ErrorContains(t, err, "has been revoked") + }) } func requireEncryptedEquals(t *testing.T, c Cipher, value, expected string) { From 381f0783d533e6a8dfd745c24c122e700ce7f0f1 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 14:48:15 +0000 Subject: [PATCH 06/34] retry insert active key on tx serialization failure --- enterprise/dbcrypt/dbcrypt.go | 34 ++++++++------ enterprise/dbcrypt/dbcrypt_internal_test.go | 51 +++++++++++++++++++++ 2 files changed, 71 insertions(+), 14 deletions(-) diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go index 441f0f2a03ba7..415d389f0bd1c 100644 --- a/enterprise/dbcrypt/dbcrypt.go +++ b/enterprise/dbcrypt/dbcrypt.go @@ -49,7 +49,8 @@ func New(ctx context.Context, db database.Store, ciphers ...Cipher) (database.St Store: db, } // nolint: gocritic // This is allowed. - if err := dbc.ensureEncrypted(dbauthz.AsSystemRestricted(ctx)); err != nil { + authCtx := dbauthz.AsSystemRestricted(ctx) + if err := dbc.ensureEncryptedWithRetry(authCtx); err != nil { return nil, xerrors.Errorf("ensure encrypted database fields: %w", err) } return dbc, nil @@ -334,6 +335,22 @@ func (db *dbCrypt) decryptField(field *string, digest sql.NullString) error { return nil } +func (db *dbCrypt) ensureEncryptedWithRetry(ctx context.Context) error { + err := db.ensureEncrypted(ctx) + if err == nil { + return nil + } + // If we get a serialization error, then we need to retry. + var pqerr *pq.Error + if !xerrors.As(err, &pqerr) { + return err + } + if pqerr.Code != "40001" { // serialization_failure + return err + } + return db.ensureEncrypted(ctx) +} + func (db *dbCrypt) ensureEncrypted(ctx context.Context) error { return db.InTx(func(s database.Store) error { // Attempt to read the encrypted test fields of the currently active keys. @@ -364,21 +381,10 @@ func (db *dbCrypt) ensureEncrypted(ctx context.Context) error { } // If we get here, then we have a new key that we need to insert. - // If this conflicts with another transaction, we do not need to retry as - // the other transaction will have inserted the key for us. - if err := db.InsertDBCryptKey(ctx, database.InsertDBCryptKeyParams{ + return db.InsertDBCryptKey(ctx, database.InsertDBCryptKeyParams{ Number: highestNumber + 1, ActiveKeyDigest: db.primaryCipherDigest, Test: testValue, - }); err != nil { - var pqErr *pq.Error - if xerrors.As(err, &pqErr) && pqErr.Code == "23505" { - // Unique constraint violation -> another transaction has inserted the key for us. - return nil - } - return err - } - - return nil + }) }, &sql.TxOptions{Isolation: sql.LevelRepeatableRead}) } diff --git a/enterprise/dbcrypt/dbcrypt_internal_test.go b/enterprise/dbcrypt/dbcrypt_internal_test.go index 7be23486694b8..994d136dd918a 100644 --- a/enterprise/dbcrypt/dbcrypt_internal_test.go +++ b/enterprise/dbcrypt/dbcrypt_internal_test.go @@ -8,10 +8,13 @@ import ( "io" "testing" + "github.com/golang/mock/gomock" + "github.com/lib/pq" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/database/dbtestutil" ) @@ -470,6 +473,46 @@ func TestNew(t *testing.T) { require.Error(t, err) require.ErrorContains(t, err, "has been revoked") }) + + t.Run("Retry", func(t *testing.T) { + t.Parallel() + // Given: a cipher is loaded + cipher := initCipher(t) + ctx, cancel := context.WithCancel(context.Background()) + testVal, err := cipher.Encrypt([]byte("coder")) + key := database.DBCryptKey{ + Number: 1, + ActiveKeyDigest: sql.NullString{String: cipher.HexDigest(), Valid: true}, + Test: b64encode(testVal), + } + require.NoError(t, err) + t.Cleanup(cancel) + + // And: a database that returns an error once when we try to serialize a key + ctrl := gomock.NewController(t) + mockDB := dbmock.NewMockStore(ctrl) + + gomock.InOrder( + // First try: we get a serialization error. + expectTx(mockDB), + mockDB.EXPECT().GetDBCryptKeys(gomock.Any()).Times(1).Return([]database.DBCryptKey{}, nil), + mockDB.EXPECT().InsertDBCryptKey(gomock.Any(), gomock.Any()).Times(1).Return(&pq.Error{Code: "40001"}), + // Second try: we get the key we wanted to insert initially. + expectTx(mockDB), + mockDB.EXPECT().GetDBCryptKeys(gomock.Any()).Times(1).Return([]database.DBCryptKey{key}, nil), + ) + + _, err = New(ctx, mockDB, cipher) + require.NoError(t, err) + }) +} + +func expectTx(mdb *dbmock.MockStore) *gomock.Call { + return mdb.EXPECT().InTx(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(f func(store database.Store) error, _ *sql.TxOptions) error { + return f(mdb) + }, + ) } func requireEncryptedEquals(t *testing.T, c Cipher, value, expected string) { @@ -511,3 +554,11 @@ func fakeBase64RandomData(t *testing.T, n int) string { require.NoError(t, err) return base64.StdEncoding.EncodeToString(b) } + +func withInTx(mTx *dbmock.MockStore) { + mTx.EXPECT().InTx(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(f func(store database.Store) error, _ *sql.TxOptions) error { + return f(mTx) + }, + ) +} From c42e6a63ac3b1268198c5473500fa5650b96b65a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 15:03:14 +0000 Subject: [PATCH 07/34] fixup! retry insert active key on tx serialization failure --- enterprise/dbcrypt/dbcrypt_internal_test.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/enterprise/dbcrypt/dbcrypt_internal_test.go b/enterprise/dbcrypt/dbcrypt_internal_test.go index 994d136dd918a..ac0897867368d 100644 --- a/enterprise/dbcrypt/dbcrypt_internal_test.go +++ b/enterprise/dbcrypt/dbcrypt_internal_test.go @@ -494,11 +494,11 @@ func TestNew(t *testing.T) { gomock.InOrder( // First try: we get a serialization error. - expectTx(mockDB), + expectInTx(mockDB), mockDB.EXPECT().GetDBCryptKeys(gomock.Any()).Times(1).Return([]database.DBCryptKey{}, nil), mockDB.EXPECT().InsertDBCryptKey(gomock.Any(), gomock.Any()).Times(1).Return(&pq.Error{Code: "40001"}), // Second try: we get the key we wanted to insert initially. - expectTx(mockDB), + expectInTx(mockDB), mockDB.EXPECT().GetDBCryptKeys(gomock.Any()).Times(1).Return([]database.DBCryptKey{key}, nil), ) @@ -507,7 +507,7 @@ func TestNew(t *testing.T) { }) } -func expectTx(mdb *dbmock.MockStore) *gomock.Call { +func expectInTx(mdb *dbmock.MockStore) *gomock.Call { return mdb.EXPECT().InTx(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( func(f func(store database.Store) error, _ *sql.TxOptions) error { return f(mdb) @@ -554,11 +554,3 @@ func fakeBase64RandomData(t *testing.T, n int) string { require.NoError(t, err) return base64.StdEncoding.EncodeToString(b) } - -func withInTx(mTx *dbmock.MockStore) { - mTx.EXPECT().InTx(gomock.Any(), gomock.Any()).Times(1).DoAndReturn( - func(f func(store database.Store) error, _ *sql.TxOptions) error { - return f(mTx) - }, - ) -} From 6a50a43cba77a130e1c01d7587842e6ef2454433 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 15:15:39 +0000 Subject: [PATCH 08/34] use database.IsSerializedError --- enterprise/dbcrypt/dbcrypt.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go index 415d389f0bd1c..6064fc66dc9f3 100644 --- a/enterprise/dbcrypt/dbcrypt.go +++ b/enterprise/dbcrypt/dbcrypt.go @@ -5,8 +5,6 @@ import ( "database/sql" "encoding/base64" - "github.com/lib/pq" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" @@ -336,19 +334,20 @@ func (db *dbCrypt) decryptField(field *string, digest sql.NullString) error { } func (db *dbCrypt) ensureEncryptedWithRetry(ctx context.Context) error { - err := db.ensureEncrypted(ctx) - if err == nil { - return nil - } - // If we get a serialization error, then we need to retry. - var pqerr *pq.Error - if !xerrors.As(err, &pqerr) { - return err - } - if pqerr.Code != "40001" { // serialization_failure - return err + var err error + for i := 0; i < 3; i++ { + err = db.ensureEncrypted(ctx) + if err == nil { + return nil + } + // If we get a serialization error, then we need to retry. + if !database.IsSerializedError(err) { + return err + } + // otherwise, retry } - return db.ensureEncrypted(ctx) + // If we get here, then we ran out of retries + return err } func (db *dbCrypt) ensureEncrypted(ctx context.Context) error { From 46b1ff4920ea8a157c3609264f77e41530833871 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 15:30:55 +0000 Subject: [PATCH 09/34] encryptFields: check for nil field or digest --- enterprise/dbcrypt/dbcrypt.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go index 6064fc66dc9f3..b928bfda9af90 100644 --- a/enterprise/dbcrypt/dbcrypt.go +++ b/enterprise/dbcrypt/dbcrypt.go @@ -270,7 +270,10 @@ func (db *dbCrypt) encryptField(field *string, digest *sql.NullString) error { } if field == nil { - return nil + return xerrors.Errorf("developer error: encryptField called with nil field") + } + if digest == nil { + return xerrors.Errorf("developer error: encryptField called with nil digest") } encrypted, err := db.ciphers[db.primaryCipherDigest].Encrypt([]byte(*field)) From 9c18168477ffc65411458af8c1de6a27f04f26bd Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 15:36:42 +0000 Subject: [PATCH 10/34] rm insertDBCryptKeyNoLock --- coderd/database/dbfake/dbfake.go | 41 ++++++++++++++------------------ 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 0c5c377960a7b..e73578a61a7df 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -672,26 +672,6 @@ func (q *FakeQuerier) isEveryoneGroup(id uuid.UUID) bool { return false } -func (q *FakeQuerier) insertDBCryptKeyNoLock(_ context.Context, arg database.InsertDBCryptKeyParams) error { - err := validateDatabaseType(arg) - if err != nil { - return err - } - - for _, key := range q.dbcryptKeys { - if key.Number == arg.Number { - return errDuplicateKey - } - } - - q.dbcryptKeys = append(q.dbcryptKeys, database.DBCryptKey{ - Number: arg.Number, - ActiveKeyDigest: sql.NullString{String: arg.ActiveKeyDigest, Valid: true}, - Test: arg.Test, - }) - return nil -} - func (q *FakeQuerier) GetActiveDBCryptKeys(_ context.Context) ([]database.DBCryptKey, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -3918,9 +3898,24 @@ func (q *FakeQuerier) InsertAuditLog(_ context.Context, arg database.InsertAudit return alog, nil } -func (q *FakeQuerier) InsertDBCryptKey(ctx context.Context, arg database.InsertDBCryptKeyParams) error { - // This only ever gets called inside a transaction, so we need to not lock. - return q.insertDBCryptKeyNoLock(ctx, arg) +func (q *FakeQuerier) InsertDBCryptKey(_ context.Context, arg database.InsertDBCryptKeyParams) error { + err := validateDatabaseType(arg) + if err != nil { + return err + } + + for _, key := range q.dbcryptKeys { + if key.Number == arg.Number { + return errDuplicateKey + } + } + + q.dbcryptKeys = append(q.dbcryptKeys, database.DBCryptKey{ + Number: arg.Number, + ActiveKeyDigest: sql.NullString{String: arg.ActiveKeyDigest, Valid: true}, + Test: arg.Test, + }) + return nil } func (q *FakeQuerier) InsertDERPMeshKey(_ context.Context, id string) error { From c54b64a2df8f1a16a575e35a6f4ab31659863fb8 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 16:44:05 +0100 Subject: [PATCH 11/34] Update enterprise/cli/dbcrypt_rotate.go Co-authored-by: Dean Sheather --- enterprise/cli/dbcrypt_rotate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/cli/dbcrypt_rotate.go b/enterprise/cli/dbcrypt_rotate.go index f145a65f73d32..05702c27df43e 100644 --- a/enterprise/cli/dbcrypt_rotate.go +++ b/enterprise/cli/dbcrypt_rotate.go @@ -93,7 +93,7 @@ func (*RootCmd) dbcryptRotate() *clibase.Cmd { if err != nil { return xerrors.Errorf("get users: %w", err) } - logger.Info(ctx, "encrypting user tokens", slog.F("count", len(users))) + logger.Info(ctx, "encrypting user tokens", slog.F("user_count", len(users))) for idx, usr := range users { err := cryptDB.InTx(func(tx database.Store) error { userLinks, err := tx.GetUserLinksByUserID(ctx, usr.ID) From 5959b3498043d6f5dcfb57b433d015fcc1aad26d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 16:44:17 +0100 Subject: [PATCH 12/34] Update enterprise/coderd/coderd.go Co-authored-by: Dean Sheather --- enterprise/coderd/coderd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 2e525ddf13dd8..5e45c00b0dc6c 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -455,7 +455,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { codersdk.FeatureHighAvailability: api.DERPServerRelayAddress != "", codersdk.FeatureMultipleGitAuth: len(api.GitAuthConfigs) > 1, codersdk.FeatureTemplateRBAC: api.RBAC, - codersdk.FeatureExternalTokenEncryption: api.ExternalTokenEncryption != nil, + codersdk.FeatureExternalTokenEncryption: len(api.ExternalTokenEncryption) != 0, codersdk.FeatureExternalProvisionerDaemons: true, codersdk.FeatureAdvancedTemplateScheduling: true, // FeatureTemplateAutostopRequirement depends on From b1546b1ddf8543e883d7d5b5dde7de8ecaf34afb Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 17:39:13 +0100 Subject: [PATCH 13/34] add unit test for ExtractAPIKeyMW --- coderd/httpmw/apikey_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index f29067dec522f..f3ceba017d773 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -153,6 +153,34 @@ func TestAPIKey(t *testing.T) { require.Equal(t, http.StatusUnauthorized, res.StatusCode) }) + t.Run("UserLinkNotFound", func(t *testing.T) { + t.Parallel() + var ( + db = dbfake.New() + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() + user = dbgen.User(t, db, database.User{ + LoginType: database.LoginTypeGithub, + }) + // Intentionally not inserting any user link + _, token = dbgen.APIKey(t, db, database.APIKey{ + UserID: user.ID, + LoginType: user.LoginType, + }) + ) + r.Header.Set(codersdk.SessionTokenHeader, token) + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ + DB: db, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusUnauthorized, res.StatusCode) + var resp codersdk.Response + require.NoError(t, json.NewDecoder(res.Body).Decode(&resp)) + require.Equal(t, resp.Message, httpmw.SignedOutErrorMessage) + }) + t.Run("InvalidSecret", func(t *testing.T) { t.Parallel() var ( From 3859e0340b00f5b2f175a86bd52bc660a87e2b3b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Sep 2023 20:10:23 +0100 Subject: [PATCH 14/34] add unit test for cli.ConnectToPostgres --- cli/server_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/cli/server_test.go b/cli/server_test.go index 7b38bb76f9e15..3adcc3c4b39dd 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -34,10 +34,13 @@ import ( "go.uber.org/goleak" "gopkg.in/yaml.v3" + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/cli" "github.com/coder/coder/v2/cli/clitest" "github.com/coder/coder/v2/cli/config" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/postgres" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/codersdk" @@ -1657,3 +1660,26 @@ func TestServerYAMLConfig(t *testing.T) { require.Equal(t, string(wantByt), string(got)) } + +func TestConnectToPostgres(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("this test does not make sense without postgres") + } + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + t.Cleanup(cancel) + + log := slogtest.Make(t, nil) + + dbURL, closeFunc, err := postgres.Open() + require.NoError(t, err) + t.Cleanup(closeFunc) + + sqlDB, err := cli.ConnectToPostgres(ctx, log, "postgres", dbURL) + require.NoError(t, err) + t.Cleanup(func() { + _ = sqlDB.Close() + }) + require.NoError(t, sqlDB.PingContext(ctx)) +} From 55a0fd01bf5e5d61c9de403d24e1c8bd52718abb Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 14:10:48 +0100 Subject: [PATCH 15/34] DON'T PANIC --- enterprise/coderd/coderd.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 5e45c00b0dc6c..c89ea8f260a70 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -72,9 +72,9 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { // This is a fatal error. var derr *dbcrypt.DecryptFailedError if xerrors.As(err, &derr) { - panic(`Coder has shut down to prevent data corruption: your configured database is encrypted with an unknown external token encryption key. Please check your configuration and try again.`) + return nil, xerrors.Errorf("database encrypted with unknown key, either add the key or see https://coder.com/docs/v2/latest/admin/encryption#disabling-encryption: %w", derr) } - return nil, xerrors.Errorf("init dbcrypt: %w", err) + return nil, xerrors.Errorf("init database encryption: %w", err) } options.Database = cryptDB } From cce02442bf2a02f761361aee0c990d49ec2a9e2e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 15:42:47 +0100 Subject: [PATCH 16/34] debug log user_ids --- enterprise/cli/dbcrypt_rotate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/enterprise/cli/dbcrypt_rotate.go b/enterprise/cli/dbcrypt_rotate.go index 05702c27df43e..d029a45603ae8 100644 --- a/enterprise/cli/dbcrypt_rotate.go +++ b/enterprise/cli/dbcrypt_rotate.go @@ -108,7 +108,7 @@ func (*RootCmd) dbcryptRotate() *clibase.Cmd { UserID: usr.ID, LoginType: usr.LoginType, }); err != nil { - return xerrors.Errorf("update user link: %w", err) + return xerrors.Errorf("update user link user_id=%s linked_id=%s: %w", userLink.UserID, userLink.LinkedID, err) } } gitAuthLinks, err := tx.GetGitAuthLinksByUserID(ctx, usr.ID) @@ -124,7 +124,7 @@ func (*RootCmd) dbcryptRotate() *clibase.Cmd { OAuthRefreshToken: gitAuthLink.OAuthRefreshToken, OAuthExpiry: gitAuthLink.OAuthExpiry, }); err != nil { - return xerrors.Errorf("update git auth link: %w", err) + return xerrors.Errorf("update git auth link user_id=%s provider_id=%s: %w", gitAuthLink.UserID, gitAuthLink.ProviderID, err) } } return nil @@ -134,7 +134,7 @@ func (*RootCmd) dbcryptRotate() *clibase.Cmd { if err != nil { return xerrors.Errorf("update user links: %w", err) } - logger.Debug(ctx, "encrypted user tokens", slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + logger.Debug(ctx, "encrypted user tokens", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) } logger.Info(ctx, "operation completed successfully") From d51ec66d17f632cb1f5af8e323c43084f01e57bf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 16:04:27 +0100 Subject: [PATCH 17/34] dbcrypt-rotate -> server dbcrypt rotate --- docs/cli.md | 1 - docs/cli/server.md | 1 + docs/cli/server_dbcrypt.md | 17 +++++++++++++ ...ypt-rotate.md => server_dbcrypt_rotate.md} | 4 ++-- docs/manifest.json | 15 ++++++++---- enterprise/cli/dbcrypt_rotate_slim.go | 20 ---------------- enterprise/cli/root.go | 1 - enterprise/cli/server.go | 4 ++++ ...ypt_rotate.go => server_dbcrypt_rotate.go} | 20 +++++++++++++--- ..._test.go => server_dbcrypt_rotate_test.go} | 4 ++-- enterprise/cli/testdata/coder_--help.golden | 1 - .../cli/testdata/coder_server_--help.golden | 1 + .../coder_server_dbcrypt_--help.golden | 9 +++++++ .../coder_server_dbcrypt_rotate_--help.golden | 24 +++++++++++++++++++ 14 files changed, 87 insertions(+), 35 deletions(-) create mode 100644 docs/cli/server_dbcrypt.md rename docs/cli/{dbcrypt-rotate.md => server_dbcrypt_rotate.md} (90%) delete mode 100644 enterprise/cli/dbcrypt_rotate_slim.go rename enterprise/cli/{dbcrypt_rotate.go => server_dbcrypt_rotate.go} (91%) rename enterprise/cli/{dbcrypt_rotate_test.go => server_dbcrypt_rotate_test.go} (98%) create mode 100644 enterprise/cli/testdata/coder_server_dbcrypt_--help.golden create mode 100644 enterprise/cli/testdata/coder_server_dbcrypt_rotate_--help.golden diff --git a/docs/cli.md b/docs/cli.md index 40e5a9430f6fb..c9ffdc7c46421 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -27,7 +27,6 @@ Coder — A tool for provisioning self-hosted development environments with Terr | ------------------------------------------------------ | ----------------------------------------------------------------------------------------------------- | | [config-ssh](./cli/config-ssh.md) | Add an SSH Host entry for your workspaces "ssh coder.workspace" | | [create](./cli/create.md) | Create a workspace | -| [dbcrypt-rotate](./cli/dbcrypt-rotate.md) | Rotate database encryption keys | | [delete](./cli/delete.md) | Delete a workspace | | [dotfiles](./cli/dotfiles.md) | Personalize your workspace by applying a canonical dotfiles repository | | [features](./cli/features.md) | List Enterprise features | diff --git a/docs/cli/server.md b/docs/cli/server.md index a2b98fd724612..fcbae2aee7ee4 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -15,6 +15,7 @@ coder server [flags] | Name | Purpose | | ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | | [create-admin-user](./server_create-admin-user.md) | Create a new admin user with the given username, email and password and adds it to every organization. | +| [dbcrypt](./server_dbcrypt.md) | Manage database encryption | | [postgres-builtin-serve](./server_postgres-builtin-serve.md) | Run the built-in PostgreSQL deployment. | | [postgres-builtin-url](./server_postgres-builtin-url.md) | Output the connection URL for the built-in PostgreSQL deployment. | diff --git a/docs/cli/server_dbcrypt.md b/docs/cli/server_dbcrypt.md new file mode 100644 index 0000000000000..7fbdbd1299c1d --- /dev/null +++ b/docs/cli/server_dbcrypt.md @@ -0,0 +1,17 @@ + + +# server dbcrypt + +Manage database encryption + +## Usage + +```console +coder server dbcrypt +``` + +## Subcommands + +| Name | Purpose | +| ------------------------------------------------- | ------------------------------- | +| [rotate](./server_dbcrypt_rotate.md) | Rotate database encryption keys | diff --git a/docs/cli/dbcrypt-rotate.md b/docs/cli/server_dbcrypt_rotate.md similarity index 90% rename from docs/cli/dbcrypt-rotate.md rename to docs/cli/server_dbcrypt_rotate.md index c3d5a34c9b98f..0bdb4e9ca7437 100644 --- a/docs/cli/dbcrypt-rotate.md +++ b/docs/cli/server_dbcrypt_rotate.md @@ -1,13 +1,13 @@ -# dbcrypt-rotate +# server dbcrypt rotate Rotate database encryption keys ## Usage ```console -coder dbcrypt-rotate [flags] --postgres-url --external-token-encryption-keys , +coder server dbcrypt rotate [flags] ``` ## Options diff --git a/docs/manifest.json b/docs/manifest.json index cbd37d12f1e50..2ff2eb7d8f81a 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -547,11 +547,6 @@ "description": "Create a workspace", "path": "cli/create.md" }, - { - "title": "dbcrypt-rotate", - "description": "Rotate database encryption keys", - "path": "cli/dbcrypt-rotate.md" - }, { "title": "delete", "description": "Delete a workspace", @@ -711,6 +706,16 @@ "description": "Create a new admin user with the given username, email and password and adds it to every organization.", "path": "cli/server_create-admin-user.md" }, + { + "title": "server dbcrypt", + "description": "Manage database encryption", + "path": "cli/server_dbcrypt.md" + }, + { + "title": "server dbcrypt rotate", + "description": "Rotate database encryption keys", + "path": "cli/server_dbcrypt_rotate.md" + }, { "title": "server postgres-builtin-serve", "description": "Run the built-in PostgreSQL deployment.", diff --git a/enterprise/cli/dbcrypt_rotate_slim.go b/enterprise/cli/dbcrypt_rotate_slim.go deleted file mode 100644 index 63c5bb9205ee0..0000000000000 --- a/enterprise/cli/dbcrypt_rotate_slim.go +++ /dev/null @@ -1,20 +0,0 @@ -//go:build slim - -package cli - -import ( - "github.com/coder/coder/v2/cli/clibase" - "golang.org/x/xerrors" -) - -func (*RootCmd) dbcryptRotate() *clibase.Cmd { - return &clibase.Cmd{ - Use: "dbcrypt-rotate --postgres-url --external-token-encryption-keys ,", - Short: "Rotate database encryption keys", - Options: clibase.OptionSet{}, - Hidden: true, - Handler: func(inv *clibase.Invocation) error { - return xerrors.Errorf("slim build does not support `coder dbcrypt-rotate`") - }, - } -} diff --git a/enterprise/cli/root.go b/enterprise/cli/root.go index 6f7887518c31b..360582a3e5193 100644 --- a/enterprise/cli/root.go +++ b/enterprise/cli/root.go @@ -17,7 +17,6 @@ func (r *RootCmd) enterpriseOnly() []*clibase.Cmd { r.licenses(), r.groups(), r.provisionerDaemons(), - r.dbcryptRotate(), } } diff --git a/enterprise/cli/server.go b/enterprise/cli/server.go index 0cd5d8e1ffc97..7fb1526c50197 100644 --- a/enterprise/cli/server.go +++ b/enterprise/cli/server.go @@ -98,5 +98,9 @@ func (r *RootCmd) Server(_ func()) *clibase.Cmd { } return api.AGPL, api, nil }) + + cmd.AddSubcommands( + r.dbcryptCmd(), + ) return cmd } diff --git a/enterprise/cli/dbcrypt_rotate.go b/enterprise/cli/server_dbcrypt_rotate.go similarity index 91% rename from enterprise/cli/dbcrypt_rotate.go rename to enterprise/cli/server_dbcrypt_rotate.go index d029a45603ae8..d1d477fd9ca28 100644 --- a/enterprise/cli/dbcrypt_rotate.go +++ b/enterprise/cli/server_dbcrypt_rotate.go @@ -19,14 +19,28 @@ import ( "golang.org/x/xerrors" ) -func (*RootCmd) dbcryptRotate() *clibase.Cmd { +func (r *RootCmd) dbcryptCmd() *clibase.Cmd { + dbcryptCmd := &clibase.Cmd{ + Use: "dbcrypt", + Short: "Manage database encryption.", + Handler: func(inv *clibase.Invocation) error { + return inv.Command.HelpHandler(inv) + }, + } + dbcryptCmd.AddSubcommands( + r.dbcryptRotateCmd(), + ) + return dbcryptCmd +} + +func (*RootCmd) dbcryptRotateCmd() *clibase.Cmd { var ( vals = new(codersdk.DeploymentValues) opts = vals.Options() ) cmd := &clibase.Cmd{ - Use: "dbcrypt-rotate --postgres-url --external-token-encryption-keys ,", - Short: "Rotate database encryption keys", + Use: "rotate", + Short: "Rotate database encryption keys.", Options: clibase.OptionSet{ *opts.ByName("Postgres Connection URL"), *opts.ByName("External Token Encryption Keys"), diff --git a/enterprise/cli/dbcrypt_rotate_test.go b/enterprise/cli/server_dbcrypt_rotate_test.go similarity index 98% rename from enterprise/cli/dbcrypt_rotate_test.go rename to enterprise/cli/server_dbcrypt_rotate_test.go index 6faaf2f8775c1..a12317a6be93a 100644 --- a/enterprise/cli/dbcrypt_rotate_test.go +++ b/enterprise/cli/server_dbcrypt_rotate_test.go @@ -50,7 +50,7 @@ func TestDBCryptRotate(t *testing.T) { require.NoError(t, err) // Encrypt all the data with the initial cipher. - inv, _ := newCLI(t, "dbcrypt-rotate", + inv, _ := newCLI(t, "server", "dbcrypt", "rotate", "--postgres-url", connectionURL, "--external-token-encryption-keys", base64.StdEncoding.EncodeToString([]byte(keyA)), ) @@ -79,7 +79,7 @@ func TestDBCryptRotate(t *testing.T) { base64.StdEncoding.EncodeToString([]byte(keyA)), ) - inv, _ = newCLI(t, "dbcrypt-rotate", + inv, _ = newCLI(t, "server", "dbcrypt", "rotate", "--postgres-url", connectionURL, "--external-token-encryption-keys", externalTokensArg, ) diff --git a/enterprise/cli/testdata/coder_--help.golden b/enterprise/cli/testdata/coder_--help.golden index de2b9c5f9e7d9..ae24592079a69 100644 --- a/enterprise/cli/testdata/coder_--help.golden +++ b/enterprise/cli/testdata/coder_--help.golden @@ -10,7 +10,6 @@ Coder v0.0.0-devel — A tool for provisioning self-hosted development environme  $ coder templates init  Subcommands - dbcrypt-rotate Rotate database encryption keys features List Enterprise features groups Manage groups licenses Add, delete, and list licenses diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 0a2e63ddd8ea8..2b701ff9ef2a9 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -6,6 +6,7 @@ Start a Coder server create-admin-user Create a new admin user with the given username, email and password and adds it to every organization. + dbcrypt Manage database encryption. postgres-builtin-serve Run the built-in PostgreSQL deployment. postgres-builtin-url Output the connection URL for the built-in PostgreSQL deployment. diff --git a/enterprise/cli/testdata/coder_server_dbcrypt_--help.golden b/enterprise/cli/testdata/coder_server_dbcrypt_--help.golden new file mode 100644 index 0000000000000..a7d758c8513b0 --- /dev/null +++ b/enterprise/cli/testdata/coder_server_dbcrypt_--help.golden @@ -0,0 +1,9 @@ +Usage: coder server dbcrypt + +Manage database encryption. + +Subcommands + rotate Rotate database encryption keys. + +--- +Run `coder --help` for a list of global options. diff --git a/enterprise/cli/testdata/coder_server_dbcrypt_rotate_--help.golden b/enterprise/cli/testdata/coder_server_dbcrypt_rotate_--help.golden new file mode 100644 index 0000000000000..4ae79673d8c91 --- /dev/null +++ b/enterprise/cli/testdata/coder_server_dbcrypt_rotate_--help.golden @@ -0,0 +1,24 @@ +Usage: coder server dbcrypt rotate [flags] + +Rotate database encryption keys. + +Options + --postgres-url string, $CODER_PG_CONNECTION_URL + URL of a PostgreSQL database. If empty, PostgreSQL binaries will be + downloaded from Maven (https://repo1.maven.org/maven2) and store all + data in the config root. Access the built-in database with "coder + server postgres-builtin-url". + +Enterprise Options +These options are only available in the Enterprise Edition. + + --external-token-encryption-keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS + Encrypt OIDC and Git authentication tokens with AES-256-GCM in the + database. The value must be a comma-separated list of base64-encoded + keys. Each key, when base64-decoded, must be exactly 32 bytes in + length. The first key will be used to encrypt new values. Subsequent + keys will be used as a fallback when decrypting. During normal + operation it is recommended to only set one key. + +--- +Run `coder --help` for a list of global options. From aa39fcc262af7e26efaa9c8b7f7a9f07d7636c6e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 16:39:03 +0100 Subject: [PATCH 18/34] refactor: move rotate logic into dbcrypt --- enterprise/cli/server_dbcrypt_rotate.go | 66 +------------------ enterprise/dbcrypt/rotate.go | 88 +++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 64 deletions(-) create mode 100644 enterprise/dbcrypt/rotate.go diff --git a/enterprise/cli/server_dbcrypt_rotate.go b/enterprise/cli/server_dbcrypt_rotate.go index d1d477fd9ca28..1452ff7f5a394 100644 --- a/enterprise/cli/server_dbcrypt_rotate.go +++ b/enterprise/cli/server_dbcrypt_rotate.go @@ -5,14 +5,12 @@ package cli import ( "bytes" "context" - "database/sql" "encoding/base64" "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" "github.com/coder/coder/v2/cli" "github.com/coder/coder/v2/cli/clibase" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/dbcrypt" @@ -95,70 +93,10 @@ func (*RootCmd) dbcryptRotateCmd() *clibase.Cmd { _ = sqlDB.Close() }() logger.Info(ctx, "connected to postgres") - - db := database.New(sqlDB) - - cryptDB, err := dbcrypt.New(ctx, db, ciphers...) - if err != nil { - return xerrors.Errorf("create cryptdb: %w", err) - } - - users, err := cryptDB.GetUsers(ctx, database.GetUsersParams{}) - if err != nil { - return xerrors.Errorf("get users: %w", err) - } - logger.Info(ctx, "encrypting user tokens", slog.F("user_count", len(users))) - for idx, usr := range users { - err := cryptDB.InTx(func(tx database.Store) error { - userLinks, err := tx.GetUserLinksByUserID(ctx, usr.ID) - if err != nil { - return xerrors.Errorf("get user links for user: %w", err) - } - for _, userLink := range userLinks { - if _, err := tx.UpdateUserLink(ctx, database.UpdateUserLinkParams{ - OAuthAccessToken: userLink.OAuthAccessToken, - OAuthRefreshToken: userLink.OAuthRefreshToken, - OAuthExpiry: userLink.OAuthExpiry, - UserID: usr.ID, - LoginType: usr.LoginType, - }); err != nil { - return xerrors.Errorf("update user link user_id=%s linked_id=%s: %w", userLink.UserID, userLink.LinkedID, err) - } - } - gitAuthLinks, err := tx.GetGitAuthLinksByUserID(ctx, usr.ID) - if err != nil { - return xerrors.Errorf("get git auth links for user: %w", err) - } - for _, gitAuthLink := range gitAuthLinks { - if _, err := tx.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ - ProviderID: gitAuthLink.ProviderID, - UserID: usr.ID, - UpdatedAt: gitAuthLink.UpdatedAt, - OAuthAccessToken: gitAuthLink.OAuthAccessToken, - OAuthRefreshToken: gitAuthLink.OAuthRefreshToken, - OAuthExpiry: gitAuthLink.OAuthExpiry, - }); err != nil { - return xerrors.Errorf("update git auth link user_id=%s provider_id=%s: %w", gitAuthLink.UserID, gitAuthLink.ProviderID, err) - } - } - return nil - }, &sql.TxOptions{ - Isolation: sql.LevelRepeatableRead, - }) - if err != nil { - return xerrors.Errorf("update user links: %w", err) - } - logger.Debug(ctx, "encrypted user tokens", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + if err := dbcrypt.Rotate(ctx, logger, sqlDB, ciphers); err != nil { + return xerrors.Errorf("rotate ciphers: %w", err) } logger.Info(ctx, "operation completed successfully") - - // Revoke old keys - for _, c := range ciphers[1:] { - if err := db.RevokeDBCryptKey(ctx, c.HexDigest()); err != nil { - return xerrors.Errorf("revoke key: %w", err) - } - logger.Info(ctx, "revoked unused key", slog.F("digest", c.HexDigest())) - } return nil }, } diff --git a/enterprise/dbcrypt/rotate.go b/enterprise/dbcrypt/rotate.go new file mode 100644 index 0000000000000..812d7c46573a2 --- /dev/null +++ b/enterprise/dbcrypt/rotate.go @@ -0,0 +1,88 @@ +package dbcrypt + +import ( + "context" + "database/sql" + + "golang.org/x/xerrors" + + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" +) + +// Rotate rotates the database encryption keys by re-encrypting all user tokens +// with the first cipher and revoking all other ciphers. +func Rotate(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Cipher) error { + db := database.New(sqlDB) + cryptDB, err := New(ctx, db, ciphers...) + if err != nil { + return xerrors.Errorf("create cryptdb: %w", err) + } + + users, err := cryptDB.GetUsers(ctx, database.GetUsersParams{}) + if err != nil { + return xerrors.Errorf("get users: %w", err) + } + log.Info(ctx, "encrypting user tokens", slog.F("user_count", len(users))) + for idx, usr := range users { + err := cryptDB.InTx(func(tx database.Store) error { + userLinks, err := tx.GetUserLinksByUserID(ctx, usr.ID) + if err != nil { + return xerrors.Errorf("get user links for user: %w", err) + } + for _, userLink := range userLinks { + if userLink.OAuthAccessTokenKeyID.String == ciphers[0].HexDigest() && userLink.OAuthRefreshTokenKeyID.String == ciphers[0].HexDigest() { + log.Debug(ctx, "skipping user link", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + continue + } + if _, err := tx.UpdateUserLink(ctx, database.UpdateUserLinkParams{ + OAuthAccessToken: userLink.OAuthAccessToken, + OAuthRefreshToken: userLink.OAuthRefreshToken, + OAuthExpiry: userLink.OAuthExpiry, + UserID: usr.ID, + LoginType: usr.LoginType, + }); err != nil { + return xerrors.Errorf("update user link user_id=%s linked_id=%s: %w", userLink.UserID, userLink.LinkedID, err) + } + } + + gitAuthLinks, err := tx.GetGitAuthLinksByUserID(ctx, usr.ID) + if err != nil { + return xerrors.Errorf("get git auth links for user: %w", err) + } + for _, gitAuthLink := range gitAuthLinks { + if gitAuthLink.OAuthAccessTokenKeyID.String == ciphers[0].HexDigest() && gitAuthLink.OAuthRefreshTokenKeyID.String == ciphers[0].HexDigest() { + log.Debug(ctx, "skipping git auth link", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + continue + } + if _, err := tx.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ + ProviderID: gitAuthLink.ProviderID, + UserID: usr.ID, + UpdatedAt: gitAuthLink.UpdatedAt, + OAuthAccessToken: gitAuthLink.OAuthAccessToken, + OAuthRefreshToken: gitAuthLink.OAuthRefreshToken, + OAuthExpiry: gitAuthLink.OAuthExpiry, + }); err != nil { + return xerrors.Errorf("update git auth link user_id=%s provider_id=%s: %w", gitAuthLink.UserID, gitAuthLink.ProviderID, err) + } + } + return nil + }, &sql.TxOptions{ + Isolation: sql.LevelRepeatableRead, + }) + if err != nil { + return xerrors.Errorf("update user links: %w", err) + } + log.Debug(ctx, "encrypted user tokens", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + } + + // Revoke old keys + for _, c := range ciphers[1:] { + if err := db.RevokeDBCryptKey(ctx, c.HexDigest()); err != nil { + return xerrors.Errorf("revoke key: %w", err) + } + log.Info(ctx, "revoked unused key", slog.F("digest", c.HexDigest())) + } + + return nil +} From e69e3ef6be75c1ed2bb2b37b6c7b571d1afb1bae Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 17:33:55 +0100 Subject: [PATCH 19/34] add decrypt/delete commands --- enterprise/cli/server_dbcrypt_rotate.go | 126 +++++++++++++++ enterprise/dbcrypt/cilutil.go | 201 ++++++++++++++++++++++++ enterprise/dbcrypt/rotate.go | 88 ----------- 3 files changed, 327 insertions(+), 88 deletions(-) create mode 100644 enterprise/dbcrypt/cilutil.go delete mode 100644 enterprise/dbcrypt/rotate.go diff --git a/enterprise/cli/server_dbcrypt_rotate.go b/enterprise/cli/server_dbcrypt_rotate.go index 1452ff7f5a394..962357b526986 100644 --- a/enterprise/cli/server_dbcrypt_rotate.go +++ b/enterprise/cli/server_dbcrypt_rotate.go @@ -11,6 +11,7 @@ import ( "cdr.dev/slog/sloggers/sloghuman" "github.com/coder/coder/v2/cli" "github.com/coder/coder/v2/cli/clibase" + "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/dbcrypt" @@ -26,6 +27,8 @@ func (r *RootCmd) dbcryptCmd() *clibase.Cmd { }, } dbcryptCmd.AddSubcommands( + r.dbcryptDecryptCmd(), + r.dbcryptDeleteCmd(), r.dbcryptRotateCmd(), ) return dbcryptCmd @@ -102,3 +105,126 @@ func (*RootCmd) dbcryptRotateCmd() *clibase.Cmd { } return cmd } + +func (*RootCmd) dbcryptDecryptCmd() *clibase.Cmd { + var ( + vals = new(codersdk.DeploymentValues) + opts = vals.Options() + ) + cmd := &clibase.Cmd{ + Use: "decrypt", + Short: "Decrypt a previously encrypted database.", + Options: clibase.OptionSet{ + *opts.ByName("Postgres Connection URL"), + *opts.ByName("External Token Encryption Keys"), + }, + Middleware: clibase.Chain( + clibase.RequireNArgs(0), + ), + Handler: func(inv *clibase.Invocation) error { + ctx, cancel := context.WithCancel(inv.Context()) + defer cancel() + logger := slog.Make(sloghuman.Sink(inv.Stdout)) + if ok, _ := inv.ParsedFlags().GetBool("verbose"); ok { + logger = logger.Leveled(slog.LevelDebug) + } + + if vals.PostgresURL == "" { + return xerrors.Errorf("no database configured") + } + + switch len(vals.ExternalTokenEncryptionKeys) { + case 0: + return xerrors.Errorf("no external token encryption keys provided") + case 1: + logger.Info(ctx, "only one key provided, data will be re-encrypted with the same key") + } + + keys := make([][]byte, 0, len(vals.ExternalTokenEncryptionKeys)) + var newKey []byte + for idx, ek := range vals.ExternalTokenEncryptionKeys { + dk, err := base64.StdEncoding.DecodeString(ek) + if err != nil { + return xerrors.Errorf("key must be base64-encoded") + } + if idx == 0 { + newKey = dk + } else if bytes.Equal(dk, newKey) { + return xerrors.Errorf("old key at index %d is the same as the new key", idx) + } + keys = append(keys, dk) + } + + ciphers, err := dbcrypt.NewCiphers(keys...) + if err != nil { + return xerrors.Errorf("create ciphers: %w", err) + } + + sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", vals.PostgresURL.Value()) + if err != nil { + return xerrors.Errorf("connect to postgres: %w", err) + } + defer func() { + _ = sqlDB.Close() + }() + logger.Info(ctx, "connected to postgres") + if err := dbcrypt.Decrypt(ctx, logger, sqlDB, ciphers); err != nil { + return xerrors.Errorf("rotate ciphers: %w", err) + } + logger.Info(ctx, "operation completed successfully") + return nil + }, + } + return cmd +} + +func (*RootCmd) dbcryptDeleteCmd() *clibase.Cmd { + var ( + vals = new(codersdk.DeploymentValues) + opts = vals.Options() + ) + cmd := &clibase.Cmd{ + Use: "delete", + Short: "Delete all encrypted data from the database. THIS IS A DESTRUCTIVE OPERATION.", + Options: clibase.OptionSet{ + *opts.ByName("Postgres Connection URL"), + }, + Middleware: clibase.Chain( + clibase.RequireNArgs(0), + ), + Handler: func(inv *clibase.Invocation) error { + ctx, cancel := context.WithCancel(inv.Context()) + defer cancel() + logger := slog.Make(sloghuman.Sink(inv.Stdout)) + if ok, _ := inv.ParsedFlags().GetBool("verbose"); ok { + logger = logger.Leveled(slog.LevelDebug) + } + + if vals.PostgresURL == "" { + return xerrors.Errorf("no database configured") + } + + if _, err := cliui.Prompt(inv, cliui.PromptOptions{ + Text: "This will delete all encrypted data from the database. Are you sure you want to continue?", + IsConfirm: true, + }); err != nil { + return err + } + + sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", vals.PostgresURL.Value()) + if err != nil { + return xerrors.Errorf("connect to postgres: %w", err) + } + defer func() { + _ = sqlDB.Close() + }() + logger.Info(ctx, "connected to postgres") + if err := dbcrypt.Delete(ctx, logger, sqlDB); err != nil { + return xerrors.Errorf("delete encrypted data: %w", err) + } + logger.Info(ctx, "operation completed successfully") + return nil + }, + } + return cmd +} diff --git a/enterprise/dbcrypt/cilutil.go b/enterprise/dbcrypt/cilutil.go new file mode 100644 index 0000000000000..bc72064806e32 --- /dev/null +++ b/enterprise/dbcrypt/cilutil.go @@ -0,0 +1,201 @@ +package dbcrypt + +import ( + "context" + "database/sql" + + "golang.org/x/xerrors" + + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" +) + +// Rotate rotates the database encryption keys by re-encrypting all user tokens +// with the first cipher and revoking all other ciphers. +func Rotate(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Cipher) error { + db := database.New(sqlDB) + cryptDB, err := New(ctx, db, ciphers...) + if err != nil { + return xerrors.Errorf("create cryptdb: %w", err) + } + + users, err := cryptDB.GetUsers(ctx, database.GetUsersParams{}) + if err != nil { + return xerrors.Errorf("get users: %w", err) + } + log.Info(ctx, "encrypting user tokens", slog.F("user_count", len(users))) + for idx, usr := range users { + err := cryptDB.InTx(func(tx database.Store) error { + userLinks, err := tx.GetUserLinksByUserID(ctx, usr.ID) + if err != nil { + return xerrors.Errorf("get user links for user: %w", err) + } + for _, userLink := range userLinks { + if userLink.OAuthAccessTokenKeyID.String == ciphers[0].HexDigest() && userLink.OAuthRefreshTokenKeyID.String == ciphers[0].HexDigest() { + log.Debug(ctx, "skipping user link", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + continue + } + if _, err := tx.UpdateUserLink(ctx, database.UpdateUserLinkParams{ + OAuthAccessToken: userLink.OAuthAccessToken, + OAuthRefreshToken: userLink.OAuthRefreshToken, + OAuthExpiry: userLink.OAuthExpiry, + UserID: usr.ID, + LoginType: usr.LoginType, + }); err != nil { + return xerrors.Errorf("update user link user_id=%s linked_id=%s: %w", userLink.UserID, userLink.LinkedID, err) + } + } + + gitAuthLinks, err := tx.GetGitAuthLinksByUserID(ctx, usr.ID) + if err != nil { + return xerrors.Errorf("get git auth links for user: %w", err) + } + for _, gitAuthLink := range gitAuthLinks { + if gitAuthLink.OAuthAccessTokenKeyID.String == ciphers[0].HexDigest() && gitAuthLink.OAuthRefreshTokenKeyID.String == ciphers[0].HexDigest() { + log.Debug(ctx, "skipping git auth link", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + continue + } + if _, err := tx.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ + ProviderID: gitAuthLink.ProviderID, + UserID: usr.ID, + UpdatedAt: gitAuthLink.UpdatedAt, + OAuthAccessToken: gitAuthLink.OAuthAccessToken, + OAuthRefreshToken: gitAuthLink.OAuthRefreshToken, + OAuthExpiry: gitAuthLink.OAuthExpiry, + }); err != nil { + return xerrors.Errorf("update git auth link user_id=%s provider_id=%s: %w", gitAuthLink.UserID, gitAuthLink.ProviderID, err) + } + } + return nil + }, &sql.TxOptions{ + Isolation: sql.LevelRepeatableRead, + }) + if err != nil { + return xerrors.Errorf("update user links: %w", err) + } + log.Debug(ctx, "encrypted user tokens", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + } + + // Revoke old keys + for _, c := range ciphers[1:] { + if err := db.RevokeDBCryptKey(ctx, c.HexDigest()); err != nil { + return xerrors.Errorf("revoke key: %w", err) + } + log.Info(ctx, "revoked unused key", slog.F("digest", c.HexDigest())) + } + + return nil +} + +// Decrypt decrypts all user tokens and revokes all ciphers. +func Decrypt(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Cipher) error { + db := database.New(sqlDB) + cdb, err := New(ctx, db, ciphers...) + if err != nil { + return xerrors.Errorf("create cryptdb: %w", err) + } + + // HACK: instead of adding logic to configure the primary cipher, we just + // set it to the empty string so that it will not encrypt anything. + cryptDB, ok := cdb.(*dbCrypt) + if !ok { + return xerrors.Errorf("developer error: dbcrypt.New did not return *dbCrypt") + } + cryptDB.primaryCipherDigest = "" + + users, err := cryptDB.GetUsers(ctx, database.GetUsersParams{}) + if err != nil { + return xerrors.Errorf("get users: %w", err) + } + log.Info(ctx, "decrypting user tokens", slog.F("user_count", len(users))) + for idx, usr := range users { + err := cryptDB.InTx(func(tx database.Store) error { + userLinks, err := tx.GetUserLinksByUserID(ctx, usr.ID) + if err != nil { + return xerrors.Errorf("get user links for user: %w", err) + } + for _, userLink := range userLinks { + if !userLink.OAuthAccessTokenKeyID.Valid && !userLink.OAuthRefreshTokenKeyID.Valid { + log.Debug(ctx, "skipping user link", slog.F("user_id", usr.ID), slog.F("current", idx+1)) + continue + } + if _, err := tx.UpdateUserLink(ctx, database.UpdateUserLinkParams{ + OAuthAccessToken: userLink.OAuthAccessToken, + OAuthRefreshToken: userLink.OAuthRefreshToken, + OAuthExpiry: userLink.OAuthExpiry, + UserID: usr.ID, + LoginType: usr.LoginType, + }); err != nil { + return xerrors.Errorf("update user link user_id=%s linked_id=%s: %w", userLink.UserID, userLink.LinkedID, err) + } + } + + gitAuthLinks, err := tx.GetGitAuthLinksByUserID(ctx, usr.ID) + if err != nil { + return xerrors.Errorf("get git auth links for user: %w", err) + } + for _, gitAuthLink := range gitAuthLinks { + if !gitAuthLink.OAuthAccessTokenKeyID.Valid && !gitAuthLink.OAuthRefreshTokenKeyID.Valid { + log.Debug(ctx, "skipping git auth link", slog.F("user_id", usr.ID), slog.F("current", idx+1)) + continue + } + if _, err := tx.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ + ProviderID: gitAuthLink.ProviderID, + UserID: usr.ID, + UpdatedAt: gitAuthLink.UpdatedAt, + OAuthAccessToken: gitAuthLink.OAuthAccessToken, + OAuthRefreshToken: gitAuthLink.OAuthRefreshToken, + OAuthExpiry: gitAuthLink.OAuthExpiry, + }); err != nil { + return xerrors.Errorf("update git auth link user_id=%s provider_id=%s: %w", gitAuthLink.UserID, gitAuthLink.ProviderID, err) + } + } + return nil + }, &sql.TxOptions{ + Isolation: sql.LevelRepeatableRead, + }) + if err != nil { + return xerrors.Errorf("update user links: %w", err) + } + log.Debug(ctx, "decrypted user tokens", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + } + + // Revoke old keys + for _, c := range ciphers[1:] { + if err := db.RevokeDBCryptKey(ctx, c.HexDigest()); err != nil { + return xerrors.Errorf("revoke key: %w", err) + } + log.Info(ctx, "revoked unused key", slog.F("digest", c.HexDigest())) + } + + return nil +} + +// nolint: gosec +const sqlDeleteEncryptedUserTokens = ` +BEGIN; +DELETE FROM user_links + WHERE oauth_access_token_key_id IS NOT NULL + OR oauth_refresh_token_key_id IS NOT NULL; +DELETE FROM git_auth_links + WHERE oauth_access_token_key_id IS NOT NULL + OR oauth_refresh_token_key_id IS NOT NULL; +COMMIT; +` + +// Delete deletes all user tokens and revokes all ciphers. +// This is a destructive operation and should only be used +// as a last resort, for example, if the database encryption key has been +// lost. +func Delete(ctx context.Context, log slog.Logger, sqlDB *sql.DB) error { + res, err := sqlDB.ExecContext(ctx, sqlDeleteEncryptedUserTokens) + if err != nil { + return xerrors.Errorf("delete user links: %w", err) + } + rows, err := res.RowsAffected() + if err != nil { + return xerrors.Errorf("get rows affected: %w", err) + } + log.Info(ctx, "deleted user tokens", slog.F("rows", rows)) + return nil +} diff --git a/enterprise/dbcrypt/rotate.go b/enterprise/dbcrypt/rotate.go deleted file mode 100644 index 812d7c46573a2..0000000000000 --- a/enterprise/dbcrypt/rotate.go +++ /dev/null @@ -1,88 +0,0 @@ -package dbcrypt - -import ( - "context" - "database/sql" - - "golang.org/x/xerrors" - - "cdr.dev/slog" - "github.com/coder/coder/v2/coderd/database" -) - -// Rotate rotates the database encryption keys by re-encrypting all user tokens -// with the first cipher and revoking all other ciphers. -func Rotate(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Cipher) error { - db := database.New(sqlDB) - cryptDB, err := New(ctx, db, ciphers...) - if err != nil { - return xerrors.Errorf("create cryptdb: %w", err) - } - - users, err := cryptDB.GetUsers(ctx, database.GetUsersParams{}) - if err != nil { - return xerrors.Errorf("get users: %w", err) - } - log.Info(ctx, "encrypting user tokens", slog.F("user_count", len(users))) - for idx, usr := range users { - err := cryptDB.InTx(func(tx database.Store) error { - userLinks, err := tx.GetUserLinksByUserID(ctx, usr.ID) - if err != nil { - return xerrors.Errorf("get user links for user: %w", err) - } - for _, userLink := range userLinks { - if userLink.OAuthAccessTokenKeyID.String == ciphers[0].HexDigest() && userLink.OAuthRefreshTokenKeyID.String == ciphers[0].HexDigest() { - log.Debug(ctx, "skipping user link", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) - continue - } - if _, err := tx.UpdateUserLink(ctx, database.UpdateUserLinkParams{ - OAuthAccessToken: userLink.OAuthAccessToken, - OAuthRefreshToken: userLink.OAuthRefreshToken, - OAuthExpiry: userLink.OAuthExpiry, - UserID: usr.ID, - LoginType: usr.LoginType, - }); err != nil { - return xerrors.Errorf("update user link user_id=%s linked_id=%s: %w", userLink.UserID, userLink.LinkedID, err) - } - } - - gitAuthLinks, err := tx.GetGitAuthLinksByUserID(ctx, usr.ID) - if err != nil { - return xerrors.Errorf("get git auth links for user: %w", err) - } - for _, gitAuthLink := range gitAuthLinks { - if gitAuthLink.OAuthAccessTokenKeyID.String == ciphers[0].HexDigest() && gitAuthLink.OAuthRefreshTokenKeyID.String == ciphers[0].HexDigest() { - log.Debug(ctx, "skipping git auth link", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) - continue - } - if _, err := tx.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ - ProviderID: gitAuthLink.ProviderID, - UserID: usr.ID, - UpdatedAt: gitAuthLink.UpdatedAt, - OAuthAccessToken: gitAuthLink.OAuthAccessToken, - OAuthRefreshToken: gitAuthLink.OAuthRefreshToken, - OAuthExpiry: gitAuthLink.OAuthExpiry, - }); err != nil { - return xerrors.Errorf("update git auth link user_id=%s provider_id=%s: %w", gitAuthLink.UserID, gitAuthLink.ProviderID, err) - } - } - return nil - }, &sql.TxOptions{ - Isolation: sql.LevelRepeatableRead, - }) - if err != nil { - return xerrors.Errorf("update user links: %w", err) - } - log.Debug(ctx, "encrypted user tokens", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) - } - - // Revoke old keys - for _, c := range ciphers[1:] { - if err := db.RevokeDBCryptKey(ctx, c.HexDigest()); err != nil { - return xerrors.Errorf("revoke key: %w", err) - } - log.Info(ctx, "revoked unused key", slog.F("digest", c.HexDigest())) - } - - return nil -} From ebf4eefaa9fdf7a3e0d2ced7404f1f5beb7d5ede Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 20:32:08 +0100 Subject: [PATCH 20/34] fixup! add decrypt/delete commands --- enterprise/cli/{server_dbcrypt_rotate.go => server_dbcrypt.go} | 0 .../cli/{server_dbcrypt_rotate_test.go => server_dbcrypt_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename enterprise/cli/{server_dbcrypt_rotate.go => server_dbcrypt.go} (100%) rename enterprise/cli/{server_dbcrypt_rotate_test.go => server_dbcrypt_test.go} (100%) diff --git a/enterprise/cli/server_dbcrypt_rotate.go b/enterprise/cli/server_dbcrypt.go similarity index 100% rename from enterprise/cli/server_dbcrypt_rotate.go rename to enterprise/cli/server_dbcrypt.go diff --git a/enterprise/cli/server_dbcrypt_rotate_test.go b/enterprise/cli/server_dbcrypt_test.go similarity index 100% rename from enterprise/cli/server_dbcrypt_rotate_test.go rename to enterprise/cli/server_dbcrypt_test.go From 2de6cc3cb334dffb51c78faa3ba4963998b4fea3 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 22:21:32 +0100 Subject: [PATCH 21/34] beef up unit tests, refactor cli --- enterprise/cli/server_dbcrypt.go | 259 ++++++++++++++++++-------- enterprise/cli/server_dbcrypt_test.go | 167 +++++++++++++---- enterprise/dbcrypt/cilutil.go | 12 +- 3 files changed, 312 insertions(+), 126 deletions(-) diff --git a/enterprise/cli/server_dbcrypt.go b/enterprise/cli/server_dbcrypt.go index 962357b526986..bedd988886907 100644 --- a/enterprise/cli/server_dbcrypt.go +++ b/enterprise/cli/server_dbcrypt.go @@ -3,16 +3,16 @@ package cli import ( - "bytes" "context" "encoding/base64" + "fmt" + "strings" "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" "github.com/coder/coder/v2/cli" "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" - "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/dbcrypt" "golang.org/x/xerrors" @@ -35,20 +35,10 @@ func (r *RootCmd) dbcryptCmd() *clibase.Cmd { } func (*RootCmd) dbcryptRotateCmd() *clibase.Cmd { - var ( - vals = new(codersdk.DeploymentValues) - opts = vals.Options() - ) + var flags rotateFlags cmd := &clibase.Cmd{ Use: "rotate", Short: "Rotate database encryption keys.", - Options: clibase.OptionSet{ - *opts.ByName("Postgres Connection URL"), - *opts.ByName("External Token Encryption Keys"), - }, - Middleware: clibase.Chain( - clibase.RequireNArgs(0), - ), Handler: func(inv *clibase.Invocation) error { ctx, cancel := context.WithCancel(inv.Context()) defer cancel() @@ -57,38 +47,47 @@ func (*RootCmd) dbcryptRotateCmd() *clibase.Cmd { logger = logger.Leveled(slog.LevelDebug) } - if vals.PostgresURL == "" { - return xerrors.Errorf("no database configured") + if err := flags.valid(); err != nil { + return err } - switch len(vals.ExternalTokenEncryptionKeys) { - case 0: - return xerrors.Errorf("no external token encryption keys provided") - case 1: - logger.Info(ctx, "only one key provided, data will be re-encrypted with the same key") + ks := [][]byte{} + dk, err := base64.StdEncoding.DecodeString(flags.New) + if err != nil { + return xerrors.Errorf("decode new key: %w", err) } + ks = append(ks, dk) - keys := make([][]byte, 0, len(vals.ExternalTokenEncryptionKeys)) - var newKey []byte - for idx, ek := range vals.ExternalTokenEncryptionKeys { - dk, err := base64.StdEncoding.DecodeString(ek) + for _, k := range flags.Old { + dk, err := base64.StdEncoding.DecodeString(k) if err != nil { - return xerrors.Errorf("key must be base64-encoded") - } - if idx == 0 { - newKey = dk - } else if bytes.Equal(dk, newKey) { - return xerrors.Errorf("old key at index %d is the same as the new key", idx) + return xerrors.Errorf("decode old key: %w", err) } - keys = append(keys, dk) + ks = append(ks, dk) } - ciphers, err := dbcrypt.NewCiphers(keys...) + ciphers, err := dbcrypt.NewCiphers(ks...) if err != nil { return xerrors.Errorf("create ciphers: %w", err) } - sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", vals.PostgresURL.Value()) + newDigest := ciphers[0].HexDigest() + oldDigests := make([]string, 0, len(ciphers)-1) + for _, c := range ciphers[1:] { + oldDigests = append(oldDigests, c.HexDigest()) + } + if len(oldDigests) == 0 { + oldDigests = append(oldDigests, "none") + } + msg := fmt.Sprintf(`Rotate external token encryptions keys?\n- New key: %s\n- Old keys: %s`, + newDigest, + strings.Join(oldDigests, ", "), + ) + if _, err := cliui.Prompt(inv, cliui.PromptOptions{Text: msg, IsConfirm: true}); err != nil { + return err + } + + sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", flags.PostgresURL) if err != nil { return xerrors.Errorf("connect to postgres: %w", err) } @@ -103,24 +102,15 @@ func (*RootCmd) dbcryptRotateCmd() *clibase.Cmd { return nil }, } + flags.attach(&cmd.Options) return cmd } func (*RootCmd) dbcryptDecryptCmd() *clibase.Cmd { - var ( - vals = new(codersdk.DeploymentValues) - opts = vals.Options() - ) + var flags decryptFlags cmd := &clibase.Cmd{ Use: "decrypt", Short: "Decrypt a previously encrypted database.", - Options: clibase.OptionSet{ - *opts.ByName("Postgres Connection URL"), - *opts.ByName("External Token Encryption Keys"), - }, - Middleware: clibase.Chain( - clibase.RequireNArgs(0), - ), Handler: func(inv *clibase.Invocation) error { ctx, cancel := context.WithCancel(inv.Context()) defer cancel() @@ -129,38 +119,32 @@ func (*RootCmd) dbcryptDecryptCmd() *clibase.Cmd { logger = logger.Leveled(slog.LevelDebug) } - if vals.PostgresURL == "" { - return xerrors.Errorf("no database configured") - } - - switch len(vals.ExternalTokenEncryptionKeys) { - case 0: - return xerrors.Errorf("no external token encryption keys provided") - case 1: - logger.Info(ctx, "only one key provided, data will be re-encrypted with the same key") + if err := flags.valid(); err != nil { + return err } - keys := make([][]byte, 0, len(vals.ExternalTokenEncryptionKeys)) - var newKey []byte - for idx, ek := range vals.ExternalTokenEncryptionKeys { - dk, err := base64.StdEncoding.DecodeString(ek) + ks := make([][]byte, 0, len(flags.Keys)) + for _, k := range flags.Keys { + dk, err := base64.StdEncoding.DecodeString(k) if err != nil { - return xerrors.Errorf("key must be base64-encoded") + return xerrors.Errorf("decode key: %w", err) } - if idx == 0 { - newKey = dk - } else if bytes.Equal(dk, newKey) { - return xerrors.Errorf("old key at index %d is the same as the new key", idx) - } - keys = append(keys, dk) + ks = append(ks, dk) } - ciphers, err := dbcrypt.NewCiphers(keys...) + ciphers, err := dbcrypt.NewCiphers(ks...) if err != nil { return xerrors.Errorf("create ciphers: %w", err) } - sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", vals.PostgresURL.Value()) + if _, err := cliui.Prompt(inv, cliui.PromptOptions{ + Text: "This will decrypt all encrypted data in the database. Are you sure you want to continue?", + IsConfirm: true, + }); err != nil { + return err + } + + sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", flags.PostgresURL) if err != nil { return xerrors.Errorf("connect to postgres: %w", err) } @@ -175,23 +159,15 @@ func (*RootCmd) dbcryptDecryptCmd() *clibase.Cmd { return nil }, } + flags.attach(&cmd.Options) return cmd } func (*RootCmd) dbcryptDeleteCmd() *clibase.Cmd { - var ( - vals = new(codersdk.DeploymentValues) - opts = vals.Options() - ) + var flags deleteFlags cmd := &clibase.Cmd{ Use: "delete", Short: "Delete all encrypted data from the database. THIS IS A DESTRUCTIVE OPERATION.", - Options: clibase.OptionSet{ - *opts.ByName("Postgres Connection URL"), - }, - Middleware: clibase.Chain( - clibase.RequireNArgs(0), - ), Handler: func(inv *clibase.Invocation) error { ctx, cancel := context.WithCancel(inv.Context()) defer cancel() @@ -200,8 +176,8 @@ func (*RootCmd) dbcryptDeleteCmd() *clibase.Cmd { logger = logger.Leveled(slog.LevelDebug) } - if vals.PostgresURL == "" { - return xerrors.Errorf("no database configured") + if err := flags.valid(); err != nil { + return err } if _, err := cliui.Prompt(inv, cliui.PromptOptions{ @@ -211,7 +187,7 @@ func (*RootCmd) dbcryptDeleteCmd() *clibase.Cmd { return err } - sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", vals.PostgresURL.Value()) + sqlDB, err := cli.ConnectToPostgres(inv.Context(), logger, "postgres", flags.PostgresURL) if err != nil { return xerrors.Errorf("connect to postgres: %w", err) } @@ -226,5 +202,130 @@ func (*RootCmd) dbcryptDeleteCmd() *clibase.Cmd { return nil }, } + flags.attach(&cmd.Options) return cmd } + +type rotateFlags struct { + PostgresURL string + New string + Old []string +} + +func (f *rotateFlags) attach(opts *clibase.OptionSet) { + *opts = append( + *opts, + clibase.Option{ + Flag: "postgres-url", + Env: "CODER_PG_CONNECTION_URL", + Description: "The connection URL for the Postgres database.", + Value: clibase.StringOf(&f.PostgresURL), + }, + clibase.Option{ + Flag: "new-key", + Env: "CODER_EXTERNAL_TOKEN_ENCRYPTION_ENCRYPT_NEW_KEY", + Description: "The new external token encryption key. Must be base64-encoded.", + Value: clibase.StringOf(&f.New), + }, + clibase.Option{ + Flag: "old-keys", + Env: "CODER_EXTERNAL_TOKEN_ENCRYPTION_ENCRYPT_OLD_KEYS", + Description: "The old external token encryption keys. Must be a comma-separated list of base64-encoded keys.", + Value: clibase.StringArrayOf(&f.Old), + }, + cliui.SkipPromptOption(), + ) +} + +func (f *rotateFlags) valid() error { + if f.New == "" { + return xerrors.Errorf("no new key provided") + } + + if val, err := base64.StdEncoding.DecodeString(f.New); err != nil { + return xerrors.Errorf("new key must be base64-encoded") + } else if len(val) != 32 { + return xerrors.Errorf("new key must be exactly 32 bytes in length") + } + + for i, k := range f.Old { + if val, err := base64.StdEncoding.DecodeString(k); err != nil { + return xerrors.Errorf("old key at index %d must be base64-encoded", i) + } else if len(val) != 32 { + return xerrors.Errorf("old key at index %d must be exactly 32 bytes in length", i) + } + + // Pedantic, but typos here will ruin your day. + if k == f.New { + return xerrors.Errorf("old key at index %d is the same as the new key", i) + } + } + + return nil +} + +type decryptFlags struct { + PostgresURL string + Keys []string +} + +func (f *decryptFlags) attach(opts *clibase.OptionSet) { + *opts = append( + *opts, + clibase.Option{ + Flag: "postgres-url", + Env: "CODER_PG_CONNECTION_URL", + Description: "The connection URL for the Postgres database.", + Value: clibase.StringOf(&f.PostgresURL), + }, + clibase.Option{ + Flag: "keys", + Env: "CODER_EXTERNAL_TOKEN_ENCRYPTION_DECRYPT_KEYS", + Description: "Keys required to decrypt existing data. Must be a comma-separated list of base64-encoded keys.", + Value: clibase.StringArrayOf(&f.Keys), + }, + cliui.SkipPromptOption(), + ) +} + +func (f *decryptFlags) valid() error { + if len(f.Keys) == 0 { + return xerrors.Errorf("no keys provided") + } + + for i, k := range f.Keys { + if val, err := base64.StdEncoding.DecodeString(k); err != nil { + return xerrors.Errorf("key at index %d must be base64-encoded", i) + } else if len(val) != 32 { + return xerrors.Errorf("key at index %d must be exactly 32 bytes in length", i) + } + } + + return nil +} + +type deleteFlags struct { + PostgresURL string + Confirm bool +} + +func (f *deleteFlags) attach(opts *clibase.OptionSet) { + *opts = append( + *opts, + clibase.Option{ + Flag: "postgres-url", + Env: "CODER_EXTERNAL_TOKEN_ENCRYPTION_POSTGRES_URL", + Description: "The connection URL for the Postgres database.", + Value: clibase.StringOf(&f.PostgresURL), + }, + cliui.SkipPromptOption(), + ) +} + +func (f *deleteFlags) valid() error { + if f.PostgresURL == "" { + return xerrors.Errorf("no database configured") + } + + return nil +} diff --git a/enterprise/cli/server_dbcrypt_test.go b/enterprise/cli/server_dbcrypt_test.go index a12317a6be93a..839880b845287 100644 --- a/enterprise/cli/server_dbcrypt_test.go +++ b/enterprise/cli/server_dbcrypt_test.go @@ -4,9 +4,9 @@ import ( "context" "database/sql" "encoding/base64" - "fmt" "testing" + "github.com/google/uuid" "github.com/lib/pq" "github.com/stretchr/testify/require" "golang.org/x/xerrors" @@ -21,19 +21,19 @@ import ( ) // nolint: paralleltest // use of t.Setenv -func TestDBCryptRotate(t *testing.T) { +func TestServerDBCrypt(t *testing.T) { if !dbtestutil.WillUsePostgres() { t.Skip("this test requires a postgres instance") } - // + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) - // + // Setup a postgres database. connectionURL, closePg, err := postgres.Open() require.NoError(t, err) t.Cleanup(closePg) - // + sqlDB, err := sql.Open("postgres", connectionURL) require.NoError(t, err) t.Cleanup(func() { @@ -52,7 +52,8 @@ func TestDBCryptRotate(t *testing.T) { // Encrypt all the data with the initial cipher. inv, _ := newCLI(t, "server", "dbcrypt", "rotate", "--postgres-url", connectionURL, - "--external-token-encryption-keys", base64.StdEncoding.EncodeToString([]byte(keyA)), + "--new-key", base64.StdEncoding.EncodeToString([]byte(keyA)), + "--yes", ) pty := ptytest.New(t) inv.Stdout = pty.Output() @@ -60,7 +61,9 @@ func TestDBCryptRotate(t *testing.T) { require.NoError(t, err) // Validate that all existing data has been encrypted with cipher A. - requireDataDecryptsWithCipher(ctx, t, db, cipherA[0], users) + for _, usr := range users { + requireEncryptedWithCipher(ctx, t, db, cipherA[0], usr.ID) + } // Create an encrypted database cryptdb, err := dbcrypt.New(ctx, db, cipherA...) @@ -73,15 +76,12 @@ func TestDBCryptRotate(t *testing.T) { keyB := mustString(t, 32) cipherBA, err := dbcrypt.NewCiphers([]byte(keyB), []byte(keyA)) require.NoError(t, err) - externalTokensArg := fmt.Sprintf( - "%s,%s", - base64.StdEncoding.EncodeToString([]byte(keyB)), - base64.StdEncoding.EncodeToString([]byte(keyA)), - ) inv, _ = newCLI(t, "server", "dbcrypt", "rotate", "--postgres-url", connectionURL, - "--external-token-encryption-keys", externalTokensArg, + "--new-key", base64.StdEncoding.EncodeToString([]byte(keyB)), + "--old-keys", base64.StdEncoding.EncodeToString([]byte(keyA)), + "--yes", ) pty = ptytest.New(t) inv.Stdout = pty.Output() @@ -89,7 +89,9 @@ func TestDBCryptRotate(t *testing.T) { require.NoError(t, err) // Validate that all data has been re-encrypted with cipher B. - requireDataDecryptsWithCipher(ctx, t, db, cipherBA[0], users) + for _, usr := range users { + requireEncryptedWithCipher(ctx, t, db, cipherBA[0], usr.ID) + } // Assert that we can revoke the old key. err = db.RevokeDBCryptKey(ctx, cipherA[0].HexDigest()) @@ -112,6 +114,72 @@ func TestDBCryptRotate(t *testing.T) { var pgErr *pq.Error require.True(t, xerrors.As(err, &pgErr), "expected a pg error") require.EqualValues(t, "23503", pgErr.Code, "expected a foreign key constraint violation error") + + // Decrypt the data using only cipher B. This should result in the key being revoked. + inv, _ = newCLI(t, "server", "dbcrypt", "decrypt", + "--postgres-url", connectionURL, + "--keys", base64.StdEncoding.EncodeToString([]byte(keyB)), + "--yes", + ) + pty = ptytest.New(t) + inv.Stdout = pty.Output() + err = inv.Run() + require.NoError(t, err) + + // Validate that both keys have been revoked. + keys, err = db.GetDBCryptKeys(ctx) + require.NoError(t, err, "failed to get db crypt keys") + require.Len(t, keys, 2, "expected exactly 2 keys") + for _, key := range keys { + require.Empty(t, key.ActiveKeyDigest.String, "expected the new key to not be active") + } + + // Validate that all data has been decrypted. + for _, usr := range users { + requireEncryptedWithCipher(ctx, t, db, &nullCipher{}, usr.ID) + } + + // Re-encrypt all existing data with a new cipher. + keyC := mustString(t, 32) + cipherC, err := dbcrypt.NewCiphers([]byte(keyC)) + require.NoError(t, err) + + inv, _ = newCLI(t, "server", "dbcrypt", "rotate", + "--postgres-url", connectionURL, + "--new-key", base64.StdEncoding.EncodeToString([]byte(keyC)), + "--yes", + ) + + pty = ptytest.New(t) + inv.Stdout = pty.Output() + err = inv.Run() + require.NoError(t, err) + + // Validate that all data has been re-encrypted with cipher C. + for _, usr := range users { + requireEncryptedWithCipher(ctx, t, db, cipherC[0], usr.ID) + } + + // Now delete all the encrypted data. + inv, _ = newCLI(t, "server", "dbcrypt", "delete", + "--postgres-url", connectionURL, + "--external-token-encryption-keys", base64.StdEncoding.EncodeToString([]byte(keyC)), + "--yes", + ) + pty = ptytest.New(t) + inv.Stdout = pty.Output() + err = inv.Run() + require.NoError(t, err) + + // Assert that no user links remain. + for _, usr := range users { + userLinks, err := db.GetUserLinksByUserID(ctx, usr.ID) + require.NoError(t, err, "failed to get user links for user %s", usr.ID) + require.Empty(t, userLinks) + gitAuthLinks, err := db.GetGitAuthLinksByUserID(ctx, usr.ID) + require.NoError(t, err, "failed to get git auth links for user %s", usr.ID) + require.Empty(t, gitAuthLinks) + } } func genData(t *testing.T, db database.Store, n int) []database.User { @@ -124,14 +192,14 @@ func genData(t *testing.T, db database.Store, n int) []database.User { _ = dbgen.UserLink(t, db, database.UserLink{ UserID: usr.ID, LoginType: usr.LoginType, - OAuthAccessToken: mustString(t, 16), - OAuthRefreshToken: mustString(t, 16), + OAuthAccessToken: "access-" + usr.ID.String(), + OAuthRefreshToken: "refresh-" + usr.ID.String(), }) _ = dbgen.GitAuthLink(t, db, database.GitAuthLink{ UserID: usr.ID, ProviderID: "fake", - OAuthAccessToken: mustString(t, 16), - OAuthRefreshToken: mustString(t, 16), + OAuthAccessToken: "access-" + usr.ID.String(), + OAuthRefreshToken: "refresh-" + usr.ID.String(), }) users = append(users, usr) } @@ -145,35 +213,56 @@ func mustString(t *testing.T, n int) string { return s } -func requireDecryptWithCipher(t *testing.T, c dbcrypt.Cipher, s string) { +func requireEncryptedEquals(t *testing.T, c dbcrypt.Cipher, expected, actual string) { t.Helper() - decodedVal, err := base64.StdEncoding.DecodeString(s) - require.NoError(t, err, "failed to decode base64 string") - _, err = c.Decrypt(decodedVal) + var decodedVal []byte + var err error + if _, ok := c.(*nullCipher); !ok { + decodedVal, err = base64.StdEncoding.DecodeString(actual) + require.NoError(t, err, "failed to decode base64 string") + } else { + // If a nullCipher is being used, we expect the value not to be encrypted. + decodedVal = []byte(actual) + } + val, err := c.Decrypt(decodedVal) require.NoError(t, err, "failed to decrypt value") + require.Equal(t, expected, string(val)) } -func requireDataDecryptsWithCipher(ctx context.Context, t *testing.T, db database.Store, c dbcrypt.Cipher, users []database.User) { +func requireEncryptedWithCipher(ctx context.Context, t *testing.T, db database.Store, c dbcrypt.Cipher, userID uuid.UUID) { t.Helper() - for _, usr := range users { - ul, err := db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{ - UserID: usr.ID, - LoginType: usr.LoginType, - }) - require.NoError(t, err, "failed to get user link for user %s", usr.ID) - requireDecryptWithCipher(t, c, ul.OAuthAccessToken) - requireDecryptWithCipher(t, c, ul.OAuthRefreshToken) + userLinks, err := db.GetUserLinksByUserID(ctx, userID) + require.NoError(t, err, "failed to get user links for user %s", userID) + for _, ul := range userLinks { + requireEncryptedEquals(t, c, "access-"+userID.String(), ul.OAuthAccessToken) + requireEncryptedEquals(t, c, "refresh-"+userID.String(), ul.OAuthRefreshToken) require.Equal(t, c.HexDigest(), ul.OAuthAccessTokenKeyID.String) require.Equal(t, c.HexDigest(), ul.OAuthRefreshTokenKeyID.String) - // - gal, err := db.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ - UserID: usr.ID, - ProviderID: "fake", - }) - require.NoError(t, err, "failed to get git auth link for user %s", usr.ID) - requireDecryptWithCipher(t, c, gal.OAuthAccessToken) - requireDecryptWithCipher(t, c, gal.OAuthRefreshToken) + } + gitAuthLinks, err := db.GetGitAuthLinksByUserID(ctx, userID) + require.NoError(t, err, "failed to get git auth links for user %s", userID) + for _, gal := range gitAuthLinks { + requireEncryptedEquals(t, c, "access-"+userID.String(), gal.OAuthAccessToken) + requireEncryptedEquals(t, c, "refresh-"+userID.String(), gal.OAuthRefreshToken) require.Equal(t, c.HexDigest(), gal.OAuthAccessTokenKeyID.String) require.Equal(t, c.HexDigest(), gal.OAuthRefreshTokenKeyID.String) } } + +// nullCipher is a dbcrypt.Cipher that does not encrypt or decrypt. +// used for testing +type nullCipher struct{} + +func (*nullCipher) Encrypt(b []byte) ([]byte, error) { + return b, nil +} + +func (*nullCipher) Decrypt(b []byte) ([]byte, error) { + return b, nil +} + +func (*nullCipher) HexDigest() string { + return "" // This co-incidentally happens to be the value of sql.NullString{}.String... +} + +var _ dbcrypt.Cipher = (*nullCipher)(nil) diff --git a/enterprise/dbcrypt/cilutil.go b/enterprise/dbcrypt/cilutil.go index bc72064806e32..88a318a940502 100644 --- a/enterprise/dbcrypt/cilutil.go +++ b/enterprise/dbcrypt/cilutil.go @@ -160,8 +160,8 @@ func Decrypt(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Ciph log.Debug(ctx, "decrypted user tokens", slog.F("user_id", usr.ID), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) } - // Revoke old keys - for _, c := range ciphers[1:] { + // Revoke _all_ keys + for _, c := range ciphers { if err := db.RevokeDBCryptKey(ctx, c.HexDigest()); err != nil { return xerrors.Errorf("revoke key: %w", err) } @@ -188,14 +188,10 @@ COMMIT; // as a last resort, for example, if the database encryption key has been // lost. func Delete(ctx context.Context, log slog.Logger, sqlDB *sql.DB) error { - res, err := sqlDB.ExecContext(ctx, sqlDeleteEncryptedUserTokens) + _, err := sqlDB.ExecContext(ctx, sqlDeleteEncryptedUserTokens) if err != nil { return xerrors.Errorf("delete user links: %w", err) } - rows, err := res.RowsAffected() - if err != nil { - return xerrors.Errorf("get rows affected: %w", err) - } - log.Info(ctx, "deleted user tokens", slog.F("rows", rows)) + log.Info(ctx, "deleted encrypted user tokens") return nil } From 777481100f5d71d997615ec09819dc26f8082684 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 22:22:18 +0100 Subject: [PATCH 22/34] update golden files --- .../coder_server_dbcrypt_--help.golden | 5 +++- ...coder_server_dbcrypt_decrypt_--help.golden | 17 +++++++++++++ .../coder_server_dbcrypt_delete_--help.golden | 15 ++++++++++++ .../coder_server_dbcrypt_rotate_--help.golden | 24 ++++++++----------- 4 files changed, 46 insertions(+), 15 deletions(-) create mode 100644 enterprise/cli/testdata/coder_server_dbcrypt_decrypt_--help.golden create mode 100644 enterprise/cli/testdata/coder_server_dbcrypt_delete_--help.golden diff --git a/enterprise/cli/testdata/coder_server_dbcrypt_--help.golden b/enterprise/cli/testdata/coder_server_dbcrypt_--help.golden index a7d758c8513b0..f74fed93b74a9 100644 --- a/enterprise/cli/testdata/coder_server_dbcrypt_--help.golden +++ b/enterprise/cli/testdata/coder_server_dbcrypt_--help.golden @@ -3,7 +3,10 @@ Usage: coder server dbcrypt Manage database encryption. Subcommands - rotate Rotate database encryption keys. + decrypt Decrypt a previously encrypted database. + delete Delete all encrypted data from the database. THIS IS A + DESTRUCTIVE OPERATION. + rotate Rotate database encryption keys. --- Run `coder --help` for a list of global options. diff --git a/enterprise/cli/testdata/coder_server_dbcrypt_decrypt_--help.golden b/enterprise/cli/testdata/coder_server_dbcrypt_decrypt_--help.golden new file mode 100644 index 0000000000000..761e1aa6f5b4e --- /dev/null +++ b/enterprise/cli/testdata/coder_server_dbcrypt_decrypt_--help.golden @@ -0,0 +1,17 @@ +Usage: coder server dbcrypt decrypt [flags] + +Decrypt a previously encrypted database. + +Options + --keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_DECRYPT_KEYS + Keys required to decrypt existing data. Must be a comma-separated list + of base64-encoded keys. + + --postgres-url string, $CODER_PG_CONNECTION_URL + The connection URL for the Postgres database. + + -y, --yes bool + Bypass prompts. + +--- +Run `coder --help` for a list of global options. diff --git a/enterprise/cli/testdata/coder_server_dbcrypt_delete_--help.golden b/enterprise/cli/testdata/coder_server_dbcrypt_delete_--help.golden new file mode 100644 index 0000000000000..619282db58e6e --- /dev/null +++ b/enterprise/cli/testdata/coder_server_dbcrypt_delete_--help.golden @@ -0,0 +1,15 @@ +Usage: coder server dbcrypt delete [flags] + +Delete all encrypted data from the database. THIS IS A DESTRUCTIVE OPERATION. + +Aliases: rm + +Options + --postgres-url string, $CODER_EXTERNAL_TOKEN_ENCRYPTION_POSTGRES_URL + The connection URL for the Postgres database. + + -y, --yes bool + Bypass prompts. + +--- +Run `coder --help` for a list of global options. diff --git a/enterprise/cli/testdata/coder_server_dbcrypt_rotate_--help.golden b/enterprise/cli/testdata/coder_server_dbcrypt_rotate_--help.golden index 4ae79673d8c91..203831631c15d 100644 --- a/enterprise/cli/testdata/coder_server_dbcrypt_rotate_--help.golden +++ b/enterprise/cli/testdata/coder_server_dbcrypt_rotate_--help.golden @@ -3,22 +3,18 @@ Usage: coder server dbcrypt rotate [flags] Rotate database encryption keys. Options - --postgres-url string, $CODER_PG_CONNECTION_URL - URL of a PostgreSQL database. If empty, PostgreSQL binaries will be - downloaded from Maven (https://repo1.maven.org/maven2) and store all - data in the config root. Access the built-in database with "coder - server postgres-builtin-url". + --new-key string, $CODER_EXTERNAL_TOKEN_ENCRYPTION_ENCRYPT_NEW_KEY + The new external token encryption key. Must be base64-encoded. + + --old-keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_ENCRYPT_OLD_KEYS + The old external token encryption keys. Must be a comma-separated list + of base64-encoded keys. -Enterprise Options -These options are only available in the Enterprise Edition. + --postgres-url string, $CODER_PG_CONNECTION_URL + The connection URL for the Postgres database. - --external-token-encryption-keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS - Encrypt OIDC and Git authentication tokens with AES-256-GCM in the - database. The value must be a comma-separated list of base64-encoded - keys. Each key, when base64-decoded, must be exactly 32 bytes in - length. The first key will be used to encrypt new values. Subsequent - keys will be used as a fallback when decrypting. During normal - operation it is recommended to only set one key. + -y, --yes bool + Bypass prompts. --- Run `coder --help` for a list of global options. From 35ca78f9b7202ab2fc11f6f19c691d5c33a1b5f7 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 6 Sep 2023 22:24:33 +0100 Subject: [PATCH 23/34] Update codersdk/deployment.go Co-authored-by: Dean Sheather --- codersdk/deployment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 87aa63825a123..dcbe2a1e94679 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1611,7 +1611,7 @@ when required by your organization's security policy.`, }, { Name: "External Token Encryption Keys", - Description: "Encrypt OIDC and Git authentication tokens with AES-256-GCM in the database. The value must be a comma-separated list of base64-encoded keys. Each key, when base64-decoded, must be exactly 32 bytes in length. The first key will be used to encrypt new values. Subsequent keys will be used as a fallback when decrypting. During normal operation it is recommended to only set one key.", + Description: "Encrypt OIDC and Git authentication tokens with AES-256-GCM in the database. The value must be a comma-separated list of base64-encoded keys. Each key, when base64-decoded, must be exactly 32 bytes in length. The first key will be used to encrypt new values. Subsequent keys will be used as a fallback when decrypting. During normal operation it is recommended to only set one key unless you are in the process of rotating keys with the `coder server dbcrypt rotate` command.", Flag: "external-token-encryption-keys", Env: "CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS", Annotations: clibase.Annotations{}.Mark(annotationEnterpriseKey, "true").Mark(annotationSecretKey, "true"), From 8b1f43cc37f9bd375e368c80465ae1ae28c1d391 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 09:21:43 +0100 Subject: [PATCH 24/34] revoke all active keys on dbcrypt delete --- enterprise/cli/server_dbcrypt_test.go | 9 +++++++++ enterprise/dbcrypt/{cilutil.go => cliutil.go} | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) rename enterprise/dbcrypt/{cilutil.go => cliutil.go} (93%) diff --git a/enterprise/cli/server_dbcrypt_test.go b/enterprise/cli/server_dbcrypt_test.go index 839880b845287..cebf014a7ce58 100644 --- a/enterprise/cli/server_dbcrypt_test.go +++ b/enterprise/cli/server_dbcrypt_test.go @@ -180,6 +180,15 @@ func TestServerDBCrypt(t *testing.T) { require.NoError(t, err, "failed to get git auth links for user %s", usr.ID) require.Empty(t, gitAuthLinks) } + + // Validate that the key has been revoked in the database. + keys, err = db.GetDBCryptKeys(ctx) + require.NoError(t, err, "failed to get db crypt keys") + require.Len(t, keys, 3, "expected exactly 3 keys") + for _, k := range keys { + require.Empty(t, k.ActiveKeyDigest.String, "expected the key to not be active") + require.NotEmpty(t, k.RevokedKeyDigest.String, "expected the key to be revoked") + } } func genData(t *testing.T, db database.Store, n int) []database.User { diff --git a/enterprise/dbcrypt/cilutil.go b/enterprise/dbcrypt/cliutil.go similarity index 93% rename from enterprise/dbcrypt/cilutil.go rename to enterprise/dbcrypt/cliutil.go index 88a318a940502..7f68e284afe77 100644 --- a/enterprise/dbcrypt/cilutil.go +++ b/enterprise/dbcrypt/cliutil.go @@ -188,10 +188,27 @@ COMMIT; // as a last resort, for example, if the database encryption key has been // lost. func Delete(ctx context.Context, log slog.Logger, sqlDB *sql.DB) error { + store := database.New(sqlDB) _, err := sqlDB.ExecContext(ctx, sqlDeleteEncryptedUserTokens) if err != nil { return xerrors.Errorf("delete user links: %w", err) } log.Info(ctx, "deleted encrypted user tokens") + + log.Info(ctx, "revoking all active keys") + keys, err := store.GetDBCryptKeys(ctx) + if err != nil { + return xerrors.Errorf("get db crypt keys: %w", err) + } + for _, k := range keys { + if !k.ActiveKeyDigest.Valid { + continue + } + if err := store.RevokeDBCryptKey(ctx, k.ActiveKeyDigest.String); err != nil { + return xerrors.Errorf("revoke key: %w", err) + } + log.Info(ctx, "revoked unused key", slog.F("digest", k.ActiveKeyDigest.String)) + } + return nil } From 270cdc13250c872c5a5670ef76f00293947923c3 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 09:30:11 +0100 Subject: [PATCH 25/34] fixup! Merge remote-tracking branch 'origin/main' into cj/dbcrypt_redux_2 --- site/src/api/typesGenerated.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 5bea0a50f58bb..f7511276a081e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -372,19 +372,6 @@ export interface DeploymentValues { readonly secure_auth_cookie?: boolean; readonly strict_transport_security?: number; // This is likely an enum in an external package ("github.com/coder/coder/v2/cli/clibase.StringArray") -<<<<<<< HEAD - readonly strict_transport_security_options?: string[] - readonly ssh_keygen_algorithm?: string - readonly metrics_cache_refresh_interval?: number - readonly agent_stat_refresh_interval?: number - readonly agent_fallback_troubleshooting_url?: string - readonly browser_only?: boolean - readonly scim_api_key?: string - // This is likely an enum in an external package ("github.com/coder/coder/v2/cli/clibase.StringArray") - readonly external_token_encryption_keys?: string[] - readonly provisioner?: ProvisionerConfig - readonly rate_limit?: RateLimitConfig -======= readonly strict_transport_security_options?: string[]; readonly ssh_keygen_algorithm?: string; readonly metrics_cache_refresh_interval?: number; @@ -392,9 +379,10 @@ export interface DeploymentValues { readonly agent_fallback_troubleshooting_url?: string; readonly browser_only?: boolean; readonly scim_api_key?: string; + // This is likely an enum in an external package ("github.com/coder/coder/v2/cli/clibase.StringArray") + readonly external_token_encryption_keys?: string[]; readonly provisioner?: ProvisionerConfig; readonly rate_limit?: RateLimitConfig; ->>>>>>> origin/main // This is likely an enum in an external package ("github.com/coder/coder/v2/cli/clibase.StringArray") readonly experiments?: string[]; readonly update_check?: boolean; From 2514ffefc4b2a6b0ddae0bd3bae6abfbc2117d6f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 09:36:42 +0100 Subject: [PATCH 26/34] update docs --- docs/admin/encryption.md | 58 +++++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/docs/admin/encryption.md b/docs/admin/encryption.md index 15a15ebb00b67..2cb197161cbae 100644 --- a/docs/admin/encryption.md +++ b/docs/admin/encryption.md @@ -37,8 +37,8 @@ Additional database fields may be encrypted in the future. > Encryption keys in use are stored in `dbcrypt_keys`. This table stores a > record of all encryption keys that have been used to encrypt data. Active keys > have a null `revoked_key_id` column, and revoked keys have a non-null -> `revoked_key_id` column. A key cannot be revoked until all rows referring to -> it have been re-encrypted with a different key. +> `revoked_key_id` column. You cannot revoke a key until you have rotated all +> values using that key to a new key. ## Enabling encryption @@ -74,6 +74,9 @@ coder: key: keys ``` +1. Restart the Coder server. The server will now encrypt all new data with the + provided key. + ## Rotating keys We recommend only having one active encryption key at a time normally. However, @@ -104,10 +107,9 @@ data: encrypted with the old key(s). 1. To re-encrypt all encrypted database fields with the new key, run - [`coder dbcrypt-rotate`](../cli/dbcrypt-rotate.md). This command will - re-encrypt all tokens with the first key in the list of external token - encryption keys. We recommend performing this action during a maintenance - window. + [`coder server dbcrypt rotate`](../cli/server_dbcrypt_rotate.md). This + command will re-encrypt all tokens with the specified new encryption key. We + recommend performing this action during a maintenance window. > Note: this command requires direct access to the database. If you are using > the built-in PostgreSQL database, you can run @@ -120,10 +122,52 @@ data: ## Disabling encryption -Disabling encryption is currently not supported. +To disable encryption, perform the following actions: + +1. Ensure you have a valid backup of your database. **Do not skip this step.** + +1. Stop all active coderd instances. This will prevent new encrypted data from + being written. + +1. Run [`coder server dbcrypt decrypt`](../cli/server_dbcrypt_decrypt.md). This + command will decrypt all encrypted user tokens and revoke all active + encryption keys. + +1. Remove all + [external token encryption keys](../cli/server.md#external-token-encryption-keys) + from Coder's configuration. + +1. Start coderd. You can now safely delete the encryption keys from your secret + store. + +## Deleting Encrypted Data + +> NOTE: This is a destructive operation. + +To delete all encrypted data from your database, perform the following actions: + +1. Ensure you have a valid backup of your database. **Do not skip this step.** + +1. Stop all active coderd instances. This will prevent new encrypted data from + being written. + +1. Run [`coder server dbcrypt delete`](../cli/server_dbcrypt_delete.md). This + command will delete all encrypted user tokens and revoke all active + encryption keys. + +1. Remove all + [external token encryption keys](../cli/server.md#external-token-encryption-keys) + from Coder's configuration. + +1. Start coderd. You can now safely delete the encryption keys from your secret + store. ## Troubleshooting - If Coder detects that the data stored in the database was not encrypted with any known keys, it will refuse to start. If you are seeing this behaviour, ensure that the encryption keys provided are correct. +- If Coder detects that the data stored in the database was encrypted with a key + that is no longer active, it will refuse to start. If you are seeing this + behaviour, ensure that the encryption keys provided are correct and that you + have not revoked any keys that are still in use. From cd351afa789d3c019dc03ac685ece59277311836 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 09:39:03 +0100 Subject: [PATCH 27/34] fixup! update docs --- docs/admin/encryption.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/admin/encryption.md b/docs/admin/encryption.md index 2cb197161cbae..01e0cf9e71361 100644 --- a/docs/admin/encryption.md +++ b/docs/admin/encryption.md @@ -6,14 +6,13 @@ preventing attackers with database access from using them to impersonate users. ## How it works -Coder allows administrators to specify up to two -[external token encryption keys](../cli/server.md#external-token-encryption-keys). +Coder allows administrators to specify [external token encryption keys](../cli/server.md#external-token-encryption-keys). If configured, Coder will use these keys to encrypt external user tokens before storing them in the database. The encryption algorithm used is AES-256-GCM with a 32-byte key length. -Coder will use the first key provided for both encryption and decryption. If a -second key is provided, Coder will use it for decryption only. This allows +Coder will use the first key provided for both encryption and decryption. If +additional keys are provided, Coder will use it for decryption only. This allows administrators to rotate encryption keys without invalidating existing tokens. The following database fields are currently encrypted: From 2f5c112779b237399f140018153042f6fdbe7c1c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 12:02:47 +0100 Subject: [PATCH 28/34] fixup! update docs --- docs/admin/encryption.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/admin/encryption.md b/docs/admin/encryption.md index 01e0cf9e71361..1a5f0f6c08fe9 100644 --- a/docs/admin/encryption.md +++ b/docs/admin/encryption.md @@ -6,7 +6,8 @@ preventing attackers with database access from using them to impersonate users. ## How it works -Coder allows administrators to specify [external token encryption keys](../cli/server.md#external-token-encryption-keys). +Coder allows administrators to specify +[external token encryption keys](../cli/server.md#external-token-encryption-keys). If configured, Coder will use these keys to encrypt external user tokens before storing them in the database. The encryption algorithm used is AES-256-GCM with a 32-byte key length. From 2450d139ebaf280fb71a6c30246629a269e77238 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 12:03:40 +0100 Subject: [PATCH 29/34] soft-enforce dbcrypt in license --- enterprise/coderd/coderd.go | 43 ++++-- enterprise/coderd/coderd_test.go | 132 ++++++++++++++++++ .../coderd/coderdenttest/coderdenttest.go | 8 +- 3 files changed, 162 insertions(+), 21 deletions(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index c89ea8f260a70..74a078247d851 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -63,21 +63,26 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { ctx, cancelFunc := context.WithCancel(ctx) - if options.ExternalTokenEncryption != nil { - cryptDB, err := dbcrypt.New(ctx, options.Database, options.ExternalTokenEncryption...) - if err != nil { - cancelFunc() - // If we fail to initialize the database, it's likely that the - // database is encrypted with an unknown external token encryption key. - // This is a fatal error. - var derr *dbcrypt.DecryptFailedError - if xerrors.As(err, &derr) { - return nil, xerrors.Errorf("database encrypted with unknown key, either add the key or see https://coder.com/docs/v2/latest/admin/encryption#disabling-encryption: %w", derr) - } - return nil, xerrors.Errorf("init database encryption: %w", err) + if options.ExternalTokenEncryption == nil { + options.ExternalTokenEncryption = make([]dbcrypt.Cipher, 0) + } + + // Database encryption is an enterprise feature, but as checking license entitlements + // depends on the database, we end up in a chicken-and-egg situation. To avoid this, + // we always enable it but only soft-enforce it. + cryptDB, err := dbcrypt.New(ctx, options.Database, options.ExternalTokenEncryption...) + if err != nil { + cancelFunc() + // If we fail to initialize the database, it's likely that the + // database is encrypted with an unknown external token encryption key. + // This is a fatal error. + var derr *dbcrypt.DecryptFailedError + if xerrors.As(err, &derr) { + return nil, xerrors.Errorf("database encrypted with unknown key, either add the key or see https://coder.com/docs/v2/latest/admin/encryption#disabling-encryption: %w", derr) } - options.Database = cryptDB + return nil, xerrors.Errorf("init database encryption: %w", err) } + options.Database = cryptDB api := &API{ ctx: ctx, @@ -455,7 +460,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { codersdk.FeatureHighAvailability: api.DERPServerRelayAddress != "", codersdk.FeatureMultipleGitAuth: len(api.GitAuthConfigs) > 1, codersdk.FeatureTemplateRBAC: api.RBAC, - codersdk.FeatureExternalTokenEncryption: len(api.ExternalTokenEncryption) != 0, + codersdk.FeatureExternalTokenEncryption: len(api.ExternalTokenEncryption) > 0, codersdk.FeatureExternalProvisionerDaemons: true, codersdk.FeatureAdvancedTemplateScheduling: true, // FeatureTemplateAutostopRequirement depends on @@ -635,6 +640,16 @@ func (api *API) updateEntitlements(ctx context.Context) error { } } + // External token encryption is soft-enforced + featureExternalTokenEncryption := entitlements.Features[codersdk.FeatureExternalTokenEncryption] + featureExternalTokenEncryption.Enabled = len(api.ExternalTokenEncryption) > 0 + if featureExternalTokenEncryption.Enabled && featureExternalTokenEncryption.Entitlement != codersdk.EntitlementEntitled { + msg := fmt.Sprintf("%s is enabled (due to setting external token encryption keys) but your license is not entitled to this feature.", codersdk.FeatureExternalTokenEncryption.Humanize()) + api.Logger.Warn(ctx, msg) + entitlements.Warnings = append(entitlements.Warnings, msg) + } + entitlements.Features[codersdk.FeatureExternalTokenEncryption] = featureExternalTokenEncryption + api.entitlementsMu.Lock() defer api.entitlementsMu.Unlock() api.entitlements = entitlements diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index 9fcb748bf9ca0..e6756887317c8 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -1,8 +1,10 @@ package coderd_test import ( + "bytes" "context" "reflect" + "strings" "testing" "time" @@ -16,6 +18,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" @@ -23,6 +26,7 @@ import ( "github.com/coder/coder/v2/enterprise/coderd" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/enterprise/dbcrypt" "github.com/coder/coder/v2/testutil" ) @@ -230,6 +234,134 @@ func TestAuditLogging(t *testing.T) { }) } +func TestExternalTokenEncryption(t *testing.T) { + t.Parallel() + + t.Run("Enabled", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + db, ps := dbtestutil.NewDB(t) + ciphers, err := dbcrypt.NewCiphers(bytes.Repeat([]byte("a"), 32)) + require.NoError(t, err) + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + EntitlementsUpdateInterval: 25 * time.Millisecond, + ExternalTokenEncryption: ciphers, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureExternalTokenEncryption: 1, + }, + }, + Options: &coderdtest.Options{ + Database: db, + Pubsub: ps, + }, + }) + keys, err := db.GetDBCryptKeys(ctx) + require.NoError(t, err) + require.Len(t, keys, 1) + require.Equal(t, ciphers[0].HexDigest(), keys[0].ActiveKeyDigest.String) + + require.Eventually(t, func() bool { + entitlements, err := client.Entitlements(context.Background()) + assert.NoError(t, err) + feature := entitlements.Features[codersdk.FeatureExternalTokenEncryption] + entitled := feature.Entitlement == codersdk.EntitlementEntitled + var warningExists bool + for _, warning := range entitlements.Warnings { + if strings.Contains(warning, codersdk.FeatureExternalTokenEncryption.Humanize()) { + warningExists = true + break + } + } + t.Logf("feature: %+v, warnings: %+v, errors: %+v", feature, entitlements.Warnings, entitlements.Errors) + return feature.Enabled && entitled && !warningExists + }, testutil.WaitShort, testutil.IntervalFast) + }) + + t.Run("Disabled", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + db, ps := dbtestutil.NewDB(t) + ciphers, err := dbcrypt.NewCiphers() + require.NoError(t, err) + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + DontAddLicense: true, + EntitlementsUpdateInterval: 25 * time.Millisecond, + ExternalTokenEncryption: ciphers, + Options: &coderdtest.Options{ + Database: db, + Pubsub: ps, + }, + }) + keys, err := db.GetDBCryptKeys(ctx) + require.NoError(t, err) + require.Empty(t, keys) + + require.Eventually(t, func() bool { + entitlements, err := client.Entitlements(context.Background()) + assert.NoError(t, err) + feature := entitlements.Features[codersdk.FeatureExternalTokenEncryption] + entitled := feature.Entitlement == codersdk.EntitlementEntitled + var warningExists bool + for _, warning := range entitlements.Warnings { + if strings.Contains(warning, codersdk.FeatureExternalTokenEncryption.Humanize()) { + warningExists = true + break + } + } + t.Logf("feature: %+v, warnings: %+v, errors: %+v", feature, entitlements.Warnings, entitlements.Errors) + return !feature.Enabled && !entitled && !warningExists + }, testutil.WaitShort, testutil.IntervalFast) + }) + + t.Run("PreviouslyEnabledButMissingFromLicense", func(t *testing.T) { + // If this test fails, it potentially means that a customer who has + // actively been using this feature is now unable _start coderd_ + // because of a licensing issue. This should never happen. + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + db, ps := dbtestutil.NewDB(t) + ciphers, err := dbcrypt.NewCiphers(bytes.Repeat([]byte("a"), 32)) + require.NoError(t, err) + + dbc, err := dbcrypt.New(ctx, db, ciphers...) // should insert key + require.NoError(t, err) + + keys, err := dbc.GetDBCryptKeys(ctx) + require.NoError(t, err) + require.Len(t, keys, 1) + + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + DontAddLicense: true, + EntitlementsUpdateInterval: 25 * time.Millisecond, + ExternalTokenEncryption: ciphers, + Options: &coderdtest.Options{ + Database: db, + Pubsub: ps, + }, + }) + + require.Eventually(t, func() bool { + entitlements, err := client.Entitlements(context.Background()) + assert.NoError(t, err) + feature := entitlements.Features[codersdk.FeatureExternalTokenEncryption] + entitled := feature.Entitlement == codersdk.EntitlementEntitled + var warningExists bool + for _, warning := range entitlements.Warnings { + if strings.Contains(warning, codersdk.FeatureExternalTokenEncryption.Humanize()) { + warningExists = true + break + } + } + t.Logf("feature: %+v, warnings: %+v, errors: %+v", feature, entitlements.Warnings, entitlements.Errors) + return feature.Enabled && !entitled && warningExists + }, testutil.WaitShort, testutil.IntervalFast) + }) +} + // testDBAuthzRole returns a context with a subject that has a role // with permissions required for test setup. func testDBAuthzRole(ctx context.Context) context.Context { diff --git a/enterprise/coderd/coderdenttest/coderdenttest.go b/enterprise/coderd/coderdenttest/coderdenttest.go index 38b92e35b1346..1c3f7c4fc83e0 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest.go +++ b/enterprise/coderd/coderdenttest/coderdenttest.go @@ -25,8 +25,7 @@ import ( ) const ( - testKeyID = "enterprise-test" - testEncryptionKey = "coder-coder-coder-coder-coder-1!" // nolint:gosec + testKeyID = "enterprise-test" ) var ( @@ -85,11 +84,6 @@ func NewWithAPI(t *testing.T, options *Options) ( err := oop.DeploymentValues.UserQuietHoursSchedule.DefaultSchedule.Set("0 0 * * *") require.NoError(t, err) } - if options.ExternalTokenEncryption == nil { - c, err := dbcrypt.NewCiphers([]byte(testEncryptionKey)) - require.NoError(t, err) - options.ExternalTokenEncryption = c - } coderAPI, err := coderd.New(context.Background(), &coderd.Options{ RBAC: true, AuditLogging: options.AuditLogging, From e56b6391421662ff45f5a52d34624df557f88f52 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 12:04:04 +0100 Subject: [PATCH 30/34] do not add external token encryption keys by default (as it will always be enabled) --- scripts/develop.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/develop.sh b/scripts/develop.sh index 327f2192ce2c4..39f81c2951bc4 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -15,7 +15,6 @@ set -euo pipefail CODER_DEV_ACCESS_URL="${CODER_DEV_ACCESS_URL:-http://127.0.0.1:3000}" DEFAULT_PASSWORD="SomeSecurePassword!" -EXTERNAL_TOKEN_ENCRYPTION_KEYS="Y29kZXItY29kZXItY29kZXItY29kZXItY29kZXItMSE=" password="${CODER_DEV_ADMIN_PASSWORD:-${DEFAULT_PASSWORD}}" use_proxy=0 @@ -137,7 +136,7 @@ fatal() { trap 'fatal "Script encountered an error"' ERR cdroot - start_cmd API "" "${CODER_DEV_SHIM}" server --http-address 0.0.0.0:3000 --swagger-enable --access-url "${CODER_DEV_ACCESS_URL}" --dangerous-allow-cors-requests=true --external-token-encryption-keys="${EXTERNAL_TOKEN_ENCRYPTION_KEYS}" "$@" + start_cmd API "" "${CODER_DEV_SHIM}" server --http-address 0.0.0.0:3000 --swagger-enable --access-url "${CODER_DEV_ACCESS_URL}" --dangerous-allow-cors-requests=true "$@" echo '== Waiting for Coder to become ready' # Start the timeout in the background so interrupting this script From 441fcbf1cd0a26f333971d6cce40e8dd1442c22d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 12:06:12 +0100 Subject: [PATCH 31/34] update golden files --- cli/testdata/coder_server_--help.golden | 4 +++- enterprise/cli/testdata/coder_server_--help.golden | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 0a2e63ddd8ea8..cd23076e30a3a 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -464,7 +464,9 @@ These options are only available in the Enterprise Edition. keys. Each key, when base64-decoded, must be exactly 32 bytes in length. The first key will be used to encrypt new values. Subsequent keys will be used as a fallback when decrypting. During normal - operation it is recommended to only set one key. + operation it is recommended to only set one key unless you are in the + process of rotating keys with the `coder server dbcrypt rotate` + command. --scim-auth-header string, $CODER_SCIM_AUTH_HEADER Enables SCIM and sets the authentication header for the built-in SCIM diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 2b701ff9ef2a9..9d6ad20e2a5e2 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -465,7 +465,9 @@ These options are only available in the Enterprise Edition. keys. Each key, when base64-decoded, must be exactly 32 bytes in length. The first key will be used to encrypt new values. Subsequent keys will be used as a fallback when decrypting. During normal - operation it is recommended to only set one key. + operation it is recommended to only set one key unless you are in the + process of rotating keys with the `coder server dbcrypt rotate` + command. --scim-auth-header string, $CODER_SCIM_AUTH_HEADER Enables SCIM and sets the authentication header for the built-in SCIM From ba1412842898662288b627ba3a0d6eb67279417d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 13:02:30 +0100 Subject: [PATCH 32/34] log encryption status on startup --- enterprise/coderd/coderd.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 74a078247d851..badfdb3d6f0da 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -66,10 +66,17 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { if options.ExternalTokenEncryption == nil { options.ExternalTokenEncryption = make([]dbcrypt.Cipher, 0) } - // Database encryption is an enterprise feature, but as checking license entitlements // depends on the database, we end up in a chicken-and-egg situation. To avoid this, // we always enable it but only soft-enforce it. + if len(options.ExternalTokenEncryption) > 0 { + var keyDigests []string + for _, cipher := range options.ExternalTokenEncryption { + keyDigests = append(keyDigests, cipher.HexDigest()) + } + options.Logger.Info(ctx, "database encryption enabled", slog.F("keys", keyDigests)) + } + cryptDB, err := dbcrypt.New(ctx, options.Database, options.ExternalTokenEncryption...) if err != nil { cancelFunc() From 2ae45c6f1cdf25f04c55473815d01b2ecc8fb038 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 14:20:08 +0100 Subject: [PATCH 33/34] modify CLI output --- enterprise/cli/server_dbcrypt.go | 35 ++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/enterprise/cli/server_dbcrypt.go b/enterprise/cli/server_dbcrypt.go index bedd988886907..481df1dae6c2e 100644 --- a/enterprise/cli/server_dbcrypt.go +++ b/enterprise/cli/server_dbcrypt.go @@ -71,17 +71,18 @@ func (*RootCmd) dbcryptRotateCmd() *clibase.Cmd { return xerrors.Errorf("create ciphers: %w", err) } - newDigest := ciphers[0].HexDigest() - oldDigests := make([]string, 0, len(ciphers)-1) - for _, c := range ciphers[1:] { - oldDigests = append(oldDigests, c.HexDigest()) + var act string + switch len(flags.Old) { + case 0: + act = "Data will be encrypted with the new key." + default: + act = "Data will be decrypted with all available keys and re-encrypted with new key." } - if len(oldDigests) == 0 { - oldDigests = append(oldDigests, "none") - } - msg := fmt.Sprintf(`Rotate external token encryptions keys?\n- New key: %s\n- Old keys: %s`, - newDigest, - strings.Join(oldDigests, ", "), + + msg := fmt.Sprintf("%s\n\n- New key: %s\n- Old keys: %s\n\nRotate external token encryption keys?\n", + act, + flags.New, + strings.Join(flags.Old, ", "), ) if _, err := cliui.Prompt(inv, cliui.PromptOptions{Text: msg, IsConfirm: true}); err != nil { return err @@ -179,9 +180,13 @@ func (*RootCmd) dbcryptDeleteCmd() *clibase.Cmd { if err := flags.valid(); err != nil { return err } + msg := `All encrypted data will be deleted from the database: +- Encrypted user OAuth access and refresh tokens +- Encrypted user Git authentication access and refresh tokens +Are you sure you want to continue?` if _, err := cliui.Prompt(inv, cliui.PromptOptions{ - Text: "This will delete all encrypted data from the database. Are you sure you want to continue?", + Text: msg, IsConfirm: true, }); err != nil { return err @@ -238,6 +243,10 @@ func (f *rotateFlags) attach(opts *clibase.OptionSet) { } func (f *rotateFlags) valid() error { + if f.PostgresURL == "" { + return xerrors.Errorf("no database configured") + } + if f.New == "" { return xerrors.Errorf("no new key provided") } @@ -289,6 +298,10 @@ func (f *decryptFlags) attach(opts *clibase.OptionSet) { } func (f *decryptFlags) valid() error { + if f.PostgresURL == "" { + return xerrors.Errorf("no database configured") + } + if len(f.Keys) == 0 { return xerrors.Errorf("no keys provided") } From b3ff024e2c915174e5420ba9d2262b6446379ffe Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 7 Sep 2023 15:11:04 +0100 Subject: [PATCH 34/34] rm unused golden file --- .../coder_dbcrypt-rotate_--help.golden | 24 ------------------- 1 file changed, 24 deletions(-) delete mode 100644 enterprise/cli/testdata/coder_dbcrypt-rotate_--help.golden diff --git a/enterprise/cli/testdata/coder_dbcrypt-rotate_--help.golden b/enterprise/cli/testdata/coder_dbcrypt-rotate_--help.golden deleted file mode 100644 index bf3dae88c08bc..0000000000000 --- a/enterprise/cli/testdata/coder_dbcrypt-rotate_--help.golden +++ /dev/null @@ -1,24 +0,0 @@ -Usage: coder dbcrypt-rotate [flags] --postgres-url --external-token-encryption-keys , - -Rotate database encryption keys - -Options - --postgres-url string, $CODER_PG_CONNECTION_URL - URL of a PostgreSQL database. If empty, PostgreSQL binaries will be - downloaded from Maven (https://repo1.maven.org/maven2) and store all - data in the config root. Access the built-in database with "coder - server postgres-builtin-url". - -Enterprise Options -These options are only available in the Enterprise Edition. - - --external-token-encryption-keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS - Encrypt OIDC and Git authentication tokens with AES-256-GCM in the - database. The value must be a comma-separated list of base64-encoded - keys. Each key, when base64-decoded, must be exactly 32 bytes in - length. The first key will be used to encrypt new values. Subsequent - keys will be used as a fallback when decrypting. During normal - operation it is recommended to only set one key. - ---- -Run `coder --help` for a list of global options.