Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: validate DNSSEC in Go's DNS resolver #13279

Open
bradfitz opened this issue Nov 16, 2015 · 25 comments
Open

net: validate DNSSEC in Go's DNS resolver #13279

bradfitz opened this issue Nov 16, 2015 · 25 comments
Milestone

Comments

@bradfitz
Copy link
Contributor

DNSSEC is being deployed.

We should support it eventually.

@mwwaters
Copy link
Contributor

mwwaters commented Oct 1, 2018

I am working on a proposal for adding DNSSEC to Go. DNSSEC validation requires multiple queries up to the DNSKEY RRSet of root.

There are a few solutions I have considered for the proposal. All solutions will have x/net/dns/dnsmessage adding necessary RRs and a new package, x/net/dns/dnssec, to perform validation. Any solution will also have to support EDNS0 in case of DNSSEC queries. The three possible solutions include:

  1. Vendor dnssec and add exported functions to net such as LookupIPAddrSecure, LookupMXSecure, etc.

  2. Add a LookupBytes function returns the byte slice of the DNS response for the caller. The net package still checks query's Header and Question. The call will have to re-parse the byte slice but can skip over the question. From exchange() up the call stack, a []byte return would need to be added.

  3. LookupRRSet function which uses duplicate structures defined in the net package. The dnssec package can take net package's structures and transform them back to dnsmessage package. Using the dnsmessage structures is not an option since the caller's dnsmessage package is different than the vendored package.

#1 is the cleanest if the extra dependency is acceptable. #2 is clean for unix, but ugly for Windows. Windows 7 and up can support DNSSEC lookups, but the bytes of the DNS response would have to be reconstructed. #3 has duplication of code and two copies made of the unpacked response.

I would lean towards #1, but do not know how serious the dependency issue is. If the dependency issue is a deal-breaker, then I can make a proposal for #3. The proposal would target Go 1.13's release cycle.

@ianlancetaylor
Copy link
Contributor

Vendoring dnssec is OK; we've already got dnsmessage in vendor/golang_org/x/net/dns/dnsmessage.

Why LookupIPAddrSecure? Why not just make LookupIPAddr use DNSSEC if available?

@mwwaters
Copy link
Contributor

mwwaters commented Oct 3, 2018

The most immediate reason is systemd-resolved's stub resolver (the 127.0.0.53 /etc/resolv.conf server) always strips out DNSSEC records. On my own computer (Ubuntu 18.04), "dig . DNSKEY" gave an empty result while "dig @8.8.8.8 . DNSKEY" gave the expected result. I had to disable systemd-resolved to get a DNSKEY lookup. Ubnutu Bug Report

To soft-fail the lookup routine, one way is to have always require valid DNSSEC if DNSKEY/RRSIG for root is successful. It is unlikely to have records stripped out for other queries if the root DNSKEY lookup is successful. To hard-fail, what about adding a parameter such as "DNSSECStrict" to Resolver? Hard-fail means either getting validation of either NSEC of a DS record or full validation.

I would also like a LookupTXTSecure function in particular, so the caller could know the TXT record was verified. Verifying TXT records in particular has a use for the MTA-STS proposal and for DMARC/DKIM records. DNS-01 challenges with ACME could also use knowledge of whether TXT records are secure, though CAs would also need a CAA record lookup. LookupTXTSecure could be added later though, in a different issue and CL.

@mikioh
Copy link
Contributor

mikioh commented Oct 3, 2018

@mwwaters,

I'm not sure what's the goal of this issue but enabling security-aware DNS stub resolver as defined in RFC 4033 sounds reasonable, I mean, not entering the territory of DNSSEC-based technologies such as DANE, DSO or other fancy features from IETF dnsop-wg.

As you mentioned, a small start is always preferable, for example, 1) add a few RRs to dns/dnsmessage package of x/net, 2) create dns/dnssec package of x/net for implementing a validator for stub resolver, 3) stop, take a breath and re-consider adaptation to the standard library; API surface (e.g., why not extending net.Resolver?), dependency management (especially for cipher suite packages), etc. I feel like having dns/dnslookup package of x/net is not so bad.

