diff --git a/proxy/internal/roundtrip/multi.go b/proxy/internal/roundtrip/multi.go index 533aefe0c..f97c241fa 100644 --- a/proxy/internal/roundtrip/multi.go +++ b/proxy/internal/roundtrip/multi.go @@ -1,23 +1,12 @@ package roundtrip import ( - "bytes" "crypto/tls" - "io" "net" "net/http" - "sort" - "strings" "time" - - log "github.com/sirupsen/logrus" ) -// upstreamLogBodyMax caps the request body bytes copied into the -// debug log line so a giant prompt or streamed payload doesn't fill the -// log. The body itself is always restored to the request unchanged. -const upstreamLogBodyMax = 4096 - // MultiTransport dispatches each request to either the embedded NetBird // http.RoundTripper or a stdlib http.Transport based on a per-request // context flag set by the reverse-proxy rewrite step. When the flag is @@ -25,15 +14,14 @@ const upstreamLogBodyMax = 4096 // embedded NetBird path — current behaviour, preserved. // // The stdlib branch is used when a target was configured with -// `direct_upstream=true`. It dials via the host's network stack, which -// is what private (`netbird proxy`) deployments and centralised proxies +// direct_upstream=true. It dials via the host's network stack, which is +// what private (`netbird proxy`) deployments and centralised proxies // fronting host-reachable upstreams (public APIs, LAN services, // localhost sidecars) want. type MultiTransport struct { embedded http.RoundTripper direct *http.Transport insecure *http.Transport - logger *log.Logger } // NewMultiTransport wires both branches. embedded is the existing NetBird @@ -41,10 +29,8 @@ type MultiTransport struct { // defaults that mirror Go's stdlib defaults plus a dial-timeout wrapper // honouring the per-request value attached via types.WithDialTimeout. // Pass embedded=nil to disable the WG branch entirely (every request -// will route direct, regardless of the context flag). logger may be -// nil; when nil the transport falls back to the logrus default -// instance. -func NewMultiTransport(embedded http.RoundTripper, logger *log.Logger) *MultiTransport { +// will route direct, regardless of the context flag). +func NewMultiTransport(embedded http.RoundTripper) *MultiTransport { dialer := &net.Dialer{ Timeout: 30 * time.Second, KeepAlive: 30 * time.Second, @@ -60,15 +46,10 @@ func NewMultiTransport(embedded http.RoundTripper, logger *log.Logger) *MultiTra insecure := direct.Clone() insecure.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //nolint:gosec // matches the embedded NetBird transport's per-target opt-in - if logger == nil { - logger = log.StandardLogger() - } - return &MultiTransport{ embedded: embedded, direct: direct, insecure: insecure, - logger: logger, } } @@ -77,7 +58,6 @@ func NewMultiTransport(embedded http.RoundTripper, logger *log.Logger) *MultiTra // honouring the existing per-request skip-TLS-verify flag. Otherwise it // goes through the embedded NetBird roundtripper. func (m *MultiTransport) RoundTrip(req *http.Request) (*http.Response, error) { - m.logUpstreamRequest(req) if DirectUpstreamFromContext(req.Context()) || m.embedded == nil { if skipTLSVerifyFromContext(req.Context()) { return m.insecure.RoundTrip(req) @@ -86,78 +66,3 @@ func (m *MultiTransport) RoundTrip(req *http.Request) (*http.Response, error) { } return m.embedded.RoundTrip(req) } - -// logUpstreamRequest emits the outbound request method, URL, headers, -// and a (capped) body snippet at info level for debugging. The body is -// read, copied into a snippet, and restored on the request so the -// actual upstream call sees it unchanged. -func (m *MultiTransport) logUpstreamRequest(req *http.Request) { - if req == nil { - return - } - body := snapshotRequestBody(req) - m.logger.Debugf("upstream request: method=%s url=%s host=%s body_length=%d headers=%s body=%s", - req.Method, req.URL.String(), req.Host, req.ContentLength, formatHeaders(req.Header), body) -} - -// formatHeaders renders the headers as a deterministic single-line -// string. Multi-valued headers are joined with commas. Sensitive -// header values (the upstream Authorization NetBird just stamped, plus -// any cookie jar that survived) are redacted so logs don't leak the -// provider API key. -func formatHeaders(h http.Header) string { - if len(h) == 0 { - return "{}" - } - keys := make([]string, 0, len(h)) - for k := range h { - keys = append(keys, k) - } - sort.Strings(keys) - var sb strings.Builder - sb.WriteByte('{') - for i, k := range keys { - if i > 0 { - sb.WriteByte(' ') - } - sb.WriteString(k) - sb.WriteByte('=') - sb.WriteString(redactHeaderValue(k, h.Values(k))) - } - sb.WriteByte('}') - return sb.String() -} - -// redactHeaderValue replaces sensitive credentials with a placeholder. -// All other header values are joined with commas verbatim. -func redactHeaderValue(name string, values []string) string { - switch strings.ToLower(name) { - case "authorization", "proxy-authorization", "x-api-key", "api-key", "cookie": - return "[redacted]" - } - return strings.Join(values, ",") -} - -// snapshotRequestBody returns a printable snippet of the request body -// (capped to upstreamLogBodyMax) and restores the body so downstream -// transports can still read it. Returns the empty string when there's -// no body or it can't be read. -func snapshotRequestBody(req *http.Request) string { - if req.Body == nil || req.Body == http.NoBody { - return "" - } - raw, err := io.ReadAll(req.Body) - if err != nil { - return "" - } - req.Body = io.NopCloser(bytes.NewReader(raw)) - // Restore GetBody so transports performing redirects or retries - // still get a fresh reader. - req.GetBody = func() (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader(raw)), nil - } - if len(raw) > upstreamLogBodyMax { - return string(raw[:upstreamLogBodyMax]) + "...[truncated]" - } - return string(raw) -} diff --git a/proxy/internal/roundtrip/multi_test.go b/proxy/internal/roundtrip/multi_test.go index 55c28e36c..62cf28d1c 100644 --- a/proxy/internal/roundtrip/multi_test.go +++ b/proxy/internal/roundtrip/multi_test.go @@ -31,7 +31,7 @@ func (s *stubRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) { func TestMultiTransport_DispatchesByContextFlag(t *testing.T) { embedded := &stubRoundTripper{body: "embedded"} - mt := NewMultiTransport(embedded, nil) + mt := NewMultiTransport(embedded) t.Run("default routes to embedded", func(t *testing.T) { embedded.called = false @@ -64,7 +64,7 @@ func TestMultiTransport_DispatchesByContextFlag(t *testing.T) { } func TestMultiTransport_NilEmbeddedAlwaysDirects(t *testing.T) { - mt := NewMultiTransport(nil, nil) + mt := NewMultiTransport(nil) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, "ok") })) diff --git a/proxy/server.go b/proxy/server.go index 4c3c229ab..f341abe09 100644 --- a/proxy/server.go +++ b/proxy/server.go @@ -542,7 +542,7 @@ func (s *Server) initNetBirdClient() { // client. The split is needed so direct_upstream targets resolve DNS via the // proxy host's resolver instead of the tunnel's DNS. func (s *Server) initReverseProxy() { - upstreamRT := roundtrip.NewMultiTransport(s.netbird, s.Logger) + upstreamRT := roundtrip.NewMultiTransport(s.netbird) s.proxy = proxy.NewReverseProxy(s.meter.RoundTripper(upstreamRT), s.ForwardedProto, s.TrustedProxies, s.Logger) }