treat logical.ErrRelativePath as 400 instead of 500 (#14328)

* treat logical.ErrRelativePath as 400 instead of 500

* add changelog entry

* return UserError for logical.ErrRelativePath

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.
+```

http/logical_test.go

@@ -17,6 +17,7 @@ import (
 	kv "github.com/hashicorp/vault-plugin-secrets-kv"
 	"github.com/hashicorp/vault/api"
 	auditFile "github.com/hashicorp/vault/builtin/audit/file"
+	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"
@@ -591,3 +592,75 @@ func TestLogical_AuditPort(t *testing.T) {
 		t.Fatalf("wrong number of audit entries: %d", count)
 	}
 }
+
+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())
+	}
+}

sdk/logical/response_util.go

@@ -120,6 +120,8 @@ func RespondErrorCommon(req *Request, resp *Response, err error) (int, error) {
 			statusCode = http.StatusPreconditionFailed
 		case errwrap.Contains(err, ErrPathFunctionalityRemoved.Error()):
 			statusCode = http.StatusNotFound
+		case errwrap.Contains(err, ErrRelativePath.Error()):
+			statusCode = http.StatusBadRequest
 		}
 	}
 

vault/request_handling.go

@@ -320,6 +320,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()
@@ -825,6 +827,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
 	}