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: report detailed DNS errors with Extended DNS Errors in the go resolver #63192

Open
mateusz834 opened this issue Sep 24, 2023 · 7 comments · May be fixed by #63201
Open

net: report detailed DNS errors with Extended DNS Errors in the go resolver #63192

mateusz834 opened this issue Sep 24, 2023 · 7 comments · May be fixed by #63201
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mateusz834
Copy link
Member

Currently when a query fails we return a "server misbehaving" error. RFC 8914 allows the resolver to include detailed errors about the failure of a DNS query. It would be nice to support it in the pure go resolver. We already support EDNS(0), so it should be fairly easy to add.

The only thing that I am not sure about is the optional EXTRA-TEXT field of the EDE Option, it would be nice to include it in the error, but it can be an arbitrary UTF-8 string. We should probably not include arbitrary characters in the error string. Not sure about that.

CC @mjl- (#63116 (comment))

@mjl-
Copy link

mjl- commented Sep 24, 2023

For seeing this in action, try dig dnssec-failed.org @1.1.1.1, dig dnssec-failed.org @8.8.8.8 or dig dnssec-failed.org @127.0.0.1 against a locally installed unbound with ede enabled:

# cat /etc/unbound/unbound.conf.d/ede.conf
server:
    ede: yes
    val-log-level: 2

I have an interest in this as part of the need for a dnssec-aware non-validating stub resolver package. Perhaps #13279 should be involved.

As for the error message, it could be prefixed with "error from remote: ...", and if deemed dangerous, we could show only non-control, ascii-only strings.

@seankhliao
Copy link
Member

maybe a custom error type to be used with errors.As and the extra text as a field not included in the default Error() string?

@mateusz834
Copy link
Member Author

mateusz834 commented Sep 25, 2023

I have an interest in this as part of the need for a dnssec-aware non-validating stub resolver package. Perhaps #13279 should be involved.

Could you elaborate more about your use case for this? What is your use case for the exported extended DNS errors type? Why there is a need for #13279, are you interested in the AD bit access?

@mjl-
Copy link

mjl- commented Sep 25, 2023 via email

@mateusz834
Copy link
Member Author

I am adding support for (outgoing) DANE and TLS-RPT to my mail server (mox). For TLS reporting, I need to be able to say why a delivery attempt over SMTP failed. One of those reasons is:
"dnssec-invalid": This indicates that no valid records were returned from the recursive resolver.

When the dnssec-invalid should be returned? On DNSSEC failures from LookupHost (A/AAAA), LookupMX or failures from the TLSA record query.

So yes, I also need the "authentic data" bit. And I'll be needing a LookupTLSA function (and others will want more lookup functions, e.g. for sshfp, smimea, etc). I started prototyping a resolver based on github.com/miekg/dns that adds an "authentic data" bool return value to each Lookup function. Though I'm starting to think it could be better to change that bool into a struct so additional result fields can be added in the future..

I don't think we will ever add LookupTLSA (see #35061).

  • Ignore /etc/hosts (for some freshly setup cloud vm's it causes the fqdn to resolve to a loopback ip instead of the actual public ips).

That can be solved by a proper DNS resolver and by querying the A/AAAA resources. LookupHost needs to support hosts file, because we emulate getaddrinfo.

  • Probably a few functions that keep behaviour closer to DNS (e.g. LookupCNAME now returns no error if a cname record doesn't exist, instead of nxdomain, that surprised me).

Also note it is broken now #59943.

I would also like a full dnssec-validating caching recursive resolver as a Go package.

#13279 (comment)

@gopherbot
Copy link

Change https://go.dev/cl/530876 mentions this issue: net: report detailed DNS errors with Extended DNS Errors

@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 25, 2023
@thanm thanm added this to the Backlog milestone Sep 25, 2023
@thanm
Copy link
Contributor

thanm commented Sep 25, 2023

@ianlancetaylor @neild

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants