DoH cannot be accessed properly through Nginx with SNI strict feature enabled

[immtelecom]

Prerequisites

• [X] I have checked the Wiki and Discussions and found no answer

• [X] I have searched other issues and found no duplicates

• [X] I want to report a bug and not ask a question

Operating system type

Linux, Other (please mention the version in the description)

CPU architecture

AMD64

Installation

Other (please mention in the description)

Setup

On one machine

AdGuard Home version

v0.107.22

Description

What did you do?

1. Setup AdGuard Home with SSL/TLS
2. Modify the configuration file to enable the strict_sni_check feature
3. Configure Nginx to proxy the /dns-query provided by AdGuard Home over HTTP

location /dns-query {
    proxy_set_header X-Real-IP $remote_addr;
    proxy_set_header Host same_with_sni;
    proxy_pass http://127.0.0.1:80;
}

Expected result

When I access through Nginx, I get the correct response

Actual result

A large number of the following log contents are generated

Jan 31 09:11:48 debian adguard[43209]: 2023/01/31 09:11:48.346725 [error] Error in the BeforeRequestHandler: getting clientid: clientid check: client server name "" doesn't match host server name "***********"

Screenshots (if applicable)

Additional information

After some more checks on the source code I fixed the problem. But I won't issue a PR, so I hope you can make changes for the next release

Here are my fixes, It should help you

