DnssecResolverApi.getClient() returns insecure dnsClient

[Delicates]

The DnssecResolverApi class extends the ResolverApi and creates its own DnssecClient with its own secure separate cache, but it never overrides the inherited getClient() function. https://github.com/MiniDNS/minidns/blob/master/minidns-hla/src/main/java/org/minidns/hla/DnssecResolverApi.java#L29

So the inherited DnssecResolverApi.getClient() returns an insecure DNS client that uses the shared global DEFAULT_CACHE. https://github.com/MiniDNS/minidns/blob/master/minidns-hla/src/main/java/org/minidns/hla/ResolverApi.java#L219 https://github.com/MiniDNS/minidns/blob/master/minidns-client/src/main/java/org/minidns/AbstractDnsClient.java#L44

This results in DNS cache entries poisoned through previous insecure DNS client queries being accepted as DNSSEC valid without throwing exceptions.

As an example of impact, currently this shared insecure cache issue breaks DNSSEC enforcement in the following FairEmail code by @M66B when tested in a DNS hijacked environment: https://github.com/M66B/FairEmail/blob/3783689572b77fc0d1870444fed403a3135f577f/app/src/main/java/eu/faircode/email/DnsHelper.java#L186

DnssecResolverApi class should override the inherited getClient() function to return the secure dnssecClient instead of the insecure dnsClient.

[Flowdalic]

I don't think there is any particular reason why we couldn't have DnssecResolverApi override getDnsClient(), if I understand what you are suggesting correctly. PRs welcome.

That said,

This results in DNS cache entries poisoned through previous insecure DNS client queries being accepted as DNSSEC valid without throwing exceptions.

should not be conceptionally possible. But first, in many cases MiniDNS does not throw an exception if validation fails. And more importantly, even with a poisoned/hijacked/injected DNS server being used, MiniDNS should never accept a DNSSEC response as valid, because MiniDNS does all the DNSSEC validation itself. This is a core design goal of MiniDNS, to get the DNSSEC validation as close to the application as possible.

Therefore, an attacker should, i.e., in the absence of bugs in MiniDNS, never be able to cause MiniDNS' DnssecQueryResult.isAuthenticData() returning true, unless the trust chain from the DNS root servers down to the query result could be validated by MiniDNS itself.

[Delicates]

should not be conceptionally possible.

Simply reporting what I have experienced with FairEmail. Telcos in Australia perform DNS hijacking on 4G/5G mobile data connections, so it's fairly easy for me to test.

When 2 FairEmail accounts are connecting to the same server while in a DNS hijacked environment:

• If both use DnssecResolverApi - both fail with DNSSEC error as expected.
• If one uses DnssecResolverApi but another uses ResolverApi - both connect without any errors which shouldn't be happening.

Also I've had scenarios where despite changing from DNS hijacked environment to a secure trusted DNS resolver - I am still unable to connect due to DNSSEC errors and have to wait a bit until cached TTL expires.

This all points to DnssecResolverApi cache being contaminated with poisoned results.

My understanding is that the telco DNS server strips away all signs of DNSSEC and returns responses that appear unsigned. This is what gets cached by the ResolverApi.getClient() queries, and it seems to affect DnssecResolverApi.getClient() queries.

The same cache is shared between ResolverApi.getClient() and DnssecResolverApi.getClient() which shouldn't be happening.

Apologies, I'm not a developer, wouldn't know how to do a PR.

I guess just need to add the following at the end of DnssecResolverApi.java:

    @Override
    public final AbstractDnsClient getClient() {
        return getDnssecClient();
    }

and update ResolverApi.java to remove final from getClient()?

[Flowdalic]

I get more and more the impression that the real problem is the lack of knowledge that DNS query results are only validated if DnssecQueryResult.isAuthenticData() returns true. IOTW, just because there was no exception doesn't mean that the DNS result could be validated.

This definitely needs to be better documented. But at the same time I am wondering if we should add DnssecResolverApi.resolveDnssecAuthenticatedOrThrow() methods.

This is what gets cached by the ResolverApi.getClient() queries and it seems to affect DnssecResolverApi.getClient() queries.

Are you sure that its a cache within MiniDNS that is affected here and not some other cache? That said, you should never have to use DnssecResolverApi.getClient(), it is only visible because DnssecResolverApi inherits ResolverApi. If you have a DnssecResolverApi instance, always use getDnssecClient().

But again, this is maybe not that relevant to your problem and I am actually currently not even sure what exactly your problem is any more, nor if DnssecResolverApi overriding getDnsClient() is the right solution.

You mention

When 2 FairEmail accounts are connecting to the same server while in a DNS hijacked environment:

• If both use DnssecResolverApi - both fail with DNSSEC error as expected.
• If one uses DnssecResolverApi but another uses ResolverApi - both connect without any errors which shouldn't be happening.

Could you show how you use MiniDNS' API exactly?

[M66B]

@Flowdalic

Could you show how you use MiniDNS' API exactly?

https://github.com/M66B/FairEmail/blob/master/app/src/main/java/eu/faircode/email/DnsHelper.java#L186

[Flowdalic]

https://github.com/M66B/FairEmail/blob/master/app/src/main/java/eu/faircode/email/DnsHelper.java#L186

