-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
fix: Obfuscate the question_name field #1471
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
I'm not sure what's best to do here since the querylog is not very useful without this info, and it's disabled by default so you need to opt-in to have this output in the console.
Maybe we should make it explicit in the docs that log.privacy
doesn't impact this?
EDIT: maybe QueryLog.LogConfig
can log a warning, or we error during config validation, if log.privacy is enabled but the querylog type is console and it includes responseAnswer
or question
fields.
@@ -36,7 +36,7 @@ func LogEntryFields(entry *LogEntry) logrus.Fields { | |||
"response_reason": entry.ResponseReason, | |||
"response_type": entry.ResponseType, | |||
"response_code": entry.ResponseCode, | |||
"question_name": entry.QuestionName, | |||
"question_name": util.Obfuscate(entry.QuestionName), | |||
"question_type": entry.QuestionType, | |||
"answer": entry.Answer, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do obfuscate this, we should also do the answer:
"answer": entry.Answer, | |
"answer": util.Obfuscate(entry.Answer), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answer is already obfuscated. That's because it's piped through util.AnswerToString
which obfuscates everything (including query type, amusingly, even though that's clear from the length).
That's not actually accurate. The documentation states "If not defined, blocky will log all available information" and if The default behavior with an empty config but the privacy bit was already logging. You can still get some value out of such a query log ("has something been blocked for client X at time Y?"), which is why I kept it on for now... |
Currently blocky attempts to obfuscate queries (which I find a little questionable, given that you can still potentially deduce what has been asked for, but alas). However question_name is not obfuscated at all, so while the answer is, the plaintext query will still end up in the log. This change routes the field through the obfuscator, which will obfuscate if needed.