From 170c33a4755a900c9b12659eed2c46db99e3062a Mon Sep 17 00:00:00 2001 From: Zach <3724288+zedkipp@users.noreply.github.com> Date: Tue, 2 Jun 2026 08:36:01 -0600 Subject: [PATCH] feat: encrypt gitsshkeys.private_key at rest via dbcrypt (#25872) Adds an optional dbcrypt wrapper around gitsshkeys.private_key. The column is encrypted on insert and update through enterprise/dbcrypt when external token encryption is configured, and decrypted on read. A new private_key_key_id column references dbcrypt_keys(active_key_digest) so revocation safety is enforced by the existing foreign key. Rows with a NULL key_id stay plaintext and remain readable. Existing plaintext rows can be backfilled by running `coder server dbcrypt rotate`. Generated with assistance from Coder Agents. --- cli/server_createadminuser.go | 12 +- coderd/database/dbgen/dbgen.go | 11 +- coderd/database/dump.sql | 8 +- coderd/database/foreign_key_constraint.go | 1 + ...515_gitsshkeys_private_key_key_id.down.sql | 3 + ...00515_gitsshkeys_private_key_key_id.up.sql | 7 + coderd/database/models.go | 2 + coderd/database/queries.sql.go | 35 ++-- coderd/database/queries/gitsshkeys.sql | 7 +- coderd/gitsshkey.go | 10 +- coderd/users.go | 11 +- docs/admin/security/audit-logs.md | 2 +- docs/admin/security/database-encryption.md | 1 + enterprise/audit/table.go | 11 +- enterprise/cli/server_dbcrypt_test.go | 20 ++ enterprise/dbcrypt/cliutil.go | 51 ++++++ enterprise/dbcrypt/dbcrypt.go | 39 ++++ enterprise/dbcrypt/dbcrypt_internal_test.go | 173 ++++++++++++++++++ 18 files changed, 362 insertions(+), 42 deletions(-) create mode 100644 coderd/database/migrations/000515_gitsshkeys_private_key_key_id.down.sql create mode 100644 coderd/database/migrations/000515_gitsshkeys_private_key_key_id.up.sql diff --git a/cli/server_createadminuser.go b/cli/server_createadminuser.go index c9a0b11b90..7c4505b91d 100644 --- a/cli/server_createadminuser.go +++ b/cli/server_createadminuser.go @@ -3,6 +3,7 @@ package cli import ( + "database/sql" "fmt" "sort" @@ -210,11 +211,12 @@ func (r *RootCmd) newCreateAdminUserCommand() *serpent.Command { return xerrors.Errorf("generate user gitsshkey: %w", err) } _, err = tx.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{ - UserID: newUser.ID, - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - PrivateKey: privateKey, - PublicKey: publicKey, + UserID: newUser.ID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + PrivateKey: privateKey, + PrivateKeyKeyID: sql.NullString{}, // Plaintext; this CLI bypasses dbcrypt. Encrypted on next rotate. + PublicKey: publicKey, }) if err != nil { return xerrors.Errorf("insert user gitsshkey: %w", err) diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 416a2b7257..7c2bb68a00 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -1021,11 +1021,12 @@ func User(t testing.TB, db database.Store, orig database.User) database.User { func GitSSHKey(t testing.TB, db database.Store, orig database.GitSSHKey) database.GitSSHKey { key, err := db.InsertGitSSHKey(genCtx, database.InsertGitSSHKeyParams{ - UserID: takeFirst(orig.UserID, uuid.New()), - CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()), - UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()), - PrivateKey: takeFirst(orig.PrivateKey, ""), - PublicKey: takeFirst(orig.PublicKey, ""), + UserID: takeFirst(orig.UserID, uuid.New()), + CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()), + UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()), + PrivateKey: takeFirst(orig.PrivateKey, ""), + PrivateKeyKeyID: takeFirst(orig.PrivateKeyKeyID, sql.NullString{}), + PublicKey: takeFirst(orig.PublicKey, ""), }) require.NoError(t, err, "insert ssh key") return key diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 0bc0874e51..ea09f4300f 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2013,9 +2013,12 @@ CREATE TABLE gitsshkeys ( created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, private_key text NOT NULL, - public_key text NOT NULL + public_key text NOT NULL, + private_key_key_id text ); +COMMENT ON COLUMN gitsshkeys.private_key_key_id IS 'The ID of the key used to encrypt the private key. If this is NULL, the private key is not encrypted.'; + CREATE TABLE group_ai_budgets ( group_id uuid NOT NULL, spend_limit_micros bigint NOT NULL, @@ -4701,6 +4704,9 @@ ALTER TABLE ONLY external_auth_links ALTER TABLE ONLY external_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_private_key_key_id_fkey FOREIGN KEY (private_key_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); diff --git a/coderd/database/foreign_key_constraint.go b/coderd/database/foreign_key_constraint.go index 5eeb24587a..8109f2564f 100644 --- a/coderd/database/foreign_key_constraint.go +++ b/coderd/database/foreign_key_constraint.go @@ -46,6 +46,7 @@ const ( ForeignKeyFkOauth2ProviderAppTokensUserID ForeignKeyConstraint = "fk_oauth2_provider_app_tokens_user_id" // ALTER TABLE ONLY oauth2_provider_app_tokens ADD CONSTRAINT fk_oauth2_provider_app_tokens_user_id FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; ForeignKeyGitAuthLinksOauthAccessTokenKeyID ForeignKeyConstraint = "git_auth_links_oauth_access_token_key_id_fkey" // ALTER TABLE ONLY external_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); ForeignKeyGitAuthLinksOauthRefreshTokenKeyID ForeignKeyConstraint = "git_auth_links_oauth_refresh_token_key_id_fkey" // ALTER TABLE ONLY external_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); + ForeignKeyGitSSHKeysPrivateKeyKeyID ForeignKeyConstraint = "gitsshkeys_private_key_key_id_fkey" // ALTER TABLE ONLY gitsshkeys ADD CONSTRAINT gitsshkeys_private_key_key_id_fkey FOREIGN KEY (private_key_key_id) REFERENCES dbcrypt_keys(active_key_digest); ForeignKeyGitSSHKeysUserID ForeignKeyConstraint = "gitsshkeys_user_id_fkey" // ALTER TABLE ONLY gitsshkeys ADD CONSTRAINT gitsshkeys_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); ForeignKeyGroupAiBudgetsGroupID ForeignKeyConstraint = "group_ai_budgets_group_id_fkey" // ALTER TABLE ONLY group_ai_budgets ADD CONSTRAINT group_ai_budgets_group_id_fkey FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE; ForeignKeyGroupMembersGroupID ForeignKeyConstraint = "group_members_group_id_fkey" // ALTER TABLE ONLY group_members ADD CONSTRAINT group_members_group_id_fkey FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE; diff --git a/coderd/database/migrations/000515_gitsshkeys_private_key_key_id.down.sql b/coderd/database/migrations/000515_gitsshkeys_private_key_key_id.down.sql new file mode 100644 index 0000000000..ca4d17f749 --- /dev/null +++ b/coderd/database/migrations/000515_gitsshkeys_private_key_key_id.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE gitsshkeys + DROP CONSTRAINT gitsshkeys_private_key_key_id_fkey, + DROP COLUMN private_key_key_id; diff --git a/coderd/database/migrations/000515_gitsshkeys_private_key_key_id.up.sql b/coderd/database/migrations/000515_gitsshkeys_private_key_key_id.up.sql new file mode 100644 index 0000000000..13f3b6fc44 --- /dev/null +++ b/coderd/database/migrations/000515_gitsshkeys_private_key_key_id.up.sql @@ -0,0 +1,7 @@ +ALTER TABLE gitsshkeys + ADD COLUMN private_key_key_id TEXT; + +ALTER TABLE ONLY gitsshkeys + ADD CONSTRAINT gitsshkeys_private_key_key_id_fkey FOREIGN KEY (private_key_key_id) REFERENCES dbcrypt_keys(active_key_digest); + +COMMENT ON COLUMN gitsshkeys.private_key_key_id IS 'The ID of the key used to encrypt the private key. If this is NULL, the private key is not encrypted.'; diff --git a/coderd/database/models.go b/coderd/database/models.go index b2d7fda98f..8a6aa5fd4d 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -4914,6 +4914,8 @@ type GitSSHKey struct { UpdatedAt time.Time `db:"updated_at" json:"updated_at"` PrivateKey string `db:"private_key" json:"private_key"` PublicKey string `db:"public_key" json:"public_key"` + // The ID of the key used to encrypt the private key. If this is NULL, the private key is not encrypted. + PrivateKeyKeyID sql.NullString `db:"private_key_key_id" json:"private_key_key_id"` } type Group struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 28b11009a3..6b7b39c021 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -12572,7 +12572,7 @@ func (q *sqlQuerier) InsertFile(ctx context.Context, arg InsertFileParams) (File const getGitSSHKey = `-- name: GetGitSSHKey :one SELECT - user_id, created_at, updated_at, private_key, public_key + user_id, created_at, updated_at, private_key, public_key, private_key_key_id FROM gitsshkeys WHERE @@ -12588,6 +12588,7 @@ func (q *sqlQuerier) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (GitSSH &i.UpdatedAt, &i.PrivateKey, &i.PublicKey, + &i.PrivateKeyKeyID, ) return i, err } @@ -12599,18 +12600,20 @@ INSERT INTO created_at, updated_at, private_key, + private_key_key_id, public_key ) VALUES - ($1, $2, $3, $4, $5) RETURNING user_id, created_at, updated_at, private_key, public_key + ($1, $2, $3, $4, $5, $6) RETURNING user_id, created_at, updated_at, private_key, public_key, private_key_key_id ` type InsertGitSSHKeyParams struct { - 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"` - PrivateKey string `db:"private_key" json:"private_key"` - PublicKey string `db:"public_key" json:"public_key"` + 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"` + PrivateKey string `db:"private_key" json:"private_key"` + PrivateKeyKeyID sql.NullString `db:"private_key_key_id" json:"private_key_key_id"` + PublicKey string `db:"public_key" json:"public_key"` } func (q *sqlQuerier) InsertGitSSHKey(ctx context.Context, arg InsertGitSSHKeyParams) (GitSSHKey, error) { @@ -12619,6 +12622,7 @@ func (q *sqlQuerier) InsertGitSSHKey(ctx context.Context, arg InsertGitSSHKeyPar arg.CreatedAt, arg.UpdatedAt, arg.PrivateKey, + arg.PrivateKeyKeyID, arg.PublicKey, ) var i GitSSHKey @@ -12628,6 +12632,7 @@ func (q *sqlQuerier) InsertGitSSHKey(ctx context.Context, arg InsertGitSSHKeyPar &i.UpdatedAt, &i.PrivateKey, &i.PublicKey, + &i.PrivateKeyKeyID, ) return i, err } @@ -12638,18 +12643,20 @@ UPDATE SET updated_at = $2, private_key = $3, - public_key = $4 + private_key_key_id = $4, + public_key = $5 WHERE user_id = $1 RETURNING - user_id, created_at, updated_at, private_key, public_key + user_id, created_at, updated_at, private_key, public_key, private_key_key_id ` type UpdateGitSSHKeyParams struct { - UserID uuid.UUID `db:"user_id" json:"user_id"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - PrivateKey string `db:"private_key" json:"private_key"` - PublicKey string `db:"public_key" json:"public_key"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + PrivateKey string `db:"private_key" json:"private_key"` + PrivateKeyKeyID sql.NullString `db:"private_key_key_id" json:"private_key_key_id"` + PublicKey string `db:"public_key" json:"public_key"` } func (q *sqlQuerier) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) (GitSSHKey, error) { @@ -12657,6 +12664,7 @@ func (q *sqlQuerier) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyPar arg.UserID, arg.UpdatedAt, arg.PrivateKey, + arg.PrivateKeyKeyID, arg.PublicKey, ) var i GitSSHKey @@ -12666,6 +12674,7 @@ func (q *sqlQuerier) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyPar &i.UpdatedAt, &i.PrivateKey, &i.PublicKey, + &i.PrivateKeyKeyID, ) return i, err } diff --git a/coderd/database/queries/gitsshkeys.sql b/coderd/database/queries/gitsshkeys.sql index a9b4353dd4..a08dabb896 100644 --- a/coderd/database/queries/gitsshkeys.sql +++ b/coderd/database/queries/gitsshkeys.sql @@ -5,10 +5,11 @@ INSERT INTO created_at, updated_at, private_key, + private_key_key_id, public_key ) VALUES - ($1, $2, $3, $4, $5) RETURNING *; + ($1, $2, $3, $4, $5, $6) RETURNING *; -- name: GetGitSSHKey :one SELECT @@ -24,9 +25,9 @@ UPDATE SET updated_at = $2, private_key = $3, - public_key = $4 + private_key_key_id = $4, + public_key = $5 WHERE user_id = $1 RETURNING *; - diff --git a/coderd/gitsshkey.go b/coderd/gitsshkey.go index de97af42cb..a35a8f51d7 100644 --- a/coderd/gitsshkey.go +++ b/coderd/gitsshkey.go @@ -1,6 +1,7 @@ package coderd import ( + "database/sql" "net/http" "github.com/coder/coder/v2/coderd/audit" @@ -53,10 +54,11 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) { } newKey, err := api.Database.UpdateGitSSHKey(ctx, database.UpdateGitSSHKeyParams{ - UserID: user.ID, - UpdatedAt: dbtime.Now(), - PrivateKey: privateKey, - PublicKey: publicKey, + UserID: user.ID, + UpdatedAt: dbtime.Now(), + PrivateKey: privateKey, + PrivateKeyKeyID: sql.NullString{}, // dbcrypt will update as required + PublicKey: publicKey, }) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/coderd/users.go b/coderd/users.go index 585360630e..3003cd05f5 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1967,11 +1967,12 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create return xerrors.Errorf("generate user gitsshkey: %w", err) } _, err = tx.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{ - UserID: user.ID, - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - PrivateKey: privateKey, - PublicKey: publicKey, + UserID: user.ID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + PrivateKey: privateKey, + PrivateKeyKeyID: sql.NullString{}, // dbcrypt will set as required + PublicKey: publicKey, }) if err != nil { return xerrors.Errorf("insert user gitsshkey: %w", err) diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 6e65c5fec5..45022ff253 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -26,7 +26,7 @@ We track the following resources: | AuditableOrganizationMember
| |
FieldTracked
created_attrue
organization_idfalse
rolestrue
updated_attrue
user_idtrue
usernametrue
| | Chat
create, write | |
FieldTracked
agent_idfalse
archivedtrue
build_idfalse
client_typefalse
created_atfalse
dynamic_toolsfalse
group_acltrue
heartbeat_atfalse
idtrue
labelstrue
last_errorfalse
last_injected_contextfalse
last_model_config_idfalse
last_read_message_idfalse
last_turn_summaryfalse
mcp_server_idstrue
modetrue
organization_idfalse
owner_idtrue
owner_namefalse
owner_usernamefalse
parent_chat_idfalse
pin_ordertrue
plan_modefalse
root_chat_idfalse
started_atfalse
statusfalse
titletrue
updated_atfalse
user_acltrue
worker_idfalse
workspace_idtrue
| | CustomRole
| |
FieldTracked
created_atfalse
display_nametrue
idfalse
is_systemfalse
member_permissionstrue
nametrue
org_permissionstrue
organization_idfalse
site_permissionstrue
updated_atfalse
user_permissionstrue
| -| GitSSHKey
create | |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| +| GitSSHKey
create | |
FieldTracked
created_atfalse
private_keytrue
private_key_key_idfalse
public_keytrue
updated_atfalse
user_idtrue
| | GroupSyncSettings
| |
FieldTracked
auto_create_missing_groupstrue
fieldtrue
legacy_group_name_mappingfalse
mappingtrue
regex_filtertrue
| | HealthSettings
| |
FieldTracked
dismissed_healthcheckstrue
idfalse
| | License
create, delete | |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| diff --git a/docs/admin/security/database-encryption.md b/docs/admin/security/database-encryption.md index 7d6f0f4cbf..dd8b536f7c 100644 --- a/docs/admin/security/database-encryption.md +++ b/docs/admin/security/database-encryption.md @@ -24,6 +24,7 @@ The following database fields are currently encrypted: - `external_auth_links.oauth_refresh_token` - `crypto_keys.secret` - `user_secrets.value` +- `gitsshkeys.private_key` Additional database fields may be encrypted in the future. diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 315f6af228..6ae2afd92c 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -84,11 +84,12 @@ var auditableResourcesTypes = map[any]map[string]Action{ "updated_at": ActionIgnore, }, &database.GitSSHKey{}: { - "user_id": ActionTrack, - "created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff. - "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. - "private_key": ActionSecret, // We don't want to expose private keys in diffs. - "public_key": ActionTrack, // Public keys are ok to expose in a diff. + "user_id": ActionTrack, + "created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff. + "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. + "private_key": ActionSecret, // We don't want to expose private keys in diffs. + "private_key_key_id": ActionIgnore, // Internal dbcrypt metadata, not useful in audit diffs. + "public_key": ActionTrack, // Public keys are ok to expose in a diff. }, &database.Template{}: { "id": ActionTrack, diff --git a/enterprise/cli/server_dbcrypt_test.go b/enterprise/cli/server_dbcrypt_test.go index 6ce112f425..3cafb4ce38 100644 --- a/enterprise/cli/server_dbcrypt_test.go +++ b/enterprise/cli/server_dbcrypt_test.go @@ -204,6 +204,12 @@ func TestServerDBCrypt(t *testing.T) { userSecrets, err := db.ListUserSecretsWithValues(ctx, usr.ID) require.NoError(t, err, "failed to get user secrets for user %s", usr.ID) require.Empty(t, userSecrets) + + // gitsshkey rows are preserved so the user can regenerate; only the ciphertext is wiped. + sshKey, err := db.GetGitSSHKey(ctx, usr.ID) + require.NoError(t, err, "expected gitsshkey row to remain for user %s", usr.ID) + require.Empty(t, sshKey.PrivateKey, "expected private_key to be cleared for user %s", usr.ID) + require.False(t, sshKey.PrivateKeyKeyID.Valid, "expected private_key_key_id to be cleared for user %s", usr.ID) } // Validate that the key has been revoked in the database. @@ -245,6 +251,13 @@ func genData(t *testing.T, db database.Store) []database.User { ProviderID: provider.ID, APIKey: "provider-key-" + usr.ID.String(), }) + // gitsshkeys are not removed by the user soft-delete trigger, + // so seed one for every user including deleted ones. + _ = dbgen.GitSSHKey(t, db, database.GitSSHKey{ + UserID: usr.ID, + PrivateKey: "private-" + usr.ID.String(), + PublicKey: "public-" + usr.ID.String(), + }) now := time.Now() _, err := db.UpsertUserAIProviderKey(context.Background(), database.UpsertUserAIProviderKeyParams{ ID: uuid.New(), @@ -325,6 +338,13 @@ func requireEncryptedWithCipher(ctx context.Context, t *testing.T, db database.S require.Equal(t, c.HexDigest(), s.ValueKeyID.String) } + sshKey, err := db.GetGitSSHKey(ctx, userID) + require.NoError(t, err, "failed to get gitsshkey for user %s", userID) + requireEncryptedEquals(t, c, "private-"+userID.String(), sshKey.PrivateKey) + require.Equal(t, c.HexDigest(), sshKey.PrivateKeyKeyID.String) + // Public key is never encrypted. + require.Equal(t, "public-"+userID.String(), sshKey.PublicKey) + providers, err := db.GetAIProviders(ctx, database.GetAIProvidersParams{ IncludeDeleted: true, IncludeDisabled: true, diff --git a/enterprise/dbcrypt/cliutil.go b/enterprise/dbcrypt/cliutil.go index 28a04a5aa9..b298828055 100644 --- a/enterprise/dbcrypt/cliutil.go +++ b/enterprise/dbcrypt/cliutil.go @@ -101,6 +101,31 @@ func Rotate(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Ciphe log.Debug(ctx, "rotated user secret", slog.F("user_id", uid), slog.F("secret_name", secret.Name), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) } + sshKey, err := cryptTx.GetGitSSHKey(ctx, uid) + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + return xerrors.Errorf("get gitsshkey for user %s: %w", uid, err) + } + if err == nil { + switch { + case sshKey.PrivateKey == "": + // Post-Delete wipes the private_key and key_id; nothing to encrypt. + log.Debug(ctx, "skipping empty gitsshkey", slog.F("user_id", uid), slog.F("current", idx+1)) + case sshKey.PrivateKeyKeyID.Valid && sshKey.PrivateKeyKeyID.String == ciphers[0].HexDigest(): + log.Debug(ctx, "skipping gitsshkey", slog.F("user_id", uid), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + default: + if _, err := cryptTx.UpdateGitSSHKey(ctx, database.UpdateGitSSHKeyParams{ + UserID: uid, + UpdatedAt: sshKey.UpdatedAt, + PrivateKey: sshKey.PrivateKey, + PrivateKeyKeyID: sql.NullString{}, // dbcrypt will re-encrypt + PublicKey: sshKey.PublicKey, + }); err != nil { + return xerrors.Errorf("rotate gitsshkey user_id=%s: %w", uid, err) + } + log.Debug(ctx, "rotated gitsshkey", slog.F("user_id", uid), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest())) + } + } + return nil }, &database.TxOptions{ Isolation: sql.LevelRepeatableRead, @@ -288,6 +313,23 @@ func Decrypt(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Ciph log.Debug(ctx, "decrypted user secret", slog.F("user_id", uid), slog.F("secret_name", secret.Name), slog.F("current", idx+1)) } + sshKey, err := tx.GetGitSSHKey(ctx, uid) + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + return xerrors.Errorf("get gitsshkey for user %s: %w", uid, err) + } + if err == nil && sshKey.PrivateKeyKeyID.Valid { + if _, err := tx.UpdateGitSSHKey(ctx, database.UpdateGitSSHKeyParams{ + UserID: uid, + UpdatedAt: sshKey.UpdatedAt, + PrivateKey: sshKey.PrivateKey, + PrivateKeyKeyID: sql.NullString{}, // clear the key ID + PublicKey: sshKey.PublicKey, + }); err != nil { + return xerrors.Errorf("decrypt gitsshkey user_id=%s: %w", uid, err) + } + log.Debug(ctx, "decrypted gitsshkey", slog.F("user_id", uid), slog.F("current", idx+1)) + } + return nil }, &database.TxOptions{ Isolation: sql.LevelRepeatableRead, @@ -382,6 +424,15 @@ DELETE FROM user_ai_provider_keys WHERE api_key_key_id IS NOT NULL; DELETE FROM user_secrets WHERE value_key_id IS NOT NULL; +-- gitsshkeys has no delete path in product code: rows are inserted on +-- user creation and only ever mutated by regenerate. dbcrypt's 'delete' +-- command is the one operation that needs to wipe encrypted content, +-- and it does so by clearing the value rather than deleting the row, +-- so users can regenerate via the UI. +UPDATE gitsshkeys + SET private_key = '', + private_key_key_id = NULL + WHERE private_key_key_id IS NOT NULL; UPDATE ai_providers SET settings = NULL, settings_key_id = NULL diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go index 44cdb5554e..38a5cc1429 100644 --- a/enterprise/dbcrypt/dbcrypt.go +++ b/enterprise/dbcrypt/dbcrypt.go @@ -930,6 +930,45 @@ func (db *dbCrypt) UpdateUserSecretByUserIDAndName(ctx context.Context, arg data return secret, nil } +func (db *dbCrypt) InsertGitSSHKey(ctx context.Context, params database.InsertGitSSHKeyParams) (database.GitSSHKey, error) { + if err := db.encryptField(¶ms.PrivateKey, ¶ms.PrivateKeyKeyID); err != nil { + return database.GitSSHKey{}, err + } + key, err := db.Store.InsertGitSSHKey(ctx, params) + if err != nil { + return database.GitSSHKey{}, err + } + if err := db.decryptField(&key.PrivateKey, key.PrivateKeyKeyID); err != nil { + return database.GitSSHKey{}, err + } + return key, nil +} + +func (db *dbCrypt) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (database.GitSSHKey, error) { + key, err := db.Store.GetGitSSHKey(ctx, userID) + if err != nil { + return database.GitSSHKey{}, err + } + if err := db.decryptField(&key.PrivateKey, key.PrivateKeyKeyID); err != nil { + return database.GitSSHKey{}, err + } + return key, nil +} + +func (db *dbCrypt) UpdateGitSSHKey(ctx context.Context, params database.UpdateGitSSHKeyParams) (database.GitSSHKey, error) { + if err := db.encryptField(¶ms.PrivateKey, ¶ms.PrivateKeyKeyID); err != nil { + return database.GitSSHKey{}, err + } + key, err := db.Store.UpdateGitSSHKey(ctx, params) + if err != nil { + return database.GitSSHKey{}, err + } + if err := db.decryptField(&key.PrivateKey, key.PrivateKeyKeyID); err != nil { + return database.GitSSHKey{}, err + } + return key, 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 == "" { diff --git a/enterprise/dbcrypt/dbcrypt_internal_test.go b/enterprise/dbcrypt/dbcrypt_internal_test.go index e5a433399b..acdb0fcbbb 100644 --- a/enterprise/dbcrypt/dbcrypt_internal_test.go +++ b/enterprise/dbcrypt/dbcrypt_internal_test.go @@ -1764,3 +1764,176 @@ func TestUserSecrets(t *testing.T) { require.ErrorAs(t, err, &derr) }) } + +func TestGitSSHKey(t *testing.T) { + t.Parallel() + ctx := context.Background() + + const ( + initialPrivate = "private-key-initial" + updatedPrivate = "private-key-updated" + publicKey = "public-key" + ) + + insertGitSSHKey := func(t *testing.T, store database.Store, ciphers []Cipher) database.GitSSHKey { + t.Helper() + user := dbgen.User(t, store, database.User{}) + key, err := store.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{ + UserID: user.ID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + PrivateKey: initialPrivate, + PublicKey: publicKey, + }) + require.NoError(t, err) + require.Equal(t, initialPrivate, key.PrivateKey) + require.Equal(t, publicKey, key.PublicKey) + if len(ciphers) > 0 { + require.True(t, key.PrivateKeyKeyID.Valid) + require.Equal(t, ciphers[0].HexDigest(), key.PrivateKeyKeyID.String) + } + return key + } + + t.Run("InsertGitSSHKeyEncryptsPrivateKey", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + key := insertGitSSHKey(t, crypt, ciphers) + + // Raw row should be ciphertext under the primary cipher. + rawKey, err := db.GetGitSSHKey(ctx, key.UserID) + require.NoError(t, err) + require.NotEqual(t, initialPrivate, rawKey.PrivateKey) + requireEncryptedEquals(t, ciphers[0], rawKey.PrivateKey, initialPrivate) + require.True(t, rawKey.PrivateKeyKeyID.Valid) + require.Equal(t, ciphers[0].HexDigest(), rawKey.PrivateKeyKeyID.String) + // Public key is not encrypted. + require.Equal(t, publicKey, rawKey.PublicKey) + }) + + t.Run("GetGitSSHKeyDecryptsEncryptedRow", func(t *testing.T) { + t.Parallel() + _, crypt, ciphers := setup(t) + key := insertGitSSHKey(t, crypt, ciphers) + + got, err := crypt.GetGitSSHKey(ctx, key.UserID) + require.NoError(t, err) + require.Equal(t, initialPrivate, got.PrivateKey) + require.True(t, got.PrivateKeyKeyID.Valid) + require.Equal(t, ciphers[0].HexDigest(), got.PrivateKeyKeyID.String) + }) + + t.Run("GetGitSSHKeyReadsPlaintextRow", func(t *testing.T) { + // Pre-existing plaintext rows (private_key_key_id IS NULL) must remain readable. + t.Parallel() + db, crypt, _ := setup(t) + user := dbgen.User(t, db, database.User{}) + inserted, err := db.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{ + UserID: user.ID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + PrivateKey: initialPrivate, + PublicKey: publicKey, + }) + require.NoError(t, err) + require.False(t, inserted.PrivateKeyKeyID.Valid) + + got, err := crypt.GetGitSSHKey(ctx, user.ID) + require.NoError(t, err) + require.Equal(t, initialPrivate, got.PrivateKey) + require.False(t, got.PrivateKeyKeyID.Valid) + }) + + t.Run("UpdateGitSSHKeyReEncrypts", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + key := insertGitSSHKey(t, crypt, ciphers) + + updated, err := crypt.UpdateGitSSHKey(ctx, database.UpdateGitSSHKeyParams{ + UserID: key.UserID, + UpdatedAt: dbtime.Now(), + PrivateKey: updatedPrivate, + PublicKey: publicKey, + }) + require.NoError(t, err) + require.Equal(t, updatedPrivate, updated.PrivateKey) + require.True(t, updated.PrivateKeyKeyID.Valid) + require.Equal(t, ciphers[0].HexDigest(), updated.PrivateKeyKeyID.String) + + rawKey, err := db.GetGitSSHKey(ctx, key.UserID) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], rawKey.PrivateKey, updatedPrivate) + require.True(t, rawKey.PrivateKeyKeyID.Valid) + require.Equal(t, ciphers[0].HexDigest(), rawKey.PrivateKeyKeyID.String) + }) + + t.Run("UpdateGitSSHKeyEncryptsPlaintextRow", func(t *testing.T) { + // A row that started life as plaintext must get encrypted on the next write. + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + _, err := db.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{ + UserID: user.ID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + PrivateKey: initialPrivate, + PublicKey: publicKey, + }) + require.NoError(t, err) + + _, err = crypt.UpdateGitSSHKey(ctx, database.UpdateGitSSHKeyParams{ + UserID: user.ID, + UpdatedAt: dbtime.Now(), + PrivateKey: updatedPrivate, + PublicKey: publicKey, + }) + require.NoError(t, err) + + rawKey, err := db.GetGitSSHKey(ctx, user.ID) + require.NoError(t, err) + requireEncryptedEquals(t, ciphers[0], rawKey.PrivateKey, updatedPrivate) + require.True(t, rawKey.PrivateKeyKeyID.Valid) + require.Equal(t, ciphers[0].HexDigest(), rawKey.PrivateKeyKeyID.String) + }) + + t.Run("GetGitSSHKeyDecryptErr", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + user := dbgen.User(t, db, database.User{}) + _, err := db.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{ + UserID: user.ID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + PrivateKey: fakeBase64RandomData(t, 32), + PrivateKeyKeyID: sql.NullString{String: ciphers[0].HexDigest(), Valid: true}, + PublicKey: publicKey, + }) + require.NoError(t, err) + + _, err = crypt.GetGitSSHKey(ctx, user.ID) + require.Error(t, err) + var derr *DecryptFailedError + require.ErrorAs(t, err, &derr) + }) + + t.Run("NoCipherPassthrough", func(t *testing.T) { + t.Parallel() + db, crypt := setupNoCiphers(t) + user := dbgen.User(t, crypt, database.User{}) + key, err := crypt.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{ + UserID: user.ID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + PrivateKey: initialPrivate, + PublicKey: publicKey, + }) + require.NoError(t, err) + require.Equal(t, initialPrivate, key.PrivateKey) + require.False(t, key.PrivateKeyKeyID.Valid) + + rawKey, err := db.GetGitSSHKey(ctx, user.ID) + require.NoError(t, err) + require.Equal(t, initialPrivate, rawKey.PrivateKey) + require.False(t, rawKey.PrivateKeyKeyID.Valid) + }) +}