Processing TSLA records for DANE fails

M66B commented 5 months ago

Again, I'm not an expert, but I believe this test is incorrect:

if (record.type == Record.TYPE.TLSA && record.name.equals(req))

https://github.com/MiniDNS/minidns/blob/master/minidns-dnssec/src/main/java/org/minidns/dane/DaneVerifier.java#L123

And should be:

if (record.type == Record.TYPE.TLSA)

Delicates commented 5 months ago

The above if statement shouldn't be changed, but I think there's an easy way to fix the failure that prompted this issue being raised by MiniDNS relying on the DNS resolver server to perform DNSSEC-validated CNAME-chasing.

This verification if statement fails when TLSA records are provisioned with CNAME or DNAME aliases. Such a DNS response could return records where:

for the record.name.equals(req) the record.type is Record.TYPE.CNAME
for all Record.TYPE.TLSA records, the record.name would not equal req.

Verification seems to be missing:

DNSSEC-secure CNAME-chasing of the TLSA records, and
DNSSEC-secure CNAME-expansion of the TLS server name (TLSA base domain).

The TLSA CNAME-chasing is defined in the original DANE RFC: https://datatracker.ietf.org/doc/html/rfc6698#appendix-A.2

This spec was further updated and explained by changing the original DANE definition of the TLSA base domain, and by requiring its CNAME-expansion in the DANE Update RFC: https://datatracker.ietf.org/doc/html/rfc7671#section-7

These gaps were flagged by @vdukhovni as the second bullet point in issue #105.

1. Easy fix by relying on DNS resolver to do CNAME-chasing of the `TLSA` records

If MiniDNS can't do it's own CNAME-chasing, you can typically rely on DNS resolvers to do it, and to return responses that contain both CNAME records as well as target records, and I think CNAME-chase records typically come first in the list.

In this code section, all records in this particular response have already been DNSSEC-validated, so woulc contain a DNSSEC-secure CNAME chain. Some initial improvement for CNAME-chasing of TLSA records probably can be easily implemented by adding else if after the above if statement inside the loop, along the lines of (apologies, I'm not a developer so my syntax is probably completely wrong):

else if (record.type == Record.TYPE.CNAME && record.name.equals(req)) {
    req = record.target;
}

It wouldn't be perfect, but it would be a step in the right direction for addressing the second point raised by Victor in issue #105.

2. Further improvement by relying on DNS resolver to do CNAME-expansion of the TLS server name (TLSA base domain)

Implementing CNAME-expansion would be a bit more complicated, but doable with one or two additional DNS queries.

Perform DNSSEC-verified query of the the hostName A record to check for existence of a CNAME
- If CNAME exists, perform CNAME-chase to determine the new CNAME-expanded base domain baseName for TLSA.
- if CNAME record doesn't exist, baseName = hostName
Run the DNSSEC-verified TLSA query under the baseName
- If a valid TLSA is not found, re-run DNSSEC-verified TLSA query under the original hostName

vdukhovni commented 5 months ago

If MiniDNS can't do it's own CNAME-chasing, you can typically rely on DNS resolvers to do it, and to return responses that contain both CNAME records as well as target records, and I think CNAME-chase records typically come first in the list.

The ordering of RRs within a DNS response section (answer, authority or additional) has no significance, and one cannot (must not) rely on the records appearing in any particular order, even the records in a single RRset (same owner, class, type and covered type if RRSIG) need not be consecutive.

The simplest way to know whether a CNAME chain leading to a final answer is secure is to obtain the answer from a local validating resolver, which will only set the AD bit if the entire chain is correctly signed. This includes possible DNAME expansion, where the synthesised CNAMEs are not directly signed, but the DNSSEC signatures on the relevant DNAME RRs are sufficient to validate the associated CNAME.

Delicates commented 5 months ago

which will only set the AD bit if the entire chain is correctly signed.

AD or AA bit.

I'm not quite clear on how MiniDNS currently implements the isAuthenticData() function. There seems to be a TODO: comment mentioning checking for AD bit in there.

Does isAuthenticData() actually ensure DNSSEC validation, or is it just a placeholder for a future implementation?

I'm really interested in being able to plug MiniDNS into a trusted and secure DNS resolver that does all the DNSSEC verification as per @vdukhovni's advice, and then MiniDNS just checks for AD or AA query response flags, like it says in the TODO comment.

Though I'm not sure if there's an API that can tell MiniDNS that the resolver is secure, or if it's a responsibility of the calling application to manage the secure resolvers that appear and disappear when a mobile system moves between networks (e.g. Android Private DNS).

vdukhovni commented 5 months ago

An RFC-compliant validating resolver only sets the AD bit when all the data in the answer section (if non-empty) and/or authority section (for NODATA, NXDOMAIN or YXDOMAIN responses) is DNSSEC-validated. If there's a chain of CNAMEs leading to an answer all the CNAMEs must be validated (or be synthesised from a signed DNAME).

There is not presently a standard API or protocol for determining whether the stub resolver library is talking to a "local" resolver. The only step in that direction I am aware of is that non-ancient glibc versions by default ignore the AD bit unless the system administrator added "options trust-ad" to the resolv.conf file, which they should only do when the network path to the configured full-service resolver(s) is trusted (loopback or internal LAN where risk of packet injection is believed to be low, but ideally only loopback).

MiniDNS / minidns