From 13ca9ead3a8269ca68104ea75af584d33c385d26 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 23 Oct 2025 15:38:49 -0500 Subject: [PATCH] chore!: ensure consistent secret token generation and hashing (#20388) This PR uses the same sha256 hashing technique as we use for APIKeys. So now all randomly generated secrets will be hashed with sha256 for consistency. This is a breaking change for the oauth tokens. Since oauth is only allowed for dev builds and experimental, this is ok. --- coderd/apidoc/docs.go | 5 +- coderd/apidoc/swagger.json | 5 +- coderd/apikey/apikey.go | 43 +++++++++++------ coderd/apikey/apikey_test.go | 19 ++++++-- coderd/database/dbauthz/dbauthz.go | 2 +- coderd/database/dbauthz/dbauthz_test.go | 4 +- coderd/database/dbgen/dbgen.go | 17 ++++--- coderd/database/dbmetrics/querymetrics.go | 3 +- coderd/database/dbmock/dbmock.go | 3 +- coderd/database/dump.sql | 2 +- ...8_oauth_app_byte_reg_access_token.down.sql | 4 ++ ...388_oauth_app_byte_reg_access_token.up.sql | 4 ++ coderd/database/models.go | 2 +- coderd/database/querier.go | 3 +- coderd/database/queries.sql.go | 4 +- coderd/httpmw/apikey.go | 6 +-- coderd/httpmw/apikey_test.go | 15 +++--- coderd/httpmw/authorize_test.go | 8 +--- coderd/httpmw/workspaceparam_test.go | 10 ++-- coderd/httpmw/workspaceproxy.go | 6 +-- coderd/oauth2_test.go | 2 +- coderd/oauth2provider/app_secrets.go | 2 +- coderd/oauth2provider/apps.go | 2 +- coderd/oauth2provider/authorize.go | 2 +- coderd/oauth2provider/registration.go | 40 +++++----------- coderd/oauth2provider/revoke.go | 7 +-- coderd/oauth2provider/secrets.go | 13 ++--- coderd/oauth2provider/tokens.go | 25 ++++------ coderd/provisionerkey/provisionerkey.go | 10 ++-- codersdk/oauth2.go | 2 +- docs/reference/api/enterprise.md | 8 +++- docs/reference/api/schemas.md | 48 ++++++++++--------- enterprise/coderd/workspaceproxy.go | 9 ++-- .../x/aibridgedserver/aibridgedserver.go | 6 +-- .../x/aibridgedserver/aibridgedserver_test.go | 7 ++- 35 files changed, 169 insertions(+), 179 deletions(-) create mode 100644 coderd/database/migrations/000388_oauth_app_byte_reg_access_token.down.sql create mode 100644 coderd/database/migrations/000388_oauth_app_byte_reg_access_token.up.sql diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 669c753b24..7dc3f969fa 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -15471,7 +15471,10 @@ const docTemplate = `{ } }, "registration_access_token": { - "type": "string" + "type": "array", + "items": { + "type": "integer" + } }, "registration_client_uri": { "type": "string" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index ba019ba82d..a1b00293be 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -14025,7 +14025,10 @@ } }, "registration_access_token": { - "type": "string" + "type": "array", + "items": { + "type": "integer" + } }, "registration_client_uri": { "type": "string" diff --git a/coderd/apikey/apikey.go b/coderd/apikey/apikey.go index 0dd874b340..89bbb7ca53 100644 --- a/coderd/apikey/apikey.go +++ b/coderd/apikey/apikey.go @@ -2,6 +2,7 @@ package apikey import ( "crypto/sha256" + "crypto/subtle" "fmt" "net" "time" @@ -44,12 +45,17 @@ type CreateParams struct { // database representation. It is the responsibility of the caller to insert it // into the database. func Generate(params CreateParams) (database.InsertAPIKeyParams, string, error) { - keyID, keySecret, err := generateKey() + // Length of an API Key ID. + keyID, err := cryptorand.String(10) if err != nil { - return database.InsertAPIKeyParams{}, "", xerrors.Errorf("generate API key: %w", err) + return database.InsertAPIKeyParams{}, "", xerrors.Errorf("generate API key ID: %w", err) } - hashed := sha256.Sum256([]byte(keySecret)) + // Length of an API Key secret. + keySecret, hashedSecret, err := GenerateSecret(22) + if err != nil { + return database.InsertAPIKeyParams{}, "", xerrors.Errorf("generate API key secret: %w", err) + } // Default expires at to now+lifetime, or use the configured value if not // set. @@ -120,7 +126,7 @@ func Generate(params CreateParams) (database.InsertAPIKeyParams, string, error) ExpiresAt: params.ExpiresAt.UTC(), CreatedAt: dbtime.Now(), UpdatedAt: dbtime.Now(), - HashedSecret: hashed[:], + HashedSecret: hashedSecret, LoginType: params.LoginType, Scopes: scopes, AllowList: params.AllowList, @@ -128,17 +134,24 @@ func Generate(params CreateParams) (database.InsertAPIKeyParams, string, error) }, token, nil } -// generateKey a new ID and secret for an API key. -func generateKey() (id string, secret string, err error) { - // Length of an API Key ID. - id, err = cryptorand.String(10) +func GenerateSecret(length int) (secret string, hashed []byte, err error) { + secret, err = cryptorand.String(length) if err != nil { - return "", "", err + return "", nil, err } - // Length of an API Key secret. - secret, err = cryptorand.String(22) - if err != nil { - return "", "", err - } - return id, secret, nil + hash := HashSecret(secret) + return secret, hash, nil +} + +// ValidateHash compares a secret against an expected hashed secret. +func ValidateHash(hashedSecret []byte, secret string) bool { + hash := HashSecret(secret) + return subtle.ConstantTimeCompare(hashedSecret, hash) == 1 +} + +// HashSecret is the single function used to hash API key secrets. +// Use this to ensure a consistent hashing algorithm. +func HashSecret(secret string) []byte { + hash := sha256.Sum256([]byte(secret)) + return hash[:] } diff --git a/coderd/apikey/apikey_test.go b/coderd/apikey/apikey_test.go index 1f5de3aa18..aa17a02561 100644 --- a/coderd/apikey/apikey_test.go +++ b/coderd/apikey/apikey_test.go @@ -1,7 +1,6 @@ package apikey_test import ( - "crypto/sha256" "strings" "testing" "time" @@ -126,8 +125,8 @@ func TestGenerate(t *testing.T) { require.Equal(t, key.ID, keytokens[0]) // Assert that the hashed secret is correct. - hashed := sha256.Sum256([]byte(keytokens[1])) - assert.ElementsMatch(t, hashed, key.HashedSecret) + equal := apikey.ValidateHash(key.HashedSecret, keytokens[1]) + require.True(t, equal, "valid secret") assert.Equal(t, tc.params.UserID, key.UserID) assert.WithinDuration(t, dbtime.Now(), key.CreatedAt, time.Second*5) @@ -173,3 +172,17 @@ func TestGenerate(t *testing.T) { }) } } + +// TestInvalid just ensures the false case is asserted by some tests. +// Otherwise, a function that just `returns true` might pass all tests incorrectly. +func TestInvalid(t *testing.T) { + t.Parallel() + + require.Falsef(t, apikey.ValidateHash([]byte{}, "secret"), "empty hash") + + secret, hash, err := apikey.GenerateSecret(10) + require.NoError(t, err) + + require.Falsef(t, apikey.ValidateHash(hash, secret+"_"), "different secret") + require.Falsef(t, apikey.ValidateHash(hash[:len(hash)-1], secret), "different hash length") +} diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 52089e55c5..16bc364a42 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2475,7 +2475,7 @@ func (q *querier) GetOAuth2ProviderAppByID(ctx context.Context, id uuid.UUID) (d return q.db.GetOAuth2ProviderAppByID(ctx, id) } -func (q *querier) GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken sql.NullString) (database.OAuth2ProviderApp, error) { +func (q *querier) GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken []byte) (database.OAuth2ProviderApp, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceOauth2App); err != nil { return database.OAuth2ProviderApp{}, err } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 070da4f278..ca82390984 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -3925,9 +3925,9 @@ func (s *MethodTestSuite) TestOAuth2ProviderApps() { })) s.Run("GetOAuth2ProviderAppByRegistrationToken", s.Subtest(func(db database.Store, check *expects) { app := dbgen.OAuth2ProviderApp(s.T(), db, database.OAuth2ProviderApp{ - RegistrationAccessToken: sql.NullString{String: "test-token", Valid: true}, + RegistrationAccessToken: []byte("test-token"), }) - check.Args(sql.NullString{String: "test-token", Valid: true}).Asserts(rbac.ResourceOauth2App, policy.ActionRead).Returns(app) + check.Args([]byte("test-token")).Asserts(rbac.ResourceOauth2App, policy.ActionRead).Returns(app) })) } diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 713cb53b6d..0f38dd1f93 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -3,7 +3,6 @@ package dbgen import ( "context" "crypto/rand" - "crypto/sha256" "database/sql" "encoding/hex" "encoding/json" @@ -20,6 +19,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" @@ -161,8 +161,8 @@ func Template(t testing.TB, db database.Store, seed database.Template) database. func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func(*database.InsertAPIKeyParams)) (key database.APIKey, token string) { id, _ := cryptorand.String(10) - secret, _ := cryptorand.String(22) - hashed := sha256.Sum256([]byte(secret)) + secret, hashed, err := apikey.GenerateSecret(22) + require.NoError(t, err) ip := seed.IPAddress if !ip.Valid { @@ -179,7 +179,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func ID: takeFirst(seed.ID, id), // 0 defaults to 86400 at the db layer LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0), - HashedSecret: takeFirstSlice(seed.HashedSecret, hashed[:]), + HashedSecret: takeFirstSlice(seed.HashedSecret, hashed), IPAddress: ip, UserID: takeFirst(seed.UserID, uuid.New()), LastUsed: takeFirst(seed.LastUsed, dbtime.Now()), @@ -194,7 +194,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func for _, fn := range munge { fn(¶ms) } - key, err := db.InsertAPIKey(genCtx, params) + key, err = db.InsertAPIKey(genCtx, params) require.NoError(t, err, "insert api key") return key, fmt.Sprintf("%s-%s", key.ID, secret) } @@ -980,16 +980,15 @@ func WorkspaceResourceMetadatums(t testing.TB, db database.Store, seed database. } func WorkspaceProxy(t testing.TB, db database.Store, orig database.WorkspaceProxy) (database.WorkspaceProxy, string) { - secret, err := cryptorand.HexString(64) + secret, hashedSecret, err := apikey.GenerateSecret(64) require.NoError(t, err, "generate secret") - hashedSecret := sha256.Sum256([]byte(secret)) proxy, err := db.InsertWorkspaceProxy(genCtx, database.InsertWorkspaceProxyParams{ ID: takeFirst(orig.ID, uuid.New()), Name: takeFirst(orig.Name, testutil.GetRandomName(t)), DisplayName: takeFirst(orig.DisplayName, testutil.GetRandomName(t)), Icon: takeFirst(orig.Icon, testutil.GetRandomName(t)), - TokenHashedSecret: hashedSecret[:], + TokenHashedSecret: hashedSecret, CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()), UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()), DerpEnabled: takeFirst(orig.DerpEnabled, false), @@ -1259,7 +1258,7 @@ func OAuth2ProviderApp(t testing.TB, db database.Store, seed database.OAuth2Prov Jwks: seed.Jwks, // pqtype.NullRawMessage{} is not comparable, use existing value SoftwareID: takeFirst(seed.SoftwareID, sql.NullString{}), SoftwareVersion: takeFirst(seed.SoftwareVersion, sql.NullString{}), - RegistrationAccessToken: takeFirst(seed.RegistrationAccessToken, sql.NullString{}), + RegistrationAccessToken: seed.RegistrationAccessToken, RegistrationClientUri: takeFirst(seed.RegistrationClientUri, sql.NullString{}), }) require.NoError(t, err, "insert oauth2 app") diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index a61d05cdef..79c8a01d67 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -5,7 +5,6 @@ package dbmetrics import ( "context" - "database/sql" "slices" "time" @@ -1104,7 +1103,7 @@ func (m queryMetricsStore) GetOAuth2ProviderAppByID(ctx context.Context, id uuid return r0, r1 } -func (m queryMetricsStore) GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken sql.NullString) (database.OAuth2ProviderApp, error) { +func (m queryMetricsStore) GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken []byte) (database.OAuth2ProviderApp, error) { start := time.Now() r0, r1 := m.s.GetOAuth2ProviderAppByRegistrationToken(ctx, registrationAccessToken) m.queryLatencies.WithLabelValues("GetOAuth2ProviderAppByRegistrationToken").Observe(time.Since(start).Seconds()) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 157883c6df..a16ba47e72 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -11,7 +11,6 @@ package dbmock import ( context "context" - sql "database/sql" reflect "reflect" time "time" @@ -2325,7 +2324,7 @@ func (mr *MockStoreMockRecorder) GetOAuth2ProviderAppByID(ctx, id any) *gomock.C } // GetOAuth2ProviderAppByRegistrationToken mocks base method. -func (m *MockStore) GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken sql.NullString) (database.OAuth2ProviderApp, error) { +func (m *MockStore) GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken []byte) (database.OAuth2ProviderApp, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetOAuth2ProviderAppByRegistrationToken", ctx, registrationAccessToken) ret0, _ := ret[0].(database.OAuth2ProviderApp) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 11f668d2fc..de55f8117f 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1537,7 +1537,7 @@ CREATE TABLE oauth2_provider_apps ( jwks jsonb, software_id text, software_version text, - registration_access_token text, + registration_access_token bytea, registration_client_uri text ); diff --git a/coderd/database/migrations/000388_oauth_app_byte_reg_access_token.down.sql b/coderd/database/migrations/000388_oauth_app_byte_reg_access_token.down.sql new file mode 100644 index 0000000000..3e56dbf873 --- /dev/null +++ b/coderd/database/migrations/000388_oauth_app_byte_reg_access_token.down.sql @@ -0,0 +1,4 @@ +ALTER TABLE oauth2_provider_apps + ALTER COLUMN registration_access_token + SET DATA TYPE text + USING encode(registration_access_token, 'escape'); diff --git a/coderd/database/migrations/000388_oauth_app_byte_reg_access_token.up.sql b/coderd/database/migrations/000388_oauth_app_byte_reg_access_token.up.sql new file mode 100644 index 0000000000..b278fed80e --- /dev/null +++ b/coderd/database/migrations/000388_oauth_app_byte_reg_access_token.up.sql @@ -0,0 +1,4 @@ +ALTER TABLE oauth2_provider_apps + ALTER COLUMN registration_access_token + SET DATA TYPE bytea + USING decode(registration_access_token, 'escape'); diff --git a/coderd/database/models.go b/coderd/database/models.go index fe9b04755b..d5db3a3819 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -3956,7 +3956,7 @@ type OAuth2ProviderApp struct { // RFC 7591: Version of the client software SoftwareVersion sql.NullString `db:"software_version" json:"software_version"` // RFC 7592: Hashed registration access token for client management - RegistrationAccessToken sql.NullString `db:"registration_access_token" json:"registration_access_token"` + RegistrationAccessToken []byte `db:"registration_access_token" json:"registration_access_token"` // RFC 7592: URI for client configuration endpoint RegistrationClientUri sql.NullString `db:"registration_client_uri" json:"registration_client_uri"` } diff --git a/coderd/database/querier.go b/coderd/database/querier.go index a1c6120780..c6ae87dbc5 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -6,7 +6,6 @@ package database import ( "context" - "database/sql" "time" "github.com/google/uuid" @@ -246,7 +245,7 @@ type sqlcQuerier interface { // RFC 7591/7592 Dynamic Client Registration queries GetOAuth2ProviderAppByClientID(ctx context.Context, id uuid.UUID) (OAuth2ProviderApp, error) GetOAuth2ProviderAppByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderApp, error) - GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken sql.NullString) (OAuth2ProviderApp, error) + GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken []byte) (OAuth2ProviderApp, error) GetOAuth2ProviderAppCodeByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderAppCode, error) GetOAuth2ProviderAppCodeByPrefix(ctx context.Context, secretPrefix []byte) (OAuth2ProviderAppCode, error) GetOAuth2ProviderAppSecretByID(ctx context.Context, id uuid.UUID) (OAuth2ProviderAppSecret, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d257d571c0..c5b8caba37 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -6206,7 +6206,7 @@ const getOAuth2ProviderAppByRegistrationToken = `-- name: GetOAuth2ProviderAppBy SELECT id, created_at, updated_at, name, icon, callback_url, redirect_uris, client_type, dynamically_registered, client_id_issued_at, client_secret_expires_at, grant_types, response_types, token_endpoint_auth_method, scope, contacts, client_uri, logo_uri, tos_uri, policy_uri, jwks_uri, jwks, software_id, software_version, registration_access_token, registration_client_uri FROM oauth2_provider_apps WHERE registration_access_token = $1 ` -func (q *sqlQuerier) GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken sql.NullString) (OAuth2ProviderApp, error) { +func (q *sqlQuerier) GetOAuth2ProviderAppByRegistrationToken(ctx context.Context, registrationAccessToken []byte) (OAuth2ProviderApp, error) { row := q.db.QueryRowContext(ctx, getOAuth2ProviderAppByRegistrationToken, registrationAccessToken) var i OAuth2ProviderApp err := row.Scan( @@ -6607,7 +6607,7 @@ type InsertOAuth2ProviderAppParams struct { Jwks pqtype.NullRawMessage `db:"jwks" json:"jwks"` SoftwareID sql.NullString `db:"software_id" json:"software_id"` SoftwareVersion sql.NullString `db:"software_version" json:"software_version"` - RegistrationAccessToken sql.NullString `db:"registration_access_token" json:"registration_access_token"` + RegistrationAccessToken []byte `db:"registration_access_token" json:"registration_access_token"` RegistrationClientUri sql.NullString `db:"registration_client_uri" json:"registration_client_uri"` } diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index 08b835ccc6..29296fea59 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -2,8 +2,6 @@ package httpmw import ( "context" - "crypto/sha256" - "crypto/subtle" "database/sql" "errors" "fmt" @@ -20,6 +18,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -188,8 +187,7 @@ func APIKeyFromRequest(ctx context.Context, db database.Store, sessionTokenFunc } // Checking to see if the secret is valid. - hashedSecret := sha256.Sum256([]byte(keySecret)) - if subtle.ConstantTimeCompare(key.HashedSecret, hashedSecret[:]) != 1 { + if !apikey.ValidateHash(key.HashedSecret, keySecret) { return nil, codersdk.Response{ Message: SignedOutErrorMessage, Detail: "API key secret is invalid.", diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index 6e00b7a453..020dc28e60 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -19,6 +19,7 @@ import ( "golang.org/x/exp/slices" "golang.org/x/oauth2" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbgen" @@ -32,10 +33,10 @@ import ( "github.com/coder/coder/v2/testutil" ) -func randomAPIKeyParts() (id string, secret string) { +func randomAPIKeyParts() (id string, secret string, hashedSecret []byte) { id, _ = cryptorand.String(10) - secret, _ = cryptorand.String(22) - return id, secret + secret, hashedSecret, _ = apikey.GenerateSecret(22) + return id, secret, hashedSecret } func TestAPIKey(t *testing.T) { @@ -171,10 +172,10 @@ func TestAPIKey(t *testing.T) { t.Run("NotFound", func(t *testing.T) { t.Parallel() var ( - db, _ = dbtestutil.NewDB(t) - id, secret = randomAPIKeyParts() - r = httptest.NewRequest("GET", "/", nil) - rw = httptest.NewRecorder() + db, _ = dbtestutil.NewDB(t) + id, secret, _ = randomAPIKeyParts() + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() ) r.Header.Set(codersdk.SessionTokenHeader, fmt.Sprintf("%s-%s", id, secret)) diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go index 030f23935e..529ba94774 100644 --- a/coderd/httpmw/authorize_test.go +++ b/coderd/httpmw/authorize_test.go @@ -2,7 +2,6 @@ package httpmw_test import ( "context" - "crypto/sha256" "fmt" "net" "net/http" @@ -142,10 +141,7 @@ func TestExtractUserRoles(t *testing.T) { } func addUser(t *testing.T, db database.Store, roles ...string) (database.User, string) { - var ( - id, secret = randomAPIKeyParts() - hashed = sha256.Sum256([]byte(secret)) - ) + id, secret, hashed := randomAPIKeyParts() if roles == nil { roles = []string{} } @@ -169,7 +165,7 @@ func addUser(t *testing.T, db database.Store, roles ...string) (database.User, s _, err = db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{ ID: id, UserID: user.ID, - HashedSecret: hashed[:], + HashedSecret: hashed, LastUsed: dbtime.Now(), ExpiresAt: dbtime.Now().Add(time.Minute), LoginType: database.LoginTypePassword, diff --git a/coderd/httpmw/workspaceparam_test.go b/coderd/httpmw/workspaceparam_test.go index 7b9871ce6d..e83cbe437e 100644 --- a/coderd/httpmw/workspaceparam_test.go +++ b/coderd/httpmw/workspaceparam_test.go @@ -2,7 +2,6 @@ package httpmw_test import ( "context" - "crypto/sha256" "encoding/json" "fmt" "net" @@ -31,10 +30,7 @@ func TestWorkspaceParam(t *testing.T) { t.Parallel() setup := func(db database.Store) (*http.Request, database.User) { - var ( - id, secret = randomAPIKeyParts() - hashed = sha256.Sum256([]byte(secret)) - ) + id, secret, hashed := randomAPIKeyParts() r := httptest.NewRequest("GET", "/", nil) r.Header.Set(codersdk.SessionTokenHeader, fmt.Sprintf("%s-%s", id, secret)) @@ -44,7 +40,7 @@ func TestWorkspaceParam(t *testing.T) { user, err := db.InsertUser(r.Context(), database.InsertUserParams{ ID: userID, Email: "testaccount@coder.com", - HashedPassword: hashed[:], + HashedPassword: hashed, Username: username, CreatedAt: dbtime.Now(), UpdatedAt: dbtime.Now(), @@ -63,7 +59,7 @@ func TestWorkspaceParam(t *testing.T) { _, err = db.InsertAPIKey(r.Context(), database.InsertAPIKeyParams{ ID: id, UserID: user.ID, - HashedSecret: hashed[:], + HashedSecret: hashed, LastUsed: dbtime.Now(), ExpiresAt: dbtime.Now().Add(time.Minute), LoginType: database.LoginTypePassword, diff --git a/coderd/httpmw/workspaceproxy.go b/coderd/httpmw/workspaceproxy.go index 1f2de1ed46..39f665210b 100644 --- a/coderd/httpmw/workspaceproxy.go +++ b/coderd/httpmw/workspaceproxy.go @@ -2,8 +2,6 @@ package httpmw import ( "context" - "crypto/sha256" - "crypto/subtle" "database/sql" "net/http" "strings" @@ -12,6 +10,7 @@ import ( "github.com/google/uuid" "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/httpapi" @@ -125,8 +124,7 @@ func ExtractWorkspaceProxy(opts ExtractWorkspaceProxyConfig) func(http.Handler) } // Do a subtle constant time comparison of the hash of the secret. - hashedSecret := sha256.Sum256([]byte(secret)) - if subtle.ConstantTimeCompare(proxy.TokenHashedSecret, hashedSecret[:]) != 1 { + if !apikey.ValidateHash(proxy.TokenHashedSecret, secret) { httpapi.Write(ctx, w, http.StatusUnauthorized, codersdk.Response{ Message: "Invalid external proxy token", Detail: "Invalid proxy token secret.", diff --git a/coderd/oauth2_test.go b/coderd/oauth2_test.go index c03dcf7e3a..72564a2a0d 100644 --- a/coderd/oauth2_test.go +++ b/coderd/oauth2_test.go @@ -620,7 +620,7 @@ func TestOAuth2ProviderTokenRefresh(t *testing.T) { CreatedAt: dbtime.Now(), ExpiresAt: expires, HashPrefix: []byte(token.Prefix), - RefreshHash: []byte(token.Hashed), + RefreshHash: token.Hashed, AppSecretID: secret.ID, APIKeyID: newKey.ID, UserID: user.ID, diff --git a/coderd/oauth2provider/app_secrets.go b/coderd/oauth2provider/app_secrets.go index 5549ece426..3eff684123 100644 --- a/coderd/oauth2provider/app_secrets.go +++ b/coderd/oauth2provider/app_secrets.go @@ -66,7 +66,7 @@ func CreateAppSecret(db database.Store, auditor *audit.Auditor, logger slog.Logg ID: uuid.New(), CreatedAt: dbtime.Now(), SecretPrefix: []byte(secret.Prefix), - HashedSecret: []byte(secret.Hashed), + HashedSecret: secret.Hashed, // DisplaySecret is the last six characters of the original unhashed secret. // This is done so they can be differentiated and it matches how GitHub // displays their client secrets. diff --git a/coderd/oauth2provider/apps.go b/coderd/oauth2provider/apps.go index 74bafb851e..81ff8b0e24 100644 --- a/coderd/oauth2provider/apps.go +++ b/coderd/oauth2provider/apps.go @@ -110,7 +110,7 @@ func CreateApp(db database.Store, accessURL *url.URL, auditor *audit.Auditor, lo Jwks: pqtype.NullRawMessage{}, SoftwareID: sql.NullString{}, SoftwareVersion: sql.NullString{}, - RegistrationAccessToken: sql.NullString{}, + RegistrationAccessToken: nil, RegistrationClientUri: sql.NullString{}, }) if err != nil { diff --git a/coderd/oauth2provider/authorize.go b/coderd/oauth2provider/authorize.go index d6b05fc516..d738e781e8 100644 --- a/coderd/oauth2provider/authorize.go +++ b/coderd/oauth2provider/authorize.go @@ -165,7 +165,7 @@ func ProcessAuthorize(db database.Store) http.HandlerFunc { // has left) then they can just retry immediately and get a new code. ExpiresAt: dbtime.Now().Add(time.Duration(10) * time.Minute), SecretPrefix: []byte(code.Prefix), - HashedSecret: []byte(code.Hashed), + HashedSecret: code.Hashed, AppID: app.ID, UserID: apiKey.UserID, ResourceUri: sql.NullString{String: params.resource, Valid: params.resource != ""}, diff --git a/coderd/oauth2provider/registration.go b/coderd/oauth2provider/registration.go index c433cae8ff..807c39371d 100644 --- a/coderd/oauth2provider/registration.go +++ b/coderd/oauth2provider/registration.go @@ -15,15 +15,14 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpapi" - "github.com/coder/coder/v2/coderd/userpassword" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/cryptorand" ) // CreateDynamicClientRegistration returns an http.HandlerFunc that handles POST /oauth2/register @@ -100,7 +99,7 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi Jwks: pqtype.NullRawMessage{RawMessage: req.JWKS, Valid: len(req.JWKS) > 0}, SoftwareID: sql.NullString{String: req.SoftwareID, Valid: req.SoftwareID != ""}, SoftwareVersion: sql.NullString{String: req.SoftwareVersion, Valid: req.SoftwareVersion != ""}, - RegistrationAccessToken: sql.NullString{String: hashedRegToken, Valid: true}, + RegistrationAccessToken: hashedRegToken, RegistrationClientUri: sql.NullString{String: fmt.Sprintf("%s/oauth2/clients/%s", accessURL.String(), clientID), Valid: true}, }) if err != nil { @@ -127,7 +126,7 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi ID: uuid.New(), CreatedAt: now, SecretPrefix: []byte(parsedSecret.Prefix), - HashedSecret: []byte(hashedSecret), + HashedSecret: hashedSecret, DisplaySecret: createDisplaySecret(clientSecret), AppID: clientID, }) @@ -224,7 +223,7 @@ func GetClientConfiguration(db database.Store) http.HandlerFunc { TokenEndpointAuthMethod: app.TokenEndpointAuthMethod.String, Scope: app.Scope.String, Contacts: app.Contacts, - RegistrationAccessToken: "", // RFC 7592: Not returned in GET responses for security + RegistrationAccessToken: nil, // RFC 7592: Not returned in GET responses for security RegistrationClientURI: app.RegistrationClientUri.String, } @@ -348,7 +347,7 @@ func UpdateClientConfiguration(db database.Store, auditor *audit.Auditor, logger TokenEndpointAuthMethod: updatedApp.TokenEndpointAuthMethod.String, Scope: updatedApp.Scope.String, Contacts: updatedApp.Contacts, - RegistrationAccessToken: updatedApp.RegistrationAccessToken.String, + RegistrationAccessToken: updatedApp.RegistrationAccessToken, RegistrationClientURI: updatedApp.RegistrationClientUri.String, } @@ -476,20 +475,14 @@ func RequireRegistrationAccessToken(db database.Store) func(http.Handler) http.H } // Verify the registration access token - if !app.RegistrationAccessToken.Valid { + if len(app.RegistrationAccessToken) == 0 { writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError, "server_error", "Client has no registration access token") return } // Compare the provided token with the stored hash - valid, err := userpassword.Compare(app.RegistrationAccessToken.String, token) - if err != nil { - writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError, - "server_error", "Failed to verify registration access token") - return - } - if !valid { + if !apikey.ValidateHash(app.RegistrationAccessToken, token) { writeOAuth2RegistrationError(ctx, rw, http.StatusUnauthorized, "invalid_token", "Invalid registration access token") return @@ -504,30 +497,19 @@ func RequireRegistrationAccessToken(db database.Store) func(http.Handler) http.H // Helper functions for RFC 7591 Dynamic Client Registration // generateClientCredentials generates a client secret for OAuth2 apps -func generateClientCredentials() (plaintext, hashed string, err error) { +func generateClientCredentials() (plaintext string, hashed []byte, err error) { // Use the same pattern as existing OAuth2 app secrets secret, err := GenerateSecret() if err != nil { - return "", "", xerrors.Errorf("generate secret: %w", err) + return "", nil, xerrors.Errorf("generate secret: %w", err) } return secret.Formatted, secret.Hashed, nil } // generateRegistrationAccessToken generates a registration access token for RFC 7592 -func generateRegistrationAccessToken() (plaintext, hashed string, err error) { - token, err := cryptorand.String(secretLength) - if err != nil { - return "", "", xerrors.Errorf("generate registration token: %w", err) - } - - // Hash the token for storage - hashedToken, err := userpassword.Hash(token) - if err != nil { - return "", "", xerrors.Errorf("hash registration token: %w", err) - } - - return token, hashedToken, nil +func generateRegistrationAccessToken() (plaintext string, hashed []byte, err error) { + return apikey.GenerateSecret(secretLength) } // writeOAuth2RegistrationError writes RFC 7591 compliant error responses diff --git a/coderd/oauth2provider/revoke.go b/coderd/oauth2provider/revoke.go index 2dbe4827b1..19f3fb803a 100644 --- a/coderd/oauth2provider/revoke.go +++ b/coderd/oauth2provider/revoke.go @@ -14,11 +14,11 @@ import ( "github.com/google/uuid" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" - "github.com/coder/coder/v2/coderd/userpassword" ) var ( @@ -120,10 +120,7 @@ func revokeRefreshTokenInTx(ctx context.Context, db database.Store, token string return xerrors.Errorf("get oauth2 provider app token by prefix: %w", err) } - equal, err := userpassword.Compare(string(dbToken.RefreshHash), parsedToken.Secret) - if err != nil { - return xerrors.Errorf("invalid refresh token: %w", err) - } + equal := apikey.ValidateHash(dbToken.RefreshHash, parsedToken.Secret) if !equal { return xerrors.Errorf("invalid refresh token") } diff --git a/coderd/oauth2provider/secrets.go b/coderd/oauth2provider/secrets.go index 56c5231b25..ee6a7b315d 100644 --- a/coderd/oauth2provider/secrets.go +++ b/coderd/oauth2provider/secrets.go @@ -6,7 +6,7 @@ import ( "golang.org/x/xerrors" - "github.com/coder/coder/v2/coderd/userpassword" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/cryptorand" ) @@ -25,7 +25,7 @@ type HashedAppSecret struct { AppSecret // Hashed is the server stored hash(secret,salt,...). Used for verifying a // secret. - Hashed string + Hashed []byte } type AppSecret struct { @@ -61,7 +61,7 @@ func ParseFormattedSecret(formatted string) (AppSecret, error) { // token, or authorization code. func GenerateSecret() (HashedAppSecret, error) { // 40 characters matches the length of GitHub's client secrets. - secret, err := cryptorand.String(secretLength) + secret, hashedSecret, err := apikey.GenerateSecret(40) if err != nil { return HashedAppSecret{}, err } @@ -74,17 +74,12 @@ func GenerateSecret() (HashedAppSecret, error) { return HashedAppSecret{}, err } - hashed, err := userpassword.Hash(secret) - if err != nil { - return HashedAppSecret{}, err - } - return HashedAppSecret{ AppSecret: AppSecret{ Formatted: fmt.Sprintf("%s_%s_%s", SecretIdentifier, prefix, secret), Secret: secret, Prefix: prefix, }, - Hashed: hashed, + Hashed: hashedSecret, }, nil } diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go index 94b24cca25..d0f1aad105 100644 --- a/coderd/oauth2provider/tokens.go +++ b/coderd/oauth2provider/tokens.go @@ -22,7 +22,6 @@ import ( "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac" - "github.com/coder/coder/v2/coderd/userpassword" "github.com/coder/coder/v2/codersdk" ) @@ -197,11 +196,9 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database if err != nil { return oauth2.Token{}, err } - equal, err := userpassword.Compare(string(dbSecret.HashedSecret), secret.Secret) - if err != nil { - return oauth2.Token{}, xerrors.Errorf("unable to compare secret: %w", err) - } - if !equal { + + equalSecret := apikey.ValidateHash(dbSecret.HashedSecret, secret.Secret) + if !equalSecret { return oauth2.Token{}, errBadSecret } @@ -218,11 +215,8 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database if err != nil { return oauth2.Token{}, err } - equal, err = userpassword.Compare(string(dbCode.HashedSecret), code.Secret) - if err != nil { - return oauth2.Token{}, xerrors.Errorf("unable to compare code: %w", err) - } - if !equal { + equalCode := apikey.ValidateHash(dbCode.HashedSecret, code.Secret) + if !equalCode { return oauth2.Token{}, errBadCode } @@ -318,7 +312,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database CreatedAt: dbtime.Now(), ExpiresAt: refreshExpiresAt, HashPrefix: []byte(refreshToken.Prefix), - RefreshHash: []byte(refreshToken.Hashed), + RefreshHash: refreshToken.Hashed, AppSecretID: dbSecret.ID, APIKeyID: newKey.ID, UserID: dbCode.UserID, @@ -356,10 +350,7 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut if err != nil { return oauth2.Token{}, err } - equal, err := userpassword.Compare(string(dbToken.RefreshHash), token.Secret) - if err != nil { - return oauth2.Token{}, xerrors.Errorf("unable to compare token: %w", err) - } + equal := apikey.ValidateHash(dbToken.RefreshHash, token.Secret) if !equal { return oauth2.Token{}, errBadToken } @@ -434,7 +425,7 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut CreatedAt: dbtime.Now(), ExpiresAt: refreshExpiresAt, HashPrefix: []byte(refreshToken.Prefix), - RefreshHash: []byte(refreshToken.Hashed), + RefreshHash: refreshToken.Hashed, AppSecretID: dbToken.AppSecretID, APIKeyID: newKey.ID, UserID: dbToken.UserID, diff --git a/coderd/provisionerkey/provisionerkey.go b/coderd/provisionerkey/provisionerkey.go index bfd70fb029..046222658e 100644 --- a/coderd/provisionerkey/provisionerkey.go +++ b/coderd/provisionerkey/provisionerkey.go @@ -1,15 +1,14 @@ package provisionerkey import ( - "crypto/sha256" "crypto/subtle" "github.com/google/uuid" "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" - "github.com/coder/coder/v2/cryptorand" ) const ( @@ -17,7 +16,7 @@ const ( ) func New(organizationID uuid.UUID, name string, tags map[string]string) (database.InsertProvisionerKeyParams, string, error) { - secret, err := cryptorand.String(secretLength) + secret, hashed, err := apikey.GenerateSecret(secretLength) if err != nil { return database.InsertProvisionerKeyParams{}, "", xerrors.Errorf("generate secret: %w", err) } @@ -31,7 +30,7 @@ func New(organizationID uuid.UUID, name string, tags map[string]string) (databas CreatedAt: dbtime.Now(), OrganizationID: organizationID, Name: name, - HashedSecret: HashSecret(secret), + HashedSecret: hashed, Tags: tags, }, secret, nil } @@ -45,8 +44,7 @@ func Validate(token string) error { } func HashSecret(secret string) []byte { - h := sha256.Sum256([]byte(secret)) - return h[:] + return apikey.HashSecret(secret) } func Compare(a []byte, b []byte) bool { diff --git a/codersdk/oauth2.go b/codersdk/oauth2.go index c514a1b712..79b2186480 100644 --- a/codersdk/oauth2.go +++ b/codersdk/oauth2.go @@ -486,6 +486,6 @@ type OAuth2ClientConfiguration struct { TokenEndpointAuthMethod string `json:"token_endpoint_auth_method"` Scope string `json:"scope,omitempty"` Contacts []string `json:"contacts,omitempty"` - RegistrationAccessToken string `json:"registration_access_token"` + RegistrationAccessToken []byte `json:"registration_access_token"` RegistrationClientURI string `json:"registration_client_uri"` } diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index cc208c27ed..131223e38e 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -1274,7 +1274,9 @@ curl -X GET http://coder-server:8080/api/v2/oauth2/clients/{client_id} \ "redirect_uris": [ "string" ], - "registration_access_token": "string", + "registration_access_token": [ + 0 + ], "registration_client_uri": "string", "response_types": [ "string" @@ -1368,7 +1370,9 @@ curl -X PUT http://coder-server:8080/api/v2/oauth2/clients/{client_id} \ "redirect_uris": [ "string" ], - "registration_access_token": "string", + "registration_access_token": [ + 0 + ], "registration_client_uri": "string", "response_types": [ "string" diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 014170ca4a..3bdf92a119 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -5350,7 +5350,9 @@ Only certain features set these fields: - FeatureManagedAgentLimit| "redirect_uris": [ "string" ], - "registration_access_token": "string", + "registration_access_token": [ + 0 + ], "registration_client_uri": "string", "response_types": [ "string" @@ -5365,28 +5367,28 @@ Only certain features set these fields: - FeatureManagedAgentLimit| ### Properties -| Name | Type | Required | Restrictions | Description | -|------------------------------|-----------------|----------|--------------|-------------| -| `client_id` | string | false | | | -| `client_id_issued_at` | integer | false | | | -| `client_name` | string | false | | | -| `client_secret_expires_at` | integer | false | | | -| `client_uri` | string | false | | | -| `contacts` | array of string | false | | | -| `grant_types` | array of string | false | | | -| `jwks` | object | false | | | -| `jwks_uri` | string | false | | | -| `logo_uri` | string | false | | | -| `policy_uri` | string | false | | | -| `redirect_uris` | array of string | false | | | -| `registration_access_token` | string | false | | | -| `registration_client_uri` | string | false | | | -| `response_types` | array of string | false | | | -| `scope` | string | false | | | -| `software_id` | string | false | | | -| `software_version` | string | false | | | -| `token_endpoint_auth_method` | string | false | | | -| `tos_uri` | string | false | | | +| Name | Type | Required | Restrictions | Description | +|------------------------------|------------------|----------|--------------|-------------| +| `client_id` | string | false | | | +| `client_id_issued_at` | integer | false | | | +| `client_name` | string | false | | | +| `client_secret_expires_at` | integer | false | | | +| `client_uri` | string | false | | | +| `contacts` | array of string | false | | | +| `grant_types` | array of string | false | | | +| `jwks` | object | false | | | +| `jwks_uri` | string | false | | | +| `logo_uri` | string | false | | | +| `policy_uri` | string | false | | | +| `redirect_uris` | array of string | false | | | +| `registration_access_token` | array of integer | false | | | +| `registration_client_uri` | string | false | | | +| `response_types` | array of string | false | | | +| `scope` | string | false | | | +| `software_id` | string | false | | | +| `software_version` | string | false | | | +| `token_endpoint_auth_method` | string | false | | | +| `tos_uri` | string | false | | | ## codersdk.OAuth2ClientRegistrationRequest diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 2ebee9d93f..4f3ce12056 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -2,7 +2,6 @@ package coderd import ( "context" - "crypto/sha256" "database/sql" "fmt" "net/http" @@ -16,6 +15,7 @@ import ( "cdr.dev/slog" agpl "github.com/coder/coder/v2/coderd" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" @@ -28,7 +28,6 @@ import ( "github.com/coder/coder/v2/coderd/workspaceapps" "github.com/coder/coder/v2/coderd/workspaceapps/appurl" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/cryptorand" "github.com/coder/coder/v2/enterprise/coderd/proxyhealth" "github.com/coder/coder/v2/enterprise/replicasync" "github.com/coder/coder/v2/enterprise/wsproxy/wsproxysdk" @@ -934,13 +933,13 @@ func (api *API) reconnectingPTYSignedToken(rw http.ResponseWriter, r *http.Reque } func generateWorkspaceProxyToken(id uuid.UUID) (token string, hashed []byte, err error) { - secret, err := cryptorand.HexString(64) + secret, hashedSecret, err := apikey.GenerateSecret(64) if err != nil { return "", nil, xerrors.Errorf("generate token: %w", err) } - hashedSecret := sha256.Sum256([]byte(secret)) + fullToken := fmt.Sprintf("%s:%s", id, secret) - return fullToken, hashedSecret[:], nil + return fullToken, hashedSecret, nil } func convertProxies(p []database.WorkspaceProxy, statuses map[uuid.UUID]proxyhealth.ProxyStatus) []codersdk.WorkspaceProxy { diff --git a/enterprise/x/aibridgedserver/aibridgedserver.go b/enterprise/x/aibridgedserver/aibridgedserver.go index ee79cd15f0..e766e69622 100644 --- a/enterprise/x/aibridgedserver/aibridgedserver.go +++ b/enterprise/x/aibridgedserver/aibridgedserver.go @@ -2,8 +2,6 @@ package aibridgedserver import ( "context" - "crypto/sha256" - "crypto/subtle" "database/sql" "encoding/json" "net/url" @@ -17,6 +15,7 @@ import ( "google.golang.org/protobuf/types/known/structpb" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" @@ -358,8 +357,7 @@ func (s *Server) IsAuthorized(ctx context.Context, in *proto.IsAuthorizedRequest } // Key secret matches. - hashedSecret := sha256.Sum256([]byte(keySecret)) - if subtle.ConstantTimeCompare(key.HashedSecret, hashedSecret[:]) != 1 { + if !apikey.ValidateHash(key.HashedSecret, keySecret) { return nil, ErrInvalidKey } diff --git a/enterprise/x/aibridgedserver/aibridgedserver_test.go b/enterprise/x/aibridgedserver/aibridgedserver_test.go index 03fec9398b..fc8f8e8afd 100644 --- a/enterprise/x/aibridgedserver/aibridgedserver_test.go +++ b/enterprise/x/aibridgedserver/aibridgedserver_test.go @@ -2,7 +2,6 @@ package aibridgedserver_test import ( "context" - "crypto/sha256" "database/sql" "encoding/json" "fmt" @@ -21,6 +20,7 @@ import ( "google.golang.org/protobuf/types/known/structpb" "google.golang.org/protobuf/types/known/timestamppb" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -138,13 +138,12 @@ func TestAuthorization(t *testing.T) { } keyID, _ := cryptorand.String(10) - keySecret, _ := cryptorand.String(22) + keySecret, keySecretHashed, _ := apikey.GenerateSecret(22) token := fmt.Sprintf("%s-%s", keyID, keySecret) - keySecretHashed := sha256.Sum256([]byte(keySecret)) apiKey := database.APIKey{ ID: keyID, LifetimeSeconds: 86400, // default in db - HashedSecret: keySecretHashed[:], + HashedSecret: keySecretHashed, IPAddress: pqtype.Inet{ IPNet: net.IPNet{ IP: net.IPv4(127, 0, 0, 1),