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

Gateway: ensure consistent caching of DNS records #480

Open
hsanjuan opened this issue Oct 6, 2023 · 4 comments
Open

Gateway: ensure consistent caching of DNS records #480

hsanjuan opened this issue Oct 6, 2023 · 4 comments
Assignees
Labels
dif/easy Someone with a little familiarity can pick up effort/hours Estimated to take one or several hours kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@hsanjuan
Copy link
Contributor

hsanjuan commented Oct 6, 2023

The gateway code uses https://github.com/multiformats/go-multiaddr-dns which by default would use net.DefaultResolver and not cache anything. It also registers two default resolvers for .eth and .crypto. In this case using DoH urls for which it uses https://github.com/libp2p/go-doh-resolver, which does include caching of results.

This means some dns results are going to be cached while others depend on what DNS resolver Go is actually using (which depends on CGO iirc). When not using CGO, Go would use its own resolver and not cache anything. When using CGO, go would use the system's resolver which may or not cache stuff depending what is on the system.

Is my understanding correct @lidel ?

Ideally we should be caching all results. This has been used before in the ecosystem: https://github.com/rs/dnscache

@hsanjuan hsanjuan added the need/triage Needs initial labeling and prioritization label Oct 6, 2023
@hsanjuan hsanjuan self-assigned this Oct 6, 2023
@lidel
Copy link
Member

lidel commented Oct 7, 2023

Yes, we seem to lack cache for the default DNS resolver from OS. Having universal cache in boxo/gateway makes sense.

cc @hacdias - you have been looking at this in context of TTL, thoughts where would be best place to wire it up?

@lidel lidel added P2 Medium: Good to have, but can wait until someone steps up dif/easy Someone with a little familiarity can pick up effort/hours Estimated to take one or several hours kind/bug A bug in existing code (including security flaws) and removed need/triage Needs initial labeling and prioritization labels Oct 7, 2023
@hacdias
Copy link
Member

hacdias commented Oct 9, 2023

I'm working on reworking the namesys package here: #459. This will allow us to bubble up TTLs to the gateway and actually use them in requests. I don't mind adding support for DNS TTLs - but perhaps in a separate PR.

I also want to mention that DNS caching has been talked here before: multiformats/go-multiaddr-dns#28

@hsanjuan
Copy link
Contributor Author

TTL bubbling is a bit different from caching. Just saying there should be a layer (perhaps above Resolver) that does the caching consistently for everything that is resolved (based on TTL, why not).

@hacdias
Copy link
Member

hacdias commented Oct 10, 2023

Yes, #459 also includes caching, but just for IPNS. If the DNSResolver were to return a TTL, namesys (which uses both IPNSResolver and DNSResolver) would cache resolved dns lookups.

@BigLep BigLep mentioned this issue Nov 9, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dif/easy Someone with a little familiarity can pick up effort/hours Estimated to take one or several hours kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
No open projects
Status: 🥞 Todo
Development

No branches or pull requests

3 participants