Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Caddyfile options to configure error log output #3958

Closed
kujenga opened this issue Jan 8, 2021 · 7 comments · Fixed by #4028
Closed

Add Caddyfile options to configure error log output #3958

kujenga opened this issue Jan 8, 2021 · 7 comments · Fixed by #4028
Labels
feature ⚙️ New feature or request help wanted 🆘 Extra attention is needed
Milestone

Comments

@kujenga
Copy link
Contributor

kujenga commented Jan 8, 2021

This issue is a request to add functionality to the Caddyfile to configure the error log output in a similar manner to how the log directive [1] allows this for "access logs". I was motived to open this issue by some of the discussion here: #3744 (comment) which resonates with needs I have run into recently. I haven't been able to find a way to do this so far, which seems to be confirmed by this somewhat recent forum post: https://caddy.community/t/where-to-put-error-log-in-caddyfile/9896/2

The specific situation I've seen motivating this request is that when you have an inbound request with sensitive headers set on it like Cookie or Authorization values that is routed to a reverse_proxy stanza, and if there is an error proxying the request to that backend service, you end up with an error message that looks like the following (I removed some of the default browser headers for simplicity):

2021/01/08 04:21:15.642	ERROR	http.log.error.log0	dial tcp [::1]:8080: connect: connection refused	{"request": {"remote_addr": "127.0.0.1:60809", "proto": "HTTP/2.0", "method": "GET", "host": "localhost", "uri": "/example", "headers": {"Accept": ["*/*"], "Accept-Encoding": ["gzip, deflate, br"], "Accept-Language": ["en-US,en;q=0.9"], "Cookie": ["secret_session_id=XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"]}, "tls": {"resumed": false, "version": 772, "cipher_suite": 4865, "proto": "h2", "proto_mutual": true, "server_name": "localhost"}}, "duration": 0.009280984, "status": 502, "err_id": "5qpcc54u3", "err_trace": "reverseproxy.statusError (reverseproxy.go:783)"}

In this case, it would be a security issue to capture the value of "Cookie": ["secret_session_id=XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"] in application logs on the server.

I understand that it may be possible to exclude this type of log with the JSON config [2] but it would be quite convenient to do this with the Caddyfile.

The equivalent functionality in the access logs would be a request>headers>Cookie delete statement similar to the examples for the access log directive: https://caddyserver.com/docs/caddyfile/directives/log#examples

[1] https://caddyserver.com/docs/caddyfile/directives/log
[2] https://caddyserver.com/docs/json/logging/

@mholt mholt added the feature ⚙️ New feature or request label Jan 8, 2021
@mholt mholt changed the title Add Caddyfile options to configure error log output [feature request] Add Caddyfile options to configure error log output Jan 8, 2021
@mholt mholt added this to the 2.x milestone Jan 8, 2021
@mholt mholt added the help wanted 🆘 Extra attention is needed label Jan 8, 2021
@kujenga
Copy link
Contributor Author

kujenga commented Jan 8, 2021

@mholt thanks for taking a look at this. I may have some time to work on adding something like this, do you have any pointers or thoughts as to what the implementation path might look like?

@francislavoie
Copy link
Member

francislavoie commented Jan 8, 2021

I'd recommend a global option syntax very similar to the log directive, but probably with an optional name argument to match how the logs work in JSON config

{
	log [<name>] {
		output <writer_module> ...
		format <encoder_module> ...
		level  <level>
		include <namespaces...>
		exclude <namespaces...>
	}
}

Where if omitted, name would default to default which is... the default logger 😛

And log should be able to be specified more than once with different names (similar to servers global directive).

Maybe also a log_sink global option like this:

{
	log_sink <writer_module> ...
}

The Caddyfile adapter should probably warn or error if there's a name conflict between a globally configured logger and a per-site logger (same names), because trying to merge them seems like a terrible idea.

@mholt
Copy link
Member

mholt commented Jan 8, 2021

Seconding @francislavoie's suggestions.

@ghoeffner
Copy link

Would this include errors from reverse_proxy where transport is fastcgi? Still trying to get these logs on a per-host basis.

@francislavoie
Copy link
Member

Would this include errors from reverse_proxy where transport is fastcgi? Still trying to get these logs on a per-host basis.

I think you'll still need to use JSON config for that, because per-host logging requires the logger_names field of HTTP servers. Making that work at the Caddyfile level would be pretty tricky.

But you should be able to configure a logger that includes http.reverse_proxy.transport.fastcgi, I guess.

