Skip to content

Commit b9cdec9

Browse files
johnstcnpull[bot]
johnstcn
authored and
pull[bot]
committed
feat(coderd): connect dbcrypt package implementation (#9523)
See also: #9522 - Adds commands `server dbcrypt {rotate,decrypt,delete}` to re-encrypt, decrypt, or delete encrypted data, respectively. - Plumbs through dbcrypt in enterprise/coderd (including unit tests). - Adds documentation in admin/encryption.md. This enables dbcrypt by default, but the feature is soft-enforced on supplying external token encryption keys. Without specifying any keys, encryption/decryption is a no-op.
1 parent 2c77f4a commit b9cdec9

36 files changed

+1600
-36
lines changed

‎cli/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
691691
options.Database = dbfake.New()
692692
options.Pubsub = pubsub.NewInMemory()
693693
} else {
694-
sqlDB, err := connectToPostgres(ctx, logger, sqlDriver, vals.PostgresURL.String())
694+
sqlDB, err := ConnectToPostgres(ctx, logger, sqlDriver, vals.PostgresURL.String())
695695
if err != nil {
696696
return xerrors.Errorf("connect to postgres: %w", err)
697697
}
@@ -1953,7 +1953,7 @@ func BuildLogger(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (slog.
19531953
}, nil
19541954
}
19551955

1956-
func connectToPostgres(ctx context.Context, logger slog.Logger, driver string, dbURL string) (*sql.DB, error) {
1956+
func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, dbURL string) (*sql.DB, error) {
19571957
logger.Debug(ctx, "connecting to postgresql")
19581958

19591959
// Try to connect for 30 seconds.

‎cli/server_createadminuser.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func (r *RootCmd) newCreateAdminUserCommand() *clibase.Cmd {
6363
newUserDBURL = url
6464
}
6565

66-
sqlDB, err := connectToPostgres(ctx, logger, "postgres", newUserDBURL)
66+
sqlDB, err := ConnectToPostgres(ctx, logger, "postgres", newUserDBURL)
6767
if err != nil {
6868
return xerrors.Errorf("connect to postgres: %w", err)
6969
}

‎cli/server_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,13 @@ import (
3434
"go.uber.org/goleak"
3535
"gopkg.in/yaml.v3"
3636

37+
"cdr.dev/slog/sloggers/slogtest"
38+
3739
"github.com/coder/coder/v2/cli"
3840
"github.com/coder/coder/v2/cli/clitest"
3941
"github.com/coder/coder/v2/cli/config"
4042
"github.com/coder/coder/v2/coderd/coderdtest"
43+
"github.com/coder/coder/v2/coderd/database/dbtestutil"
4144
"github.com/coder/coder/v2/coderd/database/postgres"
4245
"github.com/coder/coder/v2/coderd/telemetry"
4346
"github.com/coder/coder/v2/codersdk"
@@ -1657,3 +1660,26 @@ func TestServerYAMLConfig(t *testing.T) {
16571660

16581661
require.Equal(t, string(wantByt), string(got))
16591662
}
1663+
1664+
func TestConnectToPostgres(t *testing.T) {
1665+
t.Parallel()
1666+
1667+
if !dbtestutil.WillUsePostgres() {
1668+
t.Skip("this test does not make sense without postgres")
1669+
}
1670+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
1671+
t.Cleanup(cancel)
1672+
1673+
log := slogtest.Make(t, nil)
1674+
1675+
dbURL, closeFunc, err := postgres.Open()
1676+
require.NoError(t, err)
1677+
t.Cleanup(closeFunc)
1678+
1679+
sqlDB, err := cli.ConnectToPostgres(ctx, log, "postgres", dbURL)
1680+
require.NoError(t, err)
1681+
t.Cleanup(func() {
1682+
_ = sqlDB.Close()
1683+
})
1684+
require.NoError(t, sqlDB.PingContext(ctx))
1685+
}

‎cli/testdata/coder_server_--help.golden

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,16 @@ These options are only available in the Enterprise Edition.
458458
An HTTP URL that is accessible by other replicas to relay DERP
459459
traffic. Required for high availability.
460460

461+
--external-token-encryption-keys string-array, $CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS
462+
Encrypt OIDC and Git authentication tokens with AES-256-GCM in the
463+
database. The value must be a comma-separated list of base64-encoded
464+
keys. Each key, when base64-decoded, must be exactly 32 bytes in
465+
length. The first key will be used to encrypt new values. Subsequent
466+
keys will be used as a fallback when decrypting. During normal
467+
operation it is recommended to only set one key unless you are in the
468+
process of rotating keys with the `coder server dbcrypt rotate`
469+
command.
470+
461471
--scim-auth-header string, $CODER_SCIM_AUTH_HEADER
462472
Enables SCIM and sets the authentication header for the built-in SCIM
463473
server. New users are automatically created with OIDC authentication.

‎coderd/apidoc/docs.go

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/deployment_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func TestDeploymentValues(t *testing.T) {
2626
cfg.OIDC.EmailField.Set("some_random_field_you_never_expected")
2727
cfg.PostgresURL.Set(hi)
2828
cfg.SCIMAPIKey.Set(hi)
29+
cfg.ExternalTokenEncryptionKeys.Set("the_random_key_we_never_expected,an_other_key_we_never_unexpected")
2930

3031
client := coderdtest.New(t, &coderdtest.Options{
3132
DeploymentValues: cfg,
@@ -44,6 +45,7 @@ func TestDeploymentValues(t *testing.T) {
4445
require.Empty(t, scrubbed.Values.OIDC.ClientSecret.Value())
4546
require.Empty(t, scrubbed.Values.PostgresURL.Value())
4647
require.Empty(t, scrubbed.Values.SCIMAPIKey.Value())
48+
require.Empty(t, scrubbed.Values.ExternalTokenEncryptionKeys.Value())
4749
}
4850

4951
func TestDeploymentStats(t *testing.T) {

‎coderd/httpmw/apikey.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,12 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
248248
UserID: key.UserID,
249249
LoginType: key.LoginType,
250250
})
251+
if errors.Is(err, sql.ErrNoRows) {
252+
return optionalWrite(http.StatusUnauthorized, codersdk.Response{
253+
Message: SignedOutErrorMessage,
254+
Detail: "You must re-authenticate with the login provider.",
255+
})
256+
}
251257
if err != nil {
252258
return write(http.StatusInternalServerError, codersdk.Response{
253259
Message: "A database error occurred",

‎coderd/httpmw/apikey_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,34 @@ func TestAPIKey(t *testing.T) {
153153
require.Equal(t, http.StatusUnauthorized, res.StatusCode)
154154
})
155155

156+
t.Run("UserLinkNotFound", func(t *testing.T) {
157+
t.Parallel()
158+
var (
159+
db = dbfake.New()
160+
r = httptest.NewRequest("GET", "/", nil)
161+
rw = httptest.NewRecorder()
162+
user = dbgen.User(t, db, database.User{
163+
LoginType: database.LoginTypeGithub,
164+
})
165+
// Intentionally not inserting any user link
166+
_, token = dbgen.APIKey(t, db, database.APIKey{
167+
UserID: user.ID,
168+
LoginType: user.LoginType,
169+
})
170+
)
171+
r.Header.Set(codersdk.SessionTokenHeader, token)
172+
httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
173+
DB: db,
174+
RedirectToLogin: false,
175+
})(successHandler).ServeHTTP(rw, r)
176+
res := rw.Result()
177+
defer res.Body.Close()
178+
require.Equal(t, http.StatusUnauthorized, res.StatusCode)
179+
var resp codersdk.Response
180+
require.NoError(t, json.NewDecoder(res.Body).Decode(&resp))
181+
require.Equal(t, resp.Message, httpmw.SignedOutErrorMessage)
182+
})
183+
156184
t.Run("InvalidSecret", func(t *testing.T) {
157185
t.Parallel()
158186
var (

‎codersdk/deployment.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ const (
4646
FeatureExternalProvisionerDaemons FeatureName = "external_provisioner_daemons"
4747
FeatureAppearance FeatureName = "appearance"
4848
FeatureAdvancedTemplateScheduling FeatureName = "advanced_template_scheduling"
49-
FeatureTemplateAutostopRequirement FeatureName = "template_autostop_requirement"
5049
FeatureWorkspaceProxy FeatureName = "workspace_proxy"
50+
FeatureExternalTokenEncryption FeatureName = "external_token_encryption"
51+
FeatureTemplateAutostopRequirement FeatureName = "template_autostop_requirement"
5152
FeatureWorkspaceBatchActions FeatureName = "workspace_batch_actions"
5253
)
5354

@@ -65,6 +66,8 @@ var FeatureNames = []FeatureName{
6566
FeatureAdvancedTemplateScheduling,
6667
FeatureWorkspaceProxy,
6768
FeatureUserRoleManagement,
69+
FeatureExternalTokenEncryption,
70+
FeatureTemplateAutostopRequirement,
6871
FeatureWorkspaceBatchActions,
6972
}
7073

@@ -154,6 +157,7 @@ type DeploymentValues struct {
154157
AgentFallbackTroubleshootingURL clibase.URL `json:"agent_fallback_troubleshooting_url,omitempty" typescript:",notnull"`
155158
BrowserOnly clibase.Bool `json:"browser_only,omitempty" typescript:",notnull"`
156159
SCIMAPIKey clibase.String `json:"scim_api_key,omitempty" typescript:",notnull"`
160+
ExternalTokenEncryptionKeys clibase.StringArray `json:"external_token_encryption_keys,omitempty" typescript:",notnull"`
157161
Provisioner ProvisionerConfig `json:"provisioner,omitempty" typescript:",notnull"`
158162
RateLimit RateLimitConfig `json:"rate_limit,omitempty" typescript:",notnull"`
159163
Experiments clibase.StringArray `json:"experiments,omitempty" typescript:",notnull"`
@@ -1605,7 +1609,14 @@ when required by your organization's security policy.`,
16051609
Annotations: clibase.Annotations{}.Mark(annotationEnterpriseKey, "true").Mark(annotationSecretKey, "true"),
16061610
Value: &c.SCIMAPIKey,
16071611
},
1608-
1612+
{
1613+
Name: "External Token Encryption Keys",
1614+
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.",
1615+
Flag: "external-token-encryption-keys",
1616+
Env: "CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS",
1617+
Annotations: clibase.Annotations{}.Mark(annotationEnterpriseKey, "true").Mark(annotationSecretKey, "true"),
1618+
Value: &c.ExternalTokenEncryptionKeys,
1619+
},
16091620
{
16101621
Name: "Disable Path Apps",
16111622
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) {
17831794

17841795
// This only works with string values for now.
17851796
switch v := opt.Value.(type) {
1786-
case *clibase.String:
1797+
case *clibase.String, *clibase.StringArray:
17871798
err := v.Set("")
17881799
if err != nil {
17891800
panic(err)

‎codersdk/deployment_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ func TestDeploymentValues_HighlyConfigurable(t *testing.T) {
5757
"SCIM API Key": {
5858
yaml: true,
5959
},
60+
"External Token Encryption Keys": {
61+
yaml: true,
62+
},
6063
// These complex objects should be configured through YAML.
6164
"Support Links": {
6265
flag: true,

0 commit comments

Comments
 (0)