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

feat: always answer with the NS record in the authority section #87

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

fauno
Copy link
Collaborator

@fauno fauno commented Oct 30, 2024

if we don't do this, we're never going to see the resource record being found, because resolvers would recurse into the nameserver as the authority for this, even if intermediate nameservers know it.

if we don't do this, we're never going to see the resource record being
found, because resolvers would recurse into the nameserver as the
authority for this, even if intermediate nameservers know it.
@fauno fauno self-assigned this Oct 30, 2024
@fauno
Copy link
Collaborator Author

fauno commented Oct 30, 2024

this is needed for the onboarding site to confirm the dns is correctly setup, because it will recurse all nameservers up to api.distributed.press who won't know anything about this record

@fauno
Copy link
Collaborator Author

fauno commented Oct 30, 2024

Related: #69 (comment)

Copy link
Contributor

@RangerMauve RangerMauve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternately, can you request ?type=NS in the DOH request to see if they got the mapping for it?

dns/index.ts Outdated
type: dns2.Packet.TYPE.NS,
class: dns2.Packet.CLASS.IN,
ttl: TTL,
ns: 'api.distributed.press.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get the host param in apiBuilder and should pass it to the DNS init defined here

Maybe respond to CNAME with the host too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe respond to CNAME with the host too?

it should query A and AAAA addresses for the host and respond with that, because CNAME is frowned upon on the apex domain (some implementations ignore it but on the wild i've only found it on email notifications from twilio)

@RangerMauve
Copy link
Contributor

Looks like asking for NS wouldn't work.

curl --header "accept: application/dns-json" "https://cloudflare-dns.com/dns-query?name=_dnslink.docs.distributed.press&type=NS"

@fauno
Copy link
Collaborator Author

fauno commented Oct 30, 2024

Looks like asking for NS wouldn't work.

curl --header "accept: application/dns-json" "https://cloudflare-dns.com/dns-query?name=_dnslink.docs.distributed.press&type=NS"

I tested it with doh.js pointing to CF's DOH and it returned answers for NS sutty.nl though

I'll check tomorrow

@RangerMauve
Copy link
Contributor

Yeah, NS for the top level seems to work, not for the _dnslink subdomain though

@fauno
Copy link
Collaborator Author

fauno commented Oct 31, 2024

Yeah, NS for the top level seems to work, not for the _dnslink subdomain though

This is what this PR fixes! It doesn't work because the resolver goes asking for the NS record and the domain NS replies that it's api.distributed.press. which works, but actually it's saying "you should ask api.distributed.press." and it currently can't answer.

You can test it like this to see I'm asking for the NS record, returned by DO, but it continues and reaches DP, who can only respond with TXT records so not what I'm asking:

>_ drill -T _dnslink.docs.distributed.press ns
.       518400  IN      NS      a.root-servers.net.
.       518400  IN      NS      b.root-servers.net.
.       518400  IN      NS      c.root-servers.net.
.       518400  IN      NS      d.root-servers.net.
.       518400  IN      NS      e.root-servers.net.
.       518400  IN      NS      f.root-servers.net.
.       518400  IN      NS      g.root-servers.net.
.       518400  IN      NS      h.root-servers.net.
.       518400  IN      NS      i.root-servers.net.
.       518400  IN      NS      j.root-servers.net.
.       518400  IN      NS      k.root-servers.net.
.       518400  IN      NS      l.root-servers.net.
.       518400  IN      NS      m.root-servers.net.
press.  172800  IN      NS      a.nic.press.
press.  172800  IN      NS      e.nic.press.
press.  172800  IN      NS      b.nic.press.
press.  172800  IN      NS      f.nic.press.
distributed.press.      3600    IN      NS      ns1.digitalocean.com.
distributed.press.      3600    IN      NS      ns2.digitalocean.com.
distributed.press.      3600    IN      NS      ns3.digitalocean.com.
_dnslink.docs.distributed.press.        600     IN      NS      api.distributed.press.
_dnslink.docs.distributed.press.        604800  IN      TXT     "dnslink=/ipns/k51qzi5uqu5dicruj8whyde1z5vtm2tg6q1gey17nb2trkuh4b0m69sl6z85cw/"
_dnslink.docs.distributed.press.        604800  IN      TXT     "dnslink=/hyper/smqq19f3gbbz7sn9zko589az9x8ip4ky5g4nce5rnzgywfi13qso/"

@RangerMauve RangerMauve merged commit 01a4121 into hyphacoop:main Oct 31, 2024
1 check failed
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

Successfully merging this pull request may close these issues.

2 participants