@iangudger, thoughts?

@iangudger
Copy link
Contributor

I am all for adding DNSSEC and other features to both x/net/dns and the standard library. The approach listed by @mikioh sounds reasonable to me.

That said, do @mikioh or @bradfitz (or someone else?) have the bandwidth to review changes like this?

@mwwaters
Copy link
Contributor

mwwaters commented Oct 3, 2018 via email

@halturin
Copy link

halturin commented May 4, 2020

is it still a thing? any chance to have it as a part of std lib?

@mateusz834
Copy link
Member

mateusz834 commented Dec 7, 2022

Probably this assumes that we will have to put the root KSK keys in the binary, but I don't think that we will be able to implement RFC 5011 in the net package here (for root KSK rollovers).
Also i don't see any reason to validate DNSSEC inside the Go resolver. DNSSEC validation should be done by external resolvers.

@PSanetra
Copy link

PSanetra commented Dec 7, 2022

@mateusz834 how do external resolvers solve the KSK key issue? (I would assume the key might be configurable somewhere on the system, similar to TLS root certificates.)

I think the main reason to implement the validation in the Go DNS resolver is to prevent issues with MITM attacks between external resolvers and the application. See https://www.rfc-editor.org/rfc/rfc4033#section-7

@mateusz834
Copy link
Member

mateusz834 commented Dec 8, 2022

@PSanetra
They implement RFC 5011.

I don't like the idea of implementing DNSSEC directly into the net library. First of all we don't always use the go resolver. We are not able to DNSSEC validate the getaddrinfo, res_search (on Unix), on windows the DnsQuery does not seem to allow sending the DNSSEC OK bit, so this would allow only to validate the pure go resolver and will cause a inconsistentsy.

To avoid MiTM you should be running a localhost DNS server with DNSSEC validation.

@PSanetra
Copy link

PSanetra commented Dec 8, 2022

@mateusz834 yes, I agree implementing RFC 5011 seems to be too much for this standard library functionality. I would see the source of the KSK to be passed to the the Go DNS resolver or to be configured in some way, before the DNSSEC validation would be available. To use some service that implements RFC 5011 would be in the responsibility of the user.

What do you mean with "we don't always use the go resolver"? I would see use cases where the go DNS resolver is not used to be out of this issues scope.

@mateusz834
Copy link
Member

@PSanetra
We don't use the go resolver while forcing the cgo resolver (GODEBUG/build flags) or when the dynamic selection decided to force the cgo (because of something that we don't support, like unknown module in nsswitch.conf).

@pemensik
Copy link

I would oppose doing DNSSEC validation in the stub library. DNSSEC requires more records with larger responses. To have decent performance, it needs localhost cache. Which should be doing the validating and create resulting AD bit in reply.

What is more important is proper EDNS0 support, to be able to handle larger responses than legacy 512 bytes. That is what stub library has to support. Related to openshift/cluster-dns-operator#276.

@mateusz834
Copy link
Member

@pemensik Side note:

What is more important is proper EDNS0 support, to be able to handle larger responses than legacy 512 bytes. That is what stub library has to support.

