From 808aa32caf7dd1942145d5b47c3bd42d4426b068 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 24 Sep 2024 22:18:07 +0000 Subject: [PATCH 1/7] feat: add wsproxy endpoint for fetching keys --- coderd/database/dbauthz/dbauthz.go | 7 +++ coderd/database/dbauthz/dbauthz_test.go | 4 ++ coderd/database/dbmem/dbmem.go | 17 +++++++ coderd/database/dbmetrics/dbmetrics.go | 7 +++ coderd/database/dbmock/dbmock.go | 15 ++++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 38 ++++++++++++++ coderd/database/queries/crypto_keys.sql | 7 +++ enterprise/coderd/coderd.go | 1 + enterprise/coderd/workspaceproxy.go | 28 +++++++++++ enterprise/dbcrypt/dbcrypt.go | 15 ++++++ enterprise/dbcrypt/dbcrypt_internal_test.go | 29 +++++++++++ enterprise/wsproxy/wsproxysdk/wsproxysdk.go | 56 +++++++++++++++++++-- 13 files changed, 220 insertions(+), 5 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 83ad60bb8351e..6436e7c6e3425 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1405,6 +1405,13 @@ func (q *querier) GetCryptoKeys(ctx context.Context) ([]database.CryptoKey, erro return q.db.GetCryptoKeys(ctx) } +func (q *querier) GetCryptoKeysByFeature(ctx context.Context, feature database.CryptoKeyFeature) ([]database.CryptoKey, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceCryptoKey); err != nil { + return nil, err + } + return q.db.GetCryptoKeysByFeature(ctx, feature) +} + func (q *querier) GetDBCryptKeys(ctx context.Context) ([]database.DBCryptKey, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { return nil, err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 10d5a32323a5f..f3aec6c9326b0 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2302,6 +2302,10 @@ func (s *MethodTestSuite) TestCryptoKeys() { DeletesAt: sql.NullTime{Time: time.Now(), Valid: true}, }).Asserts(rbac.ResourceCryptoKey, policy.ActionUpdate) })) + s.Run("GetCryptoKeysByFeature", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.CryptoKeyFeatureWorkspaceApps). + Asserts(rbac.ResourceCryptoKey, policy.ActionRead) + })) } func (s *MethodTestSuite) TestSystemFunctions() { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index dbf8e5514ea2e..5ddfe9d5026c0 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -2429,6 +2429,23 @@ func (q *FakeQuerier) GetCryptoKeys(_ context.Context) ([]database.CryptoKey, er return keys, nil } +func (q *FakeQuerier) GetCryptoKeysByFeature(_ context.Context, feature database.CryptoKeyFeature) ([]database.CryptoKey, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + keys := make([]database.CryptoKey, 0) + for _, key := range q.cryptoKeys { + if key.Feature == feature { + keys = append(keys, key) + } + } + // We want to return the highest sequence number first. + slices.SortFunc(keys, func(i, j database.CryptoKey) int { + return int(j.Sequence - i.Sequence) + }) + return keys, nil +} + func (q *FakeQuerier) GetDBCryptKeys(_ context.Context) ([]database.DBCryptKey, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index afdd6e35a14a2..b050a4ce9afc4 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -564,6 +564,13 @@ func (m metricsStore) GetCryptoKeys(ctx context.Context) ([]database.CryptoKey, return r0, r1 } +func (m metricsStore) GetCryptoKeysByFeature(ctx context.Context, feature database.CryptoKeyFeature) ([]database.CryptoKey, error) { + start := time.Now() + r0, r1 := m.s.GetCryptoKeysByFeature(ctx, feature) + m.queryLatencies.WithLabelValues("GetCryptoKeysByFeature").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetDBCryptKeys(ctx context.Context) ([]database.DBCryptKey, error) { start := time.Now() r0, r1 := m.s.GetDBCryptKeys(ctx) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index d85f29e154f17..3c7dbd6d9b958 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1103,6 +1103,21 @@ func (mr *MockStoreMockRecorder) GetCryptoKeys(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCryptoKeys", reflect.TypeOf((*MockStore)(nil).GetCryptoKeys), arg0) } +// GetCryptoKeysByFeature mocks base method. +func (m *MockStore) GetCryptoKeysByFeature(arg0 context.Context, arg1 database.CryptoKeyFeature) ([]database.CryptoKey, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCryptoKeysByFeature", arg0, arg1) + ret0, _ := ret[0].([]database.CryptoKey) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCryptoKeysByFeature indicates an expected call of GetCryptoKeysByFeature. +func (mr *MockStoreMockRecorder) GetCryptoKeysByFeature(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCryptoKeysByFeature", reflect.TypeOf((*MockStore)(nil).GetCryptoKeysByFeature), arg0, arg1) +} + // GetDBCryptKeys mocks base method. func (m *MockStore) GetDBCryptKeys(arg0 context.Context) ([]database.DBCryptKey, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 46086fa72d072..d71c54e008350 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -134,6 +134,7 @@ type sqlcQuerier interface { GetCoordinatorResumeTokenSigningKey(ctx context.Context) (string, error) GetCryptoKeyByFeatureAndSequence(ctx context.Context, arg GetCryptoKeyByFeatureAndSequenceParams) (CryptoKey, error) GetCryptoKeys(ctx context.Context) ([]CryptoKey, error) + GetCryptoKeysByFeature(ctx context.Context, feature CryptoKeyFeature) ([]CryptoKey, error) GetDBCryptKeys(ctx context.Context) ([]DBCryptKey, error) GetDERPMeshKey(ctx context.Context) (string, error) GetDefaultOrganization(ctx context.Context) (Organization, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a8ea40a395cf1..ba23191bfbb2b 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -849,6 +849,44 @@ func (q *sqlQuerier) GetCryptoKeys(ctx context.Context) ([]CryptoKey, error) { return items, nil } +const getCryptoKeysByFeature = `-- name: GetCryptoKeysByFeature :many +SELECT feature, sequence, secret, secret_key_id, starts_at, deletes_at +FROM crypto_keys +WHERE feature = $1 +AND secret IS NOT NULL +ORDER BY sequence DESC +` + +func (q *sqlQuerier) GetCryptoKeysByFeature(ctx context.Context, feature CryptoKeyFeature) ([]CryptoKey, error) { + rows, err := q.db.QueryContext(ctx, getCryptoKeysByFeature, feature) + if err != nil { + return nil, err + } + defer rows.Close() + var items []CryptoKey + for rows.Next() { + var i CryptoKey + if err := rows.Scan( + &i.Feature, + &i.Sequence, + &i.Secret, + &i.SecretKeyID, + &i.StartsAt, + &i.DeletesAt, + ); 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 getLatestCryptoKeyByFeature = `-- name: GetLatestCryptoKeyByFeature :one SELECT feature, sequence, secret, secret_key_id, starts_at, deletes_at FROM crypto_keys diff --git a/coderd/database/queries/crypto_keys.sql b/coderd/database/queries/crypto_keys.sql index 281e303751524..71f0291b08993 100644 --- a/coderd/database/queries/crypto_keys.sql +++ b/coderd/database/queries/crypto_keys.sql @@ -3,6 +3,13 @@ SELECT * FROM crypto_keys WHERE secret IS NOT NULL; +-- name: GetCryptoKeysByFeature :many +SELECT * +FROM crypto_keys +WHERE feature = $1 +AND secret IS NOT NULL +ORDER BY sequence DESC; + -- name: GetLatestCryptoKeyByFeature :one SELECT * FROM crypto_keys diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index c030441253f88..e1d1f72a98132 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -243,6 +243,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { r.Post("/app-stats", api.workspaceProxyReportAppStats) r.Post("/register", api.workspaceProxyRegister) r.Post("/deregister", api.workspaceProxyDeregister) + r.Get("/crypto-keys", api.workspaceProxyCryptoKeys) }) r.Route("/{workspaceproxy}", func(r chi.Router) { r.Use( diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index aa431c5fe231d..8d2709e6b5203 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -710,6 +710,20 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) go api.forceWorkspaceProxyHealthUpdate(api.ctx) } +func (api *API) workspaceProxyCryptoKeys(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + keys, err := api.Database.GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, wsproxysdk.CryptoKeysResponse{ + CryptoKeys: fromDBCryptoKeys(keys), + }) +} + // @Summary Deregister workspace proxy // @ID deregister-workspace-proxy // @Security CoderSessionToken @@ -967,3 +981,17 @@ func (w *workspaceProxiesFetchUpdater) Fetch(ctx context.Context) (codersdk.Regi func (w *workspaceProxiesFetchUpdater) Update(ctx context.Context) error { return w.updateFunc(ctx) } + +func fromDBCryptoKeys(keys []database.CryptoKey) []wsproxysdk.CryptoKey { + wskeys := make([]wsproxysdk.CryptoKey, 0, len(keys)) + for _, key := range keys { + wskeys = append(wskeys, wsproxysdk.CryptoKey{ + Feature: wsproxysdk.CryptoKeyFeature(key.Feature), + Secret: key.Secret.String, + DeletesAt: key.DeletesAt.Time, + Sequence: key.Sequence, + StartsAt: key.StartsAt, + }) + } + return wskeys +} diff --git a/enterprise/dbcrypt/dbcrypt.go b/enterprise/dbcrypt/dbcrypt.go index 02c619cef52d5..979a8ad137e6d 100644 --- a/enterprise/dbcrypt/dbcrypt.go +++ b/enterprise/dbcrypt/dbcrypt.go @@ -321,6 +321,21 @@ func (db *dbCrypt) UpdateCryptoKeyDeletesAt(ctx context.Context, arg database.Up return key, nil } +func (db *dbCrypt) GetCryptoKeysByFeature(ctx context.Context, feature database.CryptoKeyFeature) ([]database.CryptoKey, error) { + keys, err := db.Store.GetCryptoKeysByFeature(ctx, feature) + if err != nil { + return nil, err + } + + for i := range keys { + if err := db.decryptField(&keys[i].Secret.String, keys[i].SecretKeyID); err != nil { + return nil, err + } + } + + return keys, 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 e744317445789..432dc90061677 100644 --- a/enterprise/dbcrypt/dbcrypt_internal_test.go +++ b/enterprise/dbcrypt/dbcrypt_internal_test.go @@ -450,6 +450,35 @@ func TestCryptoKeys(t *testing.T) { require.Equal(t, ciphers[0].HexDigest(), key.SecretKeyID.String) }) + t.Run("GetCryptoKeysByFeature", func(t *testing.T) { + t.Parallel() + db, crypt, ciphers := setup(t) + expected := dbgen.CryptoKey(t, crypt, database.CryptoKey{ + Sequence: 2, + Feature: database.CryptoKeyFeatureTailnetResume, + Secret: sql.NullString{String: "test", Valid: true}, + }) + _ = dbgen.CryptoKey(t, crypt, database.CryptoKey{ + Feature: database.CryptoKeyFeatureWorkspaceApps, + Sequence: 43, + }) + keys, err := crypt.GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureTailnetResume) + require.NoError(t, err) + require.Len(t, keys, 1) + require.Equal(t, "test", keys[0].Secret.String) + require.Equal(t, ciphers[0].HexDigest(), keys[0].SecretKeyID.String) + require.Equal(t, expected.Sequence, keys[0].Sequence) + require.Equal(t, expected.Feature, keys[0].Feature) + + keys, err = db.GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureTailnetResume) + require.NoError(t, err) + require.Len(t, keys, 1) + requireEncryptedEquals(t, ciphers[0], keys[0].Secret.String, "test") + require.Equal(t, ciphers[0].HexDigest(), keys[0].SecretKeyID.String) + require.Equal(t, expected.Sequence, keys[0].Sequence) + require.Equal(t, expected.Feature, keys[0].Feature) + }) + t.Run("DecryptErr", func(t *testing.T) { t.Parallel() db, crypt, ciphers := setup(t) diff --git a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go index b3b833fb37c3a..f6c548cd77236 100644 --- a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go +++ b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go @@ -204,7 +204,37 @@ type RegisterWorkspaceProxyRequest struct { Version string `json:"version"` } +type CryptoKeyFeature string + +const ( + CryptoKeyFeatureWorkspaceApp CryptoKeyFeature = "workspace_apps" + CryptoKeyFeatureOIDCConvert CryptoKeyFeature = "oidc_convert" + CryptoKeyFeatureTailnetResume CryptoKeyFeature = "tailnet_resume" +) + +type CryptoKey struct { + Feature CryptoKeyFeature `json:"feature"` + Secret string `json:"secret"` + DeletesAt time.Time `json:"deletes_at"` + Sequence int32 `json:"sequence"` + StartsAt time.Time `json:"starts_at"` +} + +func (c CryptoKey) Active(now time.Time) bool { + now = now.UTC() + isAfterStartsAt := !c.StartsAt.IsZero() && !now.Before(c.StartsAt) + return isAfterStartsAt && !c.Invalid(now) +} + +func (c CryptoKey) Invalid(now time.Time) bool { + now = now.UTC() + noSecret := c.Secret == "" + afterDelete := !c.DeletesAt.IsZero() && !now.Before(c.DeletesAt.UTC()) + return noSecret || afterDelete +} + type RegisterWorkspaceProxyResponse struct { + Keys []CryptoKey `json:"keys"` AppSecurityKey string `json:"app_security_key"` DERPMeshKey string `json:"derp_mesh_key"` DERPRegionID int32 `json:"derp_region_id"` @@ -371,11 +401,6 @@ func (l *RegisterWorkspaceProxyLoop) Start(ctx context.Context) (RegisterWorkspa } failedAttempts = 0 - // Check for consistency. - if originalRes.AppSecurityKey != resp.AppSecurityKey { - l.failureFn(xerrors.New("app security key has changed, proxy must be restarted")) - return - } if originalRes.DERPMeshKey != resp.DERPMeshKey { l.failureFn(xerrors.New("DERP mesh key has changed, proxy must be restarted")) return @@ -580,6 +605,27 @@ func (c *Client) DialCoordinator(ctx context.Context) (agpl.MultiAgentConn, erro return ma, nil } +type CryptoKeysResponse struct { + CryptoKeys []CryptoKey `json:"crypto_keys"` +} + +func (c *Client) CryptoKeys(ctx context.Context) (CryptoKeysResponse, error) { + res, err := c.Request(ctx, http.MethodGet, + "/api/v2/workspaceproxies/me/crypto-keys", + nil, + ) + if err != nil { + return CryptoKeysResponse{}, xerrors.Errorf("make request: %w", err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return CryptoKeysResponse{}, codersdk.ReadBodyAsError(res) + } + var resp CryptoKeysResponse + return resp, json.NewDecoder(res.Body).Decode(&resp) +} + type remoteMultiAgentHandler struct { sdk *Client logger slog.Logger From a03026b44e5d244fe41b5bebde0139c7ceb5afd4 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 24 Sep 2024 23:05:49 +0000 Subject: [PATCH 2/7] add some tests --- enterprise/coderd/workspaceproxy.go | 6 +- enterprise/coderd/workspaceproxy_test.go | 117 ++++++++++++++++++++ enterprise/wsproxy/wsproxysdk/wsproxysdk.go | 8 +- 3 files changed, 127 insertions(+), 4 deletions(-) diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 8d2709e6b5203..23c16d546329d 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -987,10 +987,10 @@ func fromDBCryptoKeys(keys []database.CryptoKey) []wsproxysdk.CryptoKey { for _, key := range keys { wskeys = append(wskeys, wsproxysdk.CryptoKey{ Feature: wsproxysdk.CryptoKeyFeature(key.Feature), - Secret: key.Secret.String, - DeletesAt: key.DeletesAt.Time, Sequence: key.Sequence, - StartsAt: key.StartsAt, + StartsAt: key.StartsAt.UTC(), + DeletesAt: key.DeletesAt.Time.UTC(), + Secret: key.Secret.String, }) } return wskeys diff --git a/enterprise/coderd/workspaceproxy_test.go b/enterprise/coderd/workspaceproxy_test.go index bf6e46c08dfb5..e2a687517473a 100644 --- a/enterprise/coderd/workspaceproxy_test.go +++ b/enterprise/coderd/workspaceproxy_test.go @@ -1,6 +1,7 @@ package coderd_test import ( + "database/sql" "fmt" "net/http" "net/http/httptest" @@ -17,6 +18,7 @@ import ( "github.com/coder/coder/v2/buildinfo" "github.com/coder/coder/v2/coderd/coderdtest" "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/dbtime" "github.com/coder/coder/v2/coderd/workspaceapps" @@ -887,3 +889,118 @@ func TestReconnectingPTYSignedToken(t *testing.T) { // validate it here. }) } + +func TestGetCryptoKeys(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + db, pubsub := dbtestutil.NewDB(t) + cclient, _, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Database: db, + Pubsub: pubsub, + IncludeProvisionerDaemon: true, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureWorkspaceProxy: 1, + }, + }, + }) + + now := time.Now().UTC() + + expectedKey1 := dbgen.CryptoKey(t, db, database.CryptoKey{ + Feature: database.CryptoKeyFeatureWorkspaceApps, + StartsAt: now.Add(-time.Hour), + Sequence: 2, + }) + key1 := fromDBCryptoKeys(expectedKey1) + + expectedKey2 := dbgen.CryptoKey(t, db, database.CryptoKey{ + Feature: database.CryptoKeyFeatureWorkspaceApps, + StartsAt: now, + Sequence: 3, + }) + key2 := fromDBCryptoKeys(expectedKey2) + + // Create a deleted key. + _ = dbgen.CryptoKey(t, db, database.CryptoKey{ + Feature: database.CryptoKeyFeatureWorkspaceApps, + StartsAt: now.Add(-time.Hour), + Secret: sql.NullString{ + String: "secret1", + Valid: false, + }, + Sequence: 1, + }) + + // Create a key with different features. + _ = dbgen.CryptoKey(t, db, database.CryptoKey{ + Feature: database.CryptoKeyFeatureTailnetResume, + StartsAt: now.Add(-time.Hour), + Sequence: 1, + }) + _ = dbgen.CryptoKey(t, db, database.CryptoKey{ + Feature: database.CryptoKeyFeatureOidcConvert, + StartsAt: now.Add(-time.Hour), + Sequence: 1, + }) + + proxy := coderdenttest.NewWorkspaceProxyReplica(t, api, cclient, &coderdenttest.ProxyOptions{ + Name: testutil.GetRandomName(t), + }) + + keys, err := proxy.SDKClient.CryptoKeys(ctx) + require.NoError(t, err) + require.NotEmpty(t, keys) + require.Equal(t, 2, len(keys.CryptoKeys)) + require.Contains(t, keys.CryptoKeys, key1) + require.Contains(t, keys.CryptoKeys, key2) + }) + + t.Run("Unauthorized", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + db, pubsub := dbtestutil.NewDB(t) + cclient, _, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Database: db, + Pubsub: pubsub, + IncludeProvisionerDaemon: true, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureWorkspaceProxy: 1, + }, + }, + }) + + _ = coderdenttest.NewWorkspaceProxyReplica(t, api, cclient, &coderdenttest.ProxyOptions{ + Name: testutil.GetRandomName(t), + }) + + client := wsproxysdk.New(cclient.URL) + client.SetSessionToken(cclient.SessionToken()) + + _, err := client.CryptoKeys(ctx) + require.Error(t, err) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusUnauthorized, sdkErr.StatusCode()) + }) +} + +func fromDBCryptoKeys(key database.CryptoKey) wsproxysdk.CryptoKey { + return wsproxysdk.CryptoKey{ + Feature: wsproxysdk.CryptoKeyFeature(key.Feature), + Sequence: key.Sequence, + StartsAt: key.StartsAt.UTC(), + DeletesAt: key.DeletesAt.Time.UTC(), + Secret: key.Secret.String, + } +} diff --git a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go index f6c548cd77236..ab51146e90735 100644 --- a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go +++ b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go @@ -234,7 +234,6 @@ func (c CryptoKey) Invalid(now time.Time) bool { } type RegisterWorkspaceProxyResponse struct { - Keys []CryptoKey `json:"keys"` AppSecurityKey string `json:"app_security_key"` DERPMeshKey string `json:"derp_mesh_key"` DERPRegionID int32 `json:"derp_region_id"` @@ -364,6 +363,7 @@ func (l *RegisterWorkspaceProxyLoop) Start(ctx context.Context) (RegisterWorkspa failedAttempts = 0 ticker = time.NewTicker(l.opts.Interval) ) + for { var respCh chan RegisterWorkspaceProxyResponse select { @@ -401,6 +401,12 @@ func (l *RegisterWorkspaceProxyLoop) Start(ctx context.Context) (RegisterWorkspa } failedAttempts = 0 + // Check for consistency. + if originalRes.AppSecurityKey != resp.AppSecurityKey { + l.failureFn(xerrors.New("app security key has changed, proxy must be restarted")) + return + } + if originalRes.DERPMeshKey != resp.DERPMeshKey { l.failureFn(xerrors.New("DERP mesh key has changed, proxy must be restarted")) return From 945c1722dec7a746926206a470025e16753c2b13 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 24 Sep 2024 23:19:27 +0000 Subject: [PATCH 3/7] apidoc --- enterprise/coderd/workspaceproxy.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 23c16d546329d..9b809ffc50667 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -710,6 +710,21 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) go api.forceWorkspaceProxyHealthUpdate(api.ctx) } +// workspaceProxyCryptoKeys is used to fetch signing keys for the workspace proxy. +// +// This is called periodically by the proxy in the background (every 10m per +// replica) to ensure that the proxy has the latest signing keys. +// +// @Summary Fetch workspace proxy signing keys +// @ID get-workspace-proxy-crypto-keys +// @Security CoderSessionToken +// @Accept json +// @Produce json +// @Tags Enterprise +// @Success 200 {object} wsproxysdk.CryptoKeysResponse +// @Router /workspaceproxies/me/crypto-keys [get] +// @x-apidocgen {"skip": true} + func (api *API) workspaceProxyCryptoKeys(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() From 749522cde816e97e505ec027adfb0b72f756a5ae Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 24 Sep 2024 23:35:05 +0000 Subject: [PATCH 4/7] Add crypto-key fetching to API documentation Introduce documentation for new API endpoint to fetch workspace proxy signing keys. This addition supports enterprise use cases involving workspace proxies by documenting the models `CryptoKey`, `CryptoKeyFeature`, and `CryptoKeysResponse`. --- coderd/apidoc/docs.go | 75 +++++++++++++++++++++++++++++ coderd/apidoc/swagger.json | 65 +++++++++++++++++++++++++ docs/reference/api/schemas.md | 60 +++++++++++++++++++++++ enterprise/coderd/workspaceproxy.go | 1 - 4 files changed, 200 insertions(+), 1 deletion(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index ebf9bf8ae075d..1e68cf91cfb34 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7540,6 +7540,37 @@ const docTemplate = `{ } } }, + "/workspaceproxies/me/crypto-keys": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Fetch workspace proxy signing keys", + "operationId": "get-workspace-proxy-crypto-keys", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/wsproxysdk.CryptoKeysResponse" + } + } + }, + "x-apidocgen": { + "skip": true + } + } + }, "/workspaceproxies/me/deregister": { "post": { "security": [ @@ -15955,6 +15986,50 @@ const docTemplate = `{ } } }, + "wsproxysdk.CryptoKey": { + "type": "object", + "properties": { + "deletes_at": { + "type": "string" + }, + "feature": { + "$ref": "#/definitions/wsproxysdk.CryptoKeyFeature" + }, + "secret": { + "type": "string" + }, + "sequence": { + "type": "integer" + }, + "starts_at": { + "type": "string" + } + } + }, + "wsproxysdk.CryptoKeyFeature": { + "type": "string", + "enum": [ + "workspace_apps", + "oidc_convert", + "tailnet_resume" + ], + "x-enum-varnames": [ + "CryptoKeyFeatureWorkspaceApp", + "CryptoKeyFeatureOIDCConvert", + "CryptoKeyFeatureTailnetResume" + ] + }, + "wsproxysdk.CryptoKeysResponse": { + "type": "object", + "properties": { + "crypto_keys": { + "type": "array", + "items": { + "$ref": "#/definitions/wsproxysdk.CryptoKey" + } + } + } + }, "wsproxysdk.DeregisterWorkspaceProxyRequest": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 3a162713ed489..0509a28f8a982 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -6668,6 +6668,31 @@ } } }, + "/workspaceproxies/me/crypto-keys": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Fetch workspace proxy signing keys", + "operationId": "get-workspace-proxy-crypto-keys", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/wsproxysdk.CryptoKeysResponse" + } + } + }, + "x-apidocgen": { + "skip": true + } + } + }, "/workspaceproxies/me/deregister": { "post": { "security": [ @@ -14590,6 +14615,46 @@ } } }, + "wsproxysdk.CryptoKey": { + "type": "object", + "properties": { + "deletes_at": { + "type": "string" + }, + "feature": { + "$ref": "#/definitions/wsproxysdk.CryptoKeyFeature" + }, + "secret": { + "type": "string" + }, + "sequence": { + "type": "integer" + }, + "starts_at": { + "type": "string" + } + } + }, + "wsproxysdk.CryptoKeyFeature": { + "type": "string", + "enum": ["workspace_apps", "oidc_convert", "tailnet_resume"], + "x-enum-varnames": [ + "CryptoKeyFeatureWorkspaceApp", + "CryptoKeyFeatureOIDCConvert", + "CryptoKeyFeatureTailnetResume" + ] + }, + "wsproxysdk.CryptoKeysResponse": { + "type": "object", + "properties": { + "crypto_keys": { + "type": "array", + "items": { + "$ref": "#/definitions/wsproxysdk.CryptoKey" + } + } + } + }, "wsproxysdk.DeregisterWorkspaceProxyRequest": { "type": "object", "properties": { diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index c8f58d591170b..757d0aaab4ccb 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -9768,6 +9768,66 @@ _None_ | `derp_map` | [tailcfg.DERPMap](#tailcfgderpmap) | false | | | | `disable_direct_connections` | boolean | false | | | +## wsproxysdk.CryptoKey + +```json +{ + "deletes_at": "string", + "feature": "workspace_apps", + "secret": "string", + "sequence": 0, + "starts_at": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------------ | ---------------------------------------------------------- | -------- | ------------ | ----------- | +| `deletes_at` | string | false | | | +| `feature` | [wsproxysdk.CryptoKeyFeature](#wsproxysdkcryptokeyfeature) | false | | | +| `secret` | string | false | | | +| `sequence` | integer | false | | | +| `starts_at` | string | false | | | + +## wsproxysdk.CryptoKeyFeature + +```json +"workspace_apps" +``` + +### Properties + +#### Enumerated Values + +| Value | +| ---------------- | +| `workspace_apps` | +| `oidc_convert` | +| `tailnet_resume` | + +## wsproxysdk.CryptoKeysResponse + +```json +{ + "crypto_keys": [ + { + "deletes_at": "string", + "feature": "workspace_apps", + "secret": "string", + "sequence": 0, + "starts_at": "string" + } + ] +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------------- | ----------------------------------------------------- | -------- | ------------ | ----------- | +| `crypto_keys` | array of [wsproxysdk.CryptoKey](#wsproxysdkcryptokey) | false | | | + ## wsproxysdk.DeregisterWorkspaceProxyRequest ```json diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 9b809ffc50667..41ea43d6fc2ec 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -724,7 +724,6 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) // @Success 200 {object} wsproxysdk.CryptoKeysResponse // @Router /workspaceproxies/me/crypto-keys [get] // @x-apidocgen {"skip": true} - func (api *API) workspaceProxyCryptoKeys(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() From 7c4cc8367d5f55e76f35e32ceab2f643abf9785c Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 24 Sep 2024 23:43:31 +0000 Subject: [PATCH 5/7] fix(apidoc): update summary for workspace proxy keys Improve clarity by changing references from "signing keys" to "crypto keys" in API documentation and comments. This aligns terminology across the codebase and documentation, reducing potential confusion. --- coderd/apidoc/docs.go | 5 +---- coderd/apidoc/swagger.json | 3 +-- enterprise/coderd/workspaceproxy.go | 3 +-- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 1e68cf91cfb34..d6cc74cafbcaa 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7547,16 +7547,13 @@ const docTemplate = `{ "CoderSessionToken": [] } ], - "consumes": [ - "application/json" - ], "produces": [ "application/json" ], "tags": [ "Enterprise" ], - "summary": "Fetch workspace proxy signing keys", + "summary": "Get workspace proxy crypto keys", "operationId": "get-workspace-proxy-crypto-keys", "responses": { "200": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 0509a28f8a982..579cb33bfaa6e 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -6675,10 +6675,9 @@ "CoderSessionToken": [] } ], - "consumes": ["application/json"], "produces": ["application/json"], "tags": ["Enterprise"], - "summary": "Fetch workspace proxy signing keys", + "summary": "Get workspace proxy crypto keys", "operationId": "get-workspace-proxy-crypto-keys", "responses": { "200": { diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 41ea43d6fc2ec..eef12b1d1b13a 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -715,10 +715,9 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) // This is called periodically by the proxy in the background (every 10m per // replica) to ensure that the proxy has the latest signing keys. // -// @Summary Fetch workspace proxy signing keys +// @Summary Get workspace proxy crypto keys // @ID get-workspace-proxy-crypto-keys // @Security CoderSessionToken -// @Accept json // @Produce json // @Tags Enterprise // @Success 200 {object} wsproxysdk.CryptoKeysResponse From 3d813888eafcceaaa2b22d73c06195e046fa02e5 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 24 Sep 2024 23:51:00 +0000 Subject: [PATCH 6/7] Fix: Filter invalid crypto keys in FakeQuerier function --- coderd/database/dbmem/dbmem.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 5ddfe9d5026c0..a7ec136b736aa 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -2435,7 +2435,7 @@ func (q *FakeQuerier) GetCryptoKeysByFeature(_ context.Context, feature database keys := make([]database.CryptoKey, 0) for _, key := range q.cryptoKeys { - if key.Feature == feature { + if key.Feature == feature && key.Secret.Valid { keys = append(keys, key) } } From 5b3f9e66a16ee0c289cb382ad25e1fac6e787c58 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 26 Sep 2024 19:48:32 +0000 Subject: [PATCH 7/7] Refactor CryptoKey methods for clarity --- enterprise/wsproxy/wsproxysdk/wsproxysdk.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go index ab51146e90735..891224216003a 100644 --- a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go +++ b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go @@ -220,17 +220,17 @@ type CryptoKey struct { StartsAt time.Time `json:"starts_at"` } -func (c CryptoKey) Active(now time.Time) bool { +func (c CryptoKey) CanSign(now time.Time) bool { now = now.UTC() isAfterStartsAt := !c.StartsAt.IsZero() && !now.Before(c.StartsAt) - return isAfterStartsAt && !c.Invalid(now) + return isAfterStartsAt && c.CanVerify(now) } -func (c CryptoKey) Invalid(now time.Time) bool { +func (c CryptoKey) CanVerify(now time.Time) bool { now = now.UTC() - noSecret := c.Secret == "" - afterDelete := !c.DeletesAt.IsZero() && !now.Before(c.DeletesAt.UTC()) - return noSecret || afterDelete + hasSecret := c.Secret != "" + beforeDelete := c.DeletesAt.IsZero() || now.Before(c.DeletesAt) + return hasSecret && beforeDelete } type RegisterWorkspaceProxyResponse struct {