caddyhttp: Make logging of credential headers opt-in (#4438)

4 files changed, 55 insertions, 15 deletions

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.