Skip to content

feat(coderd): plumb through dbcrypt package #9433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
plumb through dbcrypt in enterprise/coderd:
- 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

Co-authored-by: Kyle Carberry <kyle@coder.com>
  • Loading branch information
johnstcn and kylecarbs committed Aug 30, 2023
commit 66048de541e05aeb71228fa01db5ab1fad072800
4 changes: 2 additions & 2 deletions cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,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)
}
Expand Down Expand Up @@ -1950,7 +1950,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.
Expand Down
2 changes: 1 addition & 1 deletion cli/server_createadminuser.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,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)
}
Expand Down
8 changes: 8 additions & 0 deletions cli/testdata/coder_server_--help.golden
Original file line number Diff line number Diff line change
Expand Up @@ -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. A maximum of two keys may be provided. 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a little confusing that there's a maximum of two keys but this refers to "subsequent keys". Maybe it should say "the optional second key" instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on other comments, I'm considering removing the restriction on the number of keys. I can see a legitimate use case for needing to have three keys for a period of time.

fallback when decrypting.

--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.
Expand Down
6 changes: 6 additions & 0 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions coderd/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions coderd/httpmw/apikey.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,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",
Expand Down
17 changes: 14 additions & 3 deletions codersdk/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

// FeatureNames must be kept in-sync with the Feature enum above.
Expand All @@ -64,6 +65,8 @@ var FeatureNames = []FeatureName{
FeatureAdvancedTemplateScheduling,
FeatureWorkspaceProxy,
FeatureUserRoleManagement,
FeatureExternalTokenEncryption,
FeatureTemplateAutostopRequirement,
}

// Humanize returns the feature name in a human-readable format.
Expand Down Expand Up @@ -152,6 +155,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"`
Expand Down Expand Up @@ -1603,7 +1607,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. A maximum of two keys may be provided. 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.",
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.",
Expand Down Expand Up @@ -1781,7 +1792,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)
Expand Down
3 changes: 3 additions & 0 deletions codersdk/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions docs/api/general.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions docs/api/schemas.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Coder — A tool for provisioning self-hosted development environments with Terr
| ------------------------------------------------------ | ----------------------------------------------------------------------------------------------------- |
| [<code>config-ssh</code>](./cli/config-ssh.md) | Add an SSH Host entry for your workspaces "ssh coder.workspace" |
| [<code>create</code>](./cli/create.md) | Create a workspace |
| [<code>dbcrypt-rotate</code>](./cli/dbcrypt-rotate.md) | Rotate database encryption keys |
| [<code>delete</code>](./cli/delete.md) | Delete a workspace |
| [<code>dotfiles</code>](./cli/dotfiles.md) | Personalize your workspace by applying a canonical dotfiles repository |
| [<code>features</code>](./cli/features.md) | List Enterprise features |
Expand Down
31 changes: 31 additions & 0 deletions docs/cli/dbcrypt-rotate.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions docs/cli/server.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

131 changes: 131 additions & 0 deletions enterprise/cli/dbcrypt_rotate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
//go:build !slim

package cli

import (
"bytes"
"context"
"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 <postgres_url> --external-token-encryption-keys <new-key>,<old-key>",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

running this from the CLI that has to have direct DB access is kind of obnoxious from an operator's standpoint.

What about making it an API call you could make to an appropriately-keyed Coderd? We could still have the CLI command, but you could run it from anywhere with Coder access. We could also consider a button on the front end at some later date.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this would be a nice usability improvement, it has precedence already with the reset-password CLI command. It's a good candidate for a follow-up PR though!

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 vals.PostgresURL == "" {
return xerrors.Errorf("no database configured")
}

if vals.ExternalTokenEncryptionKeys == nil || len(vals.ExternalTokenEncryptionKeys) != 2 {
return xerrors.Errorf("dbcrypt-rotate requires exactly two external token encryption keys")
}

newKey, err := base64.StdEncoding.DecodeString(vals.ExternalTokenEncryptionKeys[0])
if err != nil {
return xerrors.Errorf("new key must be base64-encoded")
}
oldKey, err := base64.StdEncoding.DecodeString(vals.ExternalTokenEncryptionKeys[1])
if err != nil {
return xerrors.Errorf("old key must be base64-encoded")
}
if bytes.Equal(newKey, oldKey) {
return xerrors.Errorf("old and new keys must be different")
}

primaryCipher, err := dbcrypt.CipherAES256(newKey)
if err != nil {
return xerrors.Errorf("create primary cipher: %w", err)
}
secondaryCipher, err := dbcrypt.CipherAES256(oldKey)
if err != nil {
return xerrors.Errorf("create secondary cipher: %w", err)
}
ciphers := dbcrypt.NewCiphers(primaryCipher, secondaryCipher)

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)
}
for idx, usr := range users {
userLinks, err := cryptDB.GetUserLinksByUserID(ctx, usr.ID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should put comments everywhere so people forget to add something in one place and forget it here.

if err != nil {
return xerrors.Errorf("get user links for user: %w", err)
}
for _, userLink := range userLinks {
if _, err := cryptDB.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 := cryptDB.GetGitAuthLinksByUserID(ctx, usr.ID)
if err != nil {
return xerrors.Errorf("get git auth links for user: %w", err)
}
for _, gitAuthLink := range gitAuthLinks {
if _, err := cryptDB.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)
}
}
logger.Info(ctx, "encrypted user tokens", slog.F("current", idx+1), slog.F("of", len(users)))
}
logger.Info(ctx, "operation completed successfully")
return nil
},
}
return cmd
}
Loading