relaycorp / doh-jvm

Basic DNS-over-HTTPS library for the JVM
https://central.sonatype.com/search?q=doh&namespace=tech.relaycorp
Apache License 2.0
13 stars 2 forks source link

An API to tell if a query was DNSSEC verified or not #154

Open singpolyma opened 1 year ago

singpolyma commented 1 year ago

Describe the problem

I would like to know if a DNS result was DNSSEC verified.

Describe the solution you'd like

I'd like a property on the Answer or similar that tells me if it was DNSSEC verified.

Describe alternatives you've considered

I know if the DNSSEC fails then a verifying resolver will just return an error, but I'm interested in telling the difference between "there was DNSSEC and it passed" and "there was no DNSSEC"

gnarea commented 1 year ago

Thanks for taking the time to suggest this enhancement @singpolyma!

I can see how a more granular failure result can be helpful in some cases, but can you please elaborate on what you're trying to achieve?

From a DNSSEC standpoint, there could be four possible outcomes. "there was DNSSEC and it passed" and "there was no DNSSEC" correspond to secure and insecure respectively, but there are two other failure statuses that could be relevant.

This sounds like a good idea in principle, but as a high-level where the primary functionality is DoH and DNSSEC is secondary, I need to be careful we're not making things too complicated, which is why learning about your specific use case can be helpful.

singpolyma commented 1 year ago

I can see how a more granular failure result can be helpful in some cases, but can you please elaborate on what you're trying to achieve?

Sure. There are cases where DNSSEC being present and valid is necessary (such as TLSA records) and also cases where DNSSEC being present is sufficient (such as to know the IP you got really do belong to the target server) but where a valid DNSSEC-less result can be used in conjunction with a fallback verification mechanism (such as CA certified TLS certificate).

gnarea commented 1 year ago

Thanks. Apologies for the delay.

Those are two very different use cases.

The first use case seems straightforward. We could introduce a new exception (e.g., DNSSECVerificationException) and throw it instead of LookupFailureException when the ad flag in the responseMessage below is false:

https://github.com/relaycorp/doh-jvm/blob/ee1b0537b8570237083cd9e5482fe7bd662a9ddb/src/main/kotlin/tech/relaycorp/doh/DoHClient.kt#L84-L87

We'd welcome a PR for that. If you'd like to proceed with it, please make sure to follow the current coding conventions and add a unit test for every new code execution branch (I think this will just introduce an if block, so one unit test will suffice). Also, FYI, you'll be prompted to sign a CLA.

The second use case deserves further consideration and, if I'm reading it correctly, we'd be talking about a different feature request: offer the option to disable DNSSEC verification (as a new optional parameter in lookUp()).

If DNSSEC verification fails, you wouldn't get an answer (e.g., dnssec-failed.org/A), so you'd have to repeat the query with DNSSEC verification disabled. I think this library should support the option to disable DNSSEC verification, but it should be up to the user to repeat the query with DNSSEC disabled.

Also, if we're introducing a flag to disable DNSSEC verification, we should consider the other DNSSEC-related flag that someone might want to include in the query: whether to include the RRSIG records for the answers. Even if we don't implement this second flag now, we should keep in mind that it's there when we name the first parameter (e.g., verifyDnssec: boolean = true instead of dnssec : boolean = true).

singpolyma commented 1 year ago

I have no interest in disabling verification. If DNSSEC verification fails then that should be either an error or no result, of course.

I'm only interested in knowing if DNSSEC existed and was verified or else was not present. I don't want not present to be an error, just to know when it happened.

singpolyma commented 1 year ago

So I think maybe your other feature you mentioned (returning RRSIG records if present) would be the thing to do here? If there are no RRSIG then we know there was no DNSSEC.