Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Device Identifier - DNS-over-TLS and DNS-over-HTTPS #1383

Closed
iganeshk opened this issue Jan 30, 2020 · 15 comments
Closed

Device Identifier - DNS-over-TLS and DNS-over-HTTPS #1383

iganeshk opened this issue Jan 30, 2020 · 15 comments
Assignees
Milestone

Comments

@iganeshk
Copy link

iganeshk commented Jan 30, 2020

Just migrated to Adguard-Home after hoping from nextdns and pihole. Everything seems to work out of the box, including DNS-over-TLS (standard installation).

While I was with nextdns, I found their device identifier to be a really nifty option where the device could be identified by identifier-subdomain.domain.com

DNS-over-TLS
Prepend the name of your choice to the provided domain (the name should only contain a-z, A-Z, 0-9 and -).
Example: for "John-Home-Router", you would use John-Home-Router-5ff9b1.dns.nextdns.io as your DNS-over-TLS endpoint.

Was wondering if something similar could be implemented within Encryption Settings with the following possibility:

  • Ability to configure multiple sub-/domains with certificate and client name
    Example: Google-Pixel-subdomain.domain.com

This way, it would be easier to identify clients queries that were configured to use that particular domain in the DNS-o-TLS section.

@ameshkov
Copy link
Member

Oh, interesting idea, I really like it!

We could use a different approach, though. We could extend "settings -> clients -> add client" dialog and allow settings this "subdomain" identifier there. This way you don't just identify the device, but you can also have different per-client settings.

@iganeshk
Copy link
Author

That sounds cool! You could sign me up for beta testing if required 😄

@Jbbouille
Copy link

I would like to work on this feature, is it possible ?

@ameshkov
Copy link
Member

@Jbbouille we're going to start working on it soon so I guess we may overlap here

@CalmLong
Copy link

建议在这个基础上可以增加对不同的域名(客户端)配置不同的拦截模式

@ameshkov
Copy link
Member

@CalmLong

这有意义,谢谢。

我觉得如果 TLS 证书有一些”SAN“(subject alternative name),我们可能允许用几个域名。
但是我不想我们应该允许用一些证书,这是太复杂了。

@ameshkov ameshkov changed the title Device Identifier - DNS-over-TLS Device Identifier - DNS-over-TLS and DNS-over-HTTPS Nov 19, 2020
@ameshkov
Copy link
Member

ameshkov commented Nov 19, 2020

Proposed implementation:

  1. Allow wildcard certificates in the Encryption settings
  2. Improve wording in the "Server name" field: https://uploads.adguard.com/up04_AdGuard_Home_3s1g4.png We should explain that if "Server name" is not set, we should accept TLS connections for any domain.
  3. Allow setting custom strings as Client ID. These strings should be validated (i.e. needs they can be used as a domain name component): https://uploads.adguard.com/up04_AdGuard_Home_278y0.png
  4. Once this identifier is configured, you can use a special domain name while configuring your client.
    Example:
    1. AdGuard Home domain name example.org.
    2. In AdGuard Home you add a client with identifier my-client.
    3. On the client device you can now configure:
      • DNS-over-TLS: tls://my-client.example.org
      • DNS-over-QUIC: quic://my-client.example.org
      • DNS-over-HTTPS: https://example.org/dns-query-my-client
  5. Don't forget to add this description to the "Identifier" field in the UI.
  6. If some client uses an address matching the above pattern, let's add it to "Clients (runtime)" automatically.
  7. TODO: Query log and Dashboard UI for this new identifier

@ainar-g ainar-g self-assigned this Nov 25, 2020
@Belphemur
Copy link

@ameshkov
I have a suggestion for the routes.

For the DNS-over-HTTPS currently NextDNS uses:
https://dns.nextdns.io/ACCOUNT_ID/DEVICE_ID

Maybe for AdGuardHome it could be
https://example.com/dns-query/DEVICE_ID/

This would make sense with the current routing of DoH.

@ghost
Copy link

