From 0fc70d82eef32c0e0af00e500434032df5ce8431 Mon Sep 17 00:00:00 2001 From: Abhishek Manjegowda Date: Tue, 9 Dec 2025 09:50:36 +0530 Subject: [PATCH] refactor: Update Password use of crypto/rand to SecureRandomReader (#6259) * reactor: Update Password use of crypto/rand to SecureRandomReader * changes updated * make gen error changes * make gen error changes * new changes updated * new changes updated * Revert "new changes updated" This reverts commit 85ee1d21867e26e7dac9ac28ba0e541a84345e90. * Revert "new changes updated" This reverts commit b4df84452397724465e00fc48169473148a26e3f. * SecureRandomreader introduced to password * SecureRandom pkg removed * updated --- internal/auth/password/argon2.go | 7 ++++--- internal/auth/password/argon2_test.go | 5 +++-- internal/auth/password/options.go | 18 ++++++++++++++++-- internal/auth/password/options_test.go | 9 +++++++++ internal/auth/password/repository.go | 3 +++ internal/auth/password/repository_account.go | 2 +- internal/auth/password/repository_password.go | 6 +++--- internal/auth/password/repository_test.go | 16 ++++++++++++---- internal/auth/password/rewrapping_test.go | 3 ++- internal/cmd/base/initial_resources.go | 3 ++- 10 files changed, 55 insertions(+), 17 deletions(-) diff --git a/internal/auth/password/argon2.go b/internal/auth/password/argon2.go index faf89a68c..a38f091f2 100644 --- a/internal/auth/password/argon2.go +++ b/internal/auth/password/argon2.go @@ -5,7 +5,7 @@ package password import ( "context" - "crypto/rand" + "io" "strings" "github.com/hashicorp/boundary/internal/auth/password/store" @@ -152,7 +152,7 @@ type Argon2Credential struct { tableName string } -func newArgon2Credential(ctx context.Context, accountId string, password string, conf *Argon2Configuration) (*Argon2Credential, error) { +func newArgon2Credential(ctx context.Context, accountId string, password string, conf *Argon2Configuration, randReader io.Reader) (*Argon2Credential, error) { const op = "password.newArgon2Credential" if accountId == "" { return nil, errors.New(ctx, errors.InvalidParameter, op, "missing accountId") @@ -178,8 +178,9 @@ func newArgon2Credential(ctx context.Context, accountId string, password string, }, } + // Generate a random salt salt := make([]byte, conf.SaltLength) - if _, err := rand.Read(salt); err != nil { + if _, err := io.ReadFull(randReader, salt); err != nil { return nil, errors.Wrap(ctx, err, op, errors.WithCode(errors.Io)) } c.Salt = salt diff --git a/internal/auth/password/argon2_test.go b/internal/auth/password/argon2_test.go index cb0d5ad11..8a53dc757 100644 --- a/internal/auth/password/argon2_test.go +++ b/internal/auth/password/argon2_test.go @@ -5,6 +5,7 @@ package password import ( "context" + "crypto/rand" "runtime" "slices" "strconv" @@ -431,7 +432,7 @@ func TestArgon2Credential_New(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - got, err := newArgon2Credential(context.Background(), tt.args.accountId, tt.args.password, tt.args.conf) + got, err := newArgon2Credential(context.Background(), tt.args.accountId, tt.args.password, tt.args.conf, rand.Reader) if tt.wantIsErr != 0 { assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "Unexpected error %s", err) assert.Equal(tt.wantErrMsg, err.Error()) @@ -507,7 +508,7 @@ func measureCredentialCreations(ctx context.Context, t testing.TB, publicId stri defer wg.Done() <-start t.Log("Credential " + strconv.Itoa(i) + " starting") - _, err := newArgon2Credential(ctx, publicId, "password", conf) + _, err := newArgon2Credential(ctx, publicId, "password", conf, rand.Reader) assert.NoError(t, err) t.Log("Credential " + strconv.Itoa(i) + " finished") }() diff --git a/internal/auth/password/options.go b/internal/auth/password/options.go index b07a53fdc..96e9e20f4 100644 --- a/internal/auth/password/options.go +++ b/internal/auth/password/options.go @@ -3,7 +3,12 @@ package password -import "github.com/hashicorp/boundary/internal/pagination" +import ( + "crypto/rand" + "io" + + "github.com/hashicorp/boundary/internal/pagination" +) // GetOpts - iterate the inbound Options and return a struct. func GetOpts(opt ...Option) options { @@ -30,11 +35,13 @@ type options struct { withOrderByCreateTime bool ascending bool withStartPageAfterItem pagination.Item + withRandomReader io.Reader } func getDefaultOptions() options { return options{ - withConfig: NewArgon2Configuration(), + withConfig: NewArgon2Configuration(), + withRandomReader: rand.Reader, } } @@ -106,3 +113,10 @@ func WithStartPageAfterItem(item pagination.Item) Option { o.withStartPageAfterItem = item } } + +// WithRandomReader provides an option to specify a random reader. +func WithRandomReader(reader io.Reader) Option { + return func(o *options) { + o.withRandomReader = reader + } +} diff --git a/internal/auth/password/options_test.go b/internal/auth/password/options_test.go index e9b4bae73..590a460b7 100644 --- a/internal/auth/password/options_test.go +++ b/internal/auth/password/options_test.go @@ -4,6 +4,7 @@ package password import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -68,4 +69,12 @@ func Test_GetOpts(t *testing.T) { testOpts.ascending = true assert.Equal(opts, testOpts) }) + t.Run("WithRandomReader", func(t *testing.T) { + assert := assert.New(t) + reader := strings.NewReader("notrandom") + opts := GetOpts(WithRandomReader(reader)) + testOpts := getDefaultOptions() + testOpts.withRandomReader = reader + assert.Equal(opts, testOpts) + }) } diff --git a/internal/auth/password/repository.go b/internal/auth/password/repository.go index 69370fe2f..74df9165b 100644 --- a/internal/auth/password/repository.go +++ b/internal/auth/password/repository.go @@ -5,6 +5,7 @@ package password import ( "context" + "io" "strings" "github.com/hashicorp/boundary/internal/db" @@ -20,6 +21,7 @@ type Repository struct { kms *kms.Kms // defaultLimit provides a default for limiting the number of results returned from the repo defaultLimit int + randomReader io.Reader } // NewRepository creates a new Repository. The returned repository should @@ -48,6 +50,7 @@ func NewRepository(ctx context.Context, r db.Reader, w db.Writer, kms *kms.Kms, writer: w, kms: kms, defaultLimit: opts.withLimit, + randomReader: opts.withRandomReader, }, nil } diff --git a/internal/auth/password/repository_account.go b/internal/auth/password/repository_account.go index 63ab71340..905e5d661 100644 --- a/internal/auth/password/repository_account.go +++ b/internal/auth/password/repository_account.go @@ -84,7 +84,7 @@ func (r *Repository) CreateAccount(ctx context.Context, scopeId string, a *Accou if cc.MinPasswordLength > len(opts.password) { return nil, errors.New(ctx, errors.PasswordTooShort, op, fmt.Sprintf("must be longer than %v", cc.MinPasswordLength)) } - if cred, err = newArgon2Credential(ctx, a.PublicId, opts.password, cc.argon2()); err != nil { + if cred, err = newArgon2Credential(ctx, a.PublicId, opts.password, cc.argon2(), r.randomReader); err != nil { return nil, errors.Wrap(ctx, err, op) } } diff --git a/internal/auth/password/repository_password.go b/internal/auth/password/repository_password.go index 6cb4b43cd..6d667bd44 100644 --- a/internal/auth/password/repository_password.go +++ b/internal/auth/password/repository_password.go @@ -68,7 +68,7 @@ func (r *Repository) Authenticate(ctx context.Context, scopeId, authMethodId, lo if err != nil { return acct.Account, errors.Wrap(ctx, err, op, errors.WithMsg("retrieve current password configuration")) } - cred, err := newArgon2Credential(ctx, acct.PublicId, password, cc.argon2()) + cred, err := newArgon2Credential(ctx, acct.PublicId, password, cc.argon2(), r.randomReader) if err != nil { return acct.Account, errors.Wrap(ctx, err, op, errors.WithCode(errors.PasswordInvalidConfiguration)) } @@ -166,7 +166,7 @@ func (r *Repository) ChangePassword(ctx context.Context, scopeId, accountId, old if cc.MinPasswordLength > len(new) { return nil, errors.New(ctx, errors.PasswordTooShort, op, fmt.Sprintf("must be at least %d", cc.MinPasswordLength)) } - newCred, err := newArgon2Credential(ctx, accountId, new, cc.argon2()) + newCred, err := newArgon2Credential(ctx, accountId, new, cc.argon2(), r.randomReader) if err != nil { return nil, errors.Wrap(ctx, err, op) } @@ -297,7 +297,7 @@ func (r *Repository) SetPassword(ctx context.Context, scopeId, accountId, passwo if cc.MinPasswordLength > len(password) { return nil, errors.New(ctx, errors.PasswordTooShort, op, fmt.Sprintf("password must be at least %v", cc.MinPasswordLength)) } - newCred, err = newArgon2Credential(ctx, accountId, password, cc.argon2()) + newCred, err = newArgon2Credential(ctx, accountId, password, cc.argon2(), r.randomReader) if err != nil { return nil, errors.Wrap(ctx, err, op) } diff --git a/internal/auth/password/repository_test.go b/internal/auth/password/repository_test.go index 262f1ac55..dbaf2347c 100644 --- a/internal/auth/password/repository_test.go +++ b/internal/auth/password/repository_test.go @@ -5,6 +5,8 @@ package password import ( "context" + "crypto/rand" + "strings" "testing" "github.com/hashicorp/boundary/internal/db" @@ -20,6 +22,7 @@ func TestRepository_New(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) kmsCache := kms.TestKms(t, conn, wrapper) + testReader := strings.NewReader("notrandom") type args struct { r db.Reader @@ -47,21 +50,26 @@ func TestRepository_New(t *testing.T) { writer: rw, kms: kmsCache, defaultLimit: db.DefaultLimit, + randomReader: rand.Reader, }, }, { name: "valid with limit", args: args{ - r: rw, - w: rw, - kms: kmsCache, - opts: []Option{WithLimit(5)}, + r: rw, + w: rw, + kms: kmsCache, + opts: []Option{ + WithLimit(5), + WithRandomReader(testReader), + }, }, want: &Repository{ reader: rw, writer: rw, kms: kmsCache, defaultLimit: 5, + randomReader: testReader, }, }, { diff --git a/internal/auth/password/rewrapping_test.go b/internal/auth/password/rewrapping_test.go index 8068b3470..60224aad0 100644 --- a/internal/auth/password/rewrapping_test.go +++ b/internal/auth/password/rewrapping_test.go @@ -5,6 +5,7 @@ package password import ( "context" + "crypto/rand" "errors" "testing" "time" @@ -55,7 +56,7 @@ func TestRewrap_argon2ConfigRewrapFn(t *testing.T) { wrapper, _ = kmsCache.GetWrapper(context.Background(), org.GetPublicId(), 1) // actually store it - cred, err := newArgon2Credential(ctx, acct.PublicId, "this is a password", conf) + cred, err := newArgon2Credential(ctx, acct.PublicId, "this is a password", conf, rand.Reader) require.NoError(t, err) require.NoError(t, cred.encrypt(ctx, wrapper)) diff --git a/internal/cmd/base/initial_resources.go b/internal/cmd/base/initial_resources.go index 87dc30a10..8c2535c35 100644 --- a/internal/cmd/base/initial_resources.go +++ b/internal/cmd/base/initial_resources.go @@ -150,7 +150,7 @@ func (b *Server) CreateInitialPasswordAuthMethod(ctx context.Context) (*password } // Create the dev auth method - pwRepo, err := password.NewRepository(ctx, rw, rw, kmsCache) + pwRepo, err := password.NewRepository(ctx, rw, rw, kmsCache, password.WithRandomReader(b.SecureRandomReader)) if err != nil { return nil, nil, fmt.Errorf("error creating password repo: %w", err) } @@ -222,6 +222,7 @@ func (b *Server) CreateInitialPasswordAuthMethod(ctx context.Context) (*password acct, password.WithPassword(loginPassword), password.WithPublicId(accountId), + password.WithRandomReader(b.SecureRandomReader), ) if err != nil { return nil, fmt.Errorf("error saving auth account to the db: %w", err)