From 42d2d005d03bada92f50416f2a4f2edd0b0f9149 Mon Sep 17 00:00:00 2001 From: rculpepper Date: Tue, 21 Mar 2023 16:03:22 -0400 Subject: [PATCH 1/6] change testing password policy to be deterministic --- helper/random/string_generator.go | 2 +- vault/logical_system.go | 38 ++++++++++++++++--------------- 2 files changed, 21 insertions(+), 19 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..50ccc2100730 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -3019,28 +3019,30 @@ 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() - - attempts := 10 - failed := 0 - for i := 0; i < attempts; i++ { - _, err = policy.Generate(genCtx, nil) - if err != nil { - failed++ + // Attempt to construct a test password from the rules to ensure that the policy isn't impossible + var testPassword []rune + 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)) } - } - 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)) + char := charsetRule.Chars()[0] + for i := 0; i < charsetRule.MinLength(); i++ { + testPassword = append(testPassword, char) + } } - 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)) + if len(testPassword) < policy.Length { + rule, ok := policy.Rules[0].(random.CharsetRule) + if !ok { + return nil, logical.CodedError(http.StatusBadRequest, fmt.Sprintf("unexpected rule type %T", rule)) + } + + char := rule.Chars()[0] + for i := len(testPassword); i < policy.Length; i++ { + testPassword[i] = char + } } cfg := passwordPolicyConfig{ From ca56ef653dd7e93b742637dcd20ae91bee70705a Mon Sep 17 00:00:00 2001 From: rculpepper Date: Tue, 21 Mar 2023 16:34:56 -0400 Subject: [PATCH 2/6] fix panic --- vault/logical_system.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index 50ccc2100730..4a6d388be9ec 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -3041,7 +3041,7 @@ func (*SystemBackend) handlePoliciesPasswordSet(ctx context.Context, req *logica char := rule.Chars()[0] for i := len(testPassword); i < policy.Length; i++ { - testPassword[i] = char + testPassword = append(testPassword, char) } } From c3710bfda85ef99b8a92a0240ede6b5597f9ca0b Mon Sep 17 00:00:00 2001 From: rculpepper Date: Wed, 22 Mar 2023 13:35:20 -0400 Subject: [PATCH 3/6] test password against rules --- vault/logical_system.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index 4a6d388be9ec..802774eea1f6 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -3021,27 +3021,28 @@ func (*SystemBackend) handlePoliciesPasswordSet(ctx context.Context, req *logica // Attempt to construct a test password from the rules to ensure that the policy isn't impossible var testPassword []rune - for _, rule := range policy.Rules { + for i, rule := range policy.Rules { charsetRule, ok := rule.(random.CharsetRule) if !ok { return nil, logical.CodedError(http.StatusBadRequest, fmt.Sprintf("unexpected rule type %T", charsetRule)) } char := charsetRule.Chars()[0] - for i := 0; i < charsetRule.MinLength(); i++ { + for j := 0; j < charsetRule.MinLength(); j++ { testPassword = append(testPassword, char) } - } - if len(testPassword) < policy.Length { - rule, ok := policy.Rules[0].(random.CharsetRule) - if !ok { - return nil, logical.CodedError(http.StatusBadRequest, fmt.Sprintf("unexpected rule type %T", rule)) + // fill in any remaining characters with the last rule + if i == len(policy.Rules)-1 && len(testPassword) < policy.Length { + for j := len(testPassword); j < policy.Length; j++ { + testPassword = append(testPassword, char) + } } + } - char := rule.Chars()[0] - for i := len(testPassword); i < policy.Length; i++ { - testPassword = append(testPassword, char) + for _, rule := range policy.Rules { + if !rule.Pass(testPassword) { + return nil, logical.CodedError(http.StatusBadRequest, "failed to construct test password; password failed to pass all rules") } } From e026a2420624a808aea5530ae7c8526f9cc15e58 Mon Sep 17 00:00:00 2001 From: rculpepper Date: Wed, 22 Mar 2023 15:41:41 -0400 Subject: [PATCH 4/6] improve error message --- vault/logical_system.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index 802774eea1f6..fcd18789ac6f 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -3042,7 +3042,7 @@ func (*SystemBackend) handlePoliciesPasswordSet(ctx context.Context, req *logica for _, rule := range policy.Rules { if !rule.Pass(testPassword) { - return nil, logical.CodedError(http.StatusBadRequest, "failed to construct test password; password failed to pass all rules") + return nil, logical.CodedError(http.StatusBadRequest, "unable to construct test password from provided policy: are the rules impossible?") } } From 8d600c04a601a510ad823d4df1a1cc6f50cfb979 Mon Sep 17 00:00:00 2001 From: rculpepper Date: Mon, 27 Mar 2023 17:35:50 -0400 Subject: [PATCH 5/6] make test password gen more random --- vault/logical_system.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index fcd18789ac6f..197586b52b57 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" @@ -3021,25 +3022,38 @@ func (*SystemBackend) handlePoliciesPasswordSet(ctx context.Context, req *logica // Attempt to construct a test password from the rules to ensure that the policy isn't impossible var testPassword []rune - for i, rule := range policy.Rules { + + 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)) } - char := charsetRule.Chars()[0] for j := 0; j < charsetRule.MinLength(); j++ { - testPassword = append(testPassword, char) + charIndex := rand.Intn(len(charsetRule.Chars())) + testPassword = append(testPassword, charsetRule.Chars()[charIndex]) } + } - // fill in any remaining characters with the last rule - if i == len(policy.Rules)-1 && len(testPassword) < policy.Length { - for j := len(testPassword); j < policy.Length; j++ { - testPassword = append(testPassword, char) + for i := len(testPassword); i < policy.Length; i++ { + for _, rule := range policy.Rules { + if i >= 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]) } } + 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?") From 7533d27ee3b30b72b7f9ec5faeea92a63cc9e9b9 Mon Sep 17 00:00:00 2001 From: Rachel Culpepper Date: Tue, 16 May 2023 15:44:42 -0400 Subject: [PATCH 6/6] fix check on test password length --- vault/logical_system.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index 197586b52b57..e6e93c3e8d14 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -3037,7 +3037,7 @@ func (*SystemBackend) handlePoliciesPasswordSet(ctx context.Context, req *logica for i := len(testPassword); i < policy.Length; i++ { for _, rule := range policy.Rules { - if i >= policy.Length { + if len(testPassword) >= policy.Length { break } charsetRule, ok := rule.(random.CharsetRule)