search

hildjj / dohdec

Lookup and decode DNS records using DNS-over-HTTPS (DoH)
MIT License
19 stars 5 forks source link

Support option to disable DNSSEC verification #37

Closed gnarea closed 1 year ago

gnarea commented 1 year ago

I'm using dohdec as a resolver in a DNSSEC library, which means Google/Cloudflare/etc shouldn't be doing the validation. The problem is that, when DNSSEC fails, the server doesn't return any records.

Would you accept a PR that implemented an option to enable the cd flag?

hildjj commented 1 year ago

Yes, I would take a patch for this. I also have an outstanding issue (#3) to check DNSSSEC results, which I'd be happy to close with a patch that points to your library, if you like.

gnarea commented 1 year ago

Thanks for the prompt reply, @hildjj!

That's fantastic. I'll create a patch first thing tomorrow.

I'll also ping you when I've finished the docs for the DNSSEC library.

hildjj commented 1 year ago

Nice. Will your lib also add caching, or pick a direction people should go for that? I feel like a) DNS caching is pretty easy to mess up, b) it interacts with DNSSEC in a few interesting ways, and c) people are used to having a single API to call that also has OS-wide caching built-in.

(I get this isn't the right place to talk about it, but hey, we're both here.)

gnarea commented 1 year ago

Thanks for bringing that up! I hadn't thought about caching but that's definitely something I'd love to offer. I just created an issue for that: https://github.com/relaycorp/dnssec-js/issues/79

I just implemented the option to set cd=1 in #38.

I'd be happy to propose another PR to document how to use dohdec with our DNSSEC library. I'll just need to write up our docs first (and I was planning to use dohdec in the "hello world" example, btw).

hildjj commented 1 year ago

If you don't have a CLI for dnssec-js, it might be interesting to make it a dependency of the dohdec CLI, and add a few options.

If that doesn't sound interesting to you, that's completely fine, and I'm still ok with pointing people to your code instead, but it might be fun to collaborate.

gnarea commented 1 year ago

dnssec-js doesn't have a CLI -- partly to keep it agnostic of any particular resolver-- but I agree it's worth considering making it a dependency of the dohdec CLI.

However, dnssec-js requires Node.js 16 or newer because one or two crypto algorithms weren't available in v14 (can't remember which). So maybe we should hold off until Node.js 14 reaches end-of-life in April next year? (Mainly because I think dohdec still follows Node's release schedule)

hildjj commented 1 year ago

I'm willing to accelerate the v14 deprecation for the command line, but agree we should wait to do that for the library until 14 is officially EOL. Updating node to run the CLI might break someone's workflow, but a) it won't break their code, and b) it's easy to work around with nve or nvm.

gnarea commented 1 year ago

Cool, in that case I'd be happy to propose a PR. What do you think the CLI switch should be? --dnssecClientSide? Or maybe repurpose --dnssec and make it take an optional value (remote|local), defaulting to remote?

hildjj commented 1 year ago

Let's take this to #3