Re-process .well-known redirects with a recursive handler call rather than a 302 redirect (#24890)

* Re-process .well-known redirects with a recursive handler call rather than a 302 redirect

* Track when the RequestURI mismatches path (in a redirect) and add it to the audit log

* call cancelFunc
This commit is contained in:
Scott Miller
2024-01-19 09:59:58 -06:00
committed by GitHub
parent ab8887c875
commit 9bb4f9e996
5 changed files with 48 additions and 16 deletions

View File

@@ -255,6 +255,10 @@ func (f *EntryFormatter) FormatRequest(ctx context.Context, in *logical.LogInput
},
}
if req.HTTPRequest != nil && req.HTTPRequest.RequestURI != req.Path {
reqEntry.Request.RequestURI = req.HTTPRequest.RequestURI
}
if !auth.IssueTime.IsZero() {
reqEntry.Auth.TokenIssueTime = auth.IssueTime.Format(time.RFC3339)
}
@@ -472,6 +476,10 @@ func (f *EntryFormatter) FormatResponse(ctx context.Context, in *logical.LogInpu
},
}
if req.HTTPRequest != nil && req.HTTPRequest.RequestURI != req.Path {
respEntry.Request.RequestURI = req.HTTPRequest.RequestURI
}
if auth.PolicyResults != nil {
respEntry.Auth.PolicyResults = &PolicyResults{
Allowed: auth.PolicyResults.Allowed,

View File

@@ -195,6 +195,7 @@ type Request struct {
WrapTTL int `json:"wrap_ttl,omitempty"`
Headers map[string][]string `json:"headers,omitempty"`
ClientCertificateSerialNumber string `json:"client_certificate_serial_number,omitempty"`
RequestURI string `json:"request_uri,omitempty"`
}
type Response struct {

View File

@@ -347,7 +347,8 @@ func wrapGenericHandler(core *vault.Core, h http.Handler, props *vault.HandlerPr
// return an HTTP error here. This information is best effort.
hostname, _ := os.Hostname()
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var hf func(w http.ResponseWriter, r *http.Request)
hf = func(w http.ResponseWriter, r *http.Request) {
// This block needs to be here so that upon sending SIGHUP, custom response
// headers are also reloaded into the handlers.
var customHeaders map[string][]*logical.CustomHeader
@@ -422,16 +423,9 @@ func wrapGenericHandler(core *vault.Core, h http.Handler, props *vault.HandlerPr
core.Logger().Warn("error resolving potential API redirect", "error", err)
} else {
if redir != "" {
dest := url.URL{
Path: redir,
RawQuery: r.URL.RawQuery,
}
w.Header().Set("Location", dest.String())
if r.Method == http.MethodGet || r.Proto == "HTTP/1.0" {
w.WriteHeader(http.StatusFound)
} else {
w.WriteHeader(http.StatusTemporaryRedirect)
}
newReq := r.Clone(ctx)
newReq.URL.Path = redir
hf(w, newReq)
cancelFunc()
return
}
@@ -487,7 +481,8 @@ func wrapGenericHandler(core *vault.Core, h http.Handler, props *vault.HandlerPr
h.ServeHTTP(nw, r)
cancelFunc()
})
}
return http.HandlerFunc(hf)
}
func WrapForwardedForHandler(h http.Handler, l *configutil.Listener) http.Handler {

View File

@@ -110,10 +110,8 @@ func buildLogicalRequestNoAuth(perfStandby bool, ra *vault.RouterAccess, w http.
// add the HTTP request to the logical request object for later consumption.
contentType := r.Header.Get("Content-Type")
if ra != nil && ra.IsBinaryPath(r.Context(), path) {
passHTTPReq = true
origBody = r.Body
} else if path == "sys/storage/raft/snapshot" || path == "sys/storage/raft/snapshot-force" {
if (ra != nil && ra.IsBinaryPath(r.Context(), path)) ||
path == "sys/storage/raft/snapshot" || path == "sys/storage/raft/snapshot-force" {
passHTTPReq = true
origBody = r.Body
} else {
@@ -194,6 +192,11 @@ func buildLogicalRequestNoAuth(perfStandby bool, ra *vault.RouterAccess, w http.
return nil, nil, http.StatusMethodNotAllowed, nil
}
// RFC 5785 Redirect, keep the request for auditing purposes
if r.URL.Path != r.RequestURI {
passHTTPReq = true
}
requestId, err := uuid.GenerateUUID()
if err != nil {
return nil, nil, http.StatusInternalServerError, fmt.Errorf("failed to generate identifier for the request: %w", err)

View File

@@ -6,8 +6,12 @@ package router
import (
"context"
"net/http"
"strings"
"testing"
"github.com/hashicorp/vault/audit"
"github.com/hashicorp/vault/helper/testhelpers/corehelpers"
"github.com/hashicorp/vault/helper/testhelpers"
vaulthttp "github.com/hashicorp/vault/http"
@@ -90,7 +94,11 @@ func TestRouter_UnmountRollbackIsntFatal(t *testing.T) {
}
func TestWellKnownRedirect_HA(t *testing.T) {
var records *[][]byte
cluster := vault.NewTestCluster(t, &vault.CoreConfig{
AuditBackends: map[string]audit.Factory{
"noop": corehelpers.NoopAuditFactory(&records),
},
DisablePerformanceStandby: true,
LogicalBackends: map[string]logical.Factory{
"noop": func(_ context.Context, _ *logical.BackendConfig) (logical.Backend, error) {
@@ -114,6 +122,12 @@ func TestWellKnownRedirect_HA(t *testing.T) {
standbys := testhelpers.DeriveStandbyCores(t, cluster)
standby := standbys[0].Client
if err := active.Client.Sys().EnableAuditWithOptions("noop", &api.EnableAuditOptions{
Type: "noop",
}); err != nil {
t.Fatalf("failed to enable audit: %v", err)
}
if err := active.Client.Sys().Mount("noop", &api.MountInput{
Type: "noop",
}); err != nil {
@@ -143,4 +157,15 @@ func TestWellKnownRedirect_HA(t *testing.T) {
} else if resp2.StatusCode != http.StatusOK {
t.Fatal("did not get expected response from noop backend after redirect")
}
if len(*records) < 2 {
t.Fatal("audit entries not populated")
} else {
rs := *records
// Make sure RequestURI is present in the redirect audit entries
if !strings.Contains(string(rs[len(rs)-1]), "request_uri\":\"/.well-known/foo/baz") ||
!strings.Contains(string(rs[len(rs)-2]), "request_uri\":\"/.well-known/foo/baz") {
t.Fatal("did not find request_uri in audit entries")
}
}
}