Apparently it's never worked since its introduction.

.addr is non-nil exactly when .dst_addr is non-nil
(which hapens iff the query originated externally).
Now we have semantics which was probably intended by the original code
(98216295 from 2016) but that semantics is still problematic
if you need both kinds of matching in a single request from client.

This matching by destination has never made it to docs,
so let's just add this simple fixup for now, and later
we'll steer users to new policy configuration anyway.

Christopher Ng authored 2 years ago and Vladimír Čunát committed 2 years ago

This commit adds support for building on Cygwin/MSYS2.

Signed-off-by: Christopher Ng <facboy@gmail.com>

This reverts commit 0b9524b7.

The hack shouldn't be needed anymore:
https://github.com/Homebrew/homebrew-core/commit/4369052170f4360b7ad545f23b8a01a4ccb37683#diff-59a7902ada251dd9dba99b5bd323c1dba1d102d244ce766c06ce00097fb82e8fL71

This isn't an exact revert, but differences are minor.

That setting is recommended by documentation but it would still leave
the timer triggering repeatedly.  Maybe it didn't cause any practical
difference, but it was unnecessary and possibly confusing.

- apply to first (uncached) answer already
- don't extend over signature validity

Nit: the tests were using too high TTL (RFCs disallow the "sign bit").
It was working because (manual) cache-insertion was applying bounds,
but now the bounds don't get applied anymore, so it would fail.

When a whole packet is cached (instead of individual RRs),
let's simplify the way the packet's TTL gets computed.

The previous mechanism came from commit 5b383a2b,
probably a misunderstanding of:
https://datatracker.ietf.org/doc/html/rfc2308#section-5
Anyway, I see no motivation to do it, and this way we should
get rid of some weird cases where we might extend TTL of some records,
except if they were below the cache.min_ttl() setting (5s default).

In particular, avoids unintentional NXDOMAIN on grafted subtrees.
Consequently the users can drop 'NO_CACHE' flag and get caching.

I broke this in 54ab3f78 or closely around, so this never worked well
since 5.4.1, and maybe structured logging (5.4.0) had related issues.

Tom Herbers authored 2 years ago and Vladimír Čunát committed 2 years ago

It's resonable to assume that people would also want to disable DNS64 for
IPv4 source addresses if they only enable it for some IPv6 sources.

Close https://github.com/CZ-NIC/knot-resolver/pull/83

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

Fixes #760.

Also removes a warning in policy.REROUTE that is no longer true.

https://gitlab.nic.cz/knot/knot-resolver/-/jobs/635837#L219

When kresd starts without working internet connection, these would spam
logs by default every 10 seconds, which doesn't seem useful.

I can't see sufficient motivation here.  The cache will be slightly
less ready, but it's not often that you need to contact a root server.

Most importantly, kresd must work well anyway, even with empty cache.
Also, the compiled-in address set of root servers should be quite
accurate - the NS set has never changed, and the last address change
was five years ago with just one of 26 records changing.

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

(we changed the original fix a bit)

Олег Петрачев authored 2 years ago and Vladimír Čunát committed 2 years ago

(with minor cleanups from vcunat)

WARNING: You should add the boolean check kwarg to the run_command call.
         It currently defaults to false,
         but it will default to true in future releases of meson.
         See also: https://github.com/mesonbuild/meson/issues/9300

In almost all cases we already check the return code explicitly
and throw a more descriptive message than what would be the default.

Konstantin Amelichev authored 2 years ago and Vladimír Čunát committed 2 years ago

https://github.com/CZ-NIC/knot-resolver/pull/77
originally but changed by vcunat quite a bit.

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

Also change the return type of kr_pkt_has_dnssec() and lua's :dobit()

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)