Restrict concurrent password hashes (#5437)

* internal/auth/password: add resizable pool

The resizable pool wraps the permit pool with a mutex
to allow it to be resized at runtime.

* internal/auth/password: add concurrency limit

The default concurrency limit is 1, and it can be
set using the SetHashingPermits function.

* internal/cmd/config: allow configuring hashing limits

The new concurrent_password_hash_workers configuration
value can be used to loosen the constraints on the
userpass auth method hashing operation.

* internal/cmd: set password hashing constraints

Sets the password hashing concurrency constraints on
startup and config reload.

* CHANGELOG: add notice about password concurrency limit
This commit is contained in:
Johan Brandhorst-Satzkorn authored and GitHub committed 2025-01-22 13:31:42 -08:00
1 parent aad833fe09
commit 40cb9f5c9e
11 files changed
+457 -10

No files matched your search

+10 -3
View File
@@ -13,6 +13,8 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
* Fix bug in parsing IPv6 addresses. Previously setting a target address or the
initial upstream address in the config file would result in a malformed value.
([PR](https://github.com/hashicorp/boundary/pull/5221)).
* Fix an issue where, when starting a session, the connection limit always displays 0.
([PR](https://github.com/hashicorp/boundary/pull/5396)).
### New and Improved
@@ -33,11 +35,16 @@ maintainability of worker queries, and improve DB performance. ([PR](https://git
and reliability at large scale. Workers older than v0.19.0 will remain supported
until the release of v0.20.0, in accordance with
[our worker/controller compatiblity policy](https://developer.hashicorp.com/boundary/docs/enterprise/supported-versions#control-plane-and-worker-compatibility).
* Add concurrency limit on the password hashing of all password auth methods.
([PR](https://github.com/hashicorp/boundary-plugin-aws/pull/5437)).
### Bug fixes
This avoids bursty memory and CPU use during concurrent password auth method
authentication attempts. The number of concurrent hashing operations
can be set with the new `concurrent_password_hash_workers` configuration
value in the controller stanza, or the new
`BOUNDARY_CONTROLLER_CONCURRENT_PASSWORD_HASH_WORKERS` environment variable.
The default limit is 1.
* Fix an issue where, when starting a session, the connection limit always displays 0.
([PR](https://github.com/hashicorp/boundary/pull/5396)).
## 0.18.2 (2024/12/12)
### Bug fixes
+2 -1
View File
@@ -63,7 +63,7 @@ require (
github.com/posener/complete v1.2.3
github.com/prometheus/client_golang v1.18.0
github.com/ryanuber/go-glob v1.0.0
github.com/stretchr/testify v1.9.0
github.com/stretchr/testify v1.10.0
github.com/zalando/go-keyring v0.2.3
go.uber.org/atomic v1.11.0
golang.org/x/crypto v0.31.0
@@ -93,6 +93,7 @@ require (
github.com/hashicorp/dbassert v0.0.0-20231012105025-1bc1bd88e22b
github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20241126174344-f3b1a41a15fd
github.com/hashicorp/go-rate v0.0.0-20231204194614-cc8d401f70ab
github.com/hashicorp/go-secure-stdlib/permitpool v1.0.0
github.com/hashicorp/go-version v1.6.0
github.com/hashicorp/nodeenrollment v0.2.13
github.com/jackc/pgx/v5 v5.6.0
+4 -2
View File
@@ -239,6 +239,8 @@ github.com/hashicorp/go-secure-stdlib/parseutil v0.1.8 h1:iBt4Ew4XEGLfh6/bPk4rSY
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.8/go.mod h1:aiJI+PIApBRQG7FZTEBx5GiiX+HbOHilUdNxUZi4eV0=
github.com/hashicorp/go-secure-stdlib/password v0.1.3 h1:/2S3qhBDGbI0DoSgSC8m9EaiRelgGrJmApZIDb/8Xv8=
github.com/hashicorp/go-secure-stdlib/password v0.1.3/go.mod h1:JPOgAG+z70auO30+LCRhvZKxGAh8cfXorXNJWGlFiVQ=
github.com/hashicorp/go-secure-stdlib/permitpool v1.0.0 h1:U6y5MXGiDVOOtkWJ6o/tu1TxABnI0yKTQWJr7z6BpNk=
github.com/hashicorp/go-secure-stdlib/permitpool v1.0.0/go.mod h1:ecDb3o+8D4xtP0nTCufJaAVawHavy5M2eZ64Nq/8/LM=
github.com/hashicorp/go-secure-stdlib/pluginutil/v2 v2.0.6 h1:ZYv2XA+tEfFXIToR2jmBgVqQU9gERt0APbWqmUoNGnY=
github.com/hashicorp/go-secure-stdlib/pluginutil/v2 v2.0.6/go.mod h1:ggFN8dlaLWS2R1gymBbCrvXM/bkZP7hEAa4seqDwhyg=
github.com/hashicorp/go-secure-stdlib/reloadutil v0.1.1 h1:SMGUnbpAcat8rIKHkBPjfv81yC46a8eCNZ2hsR2l1EI=
@@ -484,8 +486,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
github.com/tj/assert v0.0.0-20171129193455-018094318fb0/go.mod h1:mZ9/Rh9oLWpLLDRpvE+3b7gP/C2YyLFYxNmcLnPTMe0=
github.com/tj/assert v0.0.3 h1:Df/BlaZ20mq6kuai7f5z2TvPFiwC3xaWJSDQNiIS3Rk=
github.com/tj/assert v0.0.3/go.mod h1:Ne6X72Q+TB1AteidzQncjw9PabbMp4PBMZ1k+vd1Pvk=
+29 -1
View File
@@ -17,6 +17,26 @@ import (
"google.golang.org/protobuf/proto"
)
// hashingPermitPool is the global permit pool used to restrict concurrent
// password hashing. It can be resized with SetHashingPermits.
var hashingPermitPool *resizablePermitPool
func init() {
hashingPermitPool = newResizablePermitPool(1)
}
// SetHashingPermits sets the number of concurrent password hashing operations permitted.
func SetHashingPermits(n int) error {
const op = "password.SetHashingPermits"
if n <= 0 {
return errors.New(context.Background(), errors.InvalidParameter, op, "n must be greater than 0")
}
if err := hashingPermitPool.SetPermits(n); err != nil {
return err
}
return nil
}
// Argon2Configuration is a configuration for using the argon2id key
// derivation function. It is owned by an AuthMethod.
//
@@ -163,7 +183,15 @@ func newArgon2Credential(ctx context.Context, accountId string, password string,
return nil, errors.Wrap(ctx, err, op, errors.WithCode(errors.Io))
}
c.Salt = salt
c.DerivedKey = argon2.IDKey([]byte(password), c.Salt, conf.Iterations, conf.Memory, uint8(conf.Threads), conf.KeyLength)
// Limit the number of concurrent calls to the argon2 hashing function,
// since each call consumes a significant amount of CPU and memory.
if err := hashingPermitPool.Do(ctx, func() {
c.DerivedKey = argon2.IDKey([]byte(password), c.Salt, conf.Iterations, conf.Memory, uint8(conf.Threads), conf.KeyLength)
}); err != nil {
// Context was canceled while waiting for a permit
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("context canceled while waiting for hashing permit"))
}
return c, nil
}
+96
View File
@@ -5,7 +5,12 @@ package password
import (
"context"
"runtime"
"slices"
"strconv"
"sync"
"testing"
"time"
"github.com/hashicorp/boundary/internal/auth/password/store"
"github.com/hashicorp/boundary/internal/db"
@@ -456,3 +461,94 @@ func TestArgon2Credential_New(t *testing.T) {
})
}
}
func TestArgon2Credential_ConcurrencyLimit(t *testing.T) {
// Do NOT run this concurrently with other tests in this package or elsewhere,
// since it relies on the global concurrency limit.
conn, _ := db.TestSetup(t, "postgres")
rootWrapper := db.TestWrapper(t)
o, _ := iam.TestScopes(t, iam.TestRepo(t, conn, rootWrapper))
auts := TestAuthMethods(t, conn, o.GetPublicId(), 1)
aut := auts[0]
accts := TestMultipleAccounts(t, conn, aut.PublicId, 5)
conf := testArgon2Confs(t, conn, accts[0].AuthMethodId, 1)[0]
testTimeout := time.Minute
testDeadline, ok := t.Deadline()
if ok && time.Until(testDeadline) < testTimeout {
// If the test deadline is less than the default timeout, use the deadline
testTimeout = time.Until(testDeadline)
}
deadlineCtx, deadlineCtxCancel := context.WithTimeout(context.Background(), testTimeout)
defer deadlineCtxCancel()
// Measure memory usage when using the default limit of 1
mean1, _, _ := measureCredentialCreations(deadlineCtx, t, accts[0].PublicId, conf)
// Now set the limit to 5 and measure again
require.NoError(t, SetHashingPermits(5))
t.Cleanup(func() {
require.NoError(t, SetHashingPermits(1))
})
mean5, _, _ := measureCredentialCreations(deadlineCtx, t, accts[0].PublicId, conf)
// Assert that we used less memory while the limit was in place
assert.Less(t, mean1, mean5)
}
func measureCredentialCreations(ctx context.Context, t testing.TB, publicId string, conf *Argon2Configuration) (mean float64, max, min uint64) {
wg := &sync.WaitGroup{}
start := make(chan struct{})
// Start 5 goroutines all trying to create a new credential concurrently
for i := range 5 {
wg.Add(1)
go func() {
defer wg.Done()
<-start
t.Log("Credential " + strconv.Itoa(i) + " starting")
_, err := newArgon2Credential(ctx, publicId, "password", conf)
assert.NoError(t, err)
t.Log("Credential " + strconv.Itoa(i) + " finished")
}()
}
doneDone := make(chan struct{})
go func() {
wg.Wait()
close(doneDone)
}()
// Start the goroutines
close(start)
var memStats runtime.MemStats
var measures []uint64
for {
select {
case <-ctx.Done():
t.Error("timeout")
return
case <-time.After(10 * time.Millisecond):
// Run GC and check memory use
runtime.GC()
runtime.ReadMemStats(&memStats)
measures = append(measures, memStats.HeapAlloc)
case <-doneDone:
// All credential creations done
return meanInt(measures), slices.Max(measures), slices.Min(measures)
}
}
}
func meanInt(in []uint64) float64 {
return sumInt(in) / float64(len(in))
}
func sumInt(in []uint64) float64 {
var sum float64
for _, n := range in {
sum += float64(n)
}
return sum
}
+59
View File
@@ -0,0 +1,59 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package password
import (
"context"
"sync"
"github.com/hashicorp/boundary/internal/errors"
"github.com/hashicorp/go-secure-stdlib/permitpool"
)
// resizablePermitPool is a permit pool that can be resized at runtime.
type resizablePermitPool struct {
pool *permitpool.Pool
// lock is used to synchronize access to the permit pool
// This is an RWMutex to allow an unlimited number of readers
// and a single writer, since allowing a single reader or writer
// would effectively make the pool useless.
lock *sync.RWMutex
}
// newResizablePermitPool creates a new resizable permit pool with n permits.
func newResizablePermitPool(n int) *resizablePermitPool {
return &resizablePermitPool{
pool: permitpool.New(n),
lock: &sync.RWMutex{},
}
}
// SetPermit sets the number of permits available in the pool.
func (r *resizablePermitPool) SetPermits(n int) error {
const op = "resizablePermitPool.SetPermits"
if n <= 0 {
return errors.New(context.Background(), errors.InvalidParameter, op, "n must be greater than 0")
}
// Taking a write lock ensures there are no currently acquired permits
r.lock.Lock()
defer r.lock.Unlock()
r.pool = permitpool.New(n)
return nil
}
// Do executes the provided function with a permit acquired from the pool.
// If the context is canceled while waiting to acquire a permit, an error is returned.
func (r *resizablePermitPool) Do(ctx context.Context, fn func()) error {
const op = "resizablePermitPool.Do"
// We need to ensure both the Acquire and Release happen while the read lock is held,
// so that the pool cannot be resized in between.
r.lock.RLock()
defer r.lock.RUnlock()
if err := r.pool.Acquire(ctx); err != nil {
return errors.Wrap(ctx, err, op, errors.WithMsg("failed to acquire permit"))
}
defer r.pool.Release()
fn()
return nil
}
+53
View File
@@ -0,0 +1,53 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package password
import (
"context"
"strconv"
"sync"
"testing"
"time"
"github.com/stretchr/testify/assert"
)
func TestPermitPool(t *testing.T) {
pool := newResizablePermitPool(1)
wg := &sync.WaitGroup{}
start := make(chan struct{})
ctx := context.Background()
// Start 5 goroutines all trying to acquire the permit at the same time
for i := range 5 {
wg.Add(1)
go func() {
defer wg.Done()
<-start
t.Log("Goroutine " + strconv.Itoa(i) + " starting")
err := pool.Do(ctx, func() {
// Do some expensive operation
time.Sleep(10 * time.Millisecond)
})
assert.NoError(t, err)
t.Log("Goroutine " + strconv.Itoa(i) + " finished")
}()
}
// Also start a few goroutines that attempt to resize the pool
for i := range 5 {
wg.Add(1)
go func() {
defer wg.Done()
<-start
t.Log("Resizing pool " + strconv.Itoa(i))
err := pool.SetPermits(2)
t.Log("Resized pool" + strconv.Itoa(i))
assert.NoError(t, err)
}()
}
close(start)
wg.Wait()
}
+5
View File
@@ -16,6 +16,7 @@ import (
"time"
"github.com/hashicorp/boundary/globals"
"github.com/hashicorp/boundary/internal/auth/password"
"github.com/hashicorp/boundary/internal/cmd/base"
"github.com/hashicorp/boundary/internal/cmd/config"
"github.com/hashicorp/boundary/internal/cmd/ops"
@@ -870,6 +871,10 @@ func (c *Command) Reload(newConf *config.Config) error {
}
}
if newConf != nil && newConf.Controller != nil && newConf.Controller.ConcurrentPasswordHashWorkers > 0 {
reloadErrors = stderrors.Join(reloadErrors, password.SetHashingPermits(int(newConf.Controller.ConcurrentPasswordHashWorkers)))
}
// Send a message that we reloaded. This prevents "guessing" sleep times
// in tests.
if c.reloadedCh != nil {
+39
View File
@@ -317,6 +317,14 @@ type Controller struct {
// it is rejected by the controller.
MaxPageSizeRaw any `hcl:"max_page_size"`
MaxPageSize uint `hcl:"-"`
// ConcurrentPasswordHashWorkers controls the number of concurrent password
// hashing workers is allowed. The default value is 1. Increasing this number
// will increase the authentication throughput of all userpass auth methods,
// but at the cost of bursty memory and CPU use. Can also be controlled via
// the environment variable BOUNDARY_CONTROLLER_CONCURRENT_PASSWORD_HASH_WORKERS.
ConcurrentPasswordHashWorkersRaw any `hcl:"concurrent_password_hash_workers"`
ConcurrentPasswordHashWorkers uint `hcl:"-"`
}
func (c *Controller) InitNameIfEmpty(ctx context.Context) error {
@@ -871,6 +879,37 @@ func Parse(d string) (*Config, error) {
if result.Controller.ApiRateLimiterMaxQuotas <= 0 {
result.Controller.ApiRateLimiterMaxQuotas = ratelimit.DefaultLimiterMaxQuotas()
}
switch t := result.Controller.ConcurrentPasswordHashWorkersRaw.(type) {
case string:
concurrentPasswordWorkersString, err := parseutil.ParsePath(t)
if err != nil && !errors.Is(err, parseutil.ErrNotAUrl) {
return nil, fmt.Errorf("Error parsing concurrent password hash workers: %w", err)
}
concurrentWorkers, err := strconv.Atoi(concurrentPasswordWorkersString)
if err != nil {
return nil, fmt.Errorf("Concurrent password hash workers value is not an int: %w", err)
}
if concurrentWorkers <= 0 {
return nil, fmt.Errorf("Concurrent password hash workers value must be at least 1, was %d", concurrentWorkers)
}
result.Controller.ConcurrentPasswordHashWorkers = uint(concurrentWorkers)
case int:
if t <= 0 {
return nil, fmt.Errorf("Concurrent password hash workers value must be at least 1, was %d", t)
}
result.Controller.ConcurrentPasswordHashWorkers = uint(t)
case nil:
if envVal := os.Getenv("BOUNDARY_CONTROLLER_CONCURRENT_PASSWORD_HASH_WORKERS"); envVal != "" {
concurrentPasswordWorkers, err := strconv.Atoi(envVal)
if err != nil {
return nil, fmt.Errorf("BOUNDARY_CONTROLLER_CONCURRENT_PASSWORD_HASH_WORKERS value is not an int: %w", err)
}
result.Controller.ConcurrentPasswordHashWorkers = uint(concurrentPasswordWorkers)
}
default:
return nil, fmt.Errorf("Concurrent password hash workers: unsupported type %q", reflect.TypeOf(t).String())
}
}
// Parse worker tags
+154 -3
View File
@@ -3348,7 +3348,7 @@ func TestMaxPageSize(t *testing.T) {
in: `
controller {
name = "example-controller"
max_page_size = "env://ENV_MAX_CONN"
max_page_size = "env://ENV_MAX_PAGE_SIZE"
}`,
expMaxPageSize: 8,
envMaxPageSize: "8",
@@ -3359,7 +3359,7 @@ func TestMaxPageSize(t *testing.T) {
in: `
controller {
name = "example-controller"
max_page_size = "env://ENV_MAX_CONN"
max_page_size = "env://ENV_MAX_PAGE_SIZE"
}`,
envMaxPageSize: "bogus value",
expErr: true,
@@ -3369,7 +3369,7 @@ func TestMaxPageSize(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Setenv("ENV_MAX_CONN", tt.envMaxPageSize)
t.Setenv("ENV_MAX_PAGE_SIZE", tt.envMaxPageSize)
c, err := Parse(tt.in)
if tt.expErr {
require.EqualError(t, err, tt.expErrStr)
@@ -3384,3 +3384,154 @@ func TestMaxPageSize(t *testing.T) {
})
}
}
func TestConcurrentPasswordHashWorkers(t *testing.T) {
tests := []struct {
name string
in string
envConcurrentPasswordHashWorkers string
expConcurrentPasswordHashWorkers uint
expErr bool
expErrStr string
}{
{
name: "Valid integer value",
in: `
controller {
name = "example-controller"
concurrent_password_hash_workers = 5
}`,
expConcurrentPasswordHashWorkers: 5,
expErr: false,
},
{
name: "Valid string value",
in: `
controller {
name = "example-controller"
concurrent_password_hash_workers = "5"
}`,
expConcurrentPasswordHashWorkers: 5,
expErr: false,
},
{
name: "Invalid value integer",
in: `
controller {
name = "example-controller"
concurrent_password_hash_workers = 0
}`,
expErr: true,
expErrStr: "Concurrent password hash workers value must be at least 1, was 0",
},
{
name: "Invalid value string",
in: `
controller {
name = "example-controller"
concurrent_password_hash_workers = "string bad"
}`,
expErr: true,
expErrStr: "Concurrent password hash workers value is not an int: " +
"strconv.Atoi: parsing \"string bad\": invalid syntax",
},
{
name: "Invalid value string integer",
in: `
controller {
name = "example-controller"
concurrent_password_hash_workers = "-1"
}`,
expErr: true,
expErrStr: "Concurrent password hash workers value must be at least 1, was -1",
},
{
name: "Invalid value type",
in: `
controller {
name = "example-controller"
concurrent_password_hash_workers = false
}`,
expErr: true,
expErrStr: "Concurrent password hash workers: unsupported type \"bool\"",
},
{
name: "Valid env var",
in: `
controller {
name = "example-controller"
concurrent_password_hash_workers = "env://ENV_MAX_PW_WORKERS"
}`,
expConcurrentPasswordHashWorkers: 8,
envConcurrentPasswordHashWorkers: "8",
expErr: false,
},
{
name: "Invalid env var",
in: `
controller {
name = "example-controller"
concurrent_password_hash_workers = "env://ENV_MAX_PW_WORKERS"
}`,
envConcurrentPasswordHashWorkers: "bogus value",
expErr: true,
expErrStr: "Concurrent password hash workers value is not an int: " +
"strconv.Atoi: parsing \"bogus value\": invalid syntax",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Setenv("ENV_MAX_PW_WORKERS", tt.envConcurrentPasswordHashWorkers)
c, err := Parse(tt.in)
if tt.expErr {
require.EqualError(t, err, tt.expErrStr)
require.Nil(t, c)
return
}
require.NoError(t, err)
require.NotNil(t, c)
require.NotNil(t, c.Controller)
require.Equal(t, tt.expConcurrentPasswordHashWorkers, c.Controller.ConcurrentPasswordHashWorkers)
})
}
t.Run("using environment variable", func(t *testing.T) {
t.Setenv("BOUNDARY_CONTROLLER_CONCURRENT_PASSWORD_HASH_WORKERS", "2")
in := `
controller {
name = "example-controller"
}`
c, err := Parse(in)
require.NoError(t, err)
require.NotNil(t, c)
require.NotNil(t, c.Controller)
require.EqualValues(t, 2, c.Controller.ConcurrentPasswordHashWorkers)
})
t.Run("using environment variable with invalid value", func(t *testing.T) {
t.Setenv("BOUNDARY_CONTROLLER_CONCURRENT_PASSWORD_HASH_WORKERS", "invalid")
in := `
controller {
name = "example-controller"
}`
c, err := Parse(in)
require.Error(t, err)
require.Nil(t, c)
require.Contains(t, err.Error(), "BOUNDARY_CONTROLLER_CONCURRENT_PASSWORD_HASH_WORKERS value is not an int")
})
t.Run("using environment variable and config value uses config value", func(t *testing.T) {
t.Setenv("BOUNDARY_CONTROLLER_CONCURRENT_PASSWORD_HASH_WORKERS", "2")
in := `
controller {
name = "example-controller"
concurrent_password_hash_workers = 3
}`
c, err := Parse(in)
require.NoError(t, err)
require.NotNil(t, c)
require.NotNil(t, c.Controller)
require.EqualValues(t, 3, c.Controller.ConcurrentPasswordHashWorkers)
})
}
+6
View File
@@ -495,6 +495,12 @@ func New(ctx context.Context, conf *Config) (*Controller, error) {
event.WriteSysEvent(ctx, op, "unable to ensure worker auth roots exist, may be due to multiple controllers starting at once, continuing")
}
if c.conf.RawConfig.Controller.ConcurrentPasswordHashWorkers > 0 {
if err := password.SetHashingPermits(int(c.conf.RawConfig.Controller.ConcurrentPasswordHashWorkers)); err != nil {
return nil, fmt.Errorf("unable to set number of concurrent password workers: %w", err)
}
}
if graphFactory != nil {
boundVer := version.Get().VersionNumber()
c.downstreamWorkers, err = graphFactory(ctx, "root", boundVer)