The go resolver by default uses EDNS0 now (since go 1.19, #301fd8ac8b)

@pemensik
Copy link

Nice! There are few networks where this causes issues. It would be nice to have that configurable and make possible to send queries without EDNS0. Good resolvers would handle it better, but bad ones are still present. Could it read options edns0 from /etc/resolv.conf for example? Recent glibc also added trust-ad option, that does not have equivalent?

@ianlancetaylor
Copy link
Contributor

We've been advertising EDNS0 in our DNS packets since the Go 1.19 release (https://go.dev/doc/go1.19#minor_library_changes, in the entry for the net package). We have not seen any reported problems. We can add a knob if necessary, but we would prefer to only add a knob if it is truly needed.

@mateusz834
Copy link
Member

@pemensik

Recent glibc also added trust-ad option, that does not have equivalent?

We add the AD bit to queries when trust-ad is present in the resolv.conf file (this is what glibc does), ad bit is not used in responses in any way.

@pemensik
Copy link

pemensik commented Jul 1, 2023

@mateusz834

We add the AD bit to queries when trust-ad is present in the resolv.conf file (this is what glibc does), ad bit is not used in responses in any way.

Oh, this is not true by far. If you have running dnssec-trigger+unbound or any other DNSSEC validating resolver on localhost, each client does not need to double-verify already verified signatures. AD bit is set only when that name were verified by cache.

That makes dig example.com report AD bit, but dig google.com do not. AD bit is quite useful if you have trusted channel to validating resolver. Which running it on localhost is, alternative is using DoT or DoH. Which is why trust-ad is needed. Plain UDP forwarding to 8.8.8.8 should not consider AD bit valid, because it crosses unprotected network.

I think ssh uses just AD bit to consider SSHFP records verified. That is okay.

@pemensik
Copy link

pemensik commented Jul 1, 2023

We've been advertising EDNS0 in our DNS packets since the Go 1.19 release (https://go.dev/doc/go1.19#minor_library_changes, in the entry for the net package). We have not seen any reported problems. We can add a knob if necessary, but we would prefer to only add a knob if it is truly needed.

There were strange behaviour reported on systemd:

It should never be required for proper implementations. But there might appear some broken software, which might require turning it off.

@pemensik
Copy link

pemensik commented Jul 1, 2023

Is there any Go-based DNSSEC validating implementation already? coredns does not seem to implement that. Is there any cache written in Go, which implements that? Reinventing a wheel with DNSSEC is a bad idea. That protocol is not simple to implement well.

@mateusz834
Copy link
Member

@pemensik #13279 (comment)

I don't get your point here. We are not validating DNSSEC in any way in the go resolver. We set the AD bit in the query when trust-ad is present, mostly because there are DNS resolvers that don't perform DNSSEC validation when they don't get the AD bit in the query (https://doc.powerdns.com/recursor/dnssec.html#process).

We are not exposing the AD bit in responses in any way, just setting the AD bit in the query (this is even what the dig command does by default).

@pemensik
Copy link

pemensik commented Jul 1, 2023

Exposing AD bit present in response via generic API would be more useful than implementing whole validation chain IMO. Especially in low level framework. Information whether the result is signed (and verified) should be provided by special service like Powerdns Recursor. Simple boolean check that the response were validated should be enough. If that is not yet possible, please add that.

@mateusz834
Copy link
Member

Exposing AD bit present in response via generic API would be more useful than implementing whole validation chain IMO.

True, but the net package should work well cross-platform, but there is no way to get the AD bit through the windows DNS APIs, secondly the net package does not use the DNS all the time, for example LookupHost, LookupAddr responses might come from /etc/hosts file, or even through glibc getaddrinfo/getnameinfo, this might or might not use the DNS (depending on the nss modules), i am not aware of any way to get the AD bit from getaddrinfo/getnameinfo.

@pemensik
Copy link

pemensik commented Jul 1, 2023

I think the only way to obtain AD in response is using res_nquery or res_nsearch from libresolv of glibc. Or similar DNS-only library. It kind of make sense, since AD makes sense only for DNS specific results. Unfortunately getaddrinfo and getnameinfo do not pass that information to userspace. I have looked inside glibc, there is no flag for it prepared.

If other platforms do not export that or it does not make sense like /etc/hosts file source, then false is good value. Since getaddrinfo with AF_UNSPEC makes two queries, it would have to combine two flags somehow.

@ianlancetaylor
Copy link
Contributor

It should never be required for proper implementations. But there might appear some broken software, which might require turning it off.

Let's see if such broken software appears. Let's not fix it proactively. As the release notes say, there is already a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants