LLT-7558: Domain accessible after being removed from whitelist

[tomaszpatejko]

Problem

Domain is still accessible after it has been removed from whitelist.
The following use case trigger the bug:

1. Enable libtelio with libfirewall;
2. Try to access a domain. Whitelist is empty initially so the DNS query is sent to the blocking TP-Lite DNS server. The query result is NXDOMAIN that indicates that the domain is blocked.
3. Whitelist the domain. Nonblocking DNS server responds with an IP address that is cached in DNS resolver cache.
4. Reblock the domain by clearing the whitelist with an empty domain list.
5. Try to access the domain again. Bug: the IP address is served from resolver cache that was written is step 3. Expected behaviour should be that blocking TP-Lite DNS server is queried for an IP address and response should be NXDOMAIN.

Solution

Flush DNS resolver's cache when whitelist is set. This was no stale cache entries will be present and correct TP-Lite DNS server with be consulted for blocked domain's IP address.

AI disclosure: Claude was used for the following:

• log analysis and rootcausing,
• natlab test generation,
• fix suggestion and generation.

The work was also human-reviewed.

☑️ Definition of Done checklist

• Commit history is clean (requirements)
• README.md is updated
• Functionality is covered by unit or integration tests

[matislovas]

You can squash c8ac042 commit with the 3ec2acf.

[matislovas]

Does this blocks until all requests in-flight are finished, and only than clears the cache?

[tomaszpatejko]

If I understand the code correctly, TrustDNS' resolver uses sync::Cache from moka-rs crate for its cache implementation, and the entire cache invalidation is delegated to that crate.

Moka-rs' sync::Cache::invalidate_all method does not remove any entries and just records current time as invalidation time for all the entries in the cache. Check the comment here: https://github.com/moka-rs/moka/blob/a616ec19e8d4ed938caf8b2c88090331d778d5da/src/sync/cache.rs#L1658

Actual removal of invalidated entries is delegated to Housekeeper that is triggered by get/insert methods. Housekeeper does not block reads but it blocks writes.

[matislovas]

But as I see it, it still can cause problems with requests-in-flight. Before flushing the cache, we should check, if there are some and either:

1. Wait for them to finish and put to cache.
2. Cancel them, for them not to end in cache.

[matislovas]

The only situation, where we can avoid this problem with request-in-flight is that cache time entry (used to invalidate entries) is actually request_started and NOT request_put_to_cache

[tomaszpatejko]

I think I don't understand the issue you are trying to point out here. Could you elaborate a bit more?

[matislovas]

Shouldn't we compare the redirects as well?

[tomaszpatejko]

I don't think redirects should be compared in this situation.

If I understand the comment for set_tp_lite_domain_whitelist, redirects contains pairs of DNS servers used for blocking or passing through a domain. So the change in this list does not drive the decision whether resolver cache should be flushed.

[matislovas]

My idea was to check both - the domains and redirects:

let changed_domains = configured_state.tp_lite_whitelisted_domains != domains;
let changed_redirects= configured_state.tp_lite_dns_redirects != redirects;
...
(changed_domains || changed_redirects)

redirects can influence DNAT usage, so my idea was to compare them as well.

[jjanowsk]

I think we can get rid of this discussion by flashing on every call to set_tp_lite_domain_whitelist. Realistically it's only called when: a) user added/or removed something from whitelist, b) user enabled or disabled tp. In both cases we want to flush the cache. So I don't think it's worth optimizing for some hard to imagine scenario.