Recursor: Allow logging DNSSEC bogus in any mode #4097

Merged
merged 1 commit into from Jul 5, 2016

Projects

None yet

2 participants

@pieterlexis
Member

Also allow setting this at runtime.

@Habbie Habbie and 1 other commented on an outdated diff Jul 5, 2016
pdns/pdns_recursor.cc
@@ -975,7 +975,7 @@ void startDoResolve(void *p)
pw.getHeader()->ad=0;
}
else if(state == Bogus) {
- if(sr.doLog() || g_dnssecmode == DNSSECMode::ValidateForLog) {
+ if(::arg().mustDo("dnssec-log-bogus") || sr.doLog() || g_dnssecmode == DNSSECMode::ValidateForLog) {
@Habbie
Habbie Jul 5, 2016 Member

::arg is slow (that's why the the dnssec mode is in a g_)

@pieterlexis
pieterlexis Jul 5, 2016 Member

Changed to a global variable

@Habbie Habbie commented on an outdated diff Jul 5, 2016
pdns/rec_channel_rec.cc
@@ -343,6 +345,33 @@ string doSetCarbonServer(T begin, T end)
}
template<typename T>
+string doSetDnssecLogBogus(T begin, T end)
+{
+ if (begin == end)
+ return "No DNSSEC Bogus logging setting specified\n";
+
+ if (pdns_iequals(*begin, "on") || pdns_iequals(*begin, "yes")) {
+ if (!arg().mustDo("dnssec-log-bogus")) {
@Habbie
Habbie Jul 5, 2016 Member

Because doSetDnssecLogBogus does not update arg, this code might do the wrong thing when it is called more than once

@Habbie
Habbie Jul 5, 2016 Member

Either make sure they are in sync, or declare the arg obsolete after the initial copy to g_ in serviceMain

@Habbie Habbie commented on the diff Jul 5, 2016
pdns/rec_channel_rec.cc
+ return "DNSSEC Bogus logging was already enabled\n";
+ }
+
+ if (pdns_iequals(*begin, "off") || pdns_iequals(*begin, "no")) {
+ if (g_dnssecLogBogus) {
+ L<<Logger::Warning<<"Disabeling DNSSEC Bogus logging, requested via control channel"<<endl;
+ g_dnssecLogBogus = false;
+ return "DNSSEC Bogus logging disabled\n";
+ }
+ return "DNSSEC Bogus logging was already disabled\n";
+ }
+
+ return "Unknown DNSSEC Bogus setting: '" + *begin +"'\n";
+}
+
+template<typename T>
@Habbie
Habbie Jul 5, 2016 Member

Various parts of this feel like they might be duplicating other work.

@pieterlexis
pieterlexis Jul 5, 2016 Member

possibly, not an issue at this point imho.

@Habbie
Member
Habbie commented Jul 5, 2016

Last nit. Not a blocker. Good for merge.

@pieterlexis pieterlexis added this to the rec-4.0.0 milestone Jul 5, 2016
@pieterlexis pieterlexis merged commit 7f1a3cc into PowerDNS:master Jul 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pieterlexis pieterlexis deleted the pieterlexis:DNSSEC-Log-Bogus branch Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment