Skip to content

Commit 6c2336b

Browse files
f0sself0ssel
authored
chore: shorten provisioner key (coder#14017)
1 parent 7ea1a4c commit 6c2336b

File tree

13 files changed

+103
-60
lines changed

13 files changed

+103
-60
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,6 +1680,10 @@ func (q *querier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt
16801680
return q.db.GetProvisionerJobsCreatedAfter(ctx, createdAt)
16811681
}
16821682

1683+
func (q *querier) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (database.ProvisionerKey, error) {
1684+
return fetch(q.log, q.auth, q.db.GetProvisionerKeyByHashedSecret)(ctx, hashedSecret)
1685+
}
1686+
16831687
func (q *querier) GetProvisionerKeyByID(ctx context.Context, id uuid.UUID) (database.ProvisionerKey, error) {
16841688
return fetch(q.log, q.auth, q.db.GetProvisionerKeyByID)(ctx, id)
16851689
}

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1825,6 +1825,11 @@ func (s *MethodTestSuite) TestProvisionerKeys() {
18251825
pk := dbgen.ProvisionerKey(s.T(), db, database.ProvisionerKey{OrganizationID: org.ID})
18261826
check.Args(pk.ID).Asserts(pk, policy.ActionRead).Returns(pk)
18271827
}))
1828+
s.Run("GetProvisionerKeyByHashedSecret", s.Subtest(func(db database.Store, check *expects) {
1829+
org := dbgen.Organization(s.T(), db, database.Organization{})
1830+
pk := dbgen.ProvisionerKey(s.T(), db, database.ProvisionerKey{OrganizationID: org.ID, HashedSecret: []byte("foo")})
1831+
check.Args([]byte("foo")).Asserts(pk, policy.ActionRead).Returns(pk)
1832+
}))
18281833
s.Run("GetProvisionerKeyByName", s.Subtest(func(db database.Store, check *expects) {
18291834
org := dbgen.Organization(s.T(), db, database.Organization{})
18301835
pk := dbgen.ProvisionerKey(s.T(), db, database.ProvisionerKey{OrganizationID: org.ID})

‎coderd/database/dbmem/dbmem.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3240,6 +3240,19 @@ func (q *FakeQuerier) GetProvisionerJobsCreatedAfter(_ context.Context, after ti
32403240
return jobs, nil
32413241
}
32423242

3243+
func (q *FakeQuerier) GetProvisionerKeyByHashedSecret(_ context.Context, hashedSecret []byte) (database.ProvisionerKey, error) {
3244+
q.mutex.RLock()
3245+
defer q.mutex.RUnlock()
3246+
3247+
for _, key := range q.provisionerKeys {
3248+
if bytes.Equal(key.HashedSecret, hashedSecret) {
3249+
return key, nil
3250+
}
3251+
}
3252+
3253+
return database.ProvisionerKey{}, sql.ErrNoRows
3254+
}
3255+
32433256
func (q *FakeQuerier) GetProvisionerKeyByID(_ context.Context, id uuid.UUID) (database.ProvisionerKey, error) {
32443257
q.mutex.RLock()
32453258
defer q.mutex.RUnlock()

‎coderd/database/dbmetrics/dbmetrics.go

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

‎coderd/database/dbmock/dbmock.go

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

‎coderd/database/querier.go

Lines changed: 1 addition & 0 deletions
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: 23 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/provisionerkeys.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ FROM
1919
WHERE
2020
id = $1;
2121

22+
-- name: GetProvisionerKeyByHashedSecret :one
23+
SELECT
24+
*
25+
FROM
26+
provisioner_keys
27+
WHERE
28+
hashed_secret = $1;
29+
2230
-- name: GetProvisionerKeyByName :one
2331
SELECT
2432
*

‎coderd/httpmw/provisionerdaemon.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,17 @@ func ExtractProvisionerDaemonAuthenticated(opts ExtractProvisionerAuthConfig) fu
7171
return
7272
}
7373

74-
id, keyValue, err := provisionerkey.Parse(key)
74+
err := provisionerkey.Validate(key)
7575
if err != nil {
76-
handleOptional(http.StatusUnauthorized, codersdk.Response{
76+
handleOptional(http.StatusBadRequest, codersdk.Response{
7777
Message: "provisioner daemon key invalid",
78+
Detail: err.Error(),
7879
})
7980
return
8081
}
81-
82+
hashedKey := provisionerkey.HashSecret(key)
8283
// nolint:gocritic // System must check if the provisioner key is valid.
83-
pk, err := opts.DB.GetProvisionerKeyByID(dbauthz.AsSystemRestricted(ctx), id)
84+
pk, err := opts.DB.GetProvisionerKeyByHashedSecret(dbauthz.AsSystemRestricted(ctx), hashedKey)
8485
if err != nil {
8586
if httpapi.Is404Error(err) {
8687
handleOptional(http.StatusUnauthorized, codersdk.Response{
@@ -90,12 +91,13 @@ func ExtractProvisionerDaemonAuthenticated(opts ExtractProvisionerAuthConfig) fu
9091
}
9192

9293
handleOptional(http.StatusInternalServerError, codersdk.Response{
93-
Message: "get provisioner daemon key: " + err.Error(),
94+
Message: "get provisioner daemon key",
95+
Detail: err.Error(),
9496
})
9597
return
9698
}
9799

98-
if provisionerkey.Compare(pk.HashedSecret, provisionerkey.HashSecret(keyValue)) {
100+
if provisionerkey.Compare(pk.HashedSecret, hashedKey) {
99101
handleOptional(http.StatusUnauthorized, codersdk.Response{
100102
Message: "provisioner daemon key invalid",
101103
})

‎coderd/provisionerkey/provisionerkey.go

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package provisionerkey
33
import (
44
"crypto/sha256"
55
"crypto/subtle"
6-
"fmt"
7-
"strings"
86

97
"github.com/google/uuid"
108
"golang.org/x/xerrors"
@@ -14,41 +12,36 @@ import (
1412
"github.com/coder/coder/v2/cryptorand"
1513
)
1614

15+
const (
16+
secretLength = 43
17+
)
18+
1719
func New(organizationID uuid.UUID, name string, tags map[string]string) (database.InsertProvisionerKeyParams, string, error) {
18-
id := uuid.New()
19-
secret, err := cryptorand.HexString(64)
20+
secret, err := cryptorand.String(secretLength)
2021
if err != nil {
21-
return database.InsertProvisionerKeyParams{}, "", xerrors.Errorf("generate token: %w", err)
22+
return database.InsertProvisionerKeyParams{}, "", xerrors.Errorf("generate secret: %w", err)
2223
}
23-
hashedSecret := HashSecret(secret)
24-
token := fmt.Sprintf("%s:%s", id, secret)
2524

2625
if tags == nil {
2726
tags = map[string]string{}
2827
}
2928

3029
return database.InsertProvisionerKeyParams{
31-
ID: id,
30+
ID: uuid.New(),
3231
CreatedAt: dbtime.Now(),
3332
OrganizationID: organizationID,
3433
Name: name,
35-
HashedSecret: hashedSecret,
34+
HashedSecret: HashSecret(secret),
3635
Tags: tags,
37-
}, token, nil
36+
}, secret, nil
3837
}
3938

40-
func Parse(token string) (uuid.UUID, string, error) {
41-
parts := strings.Split(token, ":")
42-
if len(parts) != 2 {
43-
return uuid.UUID{}, "", xerrors.Errorf("invalid token format")
44-
}
45-
46-
id, err := uuid.Parse(parts[0])
47-
if err != nil {
48-
return uuid.UUID{}, "", xerrors.Errorf("parse id: %w", err)
39+
func Validate(token string) error {
40+
if len(token) != secretLength {
41+
return xerrors.Errorf("must be %d characters", secretLength)
4942
}
5043

51-
return id, parts[1], nil
44+
return nil
5245
}
5346

5447
func HashSecret(secret string) []byte {

‎enterprise/cli/provisionerdaemonstart.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ func (r *RootCmd) provisionerDaemonStart() *serpent.Command {
122122
if len(rawTags) > 0 {
123123
return xerrors.New("cannot provide tags when using provisioner key")
124124
}
125-
_, _, err := provisionerkey.Parse(provisionerKey)
125+
err = provisionerkey.Validate(provisionerKey)
126126
if err != nil {
127-
return xerrors.Errorf("parse provisioner key: %w", err)
127+
return xerrors.Errorf("validate provisioner key: %w", err)
128128
}
129129
}
130130

‎enterprise/cli/provisionerkeys_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import (
44
"strings"
55
"testing"
66

7-
"github.com/google/uuid"
87
"github.com/stretchr/testify/require"
98

109
"github.com/coder/coder/v2/cli/clitest"
1110
"github.com/coder/coder/v2/coderd/coderdtest"
11+
"github.com/coder/coder/v2/coderd/provisionerkey"
1212
"github.com/coder/coder/v2/coderd/rbac"
1313
"github.com/coder/coder/v2/codersdk"
1414
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
@@ -58,10 +58,7 @@ func TestProvisionerKeys(t *testing.T) {
5858
_ = pty.ReadLine(ctx)
5959
key := pty.ReadLine(ctx)
6060
require.NotEmpty(t, key)
61-
parts := strings.Split(key, ":")
62-
require.Len(t, parts, 2, "expected 2 parts")
63-
_, err = uuid.Parse(parts[0])
64-
require.NoError(t, err, "expected token to be a uuid")
61+
require.NoError(t, provisionerkey.Validate(key))
6562

6663
inv, conf = newCLI(
6764
t,

‎enterprise/coderd/provisionerdaemons_test.go

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"io"
88
"net/http"
9-
"strings"
109
"testing"
1110

1211
"github.com/google/uuid"
@@ -612,36 +611,12 @@ func TestProvisionerDaemonServe(t *testing.T) {
612611
errStatusCode: http.StatusUnauthorized,
613612
},
614613
{
615-
name: "WrongKey",
614+
name: "InvalidKey",
616615
multiOrgFeatureEnabled: true,
617616
multiOrgExperimentEnabled: true,
618617
insertParams: insertParams,
619618
requestProvisionerKey: "provisionersftw",
620-
errStatusCode: http.StatusUnauthorized,
621-
},
622-
{
623-
name: "IdOKKeyValueWrong",
624-
multiOrgFeatureEnabled: true,
625-
multiOrgExperimentEnabled: true,
626-
insertParams: insertParams,
627-
requestProvisionerKey: insertParams.ID.String() + ":" + "wrong",
628-
errStatusCode: http.StatusUnauthorized,
629-
},
630-
{
631-
name: "IdWrongKeyValueOK",
632-
multiOrgFeatureEnabled: true,
633-
multiOrgExperimentEnabled: true,
634-
insertParams: insertParams,
635-
requestProvisionerKey: uuid.NewString() + ":" + token,
636-
errStatusCode: http.StatusUnauthorized,
637-
},
638-
{
639-
name: "KeyValueOnly",
640-
multiOrgFeatureEnabled: true,
641-
multiOrgExperimentEnabled: true,
642-
insertParams: insertParams,
643-
requestProvisionerKey: strings.Split(token, ":")[1],
644-
errStatusCode: http.StatusUnauthorized,
619+
errStatusCode: http.StatusBadRequest,
645620
},
646621
{
647622
name: "KeyAndPSK",

0 commit comments

Comments
 (0)