You would be better served by using jq to transform the logs however you want.

@mholt
Copy link
Member

mholt commented Jan 9, 2021

Yeah, I think we already made the mistake of giving users some control over the logs that are emitted; all of that should really be done separate from the web server: you can drop logs on the floor right there or store them, but I made the web server too complicated.

kujenga added a commit to kujenga/caddy that referenced this issue Jan 12, 2021
This filter is intended to be useful in scenarios where you may want to
redact a value with a static string, giving you information that the
field did previously exist and was present, but not revealing the value
itself in the logs.

This was inspired by work on adding more complete support for removing
sensitive values from logs [1]. An example use case would be the
Authorization header in request log output, for which the value should
usually not be logged, but it may be quite useful for debugging to
confirm that the header was present in the request.

[1] caddyserver#3958
kujenga added a commit to kujenga/caddy that referenced this issue Jan 12, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current default. The intent here is to
preserve the previous behavior and add optional abilites for richer
configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958
@kujenga
Copy link
Contributor Author

kujenga commented Jan 12, 2021

Thanks for the suggestions!

I've got a draft PR open here, let me know your thoughts: #3968

kujenga added a commit to kujenga/caddy that referenced this issue Jan 12, 2021
This filter is intended to be useful in scenarios where you may want to
redact a value with a static string, giving you information that the
field did previously exist and was present, but not revealing the value
itself in the logs.

This was inspired by work on adding more complete support for removing
sensitive values from logs [1]. An example use case would be the
Authorization header in request log output, for which the value should
usually not be logged, but it may be quite useful for debugging to
confirm that the header was present in the request.

[1] caddyserver#3958
kujenga added a commit to kujenga/caddy that referenced this issue Jan 12, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current default. The intent here is to
preserve the previous behavior and add optional abilites for richer
configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958
kujenga added a commit to kujenga/caddy that referenced this issue Jan 12, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current default. The intent here is to
preserve the previous behavior and add optional abilites for richer
configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This filter is intended to be useful in scenarios where you may want to
redact a value with a static string, giving you information that the
field did previously exist and was present, but not revealing the value
itself in the logs.

This was inspired by work on adding more complete support for removing
sensitive values from logs [1]. An example use case would be the
Authorization header in request log output, for which the value should
usually not be logged, but it may be quite useful for debugging to
confirm that the header was present in the request.

[1] caddyserver#3958
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This filter is intended to be useful in scenarios where you may want to
redact a value with a static string, giving you information that the
field did previously exist and was present, but not revealing the value
itself in the logs.

This was inspired by work on adding more complete support for removing
sensitive values from logs [1]. An example use case would be the
Authorization header in request log output, for which the value should
usually not be logged, but it may be quite useful for debugging to
confirm that the header was present in the request.

[1] caddyserver#3958
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This filter is intended to be useful in scenarios where you may want to
redact a value with a static string, giving you information that the
field did previously exist and was present, but not revealing the value
itself in the logs.

This was inspired by work on adding more complete support for removing
sensitive values from logs [1]. An example use case would be the
Authorization header in request log output, for which the value should
usually not be logged, but it may be quite useful for debugging to
confirm that the header was present in the request.

[1] caddyserver#3958
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This commit makes a structural change to the way arguments are passed to
global options parsers in order to facilitate maintaining state and
passing in additional parameters in the future, in a way that mirrors
how handler directives are parsed, just simplified.

The motivation to do this is to be able to keep state on which loggers
have already been configured to prevent confusing configuration
behaviors as part of the work on [1]. This functionality will be used in
the next commit.

[1] caddyserver#3958
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Feb 18, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Mar 3, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Mar 3, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Mar 9, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Mar 9, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Mar 9, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Mar 10, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
kujenga added a commit to kujenga/caddy that referenced this issue Mar 12, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes caddyserver#3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
mholt pushed a commit that referenced this issue Mar 12, 2021
This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes #3958

[1] https://caddyserver.com/docs/caddyfile/directives/log
mholt pushed a commit that referenced this issue Mar 12, 2021
This filter is intended to be useful in scenarios where you may want to
redact a value with a static string, giving you information that the
field did previously exist and was present, but not revealing the value
itself in the logs.

This was inspired by work on adding more complete support for removing
sensitive values from logs [1]. An example use case would be the
Authorization header in request log output, for which the value should
usually not be logged, but it may be quite useful for debugging to
confirm that the header was present in the request.

[1] #3958
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request help wanted 🆘 Extra attention is needed
Projects
None yet
4 participants