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
This commit is contained in:
1 parent
d81c474aeb
commit
0fc70d82ee
10 files changed
+55
-17
No files matched your search
@@ -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
|
||||
|
||||
@@ -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")
|
||||
}()
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user