Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit13ca9ea

Browse files
authored
chore!: ensure consistent secret token generation and hashing (#20388)
This PR uses the same sha256 hashing technique as we use for APIKeys. Sonow all randomly generated secrets will be hashed with sha256 forconsistency.This is a breaking change for the oauth tokens. Since oauth is onlyallowed for dev builds and experimental, this is ok.
1 parent9061493 commit13ca9ea

35 files changed

+169
-179
lines changed

‎coderd/apidoc/docs.go‎

Lines changed: 4 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json‎

Lines changed: 4 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apikey/apikey.go‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package apikey
22

33
import (
44
"crypto/sha256"
5+
"crypto/subtle"
56
"fmt"
67
"net"
78
"time"
@@ -44,12 +45,17 @@ type CreateParams struct {
4445
// database representation. It is the responsibility of the caller to insert it
4546
// into the database.
4647
funcGenerate(paramsCreateParams) (database.InsertAPIKeyParams,string,error) {
47-
keyID,keySecret,err:=generateKey()
48+
// Length of an API Key ID.
49+
keyID,err:=cryptorand.String(10)
4850
iferr!=nil {
49-
return database.InsertAPIKeyParams{},"",xerrors.Errorf("generate API key: %w",err)
51+
return database.InsertAPIKeyParams{},"",xerrors.Errorf("generate API key ID: %w",err)
5052
}
5153

52-
hashed:=sha256.Sum256([]byte(keySecret))
54+
// Length of an API Key secret.
55+
keySecret,hashedSecret,err:=GenerateSecret(22)
56+
iferr!=nil {
57+
return database.InsertAPIKeyParams{},"",xerrors.Errorf("generate API key secret: %w",err)
58+
}
5359

5460
// Default expires at to now+lifetime, or use the configured value if not
5561
// set.
@@ -120,25 +126,32 @@ func Generate(params CreateParams) (database.InsertAPIKeyParams, string, error)
120126
ExpiresAt:params.ExpiresAt.UTC(),
121127
CreatedAt:dbtime.Now(),
122128
UpdatedAt:dbtime.Now(),
123-
HashedSecret:hashed[:],
129+
HashedSecret:hashedSecret,
124130
LoginType:params.LoginType,
125131
Scopes:scopes,
126132
AllowList:params.AllowList,
127133
TokenName:params.TokenName,
128134
},token,nil
129135
}
130136

131-
// generateKey a new ID and secret for an API key.
132-
funcgenerateKey() (idstring,secretstring,errerror) {
133-
// Length of an API Key ID.
134-
id,err=cryptorand.String(10)
135-
iferr!=nil {
136-
return"","",err
137-
}
138-
// Length of an API Key secret.
139-
secret,err=cryptorand.String(22)
137+
funcGenerateSecret(lengthint) (secretstring,hashed []byte,errerror) {
138+
secret,err=cryptorand.String(length)
140139
iferr!=nil {
141-
return"","",err
140+
return"",nil,err
142141
}
143-
returnid,secret,nil
142+
hash:=HashSecret(secret)
143+
returnsecret,hash,nil
144+
}
145+
146+
// ValidateHash compares a secret against an expected hashed secret.
147+
funcValidateHash(hashedSecret []byte,secretstring)bool {
148+
hash:=HashSecret(secret)
149+
returnsubtle.ConstantTimeCompare(hashedSecret,hash)==1
150+
}
151+
152+
// HashSecret is the single function used to hash API key secrets.
153+
// Use this to ensure a consistent hashing algorithm.
154+
funcHashSecret(secretstring) []byte {
155+
hash:=sha256.Sum256([]byte(secret))
156+
returnhash[:]
144157
}

‎coderd/apikey/apikey_test.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package apikey_test
22

33
import (
4-
"crypto/sha256"
54
"strings"
65
"testing"
76
"time"
@@ -126,8 +125,8 @@ func TestGenerate(t *testing.T) {
126125
require.Equal(t,key.ID,keytokens[0])
127126

128127
// Assert that the hashed secret is correct.
129-
hashed:=sha256.Sum256([]byte(keytokens[1]))
130-
assert.ElementsMatch(t,hashed,key.HashedSecret)
128+
equal:=apikey.ValidateHash(key.HashedSecret,keytokens[1])
129+
require.True(t,equal,"valid secret")
131130

132131
assert.Equal(t,tc.params.UserID,key.UserID)
133132
assert.WithinDuration(t,dbtime.Now(),key.CreatedAt,time.Second*5)
@@ -173,3 +172,17 @@ func TestGenerate(t *testing.T) {
173172
})
174173
}
175174
}
175+
176+
// TestInvalid just ensures the false case is asserted by some tests.
177+
// Otherwise, a function that just `returns true` might pass all tests incorrectly.
178+
funcTestInvalid(t*testing.T) {
179+
t.Parallel()
180+
181+
require.Falsef(t,apikey.ValidateHash([]byte{},"secret"),"empty hash")
182+
183+
secret,hash,err:=apikey.GenerateSecret(10)
184+
require.NoError(t,err)
185+
186+
require.Falsef(t,apikey.ValidateHash(hash,secret+"_"),"different secret")
187+
require.Falsef(t,apikey.ValidateHash(hash[:len(hash)-1],secret),"different hash length")
188+
}

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2475,7 +2475,7 @@ func (q *querier) GetOAuth2ProviderAppByID(ctx context.Context, id uuid.UUID) (d
24752475
returnq.db.GetOAuth2ProviderAppByID(ctx,id)
24762476
}
24772477

2478-
func (q*querier)GetOAuth2ProviderAppByRegistrationToken(ctx context.Context,registrationAccessTokensql.NullString) (database.OAuth2ProviderApp,error) {
2478+
func (q*querier)GetOAuth2ProviderAppByRegistrationToken(ctx context.Context,registrationAccessToken[]byte) (database.OAuth2ProviderApp,error) {
24792479
iferr:=q.authorizeContext(ctx,policy.ActionRead,rbac.ResourceOauth2App);err!=nil {
24802480
return database.OAuth2ProviderApp{},err
24812481
}

‎coderd/database/dbauthz/dbauthz_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3925,9 +3925,9 @@ func (s *MethodTestSuite) TestOAuth2ProviderApps() {
39253925
}))
39263926
s.Run("GetOAuth2ProviderAppByRegistrationToken",s.Subtest(func(db database.Store,check*expects) {
39273927
app:=dbgen.OAuth2ProviderApp(s.T(),db, database.OAuth2ProviderApp{
3928-
RegistrationAccessToken:sql.NullString{String:"test-token",Valid:true},
3928+
RegistrationAccessToken:[]byte("test-token"),
39293929
})
3930-
check.Args(sql.NullString{String:"test-token",Valid:true}).Asserts(rbac.ResourceOauth2App,policy.ActionRead).Returns(app)
3930+
check.Args([]byte("test-token")).Asserts(rbac.ResourceOauth2App,policy.ActionRead).Returns(app)
39313931
}))
39323932
}
39333933

‎coderd/database/dbgen/dbgen.go‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package dbgen
33
import (
44
"context"
55
"crypto/rand"
6-
"crypto/sha256"
76
"database/sql"
87
"encoding/hex"
98
"encoding/json"
@@ -20,6 +19,7 @@ import (
2019
"github.com/stretchr/testify/require"
2120
"golang.org/x/xerrors"
2221

22+
"github.com/coder/coder/v2/coderd/apikey"
2323
"github.com/coder/coder/v2/coderd/database"
2424
"github.com/coder/coder/v2/coderd/database/db2sdk"
2525
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -161,8 +161,8 @@ func Template(t testing.TB, db database.Store, seed database.Template) database.
161161

162162
funcAPIKey(t testing.TB,db database.Store,seed database.APIKey,munge...func(*database.InsertAPIKeyParams)) (key database.APIKey,tokenstring) {
163163
id,_:=cryptorand.String(10)
164-
secret,_:=cryptorand.String(22)
165-
hashed:=sha256.Sum256([]byte(secret))
164+
secret,hashed,err:=apikey.GenerateSecret(22)
165+
require.NoError(t,err)
166166

167167
ip:=seed.IPAddress
168168
if!ip.Valid {
@@ -179,7 +179,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func
179179
ID:takeFirst(seed.ID,id),
180180
// 0 defaults to 86400 at the db layer
181181
LifetimeSeconds:takeFirst(seed.LifetimeSeconds,0),
182-
HashedSecret:takeFirstSlice(seed.HashedSecret,hashed[:]),
182+
HashedSecret:takeFirstSlice(seed.HashedSecret,hashed),
183183
IPAddress:ip,
184184
UserID:takeFirst(seed.UserID,uuid.New()),
185185
LastUsed:takeFirst(seed.LastUsed,dbtime.Now()),
@@ -194,7 +194,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func
194194
for_,fn:=rangemunge {
195195
fn(&params)
196196
}
197-
key,err:=db.InsertAPIKey(genCtx,params)
197+
key,err=db.InsertAPIKey(genCtx,params)
198198
require.NoError(t,err,"insert api key")
199199
returnkey,fmt.Sprintf("%s-%s",key.ID,secret)
200200
}
@@ -980,16 +980,15 @@ func WorkspaceResourceMetadatums(t testing.TB, db database.Store, seed database.
980980
}
981981

982982
funcWorkspaceProxy(t testing.TB,db database.Store,orig database.WorkspaceProxy) (database.WorkspaceProxy,string) {
983-
secret,err:=cryptorand.HexString(64)
983+
secret,hashedSecret,err:=apikey.GenerateSecret(64)
984984
require.NoError(t,err,"generate secret")
985-
hashedSecret:=sha256.Sum256([]byte(secret))
986985

987986
proxy,err:=db.InsertWorkspaceProxy(genCtx, database.InsertWorkspaceProxyParams{
988987
ID:takeFirst(orig.ID,uuid.New()),
989988
Name:takeFirst(orig.Name,testutil.GetRandomName(t)),
990989
DisplayName:takeFirst(orig.DisplayName,testutil.GetRandomName(t)),
991990
Icon:takeFirst(orig.Icon,testutil.GetRandomName(t)),
992-
TokenHashedSecret:hashedSecret[:],
991+
TokenHashedSecret:hashedSecret,
993992
CreatedAt:takeFirst(orig.CreatedAt,dbtime.Now()),
994993
UpdatedAt:takeFirst(orig.UpdatedAt,dbtime.Now()),
995994
DerpEnabled:takeFirst(orig.DerpEnabled,false),
@@ -1259,7 +1258,7 @@ func OAuth2ProviderApp(t testing.TB, db database.Store, seed database.OAuth2Prov
12591258
Jwks:seed.Jwks,// pqtype.NullRawMessage{} is not comparable, use existing value
12601259
SoftwareID:takeFirst(seed.SoftwareID, sql.NullString{}),
12611260
SoftwareVersion:takeFirst(seed.SoftwareVersion, sql.NullString{}),
1262-
RegistrationAccessToken:takeFirst(seed.RegistrationAccessToken, sql.NullString{}),
1261+
RegistrationAccessToken:seed.RegistrationAccessToken,
12631262
RegistrationClientUri:takeFirst(seed.RegistrationClientUri, sql.NullString{}),
12641263
})
12651264
require.NoError(t,err,"insert oauth2 app")

‎coderd/database/dbmetrics/querymetrics.go‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go‎

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/dump.sql‎

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

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp