From 5bf0adad8748e96e10529d5fc5777afc9236a7b5 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 2 Dec 2021 15:26:24 -0500 Subject: caddyhttp: Make logging of credential headers opt-in (#4438) --- modules/caddyhttp/marshalers.go | 33 ++++++++++++++++++-------- modules/caddyhttp/push/handler.go | 7 +++++- modules/caddyhttp/reverseproxy/reverseproxy.go | 13 ++++++++-- modules/caddyhttp/server.go | 17 +++++++++++-- 4 files changed, 55 insertions(+), 15 deletions(-) (limited to 'modules') diff --git a/modules/caddyhttp/marshalers.go b/modules/caddyhttp/marshalers.go index c99c94e..e6fc3a6 100644 --- a/modules/caddyhttp/marshalers.go +++ b/modules/caddyhttp/marshalers.go @@ -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 { @@ -40,7 +44,10 @@ 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)) } @@ -48,19 +55,25 @@ func (r LoggableHTTPRequest) MarshalLogObject(enc zapcore.ObjectEncoder) error { } // 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)) } diff --git a/modules/caddyhttp/push/handler.go b/modules/caddyhttp/push/handler.go index a89c0cd..75442be 100644 --- a/modules/caddyhttp/push/handler.go +++ b/modules/caddyhttp/push/handler.go @@ -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) @@ -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, diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index c629ef6..b418953 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -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() @@ -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) diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 2ddec60..98fd962 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -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 @@ -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, + }), ) }() } @@ -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. -- cgit v1.2.3