Vladimír Čunát [Tue, 30 May 2017 16:31:10 +0000 (18:31 +0200)]
kr_ranked_rrarray*: avoid duplicate RRsets on wire
Fixes https://gitlab.labs.nic.cz/knot/resolver/issues/198.
We can't let multiple "matching RRsets" to the wire, and we can't just
merge the sets from multiple queries either. The only way is to choose
either of the sets and put it on the wire. ATM the last one wins.
Common ocurrence of the bug: if www.example.cz was a CNAME for example.cz
and we ask for a non-existent type, we would get the SOA record twice
in the final answer.
A few related changes:
- don't just assert, also return error code if -DNDEBUG
- kr_ranked_rrarray_set_wire: don't do full-content comparison anymore;
see the first paragraph in this commit message for the reasons
- minor refactoring of that code, more comments, etc.
Vladimír Čunát [Tue, 23 May 2017 19:30:54 +0000 (21:30 +0200)]
Revert "iterator: don't retry if REFUSED"
This reverts commit bc2a26702e6460aee65fe170671336d670ba3eb9 (almost).
It would be best to avoid retrying with the same NS and keep trying with
others (if any), but that would require larger changes if it should work
well, so let's err on the side of sending more queries.
Vladimír Čunát [Mon, 22 May 2017 13:13:19 +0000 (15:13 +0200)]
validate: avoid keeping KR_RANK_MISSING
If a sub-query transitions the state to QUERY_DNSSEC_INSECURE,
we didn't set correct rank. This lead to caching with _MISSING rank
which is too low to be utilized on re-queries. Example: nix.mx query.
Vladimír Čunát [Fri, 19 May 2017 12:59:39 +0000 (14:59 +0200)]
ranked rrarray: don't merge RRSIGs of different types
For example, kr_rrmap_add has been relying on sets covering just a
single type (for years). It's used by cache and there it's required to
store types separately.
It seems all other parts of code should be just OK with this.
Vladimír Čunát [Fri, 19 May 2017 10:40:56 +0000 (12:40 +0200)]
iterate: tweak ranks of rrsigs
The bit about RRSIG coming from an authoritative source is still used
and possibly useful. The inconsistency was causing rrcache to fetch
a record without its signature under some circumstances, depending on
the record type and how it was obtained.
net.tls_padding() can now take a boolean in addition to a numeric
value. true means "use sensible default padding policy", false means
"never pad".
In the struct kr_context, we change tls_padding from a uint32_t to an
int32_t so that we can explicitly represent the default value (-1).
This should be a safe ABI/API change, since no one had ever set a
padding > 4096 anyway.
This depends on libknot having adopted the changes from 2dd9f406e333a1cedfa2380ffad57913cecf8efb which is first included in
2.4.3. In the event that knot-resolver is compiled against an older
version of libknot, it just uses a baked-in default of 468 as before.
See https://gitlab.labs.nic.cz/knot/resolver/merge_requests/249 for
further discussion.
GnuTLS doesn't actually log very much at log level 1.
It currently logs things at the following levels:
level 3: asserts
level 4: handshake information
level 5: record-layer information
level 9: session keys, IVs, and internal secrets
level 10: raw network read and buffering information
level 5 seems like the right layer for "verbose", without leaking too
much sensitive stuff.
Vladimír Čunát [Wed, 26 Apr 2017 15:39:05 +0000 (17:39 +0200)]
caches: improvements to STUB and future FORWARD modes
Fixes #122, mostly. CNAME chains aren't answered from cache in STUB
mode so far, because the current iterator would be unable to follow them.
Previously the caches avoided repeated lookups by checking if it has
a NS address for the query, which disabled any lookup in forwarding modes.
Now it sets the QUERY_NO_CACHE flag instead to stop repeating.
Also those more expensive kr_ta_covers_qry checks are deferred, so that
they're not done when not needed, e.g. in STUB or +cd mode.
Vladimír Čunát [Thu, 13 Apr 2017 12:40:29 +0000 (14:40 +0200)]
iterate: don't inherit _INSECURE on CNAME jumps
The jump may lead to secure zone, so let the sub-query find out by
itself. Otherwise we might cache those RRs with INSECURE rank even
though they are secure. This shouldn't harm AD flags anymore.
Vladimír Čunát [Mon, 10 Apr 2017 13:32:15 +0000 (15:32 +0200)]
iterator: improve get_initial_rank
If a server puts NS into the authority section that refers to itself,
accept it as autoritative and validate it (if applicable). This fixes
the val_nsec3_cnametocnamewctoposwc test, as unvalidated NS in the
final answer would prevent adding the AD flag. The iter_pcname test is
broken by this, but the team's consensus is to prefer this solution.
Nitpicks: cleaner style in the function, and don't force inlining anymore.
(It's no longer a trivial function and compilers should be good at
determining whether to inline static functions or not.)
Vladimír Čunát [Fri, 7 Apr 2017 13:41:41 +0000 (15:41 +0200)]
validate: fix bad usage of KR_RANK_INSECURE
It's supposed to mean that we have a proof from configured TAs that
the RR isn't secure (typically proof of missing DS at some point).
This case was just failure to find a fitting RRSIG; new KR_RANK_MISSING
is introduced for that purpose, for simplicity.
Also, make the validator more thorough about what ranks are safe to skip.
Vladimír Čunát [Thu, 6 Apr 2017 15:44:36 +0000 (17:44 +0200)]
utils: fix KEY_* defines
The argument to KEY_FLAG_RANK was (signed) char*, so for secure rank
the shift was setting the highest two bits (which are unused).
Let me end that rubbish.