DNSCrypt / dnscrypt-proxy

dnscrypt-proxy 2 - A flexible DNS proxy, with support for encrypted DNS protocols.
https://dnscrypt.info
ISC License
11.3k stars 1.01k forks source link

Local DNS over HTTPS - respond HTTP 400 error for HTTP GET requests #2012

Closed PeterDaveHello closed 2 years ago

PeterDaveHello commented 2 years ago

Output of the following commands:

./dnscrypt-proxy -version: 2.1.1

./dnscrypt-proxy -check:

[2022-01-31 19:22:00] [NOTICE] dnscrypt-proxy 2.1.1
[2022-01-31 19:22:00] [NOTICE] Source [public-resolvers] loaded
[2022-01-31 19:22:00] [NOTICE] Source [relays] loaded
[2022-01-31 19:22:00] [NOTICE] Configuration successfully checked

./dnscrypt-proxy -resolve example.com:

Resolving [example.com] using  port 53

Resolver      : 91.219.215.227

Canonical name: example.com.

IPv4 addresses: 93.184.216.34
IPv6 addresses: 2606:2800:220:1:248:1893:25c8:1946

Name servers  : b.iana-servers.net., a.iana-servers.net.
DNSSEC signed : yes
Mail servers  : 1 mail servers found

HTTPS alias   : -
HTTPS info    : -

Host info     : -
TXT records   : v=spf1 -all, yxvy9m4blrswgrsz8ndjh467n2y7mgl2

What is affected by this bug?

iOS devices or other DoH clients using HTTP GET requests to send the query can't use dnscrypt-proxy as a DNS over HTTPS server

When does this occur?

When you set an iOS v14+ device to use dnscrypt-proxy as a DNS over HTTPS server,

Where does it happen?

On the dnscrypt-proxy application

How do we replicate the issue?

Set up a dnscrypt-proxy with local DoH server enabled behind any https enabled reverse proxy, write an iOS configuration file, or use some tools like https://dns.notjakob.com to generate one, to make an iOS device to use it as a DoH server, you'll see all the requests from iOS devices got the http 400 error.

A node.js powered command line tool called dohdec-cli can also reproduce this issue.

Or, simply use curl or wget to send to HTTP GET requests like: curl -X GET 'https://dns.local/dns-query?dns=AAABAAABAAAAAAAAA3d3dwdleGFtcGxlA2NvbQAAAQAB' -v

Expected behavior (i.e. solution)

Expect that dnscrypt-proxy can respond to the query properly.

Other Comments

Requests come from iOS and dohdec-cli seemed to be HTTP GET request, with ?dns=AAABAAABAAAAA... query string, not like the requests from Firefox or dnscryt-proxy, is HTTP POST, not sure if it's because that dnscrypt-proxy doesn't support to response DoH response yet?

I set the log level to 0 in dnscrypt-proxy.toml, which is the most verbose one, but didn't see any additional info when returning 400 error.

jedisct1 commented 2 years ago

The Local DoH server was only designed to work around Firefox restrictions to enable ESNI. It was never tested nor intended to be used in other contexts. In particular, it should not be publicly exposed.

Adding support for GET shouldn't be too difficult, if this is all it takes to make it work for your use case. But I'd really recommend using doh-server instead.

jedisct1 commented 2 years ago

c10e6e0 adds support for GET request.

Totally untested, so your feedback would be useful :)

PeterDaveHello commented 2 years ago

Cool, thanks @jedisct1! Looks like it'll work in many cases. Still found some 400 errors, for query like: /dns-query?dns=AAABAAABAAAAAAABBGluaXQEcHVzaAVhcHBsZQNjb20AABwAAQAAKQIAAAAAAABQAAwATAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, not sure if it's possible to add some logs to explain the problem to help debug issues?

According to the response body, I believe that this section was triggered:

    if len(packet) < MinDNSPacketSize {                                                                                                                                                                                                       
        writer.Header().Set("Content-Type", "text/plain")                                                                                                                                                                                     
        writer.WriteHeader(400)                                                                                                                                                                                                               
        writer.Write([]byte("dnscrypt-proxy local DoH server\n"))                                                                                                                                                                             
        return                                                                                                                                                                                                                                
    }

Do you think it's possible to integrate some other golang well-implemented DoH solution, instead of trying to write another one here? Thanks again!

PeterDaveHello commented 2 years ago

@jedisct1 may I send a patch to allow the accept header to be */* instead of application/dns-message, I believe that */* means application/dns-message is acceptable. Currently, the previous behavior seems to lead the 400 error result in some cases, I checked the behavior of Google or Cloudflare DoH implementation, doesn't seem to be so strict(That's why I wonder if there's some other implementation can be integrated here directly ;))

PeterDaveHello commented 2 years ago

Using another dnscrypt-proxy to act like a DoH client to verify the implementation, also found some HTTP 499 error, I have no idea yet.

jedisct1 commented 2 years ago

499 errors seems to be returned by nginx. I've never run such a complicated setup, but I guess these may be returned by nginx when dnscrypt-proxy closes the connection because of a timeout.

Regarding the content-type, accepting anything opens the service to cross-protocol attacks. Does iOS not send a proper Accept: header?

PeterDaveHello commented 2 years ago

@jedisct1 I didn't dig deeper about iOS behavior, but with all due respect, I'm afraid that I can't manually test and check all the details and the specs, especially the edge cases, using some completely implemented library to provide this feature could also prevent more off-topic development and reinvent another wheel here, is that something you'd consider?

PeterDaveHello commented 2 years ago

Maybe another solution is to simply use https://github.com/DNSCrypt/doh-server, instead of developing the DoH feature at two places?

jedisct1 commented 2 years ago

DoH itself is very basic and with GET support, it should be compatible with virtually all clients.

But dnscrypt-proxy, no matter how it's accessed by clients, is designed for personal use, not to be exposed over the internet.

So, if you are looking to run a DoH server, you should indeed run doh-server instead. This will also give you ODoH support.

PeterDaveHello commented 2 years ago

Understood your point, just want to make sure it works fine with my devices, will see if the 499 errors were made by nginx.