feature: hide client ip/name in querylog #575
Conversation
Codecov Report
@@ Coverage Diff @@
## development #575 /- ##
===============================================
Coverage 92.74% 92.76% 0.02%
===============================================
Files 37 37
Lines 3251 3260 9
===============================================
Hits 3015 3024 9
Misses 170 170
Partials 66 66
Continue to review full report at Codecov.
|
|
Thanks for your work! If I understand it right, "hideClient" is only used in databaseWriter (mysql and postgres), but not for CSV and console. I think, it would also make sense to hide client for all logging types, what do you think? Maybe we can hide or obfuscate client/IP in QueryLoggingResolver.Resolve or LogEntry? |
|
Sounds good. I will try to implement this for CSV etc. as well. Also moving the logic to a more central place is surely a good approach. |
| for logEntry := range r.logChan { | ||
| start := time.Now() | ||
|
|
||
| if r.hideClient { | ||
| logEntry.Request.ClientIP = emptyClientIP | ||
| logEntry.Request.ClientNames = emptyClientName |
There was a problem hiding this comment.
To avoid the variables and parsing the IP on each log, you can do:
logEntry.Request.ClientIP = net.IPv4zero
logEntry.Request.ClientNames = nil| @@ -517,6 517,7 @@ type QueryLogConfig struct { | |||
| LogRetentionDays uint64 `yaml:"logRetentionDays"` | |||
| CreationAttempts int `yaml:"creationAttempts" default:"3"` | |||
| CreationCooldown Duration `yaml:"creationCooldown" default:"2s"` | |||
| HideClient bool `yaml:"hideClient" default:"false"` | |||
There was a problem hiding this comment.
There's an existing option called logPrivacy, maybe we should extend that instead of creating a new option?
And if a new option is created think naming this something that starts with "log" would be nicer to make it clear it impacts the logs, and would follow the existing convention (logLevel, logPrivacy, etc.).
There was a problem hiding this comment.
I totally agree to follow the existing naming convention. But what exactly do you mean by extending that option? I am thinking of two possibilities here:
- Use that option instead of
HideClientin the check inresolver/query_logging_resolver.go - Introduce different "levels" of privacy similar to Pi-hole (e.g. "hide client", "hide targets", "hide all")
There was a problem hiding this comment.
logPrivacy is only for logging purposes. If true, all sensitive data (requested domain, response data) will be obfuscated. Purpose: admin should not see, which domains were resolved.
I think it makes sense to introduce new configuration parameter for query logging. Currently we log client IP, client names, response reason, question (typically domain name), response code, answer and duration.
To make it more configurable, we can introduce a list of "columns" to log, something like:
queryLog:
columns:
- clientIP
- clientNames
- durationcolumn can be an enum. If not specified, we log all possible columns. If only a subset is set, all other columns are empty.
What do you think?
There was a problem hiding this comment.
To make it more configurable, we can introduce a list of "columns" to log, something like:
I think that is awesome.
There was a problem hiding this comment.
Introduce different "levels" of privacy similar to Pi-hole (e.g. "hide client", "hide targets", "hide all")
I was thinking of something like this, but what @0xERR0R suggested is great!
|
replaced with #766 |
I'd like to hide the Client IP and Name in the querylog for privacy reasons.
Resolve or LogEntry