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

[FR] Better logging for refused queries #651

Closed
ohmantics opened this issue Mar 23, 2022 · 3 comments
Closed

[FR] Better logging for refused queries #651

ohmantics opened this issue Mar 23, 2022 · 3 comments

Comments

@ohmantics
Copy link

Current behavior
Debugging ACLs is hard.

Describe the desired feature
When a query is refused/denied and sufficiently verbose logging is enabled, the reason why the query is refused should be logged. e.g.

[12345:1] debug: refused query from ip4 1.2.3.4 port 12345 (len 16) because of 'access-control: 0.0.0.0/32 refuse_non_local'

Potential use-case
Configuring these things is hard enough. Adding better logging will aid in debugging.

@ohmantics
Copy link
Author

ohmantics commented Mar 23, 2022

One possibility build be to think of the configuration language as if you're a compiler. Clang and GCC provide diagnostics that indicate file and line number information. This would require tracking the file names for the main config file and each included file. Each interesting token would save a file index and the line number. For the case of ACLs, there would also be a special location for "<built-in>" rules.

In the case that motivated me to write this feature request, I had access-control: 0.0.0.0/32 refuse_non_local when I should have had /0. So the diagnostic for this would be like:

[12345:1] debug: refused query from ip4 1.2.3.4 port 12345 (len 16) because of <built-in>:0:'access-control: 0.0.0.0/0 deny'

As the user, I would then see that the blocking ACL was not among my configured ACLs and see my error. Of course, with an improved diagnostic system like this, I might also have seen:

[12345:1] info: ACL specifies empty range at access_lists.conf:15:'access-control: 0.0.0.0/32 refuse_non_local'

@wcawijngaards
Copy link
Member

The fix creates output like this:
dropped query from 127.0.0.1 port 60176 because of 127.0.0.0/8 deny_non_local

The 0.0.0.0/32 example you give is not an empty range, it has one address in it, with zeroes. It is a valid method to put access control onto single addresses. For ipv6 with /128. And this also works for other configuration statements with netblocks.

It does not track the file and location of the definition of the access-control element, but the printout of the netblock is there. Hopefully this helps. Also dropped messages are logged, at high verbosity, of 4 or more.

@ohmantics
Copy link
Author

Amazing turnaround. Many thanks!

jedisct1 added a commit to jedisct1/unbound that referenced this issue Apr 22, 2022
* nlnet/master:
  - Fix zonemd unsupported algo check to set reason to NULL before the   check routine, but after malformed checks, to get the correct NULL   output when the digest matches.
  - Fix zonemd unsupported algo check to print unsupported reason before   zeroing it.
  - Fix zonemd unsupported algo check reason to not copy to next record,   and check for success for debug printout.
  - Fix zonemd unsupported algo check.
  - Fix zonemd check to allow unsupported algorithms to load.   If there are only unsupported algorithms, or unsupported schemes,   and no failed or successful other ZONEMD records, or malformed   or bad ZONEMD records, the unsupported records allow the zone load.
  - Fix spelling error in comment in sldns_str2wire_svcparam_key_lookup.
  - Fix NLnetLabs#651: [FR] Better logging for refused queries.
  - Minor formatting.
  fix -q doesn't work when use with 'unbound-control stats_shm'
  - Fix to describe auth-zone and other configuration at the local-zone   configuration option, to allow for more broadly view of the options.
  - Fix to ensure uniform handling of spaces and tabs when parsing RRs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants