From 017f25cfc6a4345befd91c55f575a39411171c43 Mon Sep 17 00:00:00 2001 From: temp Date: Wed, 30 Mar 2022 09:08:35 -0400 Subject: [PATCH 1/5] no-op commit due to failed cherry-picking From b06a9cc86c7514d3f2f9cb95123d0edc034d8e8c Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Tue, 1 Mar 2022 16:07:36 -0500 Subject: [PATCH 2/5] treat logical.ErrRelativePath as 400 instead of 500 (cherry picked from commit c1e527d4886939aaded77c1de010fc62b3d20eaf) --- http/logical_test.go | 73 ++++++++++++++++++++++++++++++++++++ sdk/logical/response_util.go | 2 + vault/request_handling.go | 5 +++ 3 files changed, 80 insertions(+) diff --git a/http/logical_test.go b/http/logical_test.go index 05d6bf4eacd4d..5e324211b99f5 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + credUserpass "github.com/hashicorp/vault/builtin/credential/userpass" "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/logging" @@ -499,3 +500,75 @@ func TestLogical_ShouldParseForm(t *testing.T) { } } } + +func TestLogical_ErrRelativePath(t *testing.T) { + coreConfig := &vault.CoreConfig{ + CredentialBackends: map[string]logical.Factory{ + "userpass": credUserpass.Factory, + }, + } + + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: Handler, + }) + + cluster.Start() + defer cluster.Cleanup() + + cores := cluster.Cores + + core := cores[0].Core + c := cluster.Cores[0].Client + vault.TestWaitActive(t, core) + + err := c.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{ + Type: "userpass", + }) + if err != nil { + t.Fatalf("failed to enable userpass, err: %v", err) + } + + resp, err := c.Logical().Read("auth/userpass/users/user..aaa") + + if err == nil || resp != nil { + t.Fatalf("expected read request to fail, resp: %#v, err: %v", resp, err) + } + + respErr, ok := err.(*api.ResponseError) + + if !ok { + t.Fatalf("unexpected error type, err: %#v", err) + } + + if respErr.StatusCode != 400 { + t.Errorf("expected 400 response for read, actual: %d", respErr.StatusCode) + } + + if !strings.Contains(respErr.Error(), logical.ErrRelativePath.Error()) { + t.Errorf("expected response for read to include %q", logical.ErrRelativePath.Error()) + } + + data := map[string]interface{}{ + "password": "abc123", + } + + resp, err = c.Logical().Write("auth/userpass/users/user..aaa", data) + + if err == nil || resp != nil { + t.Fatalf("expected write request to fail, resp: %#v, err: %v", resp, err) + } + + respErr, ok = err.(*api.ResponseError) + + if !ok { + t.Fatalf("unexpected error type, err: %#v", err) + } + + if respErr.StatusCode != 400 { + t.Errorf("expected 400 response for write, actual: %d", respErr.StatusCode) + } + + if !strings.Contains(respErr.Error(), logical.ErrRelativePath.Error()) { + t.Errorf("expected response for write to include %q", logical.ErrRelativePath.Error()) + } +} diff --git a/sdk/logical/response_util.go b/sdk/logical/response_util.go index a05f74684951e..83a5fac44808e 100644 --- a/sdk/logical/response_util.go +++ b/sdk/logical/response_util.go @@ -118,6 +118,8 @@ func RespondErrorCommon(req *Request, resp *Response, err error) (int, error) { statusCode = http.StatusTooManyRequests case errwrap.Contains(err, ErrMissingRequiredState.Error()): statusCode = http.StatusPreconditionFailed + case errwrap.Contains(err, ErrRelativePath.Error()): + statusCode = http.StatusBadRequest } } diff --git a/vault/request_handling.go b/vault/request_handling.go index ebccff6f3f9f8..373c866507dff 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -299,6 +299,8 @@ func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool case logical.ErrUnsupportedPath: // fail later via bad path to avoid confusing items in the log checkExists = false + case logical.ErrRelativePath: + return nil, te, err case nil: if existsResp != nil && existsResp.IsError() { return nil, te, existsResp.Error() @@ -752,6 +754,9 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp // Validate the token auth, te, ctErr := c.checkToken(ctx, req, false) + if ctErr == logical.ErrRelativePath { + return logical.ErrorResponse(ctErr.Error()), nil, ctErr + } if ctErr == logical.ErrPerfStandbyPleaseForward { return nil, nil, ctErr } From f91ab9e55e1502479d03a9dffea1d8f077073420 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Tue, 15 Mar 2022 10:07:48 -0400 Subject: [PATCH 3/5] add changelog entry (cherry picked from commit 550d3aa7f45d7483b5eb73ddcaf15e40d5ccce21) --- changelog/14328.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/14328.txt diff --git a/changelog/14328.txt b/changelog/14328.txt new file mode 100644 index 0000000000000..c5e45f0a30c83 --- /dev/null +++ b/changelog/14328.txt @@ -0,0 +1,3 @@ +```release-note:change +core: A request that fails path validation due to relative path check will now be responded to with a 400 rather than 500. +``` From 9efaa43dfd1e2d4b2842cc1293c36331748ebc3b Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 23 Mar 2022 17:12:49 -0400 Subject: [PATCH 4/5] return UserError for logical.ErrRelativePath (cherry picked from commit 154a3a7f8f28c1fb910947314f3d64387041423e) --- vault/request_handling.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/request_handling.go b/vault/request_handling.go index 373c866507dff..f39ccbbdca4ce 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -300,7 +300,7 @@ func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool // fail later via bad path to avoid confusing items in the log checkExists = false case logical.ErrRelativePath: - return nil, te, err + return nil, te, errutil.UserError{Err: err.Error()} case nil: if existsResp != nil && existsResp.IsError() { return nil, te, existsResp.Error() From 1a882aed137116315c9b13df331c35aa5233ca25 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 30 Mar 2022 14:09:26 -0400 Subject: [PATCH 5/5] add missing import --- http/logical_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/http/logical_test.go b/http/logical_test.go index 5e324211b99f5..7d79553b19f9d 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + "github.com/hashicorp/vault/api" credUserpass "github.com/hashicorp/vault/builtin/credential/userpass" "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/sdk/helper/consts"