From c4e17539196108016153c4aaf73c609edbd04aa0 Mon Sep 17 00:00:00 2001 From: Rachel Culpepper <84159930+rculpepper@users.noreply.github.com> Date: Wed, 17 May 2023 14:22:19 -0400 Subject: [PATCH] Vault-12308: Change password policy testing to be deterministic (#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 --- helper/random/string_generator.go | 2 +- vault/logical_system.go | 49 +++++++++++++++++++++---------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/helper/random/string_generator.go b/helper/random/string_generator.go index e7c087aa3c26..48f08ed8924a 100644 --- a/helper/random/string_generator.go +++ b/helper/random/string_generator.go @@ -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. diff --git a/vault/logical_system.go b/vault/logical_system.go index 88e5759bc126..e6e93c3e8d14 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -13,6 +13,7 @@ import ( "errors" "fmt" "hash" + "math/rand" "net/http" "path" "path/filepath" @@ -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{