AdguardTeam / AdGuardHome

Network-wide ads & trackers blocking DNS server
https://adguard.com/adguard-home.html
GNU General Public License v3.0
24.61k stars 1.78k forks source link

Clients not being identified using DoT #4288

Open accountForIssues opened 2 years ago

accountForIssues commented 2 years ago

Using adguardhome:edge from docker as of issue creation.

Issue Details

I added persistent clients and then I added the client ids to the access list. DoH works fine, identifies clients and returns dns. DoT on the other end rejects all queries.

I use Caddy as reverse proxy but only for DoH. DoT is directly connected via the default port. So,

DoH -> Caddy -> AGH DoT -> AGH

DoT works fine if I remove the client IDs from the access list.

AGH is served on dns.domain.tld. I add testclient as client ID in persistent clients.

Expected Behavior

A request to tls://testclient.domain.tld should be recognized as a DoT request from testclient.

Actual Behavior

DoT request from tls://testclient.domain.tld is dropped.

Relevant part of debug log: [debug] client __REDACTED_IP__ (id "") is not in access allowlist

So, it looks like the client id is being dropped from the DoT url. If the access list is empty, everything works as usual.

Adguard tls settings

tls:
  enabled: true
  server_name: dns.domain.tld
  force_https: false
  port_https: 443
  port_dns_over_tls: 853
  port_dns_over_quic: 0
  port_dnscrypt: 0
  dnscrypt_config_file: ""
  allow_unencrypted_doh: true
  strict_sni_check: false
  certificate_chain: ""
  private_key: ""
  certificate_path: certs/wildcard_.domain.tld.crt
  private_key_path: certs/wildcard_.domain.tld.key

The TLS cert is valid for *.domain.tld

Am I missing something simple ? Why is the id part being dropped from the DoT url ?

gspannu commented 2 years ago

If your AGH is served on dns.domain.tld…. Then shouldn’t your test client be tls://testclient.dns.domain.tld

accountForIssues commented 2 years ago

@gspannu I got your reply as I was typing mine.

Looks like AGH expects clients to follow clientid.dns.domain.tld format.

It's not surprising but maybe other formats can be added/supported ? It would be nice if AGH used subdomains to get the client id. Maybe I'm the one who's different :)

Also, Caddy doesn't work with SAN certificates so I had to manually generate certificates (using lego) so that dns.domain.tld and *.dns.domain.tld share the same one.

My issue is solved. The issue can be kept opened if additional client id formats can be added, otherwise you can close this issue.

ainar-g commented 2 years ago

@accountForIssues, what do you mean by “additional ClientID formats”? Can you give an example of what that would look like?

It's not surprising but maybe other formats can be added/supported ?

If you use DNS-over-HTTPS, the ClientID is put inside the path part of the URL.

gspannu commented 2 years ago

@gspannu I got your reply as I was typing mine.

Looks like AGH expects clients to follow clientid.dns.domain.tld format.

It's not surprising but maybe other formats can be added/supported ? It would be nice if AGH used subdomains to get the client id. Maybe I'm the one who's different :)

I believe AGH is following the correct domain format.

If my AGH was hosted at abc.xyz.domain.tld - I would expect DNS resolutions to happen ONLY on the specified domain

For DoT requests:
DNS should resolve on abc.xyz.domain.tld and *.abc.xyz.domain.tld
examples:
tls://abc.xyz.domain.tld
tls://client01.abc.xyz.domain.tld
tls://client02.abc.xyz.domain.tld
tls://client0x.abc.xyz.domain.tld

For DoH requests
DNS should resolve on abc.xyz.domain.tld/dns-query and abc.xyz.domain.tld/dns-query/*
examples:
https://abc.xyz.domain.tld/dns-query
https://abc.xyz.domain.tld/dns-query/client01
https://abc.xyz.domain.tld/dns-query/client02
https://abc.xyz.domain.tld/dns-query/client0x

I would not expect AGH to resolve anything on *.xyz.domain.tld or on *.domain.tld and I believe that is what AGH does today.

For DoT requests:
DNS should NOT resolve on xyz.domain.tld or domain.tld
Below examples should fail
tls://xyz.domain.tld
tls://client01.xyz.domain.tld
tls://client02.domain.tld

For DoH requests
DNS should NOT resolve on xyz.domain.tld/dns-query or domain.tld/dns-query/*
Below examples should fail:
https://xyz.domain.tld/dns-query
https://xyz.domain.tld/dns-query/client01
https://domain.tld/dns-query/client02
accountForIssues commented 2 years ago

@ainar-g

Consider this,

DOMAIN_NAME = domain.tld
AGH_SERVER = dns.domain.tld

Currently, AGH expects DoT URLs to follow the form client.AGH_SERVER. This is all fine and as expected. (In hindsight, I was probably a bit too quick in opening this issue)

But, if I want my clients to follow client.DOMAIN_NAME, then I can, in theory, do AGH_SERVER = domain.tld. However, in this case, AGH is also served on DOMAIN_NAME.

There doesn't seem to be a way to serve AGH on AGH_SERVER && having DoT URL as client.DOMAIN_NAME.

I suggest giving users an advanced setting (may be hidden in YAML) where they are able to choose, the dashboard serve URL and something like DoT_CLIENT_SUFFIX which will be used instead of AGH_SERVER to get the client ID.

So,

AGH_SERVER = dns.domain.tld # AGH is now served here
DoT_CLIENT_SUFFIX = domain.tld # AGH uses this to detect client IDs in DoT

All of this is mainly to avoid sub-subdomains and very long urls. (and give users more freedom in config) :)

ainar-g commented 2 years ago

@accountForIssues, you can technically do something like that already if you add a dns client with all settings set to defaults.

We'll see what kind of changes we can make to the current mechanism, but it is good to remember that more freedom often means more complexity, and we have already had several issues where people got confused about some aspects of ClientID, so adding even more variability doesn't exactly seem like a wise choice to me.