Jul 25, 2024 · Jul 25, 2024 · Jul 25, 2024 · Jul 25, 2024 · Jul 25, 2024 · Jul 25, 2024
diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
 return q.db.GetProvisionerJobsCreatedAfter(ctx, createdAt)
 }

 func (q *querier) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (database.ProvisionerKey, error) {
 return fetch(q.log, q.auth, q.db.GetProvisionerKeyByHashedSecret)(ctx, hashedSecret)
 }

 func (q *querier) GetProvisionerKeyByID(ctx context.Context, id uuid.UUID) (database.ProvisionerKey, error) {
 return fetch(q.log, q.auth, q.db.GetProvisionerKeyByID)(ctx, id)
 }
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
 return jobs, nil
 }

 func (q *FakeQuerier) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (database.ProvisionerKey, error) {
 q.mutex.RLock()
 defer q.mutex.RUnlock()

 for _, key := range q.provisionerKeys {
 if bytes.Equal(key.HashedSecret, hashedSecret) {
 return key, nil
 }
 }

 return database.ProvisionerKey{}, sql.ErrNoRows
 }

 func (q *FakeQuerier) GetProvisionerKeyByID(_ context.Context, id uuid.UUID) (database.ProvisionerKey, error) {
 q.mutex.RLock()
 defer q.mutex.RUnlock()
diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/provisionerkeys.sql b/coderd/database/queries/provisionerkeys.sql
 WHERE
    id = $1;

 -- name: GetProvisionerKeyByHashedSecret :one
 SELECT
    *
 FROM
    provisioner_keys
 WHERE
    hashed_secret = $1;

 -- name: GetProvisionerKeyByName :one
 SELECT
    *
diff --git a/coderd/httpmw/provisionerdaemon.go b/coderd/httpmw/provisionerdaemon.go
 return
 }

 id, keyValue,err := provisionerkey.Parse(key)
 err := provisionerkey.Validate(key)
 if err != nil {
 handleOptional(http.StatusUnauthorized, codersdk.Response{
 handleOptional(http.StatusBadGateway, codersdk.Response{
 Message: "provisioner daemon key invalid",
 Detail:  err.Error(),
 })
 return
 }

 hashedKey := provisionerkey.HashSecret(key)
 // nolint:gocritic // System must check if the provisioner key is valid.
 pk, err := opts.DB.GetProvisionerKeyByID(dbauthz.AsSystemRestricted(ctx),id)
 pk, err := opts.DB.GetProvisionerKeyByHashedSecret(dbauthz.AsSystemRestricted(ctx),hashedKey)
 if err != nil {
 if httpapi.Is404Error(err) {
 handleOptional(http.StatusUnauthorized, codersdk.Response{
 }

 handleOptional(http.StatusInternalServerError, codersdk.Response{
 Message: "get provisioner daemon key: " + err.Error(),
 Message: "get provisioner daemon key",
 Detail:  err.Error(),
 })
 return
 }

 if provisionerkey.Compare(pk.HashedSecret,provisionerkey.HashSecret(keyValue)) {
 if provisionerkey.Compare(pk.HashedSecret,hashedKey) {
 handleOptional(http.StatusUnauthorized, codersdk.Response{
 Message: "provisioner daemon key invalid",
 })
diff --git a/coderd/provisionerkey/provisionerkey.go b/coderd/provisionerkey/provisionerkey.go
 import (
 "crypto/sha256"
 "crypto/subtle"
 "fmt"
 "strings"

 "github.com/google/uuid"
 "golang.org/x/xerrors"
 )

 func New(organizationID uuid.UUID, name string, tags map[string]string) (database.InsertProvisionerKeyParams, string, error) {
 id := uuid.New()
 secret, err := cryptorand.HexString(64)
 secret, err := cryptorand.String(64)
 if err != nil {
 return database.InsertProvisionerKeyParams{}, "", xerrors.Errorf("generatetoken: %w", err)
 return database.InsertProvisionerKeyParams{}, "", xerrors.Errorf("generatesecret: %w", err)
 }
 hashedSecret := HashSecret(secret)
 token := fmt.Sprintf("%s:%s", id, secret)

 if tags == nil {
 tags = map[string]string{}
 }

 return database.InsertProvisionerKeyParams{
 ID:id,
 ID:uuid.New(),
 CreatedAt:      dbtime.Now(),
 OrganizationID: organizationID,
 Name:           name,
 HashedSecret:hashedSecret,
 HashedSecret:HashSecret(secret),
 Tags:           tags,
 },token, nil
 },secret, nil
 }

 func Parse(token string) (uuid.UUID, string, error) {
 parts := strings.Split(token, ":")
 if len(parts) != 2 {
 return uuid.UUID{}, "", xerrors.Errorf("invalid token format")
 func Validate(token string) error {
 if len(token) != 64 {
 return xerrors.Errorf("must be 64 characters")
 }

 id, err := uuid.Parse(parts[0])
 if err != nil {
 return uuid.UUID{}, "", xerrors.Errorf("parse id: %w", err)
 }

 return id, parts[1], nil
 return nil
 }

 func HashSecret(secret string) []byte {
diff --git a/enterprise/cli/provisionerdaemonstart.go b/enterprise/cli/provisionerdaemonstart.go
 if len(rawTags) > 0 {
 return xerrors.New("cannot provide tags when using provisioner key")
 }
 _, _,err:= provisionerkey.Parse(provisionerKey)
 err = provisionerkey.Validate(provisionerKey)
 if err != nil {
 return xerrors.Errorf("parse provisioner key: %w", err)
 return xerrors.Errorf("validate provisioner key: %w", err)
 }
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -1680,6 +1680,10 @@ func (q *querier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt
		return q.db.GetProvisionerJobsCreatedAfter(ctx, createdAt)
		}

		func (q *querier) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (database.ProvisionerKey, error) {
		return fetch(q.log, q.auth, q.db.GetProvisionerKeyByHashedSecret)(ctx, hashedSecret)
		}

		func (q *querier) GetProvisionerKeyByID(ctx context.Context, id uuid.UUID) (database.ProvisionerKey, error) {
		return fetch(q.log, q.auth, q.db.GetProvisionerKeyByID)(ctx, id)
		}
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -3240,6 +3240,19 @@ func (q *FakeQuerier) GetProvisionerJobsCreatedAfter(_ context.Context, after ti
		return jobs, nil
		}

		func (q *FakeQuerier) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (database.ProvisionerKey, error) {
		q.mutex.RLock()
		defer q.mutex.RUnlock()

		for _, key := range q.provisionerKeys {
		if bytes.Equal(key.HashedSecret, hashedSecret) {
		return key, nil
		}
		}

		return database.ProvisionerKey{}, sql.ErrNoRows
		}

		func (q *FakeQuerier) GetProvisionerKeyByID(_ context.Context, id uuid.UUID) (database.ProvisionerKey, error) {
		q.mutex.RLock()
		defer q.mutex.RUnlock()
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -19,6 +19,14 @@ FROM
		WHERE
		id = $1;

		-- name: GetProvisionerKeyByHashedSecret :one
		SELECT
		*
		FROM
		provisioner_keys
		WHERE
		hashed_secret = $1;

		-- name: GetProvisionerKeyByName :one
		SELECT
		*
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -71,16 +71,17 @@ func ExtractProvisionerDaemonAuthenticated(opts ExtractProvisionerAuthConfig) fu
		return
		}

		id, keyValue,err := provisionerkey.Parse(key)
		err := provisionerkey.Validate(key)
		if err != nil {
		handleOptional(http.StatusUnauthorized, codersdk.Response{
		handleOptional(http.StatusBadGateway, codersdk.Response{
Copy link Member EmyrkJul 25, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. `http.StatusBadGateway`? This should be unauthorized, no? Copy link ContributorAuthor f0sselJul 25, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. This error has a detail that specifies the length must be 64, and I thought it felt more like a 400 in that regard. Copy link ContributorAuthor f0sselJul 25, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. omg I meant`StatusBadRequest`...
		Message: "provisioner daemon key invalid",
		Detail: err.Error(),
		})
		return
		}

		hashedKey := provisionerkey.HashSecret(key)
		// nolint:gocritic // System must check if the provisioner key is valid.
		pk, err := opts.DB.GetProvisionerKeyByID(dbauthz.AsSystemRestricted(ctx),id)
		pk, err := opts.DB.GetProvisionerKeyByHashedSecret(dbauthz.AsSystemRestricted(ctx),hashedKey)
		if err != nil {
		if httpapi.Is404Error(err) {
		handleOptional(http.StatusUnauthorized, codersdk.Response{
Expand All		@@ -90,12 +91,13 @@ func ExtractProvisionerDaemonAuthenticated(opts ExtractProvisionerAuthConfig) fu
		}

		handleOptional(http.StatusInternalServerError, codersdk.Response{
		Message: "get provisioner daemon key: " + err.Error(),
		Message: "get provisioner daemon key",
		Detail: err.Error(),
		})
		return
		}

		if provisionerkey.Compare(pk.HashedSecret,provisionerkey.HashSecret(keyValue)) {
		if provisionerkey.Compare(pk.HashedSecret,hashedKey) {
		handleOptional(http.StatusUnauthorized, codersdk.Response{
		Message: "provisioner daemon key invalid",
		})
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -3,8 +3,6 @@ package provisionerkey
		import (
		"crypto/sha256"
		"crypto/subtle"
		"fmt"
		"strings"

		"github.com/google/uuid"
		"golang.org/x/xerrors"
Expand All		@@ -15,40 +13,31 @@ import (
		)

		func New(organizationID uuid.UUID, name string, tags map[string]string) (database.InsertProvisionerKeyParams, string, error) {
		id := uuid.New()
		secret, err := cryptorand.HexString(64)
		secret, err := cryptorand.String(64)
Copy link Member EmyrkJul 25, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. 64 is probably longer than we need if we are using alpha (lower & upper) + numbers. When we using hex strings, technically they are case insensitive, so you only get 16 options vs the now 62 (26 lower, 26 upper, 10 numeric) You only need ~43 characters to match the same search space as 64 hex characters. We only use22 characters for our api keys (I bet we copied someone else here). It does not really matter, but if we are shortening our keys, we can at least shorten them to 43 and keep the same level of security as 64 hex characters. Which is super common. `16^64 = 1.157920e+7762^43 = 1.182613e+77log(16^64)/log(62) = 42.9948 <-- basically 43` Thoughts on going even shorter? 64 is such an excessive level of security. Copy link ContributorAuthor f0sselJul 25, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Of course, I was leaning on better safe than sorry but let's go short - 32 sound good? Copy link Member EmyrkJul 25, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. 32 is ~190 bits of entropy. 43 gets us to 256, which is common in symmetric key encryption. At the end of the day, 32 vs 43 feels completely arbitrary and probably sufficient, but 43 feels like "random" than "32" in the fact it lines up with 256bit. 🤷‍♂️ I don't think 43 looks too bad. That's just my 2c. 43 characters long `RrMe58fCjqyf7A8uRivKq3hQDcC3y5N5rnUWSRz5QA1KH5y6hAUAJjymG8Xrjia4NY3yz3crenkmuEkT4T3N3TEiuE7aDvUZyG1TqAtiikKR5rW6WkmG2RuEjCLXi3z2B0ZRpbhZibWFfdDjRbb6fQzqy7baCBq2JQFKjDeA` 32 `VB3StT1Uj0CvP6LX2iMzX3eDACKPEBzFjaDjJSPNLygpKLUELrUU5m5fuNbTfvV73QvMTSuRLwn7x0hbX4YJQKf2GMcL3TA5G6LkDZdw1rumSZuLyMLGUYi15hvR38jA` 64 `qySKB6Zi6XLmahApVJLM5GMYP9TGy6BEyCYUC7KbVH0AE5h07W0eWg2Cqmjw7gCFpSngALfmYcVmMETAAcntnp2bzZkRC90pYRa2fC5MmvHMuQHZ7hwg5eGMnWfnbCUut5gFjyTD3iy8J9emR6B7TSCwCYrcwJGLQXBw4zPMM0AS9E89DMXy4bK4V4FtJ1qwv4C06YWqALYBpMScm7AuVYwzun0iEb5iCpQyg2fEWgfreNL2W6itRfzeS1cic1XY537TcRdc3McvbTKtN4uZRfRp68hrnYJieHHtJp4d9tNWbFFxvrECv0umzig7UUk9` Copy link ContributorAuthor f0sselJul 25, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. 43 it is 😄
		if err != nil {
		return database.InsertProvisionerKeyParams{}, "", xerrors.Errorf("generatetoken: %w", err)
		return database.InsertProvisionerKeyParams{}, "", xerrors.Errorf("generatesecret: %w", err)
		}
		hashedSecret := HashSecret(secret)
		token := fmt.Sprintf("%s:%s", id, secret)

		if tags == nil {
		tags = map[string]string{}
		}

		return database.InsertProvisionerKeyParams{
		ID:id,
		ID:uuid.New(),
		CreatedAt: dbtime.Now(),
		OrganizationID: organizationID,
		Name: name,
		HashedSecret:hashedSecret,
		HashedSecret:HashSecret(secret),
		Tags: tags,
		},token, nil
		},secret, nil
		}

		func Parse(token string) (uuid.UUID, string, error) {
		parts := strings.Split(token, ":")
		if len(parts) != 2 {
		return uuid.UUID{}, "", xerrors.Errorf("invalid token format")
		func Validate(token string) error {
		if len(token) != 64 {
		return xerrors.Errorf("must be 64 characters")
		}

		id, err := uuid.Parse(parts[0])
		if err != nil {
		return uuid.UUID{}, "", xerrors.Errorf("parse id: %w", err)
		}

		return id, parts[1], nil
		return nil
		}

		func HashSecret(secret string) []byte {
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -122,9 +122,9 @@ func (r RootCmd) provisionerDaemonStart() serpent.Command {
		if len(rawTags) > 0 {
		return xerrors.New("cannot provide tags when using provisioner key")
		}
		_, _,err:= provisionerkey.Parse(provisionerKey)
		err = provisionerkey.Validate(provisionerKey)
		if err != nil {
		return xerrors.Errorf("parse provisioner key: %w", err)
		return xerrors.Errorf("validate provisioner key: %w", err)
		}
		}

Expand Down