https://github.com/M66B/FairEmail/blob/dd7775acfacc6180bcbc6c2901ae4ddfa67d6085/app/src/main/java/eu/faircode/email/DnsHelper.java#L219

confused me a bit, but I think the code is right. However, isn't it strange that it is actually insecure if the secure boolean is true?

I did not spot anything obvious after a, due to time contraints on my side, brief look at the code. However, if you believe that DnssecResolverApi overriding getDnsClient() fixes your issues, then you could achieve the same effect without modifying MiniDNS, simply by changing

https://github.com/M66B/FairEmail/blob/dd7775acfacc6180bcbc6c2901ae4ddfa67d6085/app/src/main/java/eu/faircode/email/DnsHelper.java#L187

to something like

AbstractDnsClient client;
if (dnssec)
    client = ((DnssecResolverApi) resolver).getDnssecClient()
else
    client = resolver.getClient();

I also looked briefly at MiniDNS' code and could not spot a place where the caches between the non-DNSSEC and DNSSEC world cross. They are deliberately separated. That said, I can't rule out a bug, because software tends to have those.

Note that in my experience DNSSEC related logic tends to have its pitfalls. How about we discuss your situation in a join video call? Drop me an email if you are interested, but note that I may not have time this week.

[M66B]

... confused me a bit, but I think the code is right.

It is confusing, but it is also correct. That said, it should be clear, so I've changed it: https://github.com/M66B/FairEmail/commit/1f63fea82e59ac99e363fcff33ce4461173d101a

[Delicates]

It is confusing, but it is also correct. That said, it should be clear, so I've changed it: M66B/FairEmail@1f63fea

I have tried a new build with this change ...

to something like

AbstractDnsClient client;
if (dnssec)
    client = ((DnssecResolverApi) resolver).getDnssecClient()
else
    client = resolver.getClient();

... and with this suggested change as well.

In both cases, straight after the switch to using the DNS-hijacking server there's an initial DNSSEC error, but on subsequent connection attempt the error disappears and connection is re-established.

I am observing the following behaviour in the authoritative server logs and trusted resolver logs... Please note there are 2 accounts connecting to the same server, one enforces FNSSEC and one doesn't...

With DNSSEC and DANE enforcement using secure trusted validating DNS Resolver server

On trusted resolver the following queries come in from the client:

• A query for server name
• DNSKEY queries for all zones recursed up to the root (.) - root DNSKEY gets queried twice for some reason.
• TLSA query for the service (when enforcing DANE)

Connection is successful.

Repeated connection attempts perform the same queries except for DNSKEY chasing. Most likely cache is used for TLD and root DNSKEY, only leaf DNSKEY is re-queried because it has short TTL.

With DNSSEC and DANE enforcement using telco's insecure untrusted hijacking DNS Resolver server

When changing network to use hijacking DNS resolver server, the authoritative server sees these queries from the telco resolver:

• A query
• AAAA query

There is no DNSKEY or TLSA queries. DNSSEC validation fails. Because of this DANE validation probably never started.

On repeated connection attempt:

• TLSA query

There are no A or AAAA or DNSKEY queries (the previous cached A and AAAA responses are used until their TTL expire). There is no DNSSEC error (suggesting cached A response is treated as valid). DANE validation fails.

DANE validation keeps failing for all future connection attempts.

With DNSSEC enforcement but no DANE enforcement using telco's insecure untrusted hijacking DNS Resolver server

If DANE enforcement is disabled and only DNSSEC enforcement is enabled on first connection, authoritative server receives:

• A query
• AAAA query

There is no DNSKEY queries. DNSSEC validation fails.

On second connection attempt:

• There are no queries to authoritative server (cached responses are probably used)

There is no DNSSEC error and connection succeeds.

On subsequent connection attempts, occasional A and AAAA queries are received (when cached entries expire). DNSSEC error never pops up again.

It seems DNSSEC validation is performed only on the very first query (there's probably a race condition which of the 2 accounts queries first). The query response seems to be cached regardless of its DNSSEC validity, and on subsequent calls the cached data is used as if it was valid.

Unfortunately I don't know how to find out the IP address of the telco 4G/5G DNS resolver, so I can't check what responses it sends to the queries.

[M66B]

@Delicates the commit doesn't change the behavior in any way. It is just the same, written down in another way.

[Delicates]

@M66B I understand, but I was hoping that using resolver.getDnssecClient() instead of resolver.getClient() would separate validating and non-validating caches used by the 2 accounts.

Looks like it didn't.

Perhaps this is Android resolver cache and not MiniDNS cache that is being shared, and MiniDNS trusts Android resolver's cache? I'm running out of ideas.

The only thing to try in FairEmail is to use isAuthenticData() check for DNSSEC enforced queries as suggested by @Flowdalic. That seems to work well for the MiniDNS DANE validation.

Never the less, if @Flowdalic thinks that the current FairEmail code is correct and it should be enforcing DNSSEC, but it doesn't - then there is still a concerning issue with MiniDNS where caching seems to be breaking DNSSEC enforcement.

[M66B]

@Delicates there is a separate cache for DNSSEC and non DNSSEC because a different instance of resolver API is used.

Also, isAuthenticData() is being used, even though not directly visible, through getUnverifiedReasons().