Skip to content

Commit b38c1c8

Browse files
deansheatherdeansheather
committed
chore: change app signing key to be 96 bytes
1 parent 2e710e2 commit b38c1c8

File tree

13 files changed

+99
-59
lines changed

13 files changed

+99
-59
lines changed

‎cli/server.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -778,23 +778,31 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
778778
}
779779
}
780780

781-
// Read the app signing key from the DB. We store it hex
782-
// encoded since the config table uses strings for the value and
783-
// we don't want to deal with automatic encoding issues.
781+
// Read the app signing key from the DB. We store it hex encoded
782+
// since the config table uses strings for the value and we
783+
// don't want to deal with automatic encoding issues.
784784
appSigningKeyStr, err := tx.GetAppSigningKey(ctx)
785785
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
786786
return xerrors.Errorf("get app signing key: %w", err)
787787
}
788-
if appSigningKeyStr == "" {
789-
// Generate 64 byte secure random string.
790-
b := make([]byte, 64)
788+
// If the string in the DB is an invalid hex string or the
789+
// length is not equal to the current key length, generate a new
790+
// one.
791+
//
792+
// If the key is regenerated, old signed tokens and encrypted
793+
// strings will become invalid. New signed app tokens will be
794+
// generated automatically on failure. Any workspace app token
795+
// smuggling operations in progress may fail, although with a
796+
// helpful error.
797+
if decoded, err := hex.DecodeString(appSigningKeyStr); err != nil || len(decoded) != len(workspaceapps.SigningKey{}) {
798+
b := make([]byte, len(workspaceapps.SigningKey{}))
791799
_, err := rand.Read(b)
792800
if err != nil {
793801
return xerrors.Errorf("generate fresh app signing key: %w", err)
794802
}
795803

796804
appSigningKeyStr = hex.EncodeToString(b)
797-
err = tx.InsertAppSigningKey(ctx, appSigningKeyStr)
805+
err = tx.UpsertAppSigningKey(ctx, appSigningKeyStr)
798806
if err != nil {
799807
return xerrors.Errorf("insert freshly generated app signing key to database: %w", err)
800808
}

‎coderd/coderd.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ func New(options *Options) *API {
328328
api.workspaceAgentCache = wsconncache.New(api.dialWorkspaceAgentTailnet, 0)
329329
api.TailnetCoordinator.Store(&options.TailnetCoordinator)
330330

331-
workspaceAppServer := workspaceapps.Server{
331+
api.workspaceAppServer = &workspaceapps.Server{
332332
Logger: options.Logger.Named("workspaceapps"),
333333

334334
DashboardURL: api.AccessURL,
@@ -380,7 +380,7 @@ func New(options *Options) *API {
380380
//
381381
// Workspace apps do their own auth and must be BEFORE the auth
382382
// middleware.
383-
workspaceAppServer.SubdomainAppMW(apiRateLimiter),
383+
api.workspaceAppServer.SubdomainAppMW(apiRateLimiter),
384384
// Build-Version is helpful for debugging.
385385
func(next http.Handler) http.Handler {
386386
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -406,7 +406,7 @@ func New(options *Options) *API {
406406
// Attach workspace apps routes.
407407
r.Group(func(r chi.Router) {
408408
r.Use(apiRateLimiter)
409-
workspaceAppServer.Attach(r)
409+
api.workspaceAppServer.Attach(r)
410410
})
411411

412412
r.Route("/derp", func(r chi.Router) {
@@ -796,6 +796,7 @@ type API struct {
796796
workspaceAgentCache *wsconncache.Cache
797797
updateChecker *updatecheck.Checker
798798
WorkspaceAppsProvider workspaceapps.SignedTokenProvider
799+
workspaceAppServer *workspaceapps.Server
799800

800801
// Experiments contains the list of experiments currently enabled.
801802
// This is used to gate features that are not yet ready for production.

‎coderd/coderdtest/coderdtest.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ import (
8080
"github.com/coder/coder/testutil"
8181
)
8282

83-
// AppSigningKey is a 64-byte key used to sign JWTs for workspace app tokens in
84-
// tests.
85-
var AppSigningKey = must(workspaceapps.KeyFromString("64656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566"))
83+
// AppSigningKey is a 64-byte key used to sign JWTs and encrypt JWEs for
84+
// workspace app tokens in tests.
85+
var AppSigningKey = must(workspaceapps.KeyFromString("6465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e2077617320686572"))
8686

8787
type Options struct {
8888
// AccessURL denotes a custom access URL. By default we use the httptest

‎coderd/database/dbauthz/querier.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,9 +384,9 @@ func (q *querier) GetAppSigningKey(ctx context.Context) (string, error) {
384384
return q.db.GetAppSigningKey(ctx)
385385
}
386386

387-
func (q *querier) InsertAppSigningKey(ctx context.Context, data string) error {
387+
func (q *querier) UpsertAppSigningKey(ctx context.Context, data string) error {
388388
// No authz checks as this is done during startup
389-
return q.db.InsertAppSigningKey(ctx, data)
389+
return q.db.UpsertAppSigningKey(ctx, data)
390390
}
391391

392392
func (q *querier) GetServiceBanner(ctx context.Context) (string, error) {

‎coderd/database/dbfake/databasefake.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4451,7 +4451,7 @@ func (q *fakeQuerier) GetAppSigningKey(_ context.Context) (string, error) {
44514451
return q.appSigningKey, nil
44524452
}
44534453

4454-
func (q *fakeQuerier) InsertAppSigningKey(_ context.Context, data string) error {
4454+
func (q *fakeQuerier) UpsertAppSigningKey(_ context.Context, data string) error {
44554455
q.mutex.Lock()
44564456
defer q.mutex.Unlock()
44574457

‎coderd/database/dbgen/generator.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,23 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database
7777
secret, _ := cryptorand.String(22)
7878
hashed := sha256.Sum256([]byte(secret))
7979

80+
ip := seed.IPAddress
81+
if !ip.Valid {
82+
ip = pqtype.Inet{
83+
IPNet: net.IPNet{
84+
IP: net.IPv4(127, 0, 0, 1),
85+
Mask: net.IPv4Mask(255, 255, 255, 255),
86+
},
87+
Valid: true,
88+
}
89+
}
90+
8091
key, err := db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{
8192
ID: takeFirst(seed.ID, id),
8293
// 0 defaults to 86400 at the db layer
8394
LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0),
8495
HashedSecret: takeFirstSlice(seed.HashedSecret, hashed[:]),
85-
IPAddress: pqtype.Inet{},
96+
IPAddress: ip,
8697
UserID: takeFirst(seed.UserID, uuid.New()),
8798
LastUsed: takeFirst(seed.LastUsed, database.Now()),
8899
ExpiresAt: takeFirst(seed.ExpiresAt, database.Now().Add(time.Hour)),

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

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

‎coderd/database/queries/siteconfig.sql

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,6 @@ SELECT value FROM site_configs WHERE key = 'logo_url';
3434
-- name: GetAppSigningKey :one
3535
SELECT value FROM site_configs WHERE key = 'app_signing_key';
3636

37-
-- name: InsertAppSigningKey :exec
38-
INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1);
37+
-- name: UpsertAppSigningKey :exec
38+
INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1)
39+
ON CONFLICT (key) DO UPDATE set value = $1 WHERE site_configs.key = 'app_signing_key';

‎coderd/workspaceapps/db.go

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,12 @@ type DBTokenProvider struct {
3232
DeploymentValues *codersdk.DeploymentValues
3333
OAuth2Configs *httpmw.OAuth2Configs
3434
WorkspaceAgentInactiveTimeout time.Duration
35-
TokenSigningKey SigningKey
35+
SigningKey SigningKey
3636
}
3737

3838
var _ SignedTokenProvider = &DBTokenProvider{}
3939

40-
func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, tokenSigningKey SigningKey) SignedTokenProvider {
41-
if len(tokenSigningKey) != 64 {
42-
panic("token signing key must be 64 bytes")
43-
}
44-
40+
func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, signingKey SigningKey) SignedTokenProvider {
4541
if workspaceAgentInactiveTimeout == 0 {
4642
workspaceAgentInactiveTimeout = 1 * time.Minute
4743
}
@@ -54,15 +50,15 @@ func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authoriz
5450
DeploymentValues: cfg,
5551
OAuth2Configs: oauth2Cfgs,
5652
WorkspaceAgentInactiveTimeout: workspaceAgentInactiveTimeout,
57-
TokenSigningKey: tokenSigningKey,
53+
SigningKey: signingKey,
5854
}
5955
}
6056

6157
func (p *DBTokenProvider) TokenFromRequest(r *http.Request) (*SignedToken, bool) {
6258
// Get the existing token from the request.
6359
tokenCookie, err := r.Cookie(codersdk.DevURLSignedAppTokenCookie)
6460
if err == nil {
65-
token, err := p.TokenSigningKey.VerifySignedToken(tokenCookie.Value)
61+
token, err := p.SigningKey.VerifySignedToken(tokenCookie.Value)
6662
if err == nil {
6763
req := token.Request.Normalize()
6864
err := req.Validate()
@@ -130,9 +126,6 @@ func (p *DBTokenProvider) CreateToken(ctx context.Context, rw http.ResponseWrite
130126
token.AgentID = dbReq.Agent.ID
131127
token.AppURL = dbReq.AppURL
132128

133-
// TODO(@deansheather): return an error if the agent is offline or the app
134-
// is not running.
135-
136129
// Verify the user has access to the app.
137130
authed, err := p.authorizeRequest(r.Context(), authz, dbReq)
138131
if err != nil {
@@ -196,7 +189,7 @@ func (p *DBTokenProvider) CreateToken(ctx context.Context, rw http.ResponseWrite
196189

197190
// Sign the token.
198191
token.Expiry = time.Now().Add(DefaultTokenExpiry)
199-
tokenStr, err := p.TokenSigningKey.SignToken(token)
192+
tokenStr, err := p.SigningKey.SignToken(token)
200193
if err != nil {
201194
WriteWorkspaceApp500(p.Logger, p.AccessURL, rw, r, &appReq, err, "generate token")
202195
return nil, "", false

0 commit comments

Comments
 (0)