It's quite cheap for us, and it might help with dumb clients
overusing the lowest IP from each set.

Pavel Doležal authored 4 years ago and Vladimír Čunát committed 4 years ago

- log queries and responses as separate dnstap messages
- use "query" instead of "request" to mirror dnstap specification
- don't export "query_zone" field in "CLIENT_*" messages

The http_data structure is allocated in http_write_pkt() and the last
callback that uses it is on_pkt_write(), so it should be responsible for
freeing the memory.

This used to leak a small amount of memory on every DoH response.

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

Make it easier to delete all rules specified in daf.

When resolving just NS names and addresses which won't go to answers,
our cache can satisfy them with just parent-side records.
Now we also make iterator consistent with that, and it will cut short
if a delegation satisfies what the kr_query wants.

There's a general risk that we will never get the child-side records,
and in practice the parent-side ones are sometimes "less accurate".
This change may increase this risk (to NS addresses in particular),
but we'd better consider addressing the risk later and systematically.
A suggestion is to refresh the records asynchronously:
https://tools.ietf.org/html/draft-ietf-dnsop-ns-revalidation

---
State before this commit lead to a weird behaviour where some IPv4-only
tests in Deckard (namely `iter_pcdiff.rpl`) were failing with IPv6
turned off.

This was due to the resolvers' internal preference towards AAAA records
for NS names.  With IPv6 networking on, NS name resolution was first
done for AAAA record and the glue (containing A record for the NS name
in question) from parent zone was put into cache.  As the AAAA
resolution failed (there is no AAAA for this NS name), A was queried
next and was satisfied from cache.

With IPv6 off, there is no query for the AAAA record, so no A record
from glue gets put in to the cache.  A record is resolved first, and
resolution ignores the glue in parent zone and continue to the child
zone which might be broken (intentionally in the case of that
`iter_pcdiff.rpl` test).

It's more consistent with what we do.  Now it will look like:
[system] error while loading config: .../lib/knot-resolver/kres_modules/view.lua:28: failed to parse subnet [::1]/128 (workdir '/foo/bar')

LuaJIT FFI was using opendir() (etc.) variants with 32-bit inodes
but the C parts was using them as 64-bit inode variants.
Consequently the `struct dirent` layout didn't match and we were getting
filenames shifted by eight bytes.

Now the whole dir-listing lua function is written in C.

Petr Špaček authored 4 years ago and Tomas Krizek committed 4 years ago

Typical example of unrepresentable message is a Lua error.
E.g. error() called from kresc would lead to NULL message.

Lukas Jezek authored 4 years ago and Tomas Krizek committed 4 years ago

This change allows map() to work with systemd integration.

As a bonus the new client implementation is based on Lua cqueues
allows caller to wrap map() in worker.corroutine() and get
asynchronous execution/avoid blocking main loop.

Currently socket communication does not employ timeouts so a hang
instance will lead to hang map() call. This does not affect query
processing _if_ map() is being run in worker.corroutine.

Fixes: #554
Fixes: #620

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.