ghost commented Dec 23, 2020

@ameshkov i think for DoT, DoQ instead of subdomain it's better to use like nextdns devide-id--domain.tld the "--" let us identify the client without having to create a wildcard certificate (not possible on some configuration with some domain provider) and/or can be messy to generate easily.

edit : in my case my dns server is on a dns subdomain with a certificate who cover only the dns, like that if my dns is hacked they can't use the certificate to redirect my other domain (it's my treat model).

@ameshkov
Copy link
Member

@michaelb-ae -- is just a part of the domain name, it does not mean you don't need a wildcard certificate. Or you can use a certificate with multiple subaltnames which include your devide-id--domain.tld

adguard pushed a commit to AdguardTeam/dnsproxy that referenced this issue Jan 25, 2021
Merge in DNS/dnsproxy from 1383-quic-session to master

Updates AdguardTeam/AdGuardHome#1383.

Squashed commit of the following:

commit d0d0afa
Author: Ainar Garipov <[email protected]>
Date:   Mon Jan 25 17:29:57 2021 +0300

    proxy: add quic session to dns context
adguard pushed a commit that referenced this issue Jan 27, 2021
Merge in DNS/adguard-home from 1383-client-id to master

Updates #1383.

Squashed commit of the following:

commit ebe2678
Author: Ildar Kamalov <[email protected]>
Date:   Wed Jan 27 17:51:59 2021 +0300

    - client: check if IP is valid

commit 0c33058
Author: Ildar Kamalov <[email protected]>
Date:   Wed Jan 27 17:07:50 2021 +0300

    - client: find clients by client_id

commit 71c9593
Merge: 9104f16 3e9edd9
Author: Ainar Garipov <[email protected]>
Date:   Wed Jan 27 16:09:45 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 9104f16
Author: Ainar Garipov <[email protected]>
Date:   Wed Jan 27 13:28:50 2021 +0300

    dnsforward: imp tests

commit ed47f26
Author: Ainar Garipov <[email protected]>
Date:   Wed Jan 27 12:39:52 2021 +0300

    dnsforward: fix address

commit 98b222b
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 19:50:31 2021 +0300

    home: imp code

commit 4f39665
Merge: 199fdc0 c215b82
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 19:45:13 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 199fdc0
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 19:20:37 2021 +0300

    all: imp tests, logging, etc

commit 35ff14f
Author: Ildar Kamalov <[email protected]>
Date:   Tue Jan 26 18:55:19 2021 +0300

    + client: remove block button from clients with client_id

commit 32991a0
Author: Ildar Kamalov <[email protected]>
Date:   Tue Jan 26 18:54:25 2021 +0300

    + client: add requests count for client_id

commit 2d68df4
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 15:49:50 2021 +0300

    stats: handle client ids

commit 4e14ab3
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 13:45:25 2021 +0300

    openapi: fix example

commit ca9cf3f
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 13:37:10 2021 +0300

    openapi: improve clients find api docs

commit f79876e
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 13:18:52 2021 +0300

    home: accept ids in clients find

commit 5b72595
Merge: 607e241 abf8f65
Author: Ainar Garipov <[email protected]>
Date:   Mon Jan 25 18:34:56 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 607e241
Author: Ainar Garipov <[email protected]>
Date:   Mon Jan 25 18:30:39 2021 +0300

    dnsforward: fix quic

commit f046352
Author: Ainar Garipov <[email protected]>
Date:   Mon Jan 25 16:53:09 2021 +0300

    all: remove wildcard requirement

commit 3b67948
Author: Andrey Meshkov <[email protected]>
Date:   Mon Jan 25 16:02:28 2021 +0300

    workDir now supports symlinks

commit 0647ab4
Author: Ildar Kamalov <[email protected]>
Date:   Mon Jan 25 14:59:46 2021 +0300

    - client: remove wildcard from domain validation

commit b1aec04
Author: Ildar Kamalov <[email protected]>
Date:   Mon Jan 25 14:55:39 2021 +0300

    + client: add form to download mobileconfig

