From 16ce923ddbd025ad4e98e433bd253d5868692aee Mon Sep 17 00:00:00 2001 From: akshya96 <87045294+akshya96@users.noreply.github.com> Date: Mon, 30 Jan 2023 13:06:10 -0800 Subject: [PATCH] Brute forcing unlock user bug (#18890) * brute forcing unlock user bug * add changelog * fix changelog --- builtin/credential/userpass/path_login.go | 2 +- changelog/18890.txt | 4 ++ .../identity/userlockouts_test.go | 37 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 changelog/18890.txt diff --git a/builtin/credential/userpass/path_login.go b/builtin/credential/userpass/path_login.go index 8e3f42ea4..f41463f60 100644 --- a/builtin/credential/userpass/path_login.go +++ b/builtin/credential/userpass/path_login.go @@ -39,7 +39,7 @@ func pathLogin(b *backend) *framework.Path { } func (b *backend) pathLoginAliasLookahead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - username := strings.ToLower(d.Get("username").(string)) + username := d.Get("username").(string) if username == "" { return nil, fmt.Errorf("missing username") } diff --git a/changelog/18890.txt b/changelog/18890.txt new file mode 100644 index 000000000..056e58599 --- /dev/null +++ b/changelog/18890.txt @@ -0,0 +1,4 @@ +```release-note:bug +core: removes strings.ToLower for alias name from pathLoginAliasLookahead function in userpass. This fixes +the storage entry for locked users by having the correct alias name in path. +`` \ No newline at end of file diff --git a/vault/external_tests/identity/userlockouts_test.go b/vault/external_tests/identity/userlockouts_test.go index 0d376466e..5653776d3 100644 --- a/vault/external_tests/identity/userlockouts_test.go +++ b/vault/external_tests/identity/userlockouts_test.go @@ -374,6 +374,13 @@ func TestIdentityStore_UnlockUserTest(t *testing.T) { t.Fatal(err) } + // create another user for userpass with a different case + if _, err = standby.Logical().Write("auth/userpass/users/bSmith", map[string]interface{}{ + "password": "training", + }); err != nil { + t.Fatal(err) + } + // login failure count 1 standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{ "password": "wrongPassword", @@ -404,6 +411,36 @@ func TestIdentityStore_UnlockUserTest(t *testing.T) { t.Fatal("expected login to succeed as user is unlocked") } + // login failure count 1 for user bSmith + standby.Logical().Write("auth/userpass/login/bSmith", map[string]interface{}{ + "password": "wrongPassword", + }) + // login failure count 2 for user bSmith + standby.Logical().Write("auth/userpass/login/bSmith", map[string]interface{}{ + "password": "wrongPassword", + }) + // login : permission denied as user locked out for user bSmith + if _, err = standby.Logical().Write("auth/userpass/login/bSmith", map[string]interface{}{ + "password": "training", + }); err == nil { + t.Fatal("expected login to fail as user locked out") + } + if !strings.Contains(err.Error(), logical.ErrPermissionDenied.Error()) { + t.Fatalf("expected to see permission denied error as user locked out, got %v", err) + } + + // unlock user bSmith + if _, err = standby.Logical().Write("sys/locked-users/"+mountAccessor+"/unlock/bSmith", nil); err != nil { + t.Fatal(err) + } + + // login: should be successful as user bSmith unlocked + if _, err = standby.Logical().Write("auth/userpass/login/bSmith", map[string]interface{}{ + "password": "training", + }); err != nil { + t.Fatal("expected login to succeed as user is unlocked") + } + // unlock unlocked user if _, err = active.Logical().Write("sys/locked-users/mountAccessor/unlock/bsmith", nil); err != nil { t.Fatal(err)