From f193c833c5974760166b887a4c185973b58bfc43 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Wed, 24 Apr 2024 11:49:30 -0700 Subject: [PATCH] backport of commit b896dc1610600775635c34a9e18b17d57f1d45aa (#26623) Co-authored-by: Violet Hynes --- changelog/26607.txt | 3 ++ http/options_test.go | 114 +++++++++++++++++++++++++++++++++++++++ sdk/framework/backend.go | 8 ++- sdk/framework/path.go | 4 ++ vault/logical_system.go | 8 ++- 5 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 changelog/26607.txt diff --git a/changelog/26607.txt b/changelog/26607.txt new file mode 100644 index 000000000000..b28c3d405977 --- /dev/null +++ b/changelog/26607.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: Fix `redact_version` listener parameter being ignored for some OpenAPI related endpoints. +``` diff --git a/http/options_test.go b/http/options_test.go index cee30c85c19e..5d52a6e42dc8 100644 --- a/http/options_test.go +++ b/http/options_test.go @@ -4,8 +4,16 @@ package http import ( + "net/http" + "strings" "testing" + "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" + "github.com/hashicorp/vault/internalshared/configutil" + "github.com/hashicorp/vault/sdk/helper/consts" + "github.com/hashicorp/vault/vault" + "github.com/hashicorp/vault/version" "github.com/stretchr/testify/require" ) @@ -157,3 +165,109 @@ func TestOptions_WithRedactVersion(t *testing.T) { }) } } + +// TestRedactVersionListener tests that the version will be redacted +// from e.g. sys/health and the OpenAPI response if `redact_version` +// is set on the listener. +func TestRedactVersionListener(t *testing.T) { + conf := &vault.CoreConfig{ + EnableUI: false, + EnableRaw: true, + BuiltinRegistry: corehelpers.NewMockBuiltinRegistry(), + } + core, _, token := vault.TestCoreUnsealedWithConfig(t, conf) + + // Setup listener without redaction + ln, addr := TestListener(t) + props := &vault.HandlerProperties{ + Core: core, + ListenerConfig: &configutil.Listener{ + RedactVersion: false, + }, + } + TestServerWithListenerAndProperties(t, ln, addr, core, props) + defer ln.Close() + TestServerAuth(t, addr, token) + + testRedactVersionEndpoints(t, addr, token, version.Version) + + // Setup listener with redaction + ln, addr = TestListener(t) + props.ListenerConfig.RedactVersion = true + TestServerWithListenerAndProperties(t, ln, addr, core, props) + defer ln.Close() + TestServerAuth(t, addr, token) + + testRedactVersionEndpoints(t, addr, token, "") +} + +// testRedactVersionEndpoints tests the endpoints containing versions +// contain the expected version +func testRedactVersionEndpoints(t *testing.T, addr, token, expectedVersion string) { + client := cleanhttp.DefaultClient() + req, err := http.NewRequest("GET", addr+"/v1/auth/token?help=1", nil) + require.NoError(t, err) + + req.Header.Set(consts.AuthHeaderName, token) + resp, err := client.Do(req) + require.NoError(t, err) + + testResponseStatus(t, resp, 200) + + var actual map[string]interface{} + testResponseBody(t, resp, &actual) + + require.NotNil(t, actual["openapi"]) + openAPI, ok := actual["openapi"].(map[string]interface{}) + require.True(t, ok) + + require.NotNil(t, openAPI["info"]) + info, ok := openAPI["info"].(map[string]interface{}) + require.True(t, ok) + + require.NotNil(t, info["version"]) + version, ok := info["version"].(string) + require.True(t, ok) + require.Equal(t, expectedVersion, version) + + req, err = http.NewRequest("GET", addr+"/v1/sys/internal/specs/openapi", nil) + require.NoError(t, err) + + req.Header.Set(consts.AuthHeaderName, "") + resp, err = client.Do(req) + require.NoError(t, err) + + testResponseStatus(t, resp, 200) + testResponseBody(t, resp, &actual) + + require.NotNil(t, actual["info"]) + info, ok = openAPI["info"].(map[string]interface{}) + require.True(t, ok) + + require.NotNil(t, info["version"]) + version, ok = info["version"].(string) + require.True(t, ok) + require.Equal(t, expectedVersion, version) + + req, err = http.NewRequest("GET", addr+"/v1/sys/health", nil) + require.NoError(t, err) + + req.Header.Set(consts.AuthHeaderName, "") + resp, err = client.Do(req) + require.NoError(t, err) + + testResponseStatus(t, resp, 200) + testResponseBody(t, resp, &actual) + + require.NotNil(t, actual["version"]) + version, ok = actual["version"].(string) + require.True(t, ok) + + // sys/health is special and uses a different format to the OpenAPI + // version.GetVersion().VersionNumber() instead of version.Version + // We use substring to make sure the check works anyway. + // In practice, version.GetVersion().VersionNumber() will give something like 1.17.0-beta1 + // and version.Version gives something like 1.17.0 + require.Truef(t, strings.HasPrefix(version, expectedVersion), "version was not as expected, version=%s, expectedVersion=%s", + version, expectedVersion) +} diff --git a/sdk/framework/backend.go b/sdk/framework/backend.go index a16228eabb7a..6765b7e2d4a2 100644 --- a/sdk/framework/backend.go +++ b/sdk/framework/backend.go @@ -221,7 +221,7 @@ func (b *Backend) HandleRequest(ctx context.Context, req *logical.Request) (*log // If the path is empty and it is a help operation, handle that. if req.Path == "" && req.Operation == logical.HelpOperation { - return b.handleRootHelp(req) + return b.handleRootHelp(ctx, req) } // Find the matching route @@ -549,7 +549,7 @@ func (b *Backend) route(path string) (*Path, map[string]string) { return nil, nil } -func (b *Backend) handleRootHelp(req *logical.Request) (*logical.Response, error) { +func (b *Backend) handleRootHelp(ctx context.Context, req *logical.Request) (*logical.Response, error) { // Build a mapping of the paths and get the paths alphabetized to // make the output prettier. pathsMap := make(map[string]*Path) @@ -597,6 +597,10 @@ func (b *Backend) handleRootHelp(req *logical.Request) (*logical.Response, error vaultVersion = env.VaultVersion } + redactVersion, _, _, _ := logical.CtxRedactionSettingsValue(ctx) + if redactVersion { + vaultVersion = "" + } doc := NewOASDocument(vaultVersion) if err := documentPaths(b, requestResponsePrefix, doc); err != nil { b.Logger().Warn("error generating OpenAPI", "error", err) diff --git a/sdk/framework/path.go b/sdk/framework/path.go index 38f2e4fe9630..067b005e0fe5 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -384,6 +384,10 @@ func (p *Path) helpCallback(b *Backend) OperationFunc { vaultVersion = env.VaultVersion } } + redactVersion, _, _, _ := logical.CtxRedactionSettingsValue(ctx) + if redactVersion { + vaultVersion = "" + } doc := NewOASDocument(vaultVersion) if err := documentPath(p, b, requestResponsePrefix, doc); err != nil { b.Logger().Warn("error generating OpenAPI", "error", err) diff --git a/vault/logical_system.go b/vault/logical_system.go index 5de496987f7f..14c90422d64e 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -5259,8 +5259,14 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re context := d.Get("context").(string) + vaultVersion := version.Version + redactVersion, _, _, _ := logical.CtxRedactionSettingsValue(ctx) + if redactVersion { + vaultVersion = "" + } + // Set up target document - doc := framework.NewOASDocument(version.Version) + doc := framework.NewOASDocument(vaultVersion) // Generic mount paths will primarily be used for code generation purposes. // This will result in parameterized mount paths being returned instead of