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

Implement DNS server #41

Merged
merged 7 commits into from Feb 17, 2023
Merged

Implement DNS server #41

merged 7 commits into from Feb 17, 2023

Conversation

jackyzha0
Copy link
Contributor

@jackyzha0 jackyzha0 commented Feb 9, 2023

Closes #26

  • DP hosts a DNS server to resolve DNSLink Queries
  • Get the person who owns the domain to add an NS record that points to the DNS server

@jackyzha0 jackyzha0 marked this pull request as ready for review February 10, 2023 06:07
@fauno
Copy link
Collaborator

fauno commented Feb 10, 2023

@jackyzha0 :D

Just a few comments to take into account for the implementation:

  • The port needs to be 53, you can't change the DNS port sadly. This means DP either

    • Runs as root and drops privileges.
    • The system disables privileged ports (sysctl -w net.ipv4.ip_unprivileged_port_start=0), there's a whole discussion if this is still needed, in regards of privilege dropping producing lots of security issues. This is simpler to implement right now IMO.
    • The system has a transparent proxy on the firewall between ports 53 and 5353.
    • Runs in a container and docker/podman/etc takes care of the port mapping.
    • Runs behind an actual nameserver who acts as a reverse proxy (the DNS terminology is different). This seems safer long term because you can host nameservers (nsd, knot, etc even bind) across several hosts and they have more protections, rate limits, cache, etc.
  • Right now we're setting _dnslink.YOUR-SITE IN NS D.P.HOST. so the CMS is expecting the DNS server to run on the same host as the API. The actual domain doesn't matter, but if you end up changing where the DNS is host we would have to update all of the domains. I can "future-proof" it by setting 2-3 nameservers like nsX.distributed.press and they can all point to the same address meanwhile, so they can change afterwards and we wouldn't have to update many domains too. It's something we can decide on March :)

@jackyzha0
Copy link
Contributor Author

@fauno I think the port defaults to 53 right now, the getPort is just for testing. I think Firewall proxy makes the most sense to me? In your second bullet point, do you mean that the DNS server runs on the same host as the DP API and not the Sutty side right?

@fauno
Copy link
Collaborator

fauno commented Feb 10, 2023

I think the port defaults to 53 right now

If it defaults to 53 you'll have to run node as root or you'll get a port binding error

In your second bullet point, do you mean that the DNS server runs on the same host as the DP API and not the Sutty side right?

Yeah what I meant is that the CMS expects the DNS on the same host as the API

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.

This looks great!

One other change that should be made, the link for IPFS/Hyper should use the DNS domain instead of the public key so that DP users will be linked to the "nice" URL

v1/dns/index.ts Outdated
type: dns2.Packet.TYPE.TXT,
class: dns2.Packet.CLASS.IN,
ttl: 60,
data: `dnslink=${links.ipfs.link}`
Copy link
Contributor

Choose a reason for hiding this comment

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

The link should be converted from ipns://KEY to /ipns/KEY

Note that instead of using the link we should be using the pubKey URL

Similarly we should use dnslink=/hyper/' for hyper, and using it's public key.

Make sure the hyper one comes after IPFS dnslink

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you may have pressed enter too early for your comment on Hyper haha! Can you clarify what the link structure for hyper looks like? Is it sufficient to just do dnslink=/hyper/${links.hyper.link}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you clarify what you meant by

One other change that should be made, the link for IPFS/Hyper should use the DNS domain instead of the public key so that DP users will be linked to the "nice" URL

I thought that link would have been the nice URL but perhaps I don't understand how the URLs map in IPFS

Copy link
Contributor

Choose a reason for hiding this comment

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

an you clarify what the link structure for hyper looks like? Is it sufficient to just do dnslink=/hyper/${links.hyper.link}

Whoops, forgot to finish my sentance there. It should be dnslink=/hyper/${links.hyper.raw.slice('hyper://').length)} Note how we want the raw public key and not the DNS domain which should be placed in the link.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought that link would have been the nice URL but perhaps I don't understand how the URLs map in IPFS

At the moment DP produces links that look like ipns://IPNS_KEY_HERE like ipns://bafywhateverelsehere. This is the "raw" link for the public key and is not human readable.

The "nice" URL is one that contains the domain name (which is what this DNS server resolves) like ipns://distributed.press.

The DNS server needs to serve TXT entries which link to the raw public keys of the sites since that's how clients turn the DNS domain name into the p2p URL that they can then resolve to the actual data.

@jackyzha0 jackyzha0 changed the title Draft: DNS server Implement DNS server Feb 16, 2023
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.

Tiny changes, but feel free to merge after checking them

v1/protocols/ipfs.ts Outdated Show resolved Hide resolved
v1/dns/index.ts Outdated Show resolved Hide resolved
jackyzha0 and others added 2 commits February 17, 2023 09:35
Co-authored-by: RangerMauve <RangerMauve@hotmail.com>
Co-authored-by: RangerMauve <RangerMauve@hotmail.com>
@jackyzha0 jackyzha0 merged commit 0cfd671 into v1-staging Feb 17, 2023
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.

None yet

3 participants