Skip to content

Commit

Permalink
caddyhttp: Make logging of credential headers opt-in (#4438)
Browse files Browse the repository at this point in the history
  • Loading branch information
francislavoie authored Dec 2, 2021
1 parent 8e5aafa commit 5bf0ada
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 24 deletions.
31 changes: 22 additions & 9 deletions caddyconfig/httpcaddyfile/serveroptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,16 @@ type serverOptions struct {
ListenerAddress string

// These will all map 1:1 to the caddyhttp.Server struct
ListenerWrappersRaw []json.RawMessage
ReadTimeout caddy.Duration
ReadHeaderTimeout caddy.Duration
WriteTimeout caddy.Duration
IdleTimeout caddy.Duration
MaxHeaderBytes int
AllowH2C bool
ExperimentalHTTP3 bool
StrictSNIHost *bool
ListenerWrappersRaw []json.RawMessage
ReadTimeout caddy.Duration
ReadHeaderTimeout caddy.Duration
WriteTimeout caddy.Duration
IdleTimeout caddy.Duration
MaxHeaderBytes int
AllowH2C bool
ExperimentalHTTP3 bool
StrictSNIHost *bool
ShouldLogCredentials bool
}

func unmarshalCaddyfileServerOptions(d *caddyfile.Dispenser) (interface{}, error) {
Expand Down Expand Up @@ -134,6 +135,12 @@ func unmarshalCaddyfileServerOptions(d *caddyfile.Dispenser) (interface{}, error
}
serverOpts.MaxHeaderBytes = int(size)

case "log_credentials":
if d.NextArg() {
return nil, d.ArgErr()
}
serverOpts.ShouldLogCredentials = true

case "protocol":
for nesting := d.Nesting(); d.NextBlock(nesting); {
switch d.Val() {
Expand Down Expand Up @@ -222,6 +229,12 @@ func applyServerOptions(
server.AllowH2C = opts.AllowH2C
server.ExperimentalHTTP3 = opts.ExperimentalHTTP3
server.StrictSNIHost = opts.StrictSNIHost
if opts.ShouldLogCredentials {
if server.Logs == nil {
server.Logs = &caddyhttp.ServerLogConfig{}
}
server.Logs.ShouldLogCredentials = opts.ShouldLogCredentials
}
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
idle 30s
}
max_header_size 100MB
log_credentials
protocol {
allow_h2c
experimental_http3
Expand Down Expand Up @@ -53,6 +54,9 @@ foo.com {
}
],
"strict_sni_host": true,
"logs": {
"should_log_credentials": true
},
"experimental_http3": true,
"allow_h2c": true
}
Expand Down
33 changes: 23 additions & 10 deletions modules/caddyhttp/marshalers.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import (
)

// LoggableHTTPRequest makes an HTTP request loggable with zap.Object().
type LoggableHTTPRequest struct{ *http.Request }
type LoggableHTTPRequest struct {
*http.Request

ShouldLogCredentials bool
}

// MarshalLogObject satisfies the zapcore.ObjectMarshaler interface.
func (r LoggableHTTPRequest) MarshalLogObject(enc zapcore.ObjectEncoder) error {
Expand All @@ -40,27 +44,36 @@ 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(r.Header))
enc.AddObject("headers", LoggableHTTPHeader{
Header: r.Header,
ShouldLogCredentials: r.ShouldLogCredentials,
})
if r.TLS != nil {
enc.AddObject("tls", LoggableTLSConnState(*r.TLS))
}
return nil
}

// LoggableHTTPHeader makes an HTTP header loggable with zap.Object().
// Headers with potentially sensitive information (Cookie, Authorization,
// and Proxy-Authorization) are logged with empty values.
type LoggableHTTPHeader http.Header
// 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 == nil {
if h.Header == nil {
return nil
}
for key, val := range h {
switch strings.ToLower(key) {
case "cookie", "authorization", "proxy-authorization":
val = []string{}
for key, val := range h.Header {
if !h.ShouldLogCredentials {
switch strings.ToLower(key) {
case "cookie", "set-cookie", "authorization", "proxy-authorization":
val = []string{}
}
}
enc.AddArray(key, LoggableStringArray(val))
}
Expand Down
7 changes: 6 additions & 1 deletion modules/caddyhttp/push/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhtt
}

repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
server := r.Context().Value(caddyhttp.ServerCtxKey).(*caddyhttp.Server)
shouldLogCredentials := server.Logs != nil && server.Logs.ShouldLogCredentials

// create header for push requests
hdr := h.initializePushHeaders(r, repl)
Expand All @@ -79,7 +81,10 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhtt
zap.String("uri", r.RequestURI),
zap.String("push_method", resource.Method),
zap.String("push_target", resource.Target),
zap.Object("push_headers", caddyhttp.LoggableHTTPHeader(hdr)))
zap.Object("push_headers", caddyhttp.LoggableHTTPHeader{
Header: hdr,
ShouldLogCredentials: shouldLogCredentials,
}))
err := pusher.Push(repl.ReplaceAll(resource.Target, "."), &http.PushOptions{
Method: resource.Method,
Header: hdr,
Expand Down
13 changes: 11 additions & 2 deletions modules/caddyhttp/reverseproxy/reverseproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,9 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl *
// point the request to this upstream
h.directRequest(req, di)

server := req.Context().Value(caddyhttp.ServerCtxKey).(*caddyhttp.Server)
shouldLogCredentials := server.Logs != nil && server.Logs.ShouldLogCredentials

// do the round-trip; emit debug log with values we know are
// safe, or if there is no error, emit fuller log entry
start := time.Now()
Expand All @@ -582,14 +585,20 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl *
logger := h.logger.With(
zap.String("upstream", di.Upstream.String()),
zap.Duration("duration", duration),
zap.Object("request", caddyhttp.LoggableHTTPRequest{Request: req}),
zap.Object("request", caddyhttp.LoggableHTTPRequest{
Request: req,
ShouldLogCredentials: shouldLogCredentials,
}),
)
if err != nil {
logger.Debug("upstream roundtrip", zap.Error(err))
return err
}
logger.Debug("upstream roundtrip",
zap.Object("headers", caddyhttp.LoggableHTTPHeader(res.Header)),
zap.Object("headers", caddyhttp.LoggableHTTPHeader{
Header: res.Header,
ShouldLogCredentials: shouldLogCredentials,
}),
zap.Int("status", res.StatusCode))

// duration until upstream wrote response headers (roundtrip duration)
Expand Down
17 changes: 15 additions & 2 deletions modules/caddyhttp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,11 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// it enters any handler chain; this is necessary
// to capture the original request in case it gets
// modified during handling
loggableReq := zap.Object("request", LoggableHTTPRequest{r})
shouldLogCredentials := s.Logs != nil && s.Logs.ShouldLogCredentials
loggableReq := zap.Object("request", LoggableHTTPRequest{
Request: r,
ShouldLogCredentials: shouldLogCredentials,
})
errLog := s.errorLogger.With(loggableReq)

var duration time.Duration
Expand Down Expand Up @@ -191,7 +195,10 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
zap.Duration("duration", duration),
zap.Int("size", wrec.Size()),
zap.Int("status", wrec.Status()),
zap.Object("resp_headers", LoggableHTTPHeader(wrec.Header())),
zap.Object("resp_headers", LoggableHTTPHeader{
Header: wrec.Header(),
ShouldLogCredentials: shouldLogCredentials,
}),
)
}()
}
Expand Down Expand Up @@ -508,6 +515,12 @@ type ServerLogConfig struct {
// If true, requests to any host not appearing in the
// LoggerNames (logger_names) map will not be logged.
SkipUnmappedHosts bool `json:"skip_unmapped_hosts,omitempty"`

// If true, credentials that are otherwise omitted, will be logged.
// The definition of credentials is defined by https://fetch.spec.whatwg.org/#credentials,
// and this includes some request and response headers, i.e `Cookie`,
// `Set-Cookie`, `Authorization`, and `Proxy-Authorization`.
ShouldLogCredentials bool `json:"should_log_credentials,omitempty"`
}

// wrapLogger wraps logger in a logger named according to user preferences for the given host.
Expand Down

0 comments on commit 5bf0ada

Please sign in to comment.