ipfs / helia

An implementation of IPFS in JavaScript
https://helia.io
Other
812 stars 81 forks source link

DoH queries with numeric `type` fail #474

Closed lidel closed 2 months ago

lidel commented 3 months ago

Found DoH problem, seems we started sending a number instead of string value:

(type=16) produces no results:

$ curl -H 'accept: application/dns-json' -sS 'https://delegated-ipfs.dev/dns-query?name=_dnslink.almonit.eth&type=16'
{"AD":true,"CD":false,"RA":true,"RD":true,"TC":false,"Status":2,"Question":[{"name":"_dnslink.almonit.eth.","type":"TXT"}],"Answer":[]}

(type=TXT) produces valid results:

$ curl -H 'accept: application/dns-json' -sS 'https://delegated-ipfs.dev/dns-query?name=_dnslink.almonit.eth&type=TXT'
{"AD":true,"CD":false,"RA":true,"RD":true,"TC":false,"Status":0,"Question":[{"name":"_dnslink.almonit.eth.","type":16}],"Answer":[{"name":"_dnslink.almonit.eth","type":16,"TTL":3600,"data":"\"a=0xb365d73dcc34b2ea5E3969687954240e187B43eA\""}]

Ref. https://developers.cloudflare.com/1.1.1.1/encryption/dns-over-https/make-api-requests/dns-json/ suggests both are allowed, but the resolver.cloudflare-eth.com we use for ENS only returns results for text.

Can we switch back to text? TXT is way easier to reason that some random number.

SgtPooki commented 3 months ago

@lidel It seems like this is a bug in the delegated-ipfs.dev/dns-query server rather than helia/ipns implementation, because https://cloudflare-dns.com/dns-query accepts both:

This is a bug in https://resolver.cloudflare-eth.com/dns-query server rather than helia/ipns implementation:

╰─ ✔ ❯ curl -H 'accept: application/dns-json' -sS 'https://resolver.cloudflare-eth.com/dns-query?name=_dnslink.almonit.eth&type=16'
{"AD":true,"CD":false,"RA":true,"RD":true,"TC":false,"Status":2,"Question":[{"name":"_dnslink.almonit.eth.","type":"TXT"}],"Answer":[]}%

╰─ ✔ ❯ curl -H 'accept: application/dns-json' -sS 'https://resolver.cloudflare-eth.com/dns-query?name=_dnslink.almonit.eth&type=TXT'
{"AD":true,"CD":false,"RA":true,"RD":true,"TC":false,"Status":0,"Question":[{"name":"_dnslink.almonit.eth.","type":16}],"Answer":[{"name":"_dnslink.almonit.eth","type":16,"TTL":3600,"data":"\"a=0xb365d73dcc34b2ea5E3969687954240e187B43eA\""}]}%
SgtPooki commented 3 months ago

Also, the core type that's being used is at https://github.com/multiformats/js-dns/blob/1ab3fc712bb4834167e03708c2fe5b6e471c8039/src/index.ts#L92-L97

SgtPooki commented 3 months ago

FYI that we can allow querying with TXT instead of 16 with the changes in https://github.com/multiformats/js-dns/pull/5, but need to wait for it to propogate.

lidel commented 3 months ago

Thank you for cleaning this up upstream. In the meantime, I've come up with a quick and dirty fixup on our proxy's end: https://github.com/ipshipyard/waterworks-infra/pull/65

achingbrain commented 3 months ago

Has anyone let CloudFlare know their gateway isn't compliant with their documentation?