Skip to content

Commit

Permalink
dnsforward: use Host when available
Browse files Browse the repository at this point in the history
  • Loading branch information
@ainar-g
ainar-g committed Feb 20, 2023
1 parent 4eee3d6 commit 74873bd
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 30 deletions.
17 changes: 13 additions & 4 deletions internal/dnsforward/clientid.go
View file
Expand Up @@ -147,14 +147,23 @@ func (s *Server) clientIDFromDNSContext(pctx *proxy.DNSContext) (clientID string
return clientID, nil
}

// clientServerName returns the TLS server name based on the protocol.
// clientServerName returns the TLS server name based on the protocol. For
// DNS-over-HTTPS requests, it will return the hostname part of the Host header
// if there is one.
func clientServerName(pctx *proxy.DNSContext, proto proxy.Proto) (srvName string, err error) {
switch proto {
case proxy.ProtoHTTPS:
// Do not rely on pctx.HTTPRequest.TLS always being non-nil, since
// AdGuard Home might be behind a reverse proxy.
if connState := pctx.HTTPRequest.TLS; connState != nil {
r := pctx.HTTPRequest
if connState := r.TLS; connState != nil {
srvName = connState.ServerName
} else if r.Host != "" {
var host string
host, err = netutil.SplitHost(r.Host)
if err != nil {
return "", fmt.Errorf("parsing host: %w", err)
}

srvName = host
}
case proxy.ProtoQUIC:
qConn := pctx.QUICConnection
Expand Down
90 changes: 64 additions & 26 deletions internal/dnsforward/clientid_test.go
View file
Expand Up @@ -50,136 +50,169 @@ func TestServer_clientIDFromDNSContext(t *testing.T) {
testCases := []struct {
name string
proto proxy.Proto
hostSrvName string
confSrvName string
cliSrvName string
wantClientID string
wantErrMsg string
inclHTTPTLS bool
strictSNI bool
}{{
name: "udp",
proto: proxy.ProtoUDP,
hostSrvName: "",
confSrvName: "",
cliSrvName: "",
wantClientID: "",
wantErrMsg: "",
inclHTTPTLS: false,
strictSNI: false,
}, {
name: "tls_no_clientid",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "example.com",
wantClientID: "",
wantErrMsg: "",
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "tls_no_client_server_name",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "",
wantClientID: "",
wantErrMsg: `clientid check: client server name "" ` +
`doesn't match host server name "example.com"`,
strictSNI: true,
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "tls_no_client_server_name_no_strict",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "",
wantClientID: "",
wantErrMsg: "",
inclHTTPTLS: false,
strictSNI: false,
}, {
name: "tls_clientid",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "cli.example.com",
wantClientID: "cli",
wantErrMsg: "",
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "tls_clientid_hostname_error",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "cli.example.net",
wantClientID: "",
wantErrMsg: `clientid check: client server name "cli.example.net" ` +
`doesn't match host server name "example.com"`,
strictSNI: true,
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "tls_invalid_clientid",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "!!!.example.com",
wantClientID: "",
wantErrMsg: `clientid check: invalid clientid "!!!": ` +
`bad domain name label rune '!'`,
strictSNI: true,
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "tls_clientid_too_long",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: `abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmno` +
`pqrstuvwxyz0123456789.example.com`,
wantClientID: "",
wantErrMsg: `clientid check: invalid clientid "abcdefghijklmno` +
`pqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789": ` +
`domain name label is too long: got 72, max 63`,
strictSNI: true,
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "quic_clientid",
proto: proxy.ProtoQUIC,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "cli.example.com",
wantClientID: "cli",
wantErrMsg: "",
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "tls_clientid_issue3437",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "cli.myexample.com",
wantClientID: "",
wantErrMsg: `clientid check: client server name "cli.myexample.com" ` +
`doesn't match host server name "example.com"`,
strictSNI: true,
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "tls_case",
proto: proxy.ProtoTLS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "InSeNsItIvE.example.com",
wantClientID: "insensitive",
wantErrMsg: ``,
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "quic_case",
proto: proxy.ProtoQUIC,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "InSeNsItIvE.example.com",
wantClientID: "insensitive",
wantErrMsg: ``,
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "https_no_clientid",
proto: proxy.ProtoHTTPS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "example.com",
wantClientID: "",
wantErrMsg: "",
inclHTTPTLS: true,
strictSNI: true,
}, {
name: "https_clientid",
proto: proxy.ProtoHTTPS,
hostSrvName: "example.com",
confSrvName: "example.com",
cliSrvName: "cli.example.com",
wantClientID: "cli",
wantErrMsg: "",
inclHTTPTLS: true,
strictSNI: true,
}, {
name: "https_issue5518",
proto: proxy.ProtoHTTPS,
confSrvName: "example.com",
cliSrvName: "cli.example.com",
wantClientID: "cli",
wantErrMsg: "",
inclHTTPTLS: false,
strictSNI: true,
}, {
name: "https_no_host",
proto: proxy.ProtoHTTPS,
confSrvName: "example.com",
cliSrvName: "example.com",
wantClientID: "",
wantErrMsg: "",
inclHTTPTLS: false,
strictSNI: true,
}}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
tlsConf := TLSConfig{
ServerName: tc.hostSrvName,
ServerName: tc.confSrvName,
StrictSNICheck: tc.strictSNI,
}

Expand All @@ -195,7 +228,7 @@ func TestServer_clientIDFromDNSContext(t *testing.T) {

switch tc.proto {
case proxy.ProtoHTTPS:
httpReq = newHTTPReq(tc.cliSrvName)
httpReq = newHTTPReq(tc.cliSrvName, tc.inclHTTPTLS)
case proxy.ProtoQUIC:
qconn = testQUICConnection{
serverName: tc.cliSrvName,
Expand All @@ -222,20 +255,25 @@ func TestServer_clientIDFromDNSContext(t *testing.T) {
}

// newHTTPReq is a helper to create HTTP requests for tests.
func newHTTPReq(cliSrvName string) (r *http.Request) {
func newHTTPReq(cliSrvName string, inclTLS bool) (r *http.Request) {
u := &url.URL{
Path: "/dns-query",
}

return &http.Request{
r = &http.Request{
ProtoMajor: 1,
ProtoMinor: 1,
URL: u,
Host: cliSrvName,
TLS: &tls.ConnectionState{
}

if inclTLS {
r.TLS = &tls.ConnectionState{
ServerName: cliSrvName,
},
}
}

return r
}

func TestClientIDFromDNSContextHTTPS(t *testing.T) {
Expand Down

0 comments on commit 74873bd

Please sign in to comment.