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. +``` diff --git a/http/logical_test.go b/http/logical_test.go index 05d6bf4eacd4d..7d79553b19f9d 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -14,6 +14,8 @@ 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" "github.com/hashicorp/vault/sdk/helper/logging" @@ -499,3 +501,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..f39ccbbdca4ce 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, errutil.UserError{Err: err.Error()} 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 }