Skip to content

Commit

Permalink
Vault-12308: Change password policy testing to be deterministic (hash…
Browse files Browse the repository at this point in the history
…icorp#20625)

* change testing password policy to be deterministic

* fix panic

* test password against rules

* improve error message

* make test password gen more random

* fix check on test password length
  • Loading branch information
rculpepper committed May 17, 2023
1 parent 7c66970 commit c4e1753
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 17 deletions.
2 changes: 1 addition & 1 deletion helper/random/string_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func sortCharset(chars string) string {
return string(r)
}

// StringGenerator generats random strings from the provided charset & adhering to a set of rules. The set of rules
// StringGenerator generates random strings from the provided charset & adhering to a set of rules. The set of rules
// are things like CharsetRule which requires a certain number of characters from a sub-charset.
type StringGenerator struct {
// Length of the string to generate.
Expand Down
49 changes: 33 additions & 16 deletions vault/logical_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"errors"
"fmt"
"hash"
"math/rand"
"net/http"
"path"
"path/filepath"
Expand Down Expand Up @@ -3019,28 +3020,44 @@ func (*SystemBackend) handlePoliciesPasswordSet(ctx context.Context, req *logica
fmt.Sprintf("passwords must be between %d and %d characters", minPasswordLength, maxPasswordLength))
}

// Generate some passwords to ensure that we're confident that the policy isn't impossible
timeout := 1 * time.Second
genCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
// Attempt to construct a test password from the rules to ensure that the policy isn't impossible
var testPassword []rune

attempts := 10
failed := 0
for i := 0; i < attempts; i++ {
_, err = policy.Generate(genCtx, nil)
if err != nil {
failed++
for _, rule := range policy.Rules {
charsetRule, ok := rule.(random.CharsetRule)
if !ok {
return nil, logical.CodedError(http.StatusBadRequest, fmt.Sprintf("unexpected rule type %T", charsetRule))
}

for j := 0; j < charsetRule.MinLength(); j++ {
charIndex := rand.Intn(len(charsetRule.Chars()))
testPassword = append(testPassword, charsetRule.Chars()[charIndex])
}
}

if failed == attempts {
return nil, logical.CodedError(http.StatusBadRequest,
fmt.Sprintf("unable to generate password from provided policy in %s: are the rules impossible?", timeout))
for i := len(testPassword); i < policy.Length; i++ {
for _, rule := range policy.Rules {
if len(testPassword) >= policy.Length {
break
}
charsetRule, ok := rule.(random.CharsetRule)
if !ok {
return nil, logical.CodedError(http.StatusBadRequest, fmt.Sprintf("unexpected rule type %T", charsetRule))
}

charIndex := rand.Intn(len(charsetRule.Chars()))
testPassword = append(testPassword, charsetRule.Chars()[charIndex])
}
}

if failed > 0 {
return nil, logical.CodedError(http.StatusBadRequest,
fmt.Sprintf("failed to generate passwords %d times out of %d attempts in %s - is the policy too restrictive?", failed, attempts, timeout))
rand.Shuffle(policy.Length, func(i, j int) {
testPassword[i], testPassword[j] = testPassword[j], testPassword[i]
})

for _, rule := range policy.Rules {
if !rule.Pass(testPassword) {
return nil, logical.CodedError(http.StatusBadRequest, "unable to construct test password from provided policy: are the rules impossible?")
}
}

cfg := passwordPolicyConfig{
Expand Down

0 comments on commit c4e1753

Please sign in to comment.