Vladimír Čunát authored 4 years ago and Petr Špaček committed 4 years ago

It failed on a CNAME to a sibling name that's a zone cut.
Fixed by a minimalistic approach - tweaking the conditions
to always ask each CNAME step separately when forwarding.

FIXME: see FIXMEs in diff, document the API change, re-review.

Fixes bug introduced in b65e8977.
open() was creating a file which could not be open again because of
insufficient permissions. This somehow worked because lockfile_release()
unlinks the file, but it broke terribly if lockfile_release() was not
called because of crash or something like that.

Fixes: merge request !1042

Vladimír Čunát authored 4 years ago and Lukas Jezek committed 4 years ago

This seems generally considered to be a good thing (for DNS servers).
We don't do it on client side; I can't see an easy way there.

Vladimír Čunát authored 4 years ago and Petr Špaček committed 4 years ago

Restrict tried RRSIGs by qry_uid equality.
I see no use case against and it could be confusing.
(Also rewrite the conditions around to positive form.)

An assertion in cache noticed an NSEC with _SECURE rank but no RRSIG
(in practice).  It was a side-effect of still not keeping RRSIGs with
their RRs in some places.  It wasn't a security problem, as it doesn't
really matter where the signatures came from.  Theoretically it
might've lead to incorrect caching (missing usable RRSIGs), as cache
was restricting qry_uid to match, but that hasn't been noticed
in practice.

When using DoH, it seems /dns-query is a more common convetion for
an endpoint name. Let's use it in addition to /doh, since it doesn't
hurt anything and makes kresd more alike the other DoH implementations
out there. It'll also play more nicely with kdig, which uses /dns-query
as default as well.

(Tiny nitpicks addressed by vcunat.)

It hasn't been released yet, but this patch fixes build against
current Knot master already.

Vladimír Čunát authored 4 years ago and Petr Špaček committed 4 years ago

This is about situations when validator *thinks* it's in a signed zone
but an unsigned answer comes in. The assumption was that RRSIGs didn't
make it through some middle-boxes and it retried with explicit QTYPE=RRSIG.

There were two issues with that.
1. It seems that in most cases the cause of the situation is that
   we skipped over a zone cut that transitioned to insecure state,
   so the signatures correctly don't exist.
2. An explicit RRSIG query appears to be more trouble than worth;
   it seems reasonable for servers not to answer it (fully);
   see RFC 8482 sect. 7.

The new approach simply tries to find a proof that the name is insecure,
by spawning a QTYPE=DS sub-query on that name.  That fixes some
real-life cases; usually this happens in iteration mode where one IP
address serves zones on both sides of a cut that transitions to insecure.
For details see new comments in that rrsig_not_found() function.

The change resulted in the iterator fallback not making sense anymore
so it was removed.

In particular this gets rid of last light user data inside kresd.

It was still causing problems on some systems, for example Debian Sid.
The error was the same: "bad light userdata pointer" from luajit,
but note that the problem can still be triggered by lua libraries,
e.g. cqueues.

We don't use it anymore, and on some systems it's apparently
not an integer.

Vladimír Čunát authored 4 years ago and Petr Špaček committed 4 years ago

When signer name isn't a prefix of owner, the signature does not make
sense and it's no use trying to use that signer name in any way.

We generally don't force queries on every level of the path,
so this signer confusion could "introduce SERVFAILs" if we
skip over a transition to insecure.

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

When the effective user is root, no capabilities are dropped. This
change has no effect when running as non-privileged user or when
switching to non-privileged user via user() in config.

Dropping capabilities as a root user resulted in the following
unexpected behaviour:

1. When using trust anchor update, r/w access to root keys is neeeded.
   These are typically owned by knot-resolver user. When kresd is
   executed as root and capabilities are dropped, this file was no longer
   writable, because it is owned by knot-resolver, not root.
2. It is impossible to recreate/resize cache due to the same permission
   issue as above.

If you want to drop capabilities when starting kresd as a root user,
you can switch the user with the `user()` command. This changes the
effective user ID and drops any capabilities as well.

Also utilize table indexing.
This was a "regression" from extending RPZ support in 5.1.0.
NS and SOA are even mandatory, as RPZ is supposed to be a valid zone:
https://tools.ietf.org/html/draft-ietf-dnsop-dns-rpz-00#section-2

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

On 32-bit systems the insufficient GC could commonly happen:
https://lists.nic.cz/pipermail/knot-resolver-users/2020/000265.html

The meaning of -f parameter got slightly changed, so that the buggy
computation could be greatly simplified.  GC seems to make sense when
most of cache space is used, in which case the difference is small.

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

There were the usual effects: very incorrect timestamps (even by days),
and I suspect we could even lose some logs completely.

If the TLS handshake process fatally fails (e.g. no matching cipher
suite / cert), sent an alert to notify the peer.

Apparently the original implementation in 14de9110 didn't think of this.
Noticed by Fantomas:
https://forum.turris.cz/t/kresd-returns-nxdomain-for-local-mx-records/12991

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

We had some minor API changes since the last release.
Perhaps we haven't been bumping this often enough.

This also fixes the same for DNAMEs - soon, when they get supported.

Only the special case is changed - xNAMEs when no TAs exist.
Overall the TA handling in kresd is buggy; fortunately in practice it
seems very rare to run in other configuration than single root TA.

Formerly multiple instances could use the same seed,
which prevented the retry logic in Lua modules (e.g. prefill) from
retrying at different times.

AFAIK security impact is zero aside from potential thundering-herd
problem with many kresd instances.