Hi Robert,
It is fixed in commit
https://github.com/NLnetLabs/unbound/commit/bd1813b126b047ccec490fadf1485c385eaed9d4
Those flags should be passed to the cache fill missing rrset cache
lookup also for RR type AAAA. Those are also tagged with the flag, and
treated similarly to RR type A.
That is correct, the flag is part of the hash key for the rrset cache
lookup.
Thank you for the code improvement! It was not caught by two other code
reviews.
Best regards, Wouter
On 10/10/2024 21:14, Robert Edmonds wrote:
Hi, Wouter:
The harden-unverified-glue patch sounds interesting so I started reading
the code. It looks like this was added in commit 1e0cf1e86b.
I see where an extra 'flags' parameter was added to
the cache_fill_missing() function in order to pass the
PACKED_RRSET_UNVERIFIED_GLUE flag through to the rrset_cache_lookup()
function.
However, I only see the flags being passed through for the
LDNS_RR_TYPE_A case here [0]:
akey = rrset_cache_lookup(env->rrset_cache, ns->name,
ns->namelen, LDNS_RR_TYPE_A, qclass, flags, now, 0);
^^^^^^
And not for the LDNS_RR_TYPE_AAAA case here [1]:
akey = rrset_cache_lookup(env->rrset_cache, ns->name,
ns->namelen, LDNS_RR_TYPE_AAAA, qclass, 0, now, 0);
^^
Is this a bug or is there some reason that A vs. AAAA records are
treated differently?
Also, do I understand correctly that the 'flags' parameter is part
of the hash key, so that the RRsets stored with 'flags' set to
PACKED_RRSET_UNVERIFIED_GLUE can never be retrieved unless that exact
'flags' value is specified by a subsequent lookup?
Thanks!
[0]
https://github.com/NLnetLabs/unbound/blame/8b7782e8fc11148abb8abe115f2a12b7974d9619/services/cache/dns.c#L379-L380
[1]
https://github.com/NLnetLabs/unbound/blame/8b7782e8fc11148abb8abe115f2a12b7974d9619/services/cache/dns.c#L400-L401
Wouter Wijngaards via Unbound-users wrote:
Hi Paul,
The unverified glue is about glue that is outside of the strict or narrow
glue definition of in zone glue, not about glue that is outside of bailiwick
or outside of parent zone, of course, that is already scrubbed. Something
about policies at registrars and ownership of domains. So, not the case you
cite; that would already be scrubbed by code in earlier releases.
There are defaults for those options, that are values suggested by the
researchers. I guess it is possible to toggle these kinds options if the
server is placed in a situation where they do not apply.
The naming of the redis options could be changed, if that is nicer for the
users of it. Aliases make for neat backwards compatibility, that is nice.
Best regards, Wouter
On 10/10/2024 15:55, Paul Wouters wrote:
On Thu, 10 Oct 2024, Wouter Wijngaards via Unbound-users wrote:
This release has an option to harden against unverified glue, it
is enabled with `harden-unverified-glue: yes`. It was contributed
by Karthik Umashankar from Microsoft. This protects Unbound against
bad glue, that is out of zone, by performing a lookup for it.
I am quite surprised that this wasn't dropped before this release?
Do you mean to say unverified (no DNSSEC signed) glue records that
were out of zone / bailiwick were just added to the cache before?
If so, that would be CVE worthy. And shouldn't need an option to
enable/disable.
Because it uses the original information as a last resort if nothing
works, it should not give lookup failures, and add protection.
So this is different from an A lookup for nohats.ca. that contains
glue for cnn.com ? Those are unused and not placed in the cache?
There are options to configure the scrubbing for NS records and
the CNAME scrubbing and the max global quota lookup limit from
previous security fix releases. They can be configured with the
options `iter-scrub-ns`, `iter-scrub-cname` and `max-global-quota`.
I am not sure I understand enough to give these sane values?
For redis use, with cachedb, it is possible to specify the
timeout for the initial connection separately from the timeout
for commands. With the options `redis-command-timeout: 20` and
`redis-connect-timeout: 200` they can be set separately, for
a longer connect attempt, but a short command timeout to keep
resolution faster.
Maybe make valkey-* aliases and slowly phase out the redis- options?
Paul
