Unbound 1.22.0rc1 pre-release

[Wouter Wijngaards]

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
>