diff --git a/internal/dnsforward/clientid.go b/internal/dnsforward/clientid.go
index fb5eefda..ecb844b8 100644
--- a/internal/dnsforward/clientid.go
+++ b/internal/dnsforward/clientid.go
@@ -158,9 +158,9 @@ func clientServerName(pctx *proxy.DNSContext, proto proxy.Proto) (srvName string
                // See https://github.com/lucas-clemente/quic-go/issues/2879.
                //
                // TODO(a.garipov): Remove this crutch once they fix it.
+               var host string
                r := pctx.HTTPRequest
                if r.ProtoAtLeast(3, 0) {
-                       var host string
                        host, err = netutil.SplitHost(r.Host)
                        if err != nil {
                                return "", fmt.Errorf("parsing host: %w", err)
@@ -169,6 +169,12 @@ func clientServerName(pctx *proxy.DNSContext, proto proxy.Proto) (srvName string
                        srvName = host
                } else if connState := r.TLS; connState != nil {
                        srvName = r.TLS.ServerName
+               } else {
+                       if host, err = netutil.SplitHost(r.Host); err != nil {
+                               srvName = r.Host
+                       } else {
+                               srvName = host
+                       }
                }
        case proxy.ProtoQUIC:
                qConn := pctx.QUICConnection

[ainar-g]

Hello. This seems like it's working as intended. If you have Nginx handling the TLS connection logic, it should also handle the check, as the TLS data isn't sent over a plain TCP connection.

[immtelecom]

Let me tell you why I'm doing this

1. I didn't want to expose my admin page, so I used an Nginx reverse proxy for DoH to solve this problem
2. I have both DoT and DoQ services turned on, which use SNI certificates, which means that any scanner on the public network can scan my machine for certificates for a particular domain, which is not good

These two points ensure that my DNS service is not accessed without authorization and that only people I have explicitly told about the domain can use it

[ainar-g]

I see, thanks. That makes sense. I guess a way to go around that would be to either add a separate setting to disable the SNI check for DoH only. Using the Host header really doesn't seem like a good idea, and we're about to remove that crutch for DoH3, since the bug in the external library we used to use seems to have been fixed.

[immtelecom]

I got a new error in the latest v0.107.24 version

Feb 19 21:19:01 debian adguard[4758]: 2023/02/19 21:19:01.759392 4758#436 [debug] request came from proxy server 127.0.0.1:9706
Feb 19 21:19:01 debian adguard[4758]: 2023/02/19 21:19:01.759448 4758#436 [debug] github.com/AdguardTeam/dnsproxy/proxy.(*Proxy).logDNSMessage(): IN: ;; opcode: QUERY, status: NOERROR, id: 0
Feb 19 21:19:01 debian adguard[4758]: ;; flags: rd; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 0
Feb 19 21:19:01 debian adguard[4758]: ;; QUESTION SECTION:
Feb 19 21:19:01 debian adguard[4758]: ;test.        IN         A
Feb 19 21:19:01 debian adguard[4758]: 2023/02/19 21:19:01.759619 4758#436 [debug] web: plain: http: panic serving 127.0.0.1:9706: runtime error: invalid memory address or nil pointer dereference
Feb 19 21:19:01 debian adguard[4758]: goroutine 436 [running]:
Feb 19 21:19:01 debian adguard[4758]: net/http.(*conn).serve.func1()
Feb 19 21:19:01 debian adguard[4758]:         net/http/server.go:1850 +0xbf
Feb 19 21:19:01 debian adguard[4758]: panic({0xc96700, 0x1e6d290})
Feb 19 21:19:01 debian adguard[4758]:         runtime/panic.go:890 +0x262
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/AdGuardHome/internal/dnsforward.clientServerName(0x1b4?, {0xd9fe7a, 0x5})
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/AdGuardHome/internal/dnsforward/clientid.go:154 +0x2ba
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/AdGuardHome/internal/dnsforward.(*Server).clientIDFromDNSContext(0xc0002c8000, 0xc000bdbb00?)
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/AdGuardHome/internal/dnsforward/clientid.go:133 +0x13e
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/AdGuardHome/internal/dnsforward.(*Server).beforeRequestHandler(0xc0002c8000, 0xd80b80?, 0xc000bdbb00)
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/AdGuardHome/internal/dnsforward/filter.go:22 +0x3e
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/dnsproxy/proxy.(*Proxy).handleDNSRequest(0xc0001db500, 0xc000bdbb00)
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/dnsproxy@v0.47.1-0.20230207130636-533058b17239/proxy/server.go:91 +0xb0
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/dnsproxy/proxy.(*Proxy).ServeHTTP(0xc0001db500, {0x19047d0?, 0xc000295ea0}, 0xc000bdba00)
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/dnsproxy@v0.47.1-0.20230207130636-533058b17239/proxy/server_https.go:161 +0x95c
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/AdGuardHome/internal/dnsforward.(*Server).ServeHTTP(0x6e13af?, {0x19047d0, 0xc000295ea0}, 0xda0810?)
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/AdGuardHome/internal/dnsforward/dnsforward.go:683 +0x45
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/AdGuardHome/internal/dnsforward.(*Server).handleDoH(0xc0002c8000, {0x19047d0, 0xc000295ea0}, 0xc000bdba00)
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/AdGuardHome/internal/dnsforward/http.go:711 +0x65
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/AdGuardHome/internal/home.postInstall.func1({0x19047d0, 0xc000295ea0}, 0xc000bdba00)
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/AdGuardHome/internal/home/control.go:383 +0x102
Feb 19 21:19:01 debian adguard[4758]: net/http.HandlerFunc.ServeHTTP(0xc001d4e420?, {0x19047d0?, 0xc000295ea0?}, 0xce8d60?)
Feb 19 21:19:01 debian adguard[4758]:         net/http/server.go:2109 +0x2f
Feb 19 21:19:01 debian adguard[4758]: net/http.(*ServeMux).ServeHTTP(0xce8d60?, {0x19047d0, 0xc000295ea0}, 0xc000bdba00)
Feb 19 21:19:01 debian adguard[4758]:         net/http/server.go:2487 +0x149
Feb 19 21:19:01 debian adguard[4758]: github.com/AdguardTeam/AdGuardHome/internal/home.limitRequestBody.func1({0x19047d0, 0xc000295ea0}, 0xc000bdb900)
Feb 19 21:19:01 debian adguard[4758]:         github.com/AdguardTeam/AdGuardHome/internal/home/middlewares.go:75 +0x379
Feb 19 21:19:01 debian adguard[4758]: net/http.HandlerFunc.ServeHTTP(0x7f80423e7d60?, {0x19047d0?, 0xc000295ea0?}, 0x40?)
Feb 19 21:19:01 debian adguard[4758]:         net/http/server.go:2109 +0x2f
Feb 19 21:19:01 debian adguard[4758]: golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP({{0x18fe020?, 0xc0002ee630?}, 0xc000334410?}, {0x19047d0, 0xc000295ea0}, 0xc000bdb900)
Feb 19 21:19:01 debian adguard[4758]:         golang.org/x/net@v0.7.0/http2/h2c/h2c.go:125 +0x59b
Feb 19 21:19:01 debian adguard[4758]: net/http.serverHandler.ServeHTTP({0xc00054e270?}, {0x19047d0, 0xc000295ea0}, 0xc000bdb900)
Feb 19 21:19:01 debian adguard[4758]:         net/http/server.go:2947 +0x30c
Feb 19 21:19:01 debian adguard[4758]: net/http.(*conn).serve(0xc000bb6820, {0x19053b0, 0xc0056f9770})
Feb 19 21:19:01 debian adguard[4758]:         net/http/server.go:1991 +0x607
Feb 19 21:19:01 debian adguard[4758]: created by net/http.(*Server).Serve
Feb 19 21:19:01 debian adguard[4758]:         net/http/server.go:3102 +0x4db

[kamontat]

I got this error as well, I use Kubernetes with Traefik Proxy.

set allow_unencrypted_doh=true and follow https traffic to adguard http.

[ainar-g]

@ImmTelecom, @kamontat, Could you test the latest version, v0.108.0-a.440+a556ce8f, from the Edge channel? It should fix the panic and it also takes the Host header into account like the OP requested. If that works for you, we'll make a hotfix release soon.

[L8X]

@ImmTelecom, @kamontat, Could you test the latest version, v0.108.0-a.440+a556ce8f, from the Edge channel? It should fix the panic and it also takes the Host header into account like the OP requested. If that works for you, we'll make a hotfix release soon.

I can confirm on v0.108.0-a.440+a556ce8f that it does not work.

[kamontat]

I tested on v0.108.0-a.443+ff9b24ad, it's working.

[immtelecom]

I tested v0.107.25 and it works fine