-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Log Authorization header as "[REDACTED]"? #5669
Comments
Hmm, yeah, maybe. I don't like |
What about...
😅 (I jest.) Do we want to add an artificial value though? I feel like that could throw off some things. |
I'm kinda leaning towards doing nothing here. We should probably just make it as clear as possible in the docs. Though I think it's already pretty decent, it's in the paragraph underneath the TOC on https://caddyserver.com/docs/caddyfile/directives/log: |
How about A missing auth header is an incredibly common error pattern.
|
I'm actually leaning toward the Unicode I jokingly posted above, or doing nothing at all, to avoid confusion. What if the header's real value is "REDACTED"? A missing header wouldn't be logged at all, it wouldn't be empty... |
It's your call, but I disagree. (With doing nothing. The unicode would be slightly weird but alright for me.) "[redacted]" is well-known in our industry as a placeholder for information that was removed for privacy reasons. Similarly well-known is the fact that when you find an empty string where you expected a value then more often than not something went wrong.
Let's say you forgot to set the auth token in the configuration of some client. The client would send |
I feel like the client would not set an empty header though? Are there popular clients that do this? I just feel like using an arbitrary string is confusing. I'm not sure what standard makes "[redacted]" more well-known than "redacted" or "REDACTED" or "xxx" or "**" or "???" or anything else for that matter. For example, password inputs definitely don't use |
So I just tested this in the two scripting environments I had at hand. PHP with The result was that cURL does not send an empty header this way. If you really want to send an empty header with curl, apparently you have to use a semicolon: Node.js does happily send the empty header.
I didn't mean the format
We're talking about logs, not about password fields. I would collect examples from the Caddy forum, but Caddy users are explicitly discouraged to redact any information, so I'll turn to Serverfault instead. If you search Serverfault for xxx you will see it mostly be used in prose as a placeholder for stuff that doesn't matter in the context. If instead you search for "redacted", you will see lots and lots of examples where people are providing logs with sensitive information redacted. |
Thanks for trying it out. I guess it kind of depends on the client (sample size 2).
It's mostly the arbitrary-ness of it that I find difficult to settle on... and adding extra bytes to the log unnecessarily. 🤷♂️
But they both redact, and that's what I was trying to illustrate. People are much more used to password field redaction I'd bet.
Just want to clarify: they're discouraged from redacting non-secret information like domain names and such. Secrets absolutely should be redacted! 💯 Anyway, I dunno -- I'll think about it. I don't have a strong technical reason not to do this other than what I've mentioned, I just try to find the most elegant, standards-conforming solutions when possible (especially when it involves external systems, which logs often do). |
I also bumped on the issue, I was hunting for the value of my auth. header and thought it was empty because of this empty log message. For what it's worth redact, a fairly popular rust library, is redacting by printing the |
Maybe let's just go with EDIT: I also kinda like |
I just spend an hour debugging why my application seemingly isn't sending
Authorization
headers, until I found out that Caddy logs them as empty arrays.Would it be possible/better to log them as the string "[REDACTED]" which is commonly used to replace sensitive values in log output?
The text was updated successfully, but these errors were encountered: