We're still run into people who thought that the example config
is a suitable default.  Example where it caused practical issues:
https://lists.nic.cz/hyperkitty/list/knot-resolver-users@lists.nic.cz/thread/WQDJJ3LLEIZ5U3VVSCITW6DZPICW4L7U/

The main thing is the "failed to open socket" message.
But let's also elevate other fatal one-off logs to ERROR level.

Usually in configuration the module is loaded in a separate command
from passing configuration to it.  For dnstap this loading would
immediately lead to opening the default socket path, even if the
configuration actually specifies (a different) path later.

Users can still force using the default by passing an empty table:
`dnstap.config({})` or `modules = { dnstap = {}}`
(though I doubt the utility of the default /tmp/dnstap.sock anyway)

Vladimír Čunát authored 3 years ago and Tomas Krizek committed 3 years ago

We've been notified about possibility of "cache poisoning" this way,
so let's document this drawback to make the expectations clearer.

The approach of the code was rather hacky, simulating some packets
arriving from upstream and making the module stack CONSUME that.
Instead we take a direct approach now: use the simplified validator API
and then insert into cache directly.

One effect is improved performance, and consequently roughly halving
the lag which happens when prefill module invokes this.
(With root zone the lag goes down to 0.1 s from over 0.2 s,
 on my relatively fast CPU.  Fortunately it's just once a day.)

The following actions will now be logged in debug level (or request
tracing): ANSWER, DENY, DENY_MSG, DROP, REFUSE, TC

This can be useful for RPZ and other policy debugging.

Purposefully ommitted actions:
PASS - since it's the same as normal processing
REROUTE - the action itself comes from renumber module
STUB,FORWARD,TLS_FORWARD - this could be more confusing than useful
  (e.g. when response comes from cache)

To allow for easier debugging, each origin of an extended DNS error has
a unique 4-byte identifier that is included in the extra_text message.

The identifiers are random 4-letter base32 strings, generated with:
base32 /dev/random | head -c 4

Unspecified mask is already returned as full bitlen by
kr_straddr_subnet().

Tomas Krizek authored 3 years ago and Vladimír Čunát committed 3 years ago

Answers to EDNS requests from certain lua policies that use the
answer_clear() function would lack OPT RR and thus violate the MUST
condition in RFC6891.6.1.1.

On some platforms in CI, even 8s doesn't seem sufficient enough to
guarantee stability. Hopefully this improves the situation.

Vladimír Čunát authored 3 years ago and Oto Šťáva committed 3 years ago

Overriding records makes more sense on a particular name
than in a whole sub-tree.

Josh Soref authored 3 years ago and Tomas Krizek committed 3 years ago

Due to the typo in the EDNS keepalive init funcion name, the module
wouldn't be properly initialized after loading and wouldn't be
functional.

Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>

Josh Soref authored 3 years ago and Tomas Krizek committed 3 years ago

Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>

Štěpán Balážik authored 3 years ago and Tomas Krizek committed 3 years ago

Previously we primed for A/AAAA addresses of root servers even when
the respective IP version was disabled from configuration.