Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vault-12308: Change password policy testing to be deterministic #20625

Merged
merged 6 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"

Check warning

Code scanning / Semgrep Scanner

Semgrep Finding: go.lang.security.audit.crypto.math_random.math-random-used

Do not use `math/rand`. Use `crypto/rand` instead.
"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