... and 12 more commits
@ainar-g
Copy link
Contributor

ainar-g commented Jan 27, 2021

@ameshkov, as I understand it, now we need to properly document it and close the issue?

@ameshkov
Copy link
Member

@ainar-g well, it kinda is documented already: https://github.com/AdguardTeam/AdGuardHome/wiki/Clients

@ainar-g
Copy link
Contributor

ainar-g commented Jan 27, 2021

Ah, I see. Then the remaining bugs and improvements should probably become separate issues, like #2607.

@ainar-g ainar-g closed this as completed Jan 27, 2021
@ghost
Copy link

ghost commented Jan 27, 2021

@ainar-g @ameshkov maybe in the doc Client ID zone i haven't seen any number of version so a fresh user may ask why it don't work if he test it before the version who support it is out.

@ameshkov
Copy link
Member

@Macqael thx, I've added a note about it to Wiki

heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Merge in DNS/adguard-home from 1383-client-id to master

Updates AdguardTeam#1383.

Squashed commit of the following:

commit ebe2678
Author: Ildar Kamalov <[email protected]>
Date:   Wed Jan 27 17:51:59 2021 +0300

    - client: check if IP is valid

commit 0c33058
Author: Ildar Kamalov <[email protected]>
Date:   Wed Jan 27 17:07:50 2021 +0300

    - client: find clients by client_id

commit 71c9593
Merge: 9104f16 3e9edd9
Author: Ainar Garipov <[email protected]>
Date:   Wed Jan 27 16:09:45 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 9104f16
Author: Ainar Garipov <[email protected]>
Date:   Wed Jan 27 13:28:50 2021 +0300

    dnsforward: imp tests

commit ed47f26
Author: Ainar Garipov <[email protected]>
Date:   Wed Jan 27 12:39:52 2021 +0300

    dnsforward: fix address

commit 98b222b
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 19:50:31 2021 +0300

    home: imp code

commit 4f39665
Merge: 199fdc0 c215b82
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 19:45:13 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 199fdc0
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 19:20:37 2021 +0300

    all: imp tests, logging, etc

commit 35ff14f
Author: Ildar Kamalov <[email protected]>
Date:   Tue Jan 26 18:55:19 2021 +0300

    + client: remove block button from clients with client_id

commit 32991a0
Author: Ildar Kamalov <[email protected]>
Date:   Tue Jan 26 18:54:25 2021 +0300

    + client: add requests count for client_id

commit 2d68df4
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 15:49:50 2021 +0300

    stats: handle client ids

commit 4e14ab3
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 13:45:25 2021 +0300

    openapi: fix example

commit ca9cf3f
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 13:37:10 2021 +0300

    openapi: improve clients find api docs

commit f79876e
Author: Ainar Garipov <[email protected]>
Date:   Tue Jan 26 13:18:52 2021 +0300

    home: accept ids in clients find

commit 5b72595
Merge: 607e241 abf8f65
Author: Ainar Garipov <[email protected]>
Date:   Mon Jan 25 18:34:56 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 607e241
Author: Ainar Garipov <[email protected]>
Date:   Mon Jan 25 18:30:39 2021 +0300

    dnsforward: fix quic

commit f046352
Author: Ainar Garipov <[email protected]>
Date:   Mon Jan 25 16:53:09 2021 +0300

    all: remove wildcard requirement

commit 3b67948
Author: Andrey Meshkov <[email protected]>
Date:   Mon Jan 25 16:02:28 2021 +0300

    workDir now supports symlinks

commit 0647ab4
Author: Ildar Kamalov <[email protected]>
Date:   Mon Jan 25 14:59:46 2021 +0300

    - client: remove wildcard from domain validation

commit b1aec04
Author: Ildar Kamalov <[email protected]>
Date:   Mon Jan 25 14:55:39 2021 +0300

    + client: add form to download mobileconfig

... and 12 more commits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants