Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import (
"github.com/prometheus/client_golang/prometheus"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"

"github.com/caddyserver/caddy/v2/internal"
)

// testCertMagicStorageOverride is a package-level test hook. Tests may set
Expand Down Expand Up @@ -800,7 +802,7 @@ func (h adminHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
zap.String("uri", r.RequestURI),
zap.String("remote_ip", ip),
zap.String("remote_port", port),
zap.Reflect("headers", r.Header),
zap.Object("headers", internal.LoggableHTTPHeader{Header: r.Header}),
)
if r.TLS != nil {
log = log.With(
Expand Down
47 changes: 47 additions & 0 deletions admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
"github.com/caddyserver/certmagic"
"github.com/prometheus/client_golang/prometheus"
dto "github.com/prometheus/client_model/go"
"go.uber.org/zap"
"go.uber.org/zap/zaptest/observer"
)

var testCfg = []byte(`{
Expand Down Expand Up @@ -242,6 +244,51 @@ func TestAdminHandlerErrorHandling(t *testing.T) {
}
}

func TestAdminHandlerServeHTTPRedactsSensitiveHeadersInLogs(t *testing.T) {
core, logs := observer.New(zap.InfoLevel)

defaultLoggerMu.Lock()
origLogger := defaultLogger.logger
defaultLogger.logger = zap.New(core)
defaultLoggerMu.Unlock()
t.Cleanup(func() {
defaultLoggerMu.Lock()
defaultLogger.logger = origLogger
defaultLoggerMu.Unlock()
})

handler := adminHandler{
mux: http.NewServeMux(),
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.Header.Set("Authorization", "Bearer secret")
req.Header.Set("Cookie", "session=secret")
req.Header.Set("X-Test", "ok")
rr := httptest.NewRecorder()

handler.ServeHTTP(rr, req)

if logs.Len() == 0 {
t.Fatal("expected request log entry")
}

ctx := logs.All()[0].ContextMap()
headers, ok := ctx["headers"].(map[string]any)
if !ok {
t.Fatalf("expected headers field in log context, got %T", ctx["headers"])
}

if got := headers["Authorization"]; !reflect.DeepEqual(got, []any{"REDACTED"}) {
t.Fatalf("expected redacted Authorization header, got %#v", got)
}
if got := headers["Cookie"]; !reflect.DeepEqual(got, []any{"REDACTED"}) {
t.Fatalf("expected redacted Cookie header, got %#v", got)
}
if got := headers["X-Test"]; !reflect.DeepEqual(got, []any{"ok"}) {
t.Fatalf("expected X-Test header to remain visible, got %#v", got)
}
}

func initAdminMetrics() {
if adminMetrics.requestErrors != nil {
prometheus.Unregister(adminMetrics.requestErrors)
Expand Down
54 changes: 54 additions & 0 deletions internal/logmarshalers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package internal

import (
"net/http"
"strings"

"go.uber.org/zap/zapcore"
)

// LoggableHTTPHeader makes an HTTP header loggable with zap.Object().
// Headers with potentially sensitive information (Cookie, Set-Cookie,
// Authorization, and Proxy-Authorization) are logged with empty values.
type LoggableHTTPHeader struct {
http.Header

ShouldLogCredentials bool
}

// MarshalLogObject satisfies the zapcore.ObjectMarshaler interface.
func (h LoggableHTTPHeader) MarshalLogObject(enc zapcore.ObjectEncoder) error {
if h.Header == nil {
return nil
}
for key, val := range h.Header {
if !h.ShouldLogCredentials {
switch strings.ToLower(key) {
case "cookie", "set-cookie", "authorization", "proxy-authorization":
val = []string{"REDACTED"} // see #5669. I still think ▒▒▒▒ would be cool.
}
}
enc.AddArray(key, LoggableStringArray(val))
}
return nil
}

// LoggableStringArray makes a slice of strings marshalable for logging.
type LoggableStringArray []string

// MarshalLogArray satisfies the zapcore.ArrayMarshaler interface.
func (sa LoggableStringArray) MarshalLogArray(enc zapcore.ArrayEncoder) error {
if sa == nil {
return nil
}
for _, s := range sa {
enc.AppendString(s)
}
return nil
}

// Interface guards
var (
_ zapcore.ObjectMarshaler = (*LoggableHTTPHeader)(nil)
_ zapcore.ArrayMarshaler = (*LoggableStringArray)(nil)
)
47 changes: 6 additions & 41 deletions modules/caddyhttp/marshalers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ import (
"crypto/tls"
"net"
"net/http"
"strings"

"go.uber.org/zap/zapcore"

"github.com/caddyserver/caddy/v2/internal"
)

// LoggableHTTPRequest makes an HTTP request loggable with zap.Object().
Expand All @@ -47,12 +48,12 @@ func (r LoggableHTTPRequest) MarshalLogObject(enc zapcore.ObjectEncoder) error {
enc.AddString("method", r.Method)
enc.AddString("host", r.Host)
enc.AddString("uri", r.RequestURI)
enc.AddObject("headers", LoggableHTTPHeader{
enc.AddObject("headers", internal.LoggableHTTPHeader{
Header: r.Header,
ShouldLogCredentials: r.ShouldLogCredentials,
})
if r.TransferEncoding != nil {
enc.AddArray("transfer_encoding", LoggableStringArray(r.TransferEncoding))
enc.AddArray("transfer_encoding", internal.LoggableStringArray(r.TransferEncoding))
}
if r.TLS != nil {
enc.AddObject("tls", LoggableTLSConnState(*r.TLS))
Expand All @@ -61,44 +62,10 @@ func (r LoggableHTTPRequest) MarshalLogObject(enc zapcore.ObjectEncoder) error {
}

// LoggableHTTPHeader makes an HTTP header loggable with zap.Object().
// Headers with potentially sensitive information (Cookie, Set-Cookie,
// Authorization, and Proxy-Authorization) are logged with empty values.
type LoggableHTTPHeader struct {
http.Header

ShouldLogCredentials bool
}

// MarshalLogObject satisfies the zapcore.ObjectMarshaler interface.
func (h LoggableHTTPHeader) MarshalLogObject(enc zapcore.ObjectEncoder) error {
if h.Header == nil {
return nil
}
for key, val := range h.Header {
if !h.ShouldLogCredentials {
switch strings.ToLower(key) {
case "cookie", "set-cookie", "authorization", "proxy-authorization":
val = []string{"REDACTED"} // see #5669. I still think ▒▒▒▒ would be cool.
}
}
enc.AddArray(key, LoggableStringArray(val))
}
return nil
}
type LoggableHTTPHeader = internal.LoggableHTTPHeader

// LoggableStringArray makes a slice of strings marshalable for logging.
type LoggableStringArray []string

// MarshalLogArray satisfies the zapcore.ArrayMarshaler interface.
func (sa LoggableStringArray) MarshalLogArray(enc zapcore.ArrayEncoder) error {
if sa == nil {
return nil
}
for _, s := range sa {
enc.AppendString(s)
}
return nil
}
type LoggableStringArray = internal.LoggableStringArray

// LoggableTLSConnState makes a TLS connection state loggable with zap.Object().
type LoggableTLSConnState tls.ConnectionState
Expand All @@ -121,7 +88,5 @@ func (t LoggableTLSConnState) MarshalLogObject(enc zapcore.ObjectEncoder) error
// Interface guards
var (
_ zapcore.ObjectMarshaler = (*LoggableHTTPRequest)(nil)
_ zapcore.ObjectMarshaler = (*LoggableHTTPHeader)(nil)
_ zapcore.ArrayMarshaler = (*LoggableStringArray)(nil)
_ zapcore.ObjectMarshaler = (*LoggableTLSConnState)(nil)
)
26 changes: 13 additions & 13 deletions modules/logging/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (

"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
"github.com/caddyserver/caddy/v2/modules/caddyhttp"
"github.com/caddyserver/caddy/v2/internal"
)

func init() {
Expand Down Expand Up @@ -100,8 +100,8 @@ func (f *HashFilter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {

// Filter filters the input field with the replacement value.
func (f *HashFilter) Filter(in zapcore.Field) zapcore.Field {
if array, ok := in.Interface.(caddyhttp.LoggableStringArray); ok {
newArray := make(caddyhttp.LoggableStringArray, len(array))
if array, ok := in.Interface.(internal.LoggableStringArray); ok {
newArray := make(internal.LoggableStringArray, len(array))
for i, s := range array {
newArray[i] = hash(s)
}
Expand Down Expand Up @@ -241,8 +241,8 @@ func (m *IPMaskFilter) Provision(ctx caddy.Context) error {

// Filter filters the input field.
func (m IPMaskFilter) Filter(in zapcore.Field) zapcore.Field {
if array, ok := in.Interface.(caddyhttp.LoggableStringArray); ok {
newArray := make(caddyhttp.LoggableStringArray, len(array))
if array, ok := in.Interface.(internal.LoggableStringArray); ok {
newArray := make(internal.LoggableStringArray, len(array))
for i, s := range array {
newArray[i] = m.mask(s)
}
Expand Down Expand Up @@ -392,8 +392,8 @@ func (m *QueryFilter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {

// Filter filters the input field.
func (m QueryFilter) Filter(in zapcore.Field) zapcore.Field {
if array, ok := in.Interface.(caddyhttp.LoggableStringArray); ok {
newArray := make(caddyhttp.LoggableStringArray, len(array))
if array, ok := in.Interface.(internal.LoggableStringArray); ok {
newArray := make(internal.LoggableStringArray, len(array))
for i, s := range array {
newArray[i] = m.processQueryString(s)
}
Expand Down Expand Up @@ -523,7 +523,7 @@ func (m *CookieFilter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {

// Filter filters the input field.
func (m CookieFilter) Filter(in zapcore.Field) zapcore.Field {
cookiesSlice, ok := in.Interface.(caddyhttp.LoggableStringArray)
cookiesSlice, ok := in.Interface.(internal.LoggableStringArray)
if !ok {
return in
}
Expand Down Expand Up @@ -559,7 +559,7 @@ OUTER:
transformedRequest.AddCookie(c)
}

in.Interface = caddyhttp.LoggableStringArray(transformedRequest.Header["Cookie"])
in.Interface = internal.LoggableStringArray(transformedRequest.Header["Cookie"])

return in
}
Expand Down Expand Up @@ -613,8 +613,8 @@ func (m *RegexpFilter) Provision(ctx caddy.Context) error {

// Filter filters the input field with the replacement value if it matches the regexp.
func (f *RegexpFilter) Filter(in zapcore.Field) zapcore.Field {
if array, ok := in.Interface.(caddyhttp.LoggableStringArray); ok {
newArray := make(caddyhttp.LoggableStringArray, len(array))
if array, ok := in.Interface.(internal.LoggableStringArray); ok {
newArray := make(internal.LoggableStringArray, len(array))
for i, s := range array {
newArray[i] = f.regexp.ReplaceAllString(s, f.Value)
}
Expand Down Expand Up @@ -783,8 +783,8 @@ func (f *MultiRegexpFilter) Validate() error {
// Filter applies all regexp operations sequentially to the input field.
// Input is sanitized and validated for security.
func (f *MultiRegexpFilter) Filter(in zapcore.Field) zapcore.Field {
if array, ok := in.Interface.(caddyhttp.LoggableStringArray); ok {
newArray := make(caddyhttp.LoggableStringArray, len(array))
if array, ok := in.Interface.(internal.LoggableStringArray); ok {
newArray := make(internal.LoggableStringArray, len(array))
for i, s := range array {
newArray[i] = f.processString(s)
}
Expand Down
32 changes: 16 additions & 16 deletions modules/logging/filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"go.uber.org/zap/zapcore"

"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/modules/caddyhttp"
"github.com/caddyserver/caddy/v2/internal"
)

func TestIPMaskSingleValue(t *testing.T) {
Expand Down Expand Up @@ -55,11 +55,11 @@ func TestIPMaskMultiValue(t *testing.T) {
f := IPMaskFilter{IPv4MaskRaw: 16, IPv6MaskRaw: 32}
f.Provision(caddy.Context{})

out := f.Filter(zapcore.Field{Interface: caddyhttp.LoggableStringArray{
out := f.Filter(zapcore.Field{Interface: internal.LoggableStringArray{
"255.255.255.255",
"244.244.244.244",
}})
arr, ok := out.Interface.(caddyhttp.LoggableStringArray)
arr, ok := out.Interface.(internal.LoggableStringArray)
if !ok {
t.Fatalf("field is wrong type: %T", out.Integer)
}
Expand All @@ -70,11 +70,11 @@ func TestIPMaskMultiValue(t *testing.T) {
t.Fatalf("field entry 1 has not been filtered: %s", arr[1])
}

out = f.Filter(zapcore.Field{Interface: caddyhttp.LoggableStringArray{
out = f.Filter(zapcore.Field{Interface: internal.LoggableStringArray{
"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
"ff00:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
}})
arr, ok = out.Interface.(caddyhttp.LoggableStringArray)
arr, ok = out.Interface.(internal.LoggableStringArray)
if !ok {
t.Fatalf("field is wrong type: %T", out.Integer)
}
Expand Down Expand Up @@ -120,11 +120,11 @@ func TestQueryFilterMultiValue(t *testing.T) {
t.Fatalf("the filter must be valid")
}

out := f.Filter(zapcore.Field{Interface: caddyhttp.LoggableStringArray{
out := f.Filter(zapcore.Field{Interface: internal.LoggableStringArray{
"/path1?foo=a&foo=b&bar=c&bar=d&baz=e&hash=hashed",
"/path2?foo=c&foo=d&bar=e&bar=f&baz=g&hash=hashed",
}})
arr, ok := out.Interface.(caddyhttp.LoggableStringArray)
arr, ok := out.Interface.(internal.LoggableStringArray)
if !ok {
t.Fatalf("field is wrong type: %T", out.Interface)
}
Expand Down Expand Up @@ -162,11 +162,11 @@ func TestCookieFilter(t *testing.T) {
{hashAction, "hash", ""},
}}

out := f.Filter(zapcore.Field{Interface: caddyhttp.LoggableStringArray{
out := f.Filter(zapcore.Field{Interface: internal.LoggableStringArray{
"foo=a; foo=b; bar=c; bar=d; baz=e; hash=hashed",
}})
outval := out.Interface.(caddyhttp.LoggableStringArray)
expected := caddyhttp.LoggableStringArray{
outval := out.Interface.(internal.LoggableStringArray)
expected := internal.LoggableStringArray{
"foo=REDACTED; foo=REDACTED; baz=e; hash=1a06df82",
}
if outval[0] != expected[0] {
Expand Down Expand Up @@ -204,8 +204,8 @@ func TestRegexpFilterMultiValue(t *testing.T) {
f := RegexpFilter{RawRegexp: `secret`, Value: "REDACTED"}
f.Provision(caddy.Context{})

out := f.Filter(zapcore.Field{Interface: caddyhttp.LoggableStringArray{"foo-secret-bar", "bar-secret-foo"}})
arr, ok := out.Interface.(caddyhttp.LoggableStringArray)
out := f.Filter(zapcore.Field{Interface: internal.LoggableStringArray{"foo-secret-bar", "bar-secret-foo"}})
arr, ok := out.Interface.(internal.LoggableStringArray)
if !ok {
t.Fatalf("field is wrong type: %T", out.Integer)
}
Expand All @@ -229,8 +229,8 @@ func TestHashFilterSingleValue(t *testing.T) {
func TestHashFilterMultiValue(t *testing.T) {
f := HashFilter{}

out := f.Filter(zapcore.Field{Interface: caddyhttp.LoggableStringArray{"foo", "bar"}})
arr, ok := out.Interface.(caddyhttp.LoggableStringArray)
out := f.Filter(zapcore.Field{Interface: internal.LoggableStringArray{"foo", "bar"}})
arr, ok := out.Interface.(internal.LoggableStringArray)
if !ok {
t.Fatalf("field is wrong type: %T", out.Integer)
}
Expand Down Expand Up @@ -292,11 +292,11 @@ func TestMultiRegexpFilterMultiValue(t *testing.T) {
t.Fatalf("unexpected error provisioning: %v", err)
}

out := f.Filter(zapcore.Field{Interface: caddyhttp.LoggableStringArray{
out := f.Filter(zapcore.Field{Interface: internal.LoggableStringArray{
"foo-secret-123",
"bar-secret-456",
}})
arr, ok := out.Interface.(caddyhttp.LoggableStringArray)
arr, ok := out.Interface.(internal.LoggableStringArray)
if !ok {
t.Fatalf("field is wrong type: %T", out.Interface)
}
Expand Down
Loading