diff --git a/api/client.go b/api/client.go index 731ab977b4..55f0fcfac8 100644 --- a/api/client.go +++ b/api/client.go @@ -70,6 +70,9 @@ const ( // SSRF protection. RequestHeaderName = "X-Vault-Request" + SnapshotHeaderName = "X-Vault-Recover-Snapshot-Id" + RecoverSourcePathHeaderName = "X-Vault-Recover-Source-Path" + TLSErrorString = "This error usually means that the server is running with TLS disabled\n" + "but the client is configured to use TLS. Please either enable TLS\n" + "on the server or run the client with -address set to an address\n" + diff --git a/api/logical.go b/api/logical.go index b138de6737..b4bb47ce2f 100644 --- a/api/logical.go +++ b/api/logical.go @@ -309,8 +309,9 @@ func (c *Logical) Recover(ctx context.Context, path string, snapshotID string) ( func (c *Logical) RecoverFromPath(ctx context.Context, newPath string, snapshotID string, originalPath string) (*Secret, error) { r := c.c.NewRequest(http.MethodPut, "/v1/"+newPath) r.Params.Set("recover_snapshot_id", snapshotID) + r.Headers.Set(SnapshotHeaderName, snapshotID) if originalPath != "" && originalPath != newPath { - r.Params.Set("recover_source_path", url.QueryEscape(originalPath)) + r.Headers.Set(RecoverSourcePathHeaderName, originalPath) } return c.write(ctx, originalPath, r) } diff --git a/changelog/_8834.txt b/changelog/_8834.txt new file mode 100644 index 0000000000..b3a888e45e --- /dev/null +++ b/changelog/_8834.txt @@ -0,0 +1,3 @@ +```release-note:change +Secrets Recovery (enterprise): Deprecate the `recover_snapshot_id` query parameter to pass the snapshot ID for recover operations, in favor of a `X-Vault-Recover-Snapshot-Id` header. Vault will still accept the query parameter for backward compatibility. Also support setting the HTTP method to `RECOVER` for recover operations, in addition to `POST` and `PUT`. +``` diff --git a/http/handler.go b/http/handler.go index 86726d4e2d..b114ba2c16 100644 --- a/http/handler.go +++ b/http/handler.go @@ -84,10 +84,15 @@ const ( VaultSnapshotReadParam = "read_snapshot_id" // VaultSnapshotRecoverParam is the query parameter sent when Vault should // recover the data from a loaded snapshot + // Deprecated: use VaultSnapshotRecoverHeader VaultSnapshotRecoverParam = "recover_snapshot_id" - // VaultRecoverSourcePathParam contains an optional source path + // VaultRecoverSourcePathHeader contains an optional source path // to read the data from when performing a recover operation - VaultRecoverSourcePathParam = consts.RecoverSourcePathParam + VaultRecoverSourcePathHeader = consts.RecoverSourcePathHeader + // VaultSnapshotRecoverHeader holds the snapshot ID to use for a read, list, or + // recover from snapshot operation. This replaces the use of query parameters + // to pass the snapshot ID + VaultSnapshotRecoverHeader = "X-Vault-Recover-Snapshot-Id" // CustomMaxJSONDepth specifies the maximum nesting depth of a JSON object. // This limit is designed to prevent stack exhaustion attacks from deeply // nested JSON payloads, which could otherwise lead to a denial-of-service @@ -1505,7 +1510,7 @@ func requiresSnapshot(r *http.Request) bool { case http.MethodGet, "LIST": return query.Has(VaultSnapshotReadParam) case http.MethodPut, http.MethodPost: - return query.Has(VaultSnapshotRecoverParam) + return query.Has(VaultSnapshotRecoverParam) || r.Header.Get(VaultSnapshotRecoverParam) != "" } return false } diff --git a/http/logical.go b/http/logical.go index c7b52450be..c48358e5fe 100644 --- a/http/logical.go +++ b/http/logical.go @@ -12,7 +12,6 @@ import ( "mime" "net" "net/http" - "net/url" "strconv" "strings" "time" @@ -98,8 +97,12 @@ func buildLogicalRequestNoAuth(perfStandby bool, ra *vault.RouterAccess, w http. responseWriter = w } - case "POST", "PUT": - op = logical.UpdateOperation + case "POST", "PUT", "RECOVER": + if r.Method == "RECOVER" { + op = logical.RecoverOperation + } else { + op = logical.UpdateOperation + } // Buffer the request body in order to allow us to peek at the beginning // without consuming it. This approach involves no copying. @@ -219,25 +222,28 @@ func buildLogicalRequestNoAuth(perfStandby bool, ra *vault.RouterAccess, w http. data = nil } } - case logical.UpdateOperation: - queryVals := r.URL.Query() - if queryVals.Has(VaultSnapshotRecoverParam) { - snapshotID := queryVals.Get(VaultSnapshotRecoverParam) - if snapshotID != "" { - requiredSnapshotID = snapshotID - op = logical.RecoverOperation + case logical.UpdateOperation, logical.RecoverOperation: + snapshotHeaderID := r.Header.Get(VaultSnapshotRecoverHeader) + if snapshotHeaderID != "" { + requiredSnapshotID = snapshotHeaderID + } else { + queryVals := r.URL.Query() + if queryVals.Has(VaultSnapshotRecoverParam) { + snapshotID := queryVals.Get(VaultSnapshotRecoverParam) + if snapshotID != "" { + requiredSnapshotID = snapshotID + } } } - if op == logical.RecoverOperation { - if queryVals.Has(VaultRecoverSourcePathParam) { - sourcePath := queryVals.Get(VaultRecoverSourcePathParam) - if sourcePath != "" { - unescapedPath, err := url.QueryUnescape(sourcePath) - if err != nil { - return nil, nil, http.StatusBadRequest, fmt.Errorf("failed to unescape %s query parameter: %w", VaultRecoverSourcePathParam, err) - } - recoverSourcePath = trimPath(ns, unescapedPath) - } + if requiredSnapshotID == "" && op == logical.RecoverOperation { + return nil, nil, http.StatusBadRequest, fmt.Errorf("missing required snapshot ID") + } + + if requiredSnapshotID != "" { + op = logical.RecoverOperation + sourcePath := r.Header.Get(VaultRecoverSourcePathHeader) + if sourcePath != "" { + recoverSourcePath = trimPath(ns, sourcePath) } } } diff --git a/http/logical_test.go b/http/logical_test.go index c42123d4d0..98ced990c3 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -1071,9 +1071,11 @@ func TestLogical_SnapshotParams(t *testing.T) { method string url string body []byte + headers map[string]string wantData map[string]interface{} wantOperation logical.Operation wantRequiresSnapshotID string + wantError bool }{ { name: "normal get", @@ -1109,6 +1111,31 @@ func TestLogical_SnapshotParams(t *testing.T) { wantOperation: logical.ReadOperation, wantRequiresSnapshotID: "1234", }, + { + name: "snapshot list header", + method: http.MethodGet, + url: "https://example.com?list=true", + body: nil, + headers: map[string]string{ + VaultSnapshotRecoverHeader: "1234", + }, + wantData: nil, + wantOperation: logical.ListOperation, + wantRequiresSnapshotID: "", + }, + { + name: "snapshot read header", + method: http.MethodGet, + url: "https://example.com", + headers: map[string]string{ + VaultSnapshotRecoverHeader: "1234", + }, + body: nil, + wantData: nil, + wantOperation: logical.ReadOperation, + wantRequiresSnapshotID: "", + }, + { name: "normal update", method: http.MethodPost, @@ -1130,6 +1157,44 @@ func TestLogical_SnapshotParams(t *testing.T) { wantOperation: logical.RecoverOperation, wantRequiresSnapshotID: "1234", }, + { + name: "snapshot update header", + method: http.MethodPost, + url: "https://example.com", + body: []byte(`{"other_data":"abcd"}`), + headers: map[string]string{ + VaultSnapshotRecoverHeader: "1234", + }, + wantData: map[string]interface{}{ + "other_data": "abcd", + }, + wantOperation: logical.RecoverOperation, + wantRequiresSnapshotID: "1234", + }, + { + name: "recover operation no snapshot", + method: "RECOVER", + url: "https://example.com", + body: []byte(`{"other_data":"abcd"}`), + headers: map[string]string{ + "other_header": "value", + }, + wantError: true, + }, + { + name: "recover operation with snapshot", + method: "RECOVER", + url: "https://example.com", + body: []byte(`{"other_data":"abcd"}`), + headers: map[string]string{ + VaultSnapshotRecoverHeader: "1234", + }, + wantOperation: logical.RecoverOperation, + wantData: map[string]interface{}{ + "other_data": "abcd", + }, + wantRequiresSnapshotID: "1234", + }, } for _, tc := range testCases { @@ -1137,8 +1202,14 @@ func TestLogical_SnapshotParams(t *testing.T) { req, _ := http.NewRequest(tc.method, tc.url, bytes.NewReader(tc.body)) req = req.WithContext(namespace.RootContext(nil)) req.Header.Add(consts.AuthHeaderName, rootToken) - + for k, v := range tc.headers { + req.Header.Add(k, v) + } lreq, _, status, err := buildLogicalRequest(core, nil, req, "") + if tc.wantError { + require.Error(t, err) + return + } require.NoError(t, err) require.Equal(t, 0, status) require.Equal(t, tc.wantOperation, lreq.Operation) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 84bccca33e..3a232537b1 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -403,15 +403,21 @@ func documentPath(p *Path, backend *Backend, requestResponsePrefix string, doc * Name: "recover_snapshot_id", Description: "Triggers a recover operation using the given snapshot ID. Request body is ignored when a recover operation is requested.", In: "query", + Deprecated: true, + Schema: &OASSchema{Type: "string"}, + }, OASParameter{ + Name: "X-Vault-Recover-Snapshot-Id", + Description: "Triggers a recover operation using the given snapshot ID. Request body is ignored when a recover operation is requested.", + In: "header", Schema: &OASSchema{Type: "string"}, }) // If there are path fields, it will also be possible to recover from a path with // different path field values than the request path. if len(pathFields) > 0 { op.Parameters = append(op.Parameters, OASParameter{ - Name: "recover_source_path", - Description: "The source path to recover from. Only used if recover_snapshot_id parameter is also supplied. If not specified, the source path is assumed to be the same as the request path.", - In: "query", + Name: "X-Vault-Recover-Source-Path", + Description: "The source path to recover from. Only used if a snapshot ID is also supplied. If not specified, the source path is assumed to be the same as the request path.", + In: "header", Schema: &OASSchema{Type: "string"}, }) } diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index 60515c6548..885b339059 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -71,14 +71,23 @@ "name": "recover_snapshot_id", "description": "Triggers a recover operation using the given snapshot ID. Request body is ignored when a recover operation is requested.", "in": "query", + "schema": { + "type": "string" + }, + "deprecated": true + }, + { + "name": "X-Vault-Recover-Snapshot-Id", + "description": "Triggers a recover operation using the given snapshot ID. Request body is ignored when a recover operation is requested.", + "in": "header", "schema": { "type": "string" } }, { - "name": "recover_source_path", - "description": "The source path to recover from. Only used if recover_snapshot_id parameter is also supplied. If not specified, the source path is assumed to be the same as the request path.", - "in": "query", + "name": "X-Vault-Recover-Source-Path", + "description": "The source path to recover from. Only used if a snapshot ID is also supplied. If not specified, the source path is assumed to be the same as the request path.", + "in": "header", "schema": { "type": "string" } diff --git a/sdk/framework/testdata/operations_recover.json b/sdk/framework/testdata/operations_recover.json index cbb81ad093..38ac86ec21 100644 --- a/sdk/framework/testdata/operations_recover.json +++ b/sdk/framework/testdata/operations_recover.json @@ -43,11 +43,20 @@ "parameters": [ { "name": "recover_snapshot_id", + "deprecated": true, "description": "Triggers a recover operation using the given snapshot ID. Request body is ignored when a recover operation is requested.", "in": "query", "schema": { "type": "string" } + }, + { + "name": "X-Vault-Recover-Snapshot-Id", + "description": "Triggers a recover operation using the given snapshot ID. Request body is ignored when a recover operation is requested.", + "in": "header", + "schema": { + "type": "string" + } } ], "requestBody": { diff --git a/sdk/helper/consts/consts.go b/sdk/helper/consts/consts.go index bee1cd5958..c71cd117cb 100644 --- a/sdk/helper/consts/consts.go +++ b/sdk/helper/consts/consts.go @@ -48,5 +48,5 @@ const ( DRReplicationPathTarget = "dr" - RecoverSourcePathParam = "recover_source_path" + RecoverSourcePathHeader = "X-Vault-Recover-Source-Path" )