-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
$dnsrewrite modifier #2102
Comments
This also opens up the possibility of being able to subscribe to DNS rewrites like blocklist/allowlists which makes it easier when you run 2 instances of Adguard and have to keep the DNS rewrite rules in sync. |
As an example, this is the rule chain that user asks in #2105:
|
One of the edge case I could think of is how would you distinguish whether a rewrite is for Couple of suggestions to make DNS rewrite more powerful
|
Well, for the cases that complicated I'd suggest using regular expressions instead:
Could you please explain why this is useful? |
The use case for me is to add AAAA record for Custom domains are served by another load balancer that isn't accessible over IPv6 or its IPv6 address hasn't been publicly documented so far. Custom domains point to Steps to reproduce: Step 0: Requires you are on IPv6 enabled network |
That's not a problem, there will be no additional query for the CNAME anyway so we won't rewrite it (CNAME resolution is done on the resolver side, and it returns both A/AAAA and CNAME records right away). |
@ainar-g check the spec in the issue description, I've finally updated it. |
@ameshkov So came across this thread https://old.reddit.com/r/firefox/comments/jysc04/clearurls_remove_url_tracking_elements_should_be/gd6rxtb/ where OP mentions that Ublock origin dev declined to support redirects as it could potentially be abused by malicious lists. This made me think, should we have an option to disable importing rewrite rules from remote lists. It could be an option when adding new/editing an existing list or in YAML config file for now and later surfaced to UI should there be a demand. Personally I would prefer to exclude all lists except my own when it comes to importing rewrite rules. Took a look at the update spec and have a couple of questions,
Unless I am missing something, I don't see any plans to keep the special Don't see the option to ignore a rewrite if the rewrite domain is a CNAME record for another domain. Is it something you prefer to address on a separate feature request? |
Well, we could do what we do in other AdGuard products. In AdGuard when you add a custom filter list, you have a choice whether you trust the list to use potentially dangerous rules or not, and then the list will be limited to simple rules only. This should be a separate feature request, though.
Here's how this could be achieved:
Could you please elaborate? |
Will raise a separate feature request for the option to trust dangerous rules. Thank you for the example. That helps.
You can ignore this request as I got it mixed up with how Firefox DoH deals when it encounters domain names with CNAME but to give you a context, what I was trying to do is add
This works great as long as you are accessing any github.io domain directly but doesn't work for custom domains as the IPv6 load balancer don't have certs for them. Below is a github hosted domain
Initially I thought that it was AGH rewriting the domain So what actually happened was Firefox made a DNS resolution request for This can be confirmed from logs as well as DNS section in Querying AGH directly doesn't rewrite. Returns no
This might be intended behaviour on Firefox side. Unbound does something similar. See
|
Merge in DNS/urlfilter from 2102-dnstype to master Updates AdguardTeam/AdGuardHome#2102. Squashed commit of the following: commit 116ad95 Author: Ainar Garipov <[email protected]> Date: Tue Nov 24 14:17:56 2020 +0300 all: improve documentation commit 2d6df3d Author: Ainar Garipov <[email protected]> Date: Tue Nov 24 13:44:11 2020 +0300 rules: improve dns type matching commit 1032d62 Author: Ainar Garipov <[email protected]> Date: Tue Nov 24 13:10:44 2020 +0300 all: improve documentation, tests; simplify code commit 8369c94 Author: Ainar Garipov <[email protected]> Date: Mon Nov 23 15:11:13 2020 +0300 all: add the $dnstype modifier
Merge in DNS/adguard-home from 2337-dnstype to master Updates #2102. Updates #2337. Squashed commit of the following: commit ac4b752 Author: Ainar Garipov <[email protected]> Date: Tue Nov 24 17:50:33 2020 +0300 dnsfilter: add $dnstype handling
I think that the currently proposed syntax is a bit
tricky to parse and has a few unwanted
properties. For example, if there is a host named
$dnsrewrite=RCODE[:RR:arg] And, for full A, AAAA, and CNAME rewrites: $dnsrewrite=NEWCNAME:host (Alternatively, we could call this one @ameshkov's examples from the OP thus become: |example.org^$dnsrewrite=NOERROR:A:127.0.0.1 |example.org^$dnsrewrite=NEWCNAME:example.net ||*^$dnstype=HTTPS,dnsrewrite=REFUSED |example.org^$dnsrewrite=NOERROR:A:127.0.0.1 |example.org^$dnsrewrite=NOERROR:A:127.0.0.2 |example.org^$dnsrewrite=NOERROR:AAAA:[::1] |example.org^$dnsrewrite=NOERROR:AAAA:[::2] This syntax also seems more easily extensible, at least to me. What do you all think? |
@ainar-g I have a couple of questions about the proposed syntax. First, what would be the difference between Also, why can't we simplify the life of filter maintainers and allow using shorter syntax for the most popular use cases? These rules won't introduce any extra ambiguity and are perfectly readable:
Regarding domain names, the example with |
Just to be clear, you are proposing this shorthand form only for A and AAAA records as opposed to the NEWCNAME behaviour, right? I think we could do that, although I personally would not use that form. |
I just don't really understand what's the necessity in rewriting just the
Why, it's not opposed to anything. The full syntax would be: Also, I am still not convinced that making people write I fully support the full syntax, though. It's clearer that what I've written in the first place. My only concern is lack of shorthands. |
After some discussion, here is my updated proposal:
Obviously, all of that shall be documented with lots of examples based on real-life use-cases. |
Works for me. Could you please change the spec in the OP? |
Wait, not everything works for me:
I still think that CNAME should make AGH chase the first CNAME and add it's records. If we ever need to specify CNAME that won't chase it, we can add a new type. However, I really doubt we will ever need this. |
@ameshkov, okay, but you owe me one if there is a legit case for it :-) . |
Merge in DNS/adguard-home from 2337-dnstype to master Updates #2102. Updates #2337. Squashed commit of the following: commit ac4b752 Author: Ainar Garipov <[email protected]> Date: Tue Nov 24 17:50:33 2020 +0300 dnsfilter: add $dnstype handling
Merge in DNS/adguard-home from 2102-rules-result to master Updates #2102. Squashed commit of the following: commit 47b2aa9 Author: Ainar Garipov <[email protected]> Date: Thu Dec 17 13:12:27 2020 +0300 querylog: remove pre-v0.99.3 compatibility code commit 2af0ee4 Author: Ainar Garipov <[email protected]> Date: Thu Dec 17 13:00:27 2020 +0300 all: improve documentation commit 3add300 Author: Ainar Garipov <[email protected]> Date: Wed Dec 16 18:30:01 2020 +0300 all: improve changelog commit e04ef70 Author: Ainar Garipov <[email protected]> Date: Wed Dec 16 17:56:53 2020 +0300 all: improve code and documentation commit 4f04845 Author: Ainar Garipov <[email protected]> Date: Wed Dec 16 17:01:08 2020 +0300 all: document changes, improve api commit bc59b76 Author: Ainar Garipov <[email protected]> Date: Tue Dec 15 18:22:01 2020 +0300 all: allow multiple rules in dns filter results
Merge in DNS/urlfilter from 2102-dnsrewrite to master Updates AdguardTeam/AdGuardHome#2102. Squashed commit of the following: commit 44ef59e Author: Ainar Garipov <[email protected]> Date: Fri Dec 18 16:27:01 2020 +0300 all: improve tests commit f863a56 Author: Ainar Garipov <[email protected]> Date: Fri Dec 18 15:14:06 2020 +0300 all: improve matching api commit 6d5a48d Author: Ainar Garipov <[email protected]> Date: Mon Dec 14 16:41:19 2020 +0300 rules: improve code commit d0ff0b9 Author: Ainar Garipov <[email protected]> Date: Fri Dec 11 20:40:42 2020 +0300 all: improve performance, add more tests commit 1f16df5 Merge: e4be9a4 ad2a7d2 Author: Ainar Garipov <[email protected]> Date: Fri Dec 11 19:53:41 2020 +0300 Merge branch 'master' into 2102-dnsrewrite commit e4be9a4 Merge: f80f32b e5d7af3 Author: Ainar Garipov <[email protected]> Date: Thu Dec 10 20:20:26 2020 +0300 Merge branch 'master' into 2102-dnsrewrite commit f80f32b Merge: 264c0ca e948b30 Author: Ainar Garipov <[email protected]> Date: Thu Dec 10 16:39:38 2020 +0300 Merge branch 'master' into 2102-dnsrewrite commit 264c0ca Author: Ainar Garipov <[email protected]> Date: Thu Dec 10 15:57:37 2020 +0300 all: improve matching commit d33afd8 Author: Ainar Garipov <[email protected]> Date: Tue Dec 8 18:50:13 2020 +0300 all: improve api commit dfe7e0e Author: Ainar Garipov <[email protected]> Date: Tue Dec 8 13:53:06 2020 +0300 all: improve naming, tweak algorithms commit 964d234 Author: Ainar Garipov <[email protected]> Date: Fri Dec 4 16:56:29 2020 +0300 all: add the $dnsrewrite modifier
Merge in DNS/adguard-home from 2102-dnsrewrite to master Updates #2102. Squashed commit of the following: commit 8490fc1 Merge: d9448dd e7f7799 Author: Ainar Garipov <[email protected]> Date: Mon Dec 21 16:44:00 2020 +0300 Merge branch 'master' into 2102-dnsrewrite commit d9448dd Author: Ainar Garipov <[email protected]> Date: Mon Dec 21 15:44:54 2020 +0300 querylog: support dnsrewrite rules commit 40aa5d3 Author: Ainar Garipov <[email protected]> Date: Fri Dec 18 19:27:40 2020 +0300 all: improve documentation commit f776a0c Author: Ainar Garipov <[email protected]> Date: Fri Dec 18 19:09:08 2020 +0300 dnsfilter: prevent panics, improve docs commit e14073b Author: Ainar Garipov <[email protected]> Date: Fri Dec 4 15:51:02 2020 +0300 all: add $dnsrewrite handling
As an update: we've just merged the first batch into AdGuardHome's
|
Merge in DNS/urlfilter from 2102-dnsrewrite-2 to master Updates AdguardTeam/AdGuardHome#2102. Updates AdguardTeam/AdGuardHome#2452. Squashed commit of the following: commit bc2d41b Author: Ainar Garipov <[email protected]> Date: Tue Dec 22 14:32:26 2020 +0300 all: handle ptr, mx, https, and svcb dns rewrites
Merge in DNS/adguard-home from 2102-dnsrewrite-2 to master Updates #2102. Updates #2452. Squashed commit of the following: commit b41e577 Author: Ainar Garipov <[email protected]> Date: Tue Dec 22 20:40:56 2020 +0300 dnsforward: improve naming commit 70b832c Author: Ainar Garipov <[email protected]> Date: Tue Dec 22 19:36:21 2020 +0300 all: support more $dnsrewrite rr types
Merge in DNS/adguard-home from 2102-dnsrewrite-tests to master Updates #2102. Squashed commit of the following: commit 894ff4b Author: Ainar Garipov <[email protected]> Date: Thu Jan 14 14:49:28 2021 +0300 dnsforward: add dnsrewrite tests
@ameshkov, I think this one is basically done? I propose we close this issue. |
Yep |
… results Merge in DNS/adguard-home from 2102-rules-result to master Updates AdguardTeam#2102. Squashed commit of the following: commit 47b2aa9 Author: Ainar Garipov <[email protected]> Date: Thu Dec 17 13:12:27 2020 +0300 querylog: remove pre-v0.99.3 compatibility code commit 2af0ee4 Author: Ainar Garipov <[email protected]> Date: Thu Dec 17 13:00:27 2020 +0300 all: improve documentation commit 3add300 Author: Ainar Garipov <[email protected]> Date: Wed Dec 16 18:30:01 2020 +0300 all: improve changelog commit e04ef70 Author: Ainar Garipov <[email protected]> Date: Wed Dec 16 17:56:53 2020 +0300 all: improve code and documentation commit 4f04845 Author: Ainar Garipov <[email protected]> Date: Wed Dec 16 17:01:08 2020 +0300 all: document changes, improve api commit bc59b76 Author: Ainar Garipov <[email protected]> Date: Tue Dec 15 18:22:01 2020 +0300 all: allow multiple rules in dns filter results
Merge in DNS/adguard-home from 2102-dnsrewrite to master Updates AdguardTeam#2102. Squashed commit of the following: commit 8490fc1 Merge: d9448dd e7f7799 Author: Ainar Garipov <[email protected]> Date: Mon Dec 21 16:44:00 2020 +0300 Merge branch 'master' into 2102-dnsrewrite commit d9448dd Author: Ainar Garipov <[email protected]> Date: Mon Dec 21 15:44:54 2020 +0300 querylog: support dnsrewrite rules commit 40aa5d3 Author: Ainar Garipov <[email protected]> Date: Fri Dec 18 19:27:40 2020 +0300 all: improve documentation commit f776a0c Author: Ainar Garipov <[email protected]> Date: Fri Dec 18 19:09:08 2020 +0300 dnsfilter: prevent panics, improve docs commit e14073b Author: Ainar Garipov <[email protected]> Date: Fri Dec 4 15:51:02 2020 +0300 all: add $dnsrewrite handling
Merge in DNS/adguard-home from 2102-dnsrewrite-2 to master Updates AdguardTeam#2102. Updates AdguardTeam#2452. Squashed commit of the following: commit b41e577 Author: Ainar Garipov <[email protected]> Date: Tue Dec 22 20:40:56 2020 +0300 dnsforward: improve naming commit 70b832c Author: Ainar Garipov <[email protected]> Date: Tue Dec 22 19:36:21 2020 +0300 all: support more $dnsrewrite rr types
Merge in DNS/adguard-home from 2102-dnsrewrite-tests to master Updates AdguardTeam#2102. Squashed commit of the following: commit 894ff4b Author: Ainar Garipov <[email protected]> Date: Thu Jan 14 14:49:28 2021 +0300 dnsforward: add dnsrewrite tests
Where the idea comes from: #2098
Example:
||example.org^$dnsrewrite=127.0.0.1
The idea is to benefit from rules syntax and other modifiers. For instance, we'll be able to combine these rules with other modifiers.
We'll need to carefully think it through, edge cases, etc.
Spec:
dnsrewrite
The
$dnsrewrite
modifier allows specifying the exact DNS records that will be returned as a response to a DNS query. This is a very powerful and flexible modifier so please be cautious with it.The
$dnstype
modifier allows specifying one or several DNS query types the rule will be working for.The syntax is:
Alternatively, you can specify just the status to respond with. In this case, AdGuard Home will respond with an empty DNS response with the specified status code.
Where:
response_content
is the contents of a DNS record.KEYWORD
is an all-caps keyword. For example,REFUSED
.SHORTHAND
is either an IPv4 (A
record), an IPv6 (AAAA
record), or a hostname (CNAME
rewrite, see below).RCODE
is a DNS response code.RR
is a DNS record type.RR
will be rewritten to the specifiedresponse_content
response.RR
will be rewritten to an emptyNOERROR
response (unless they're modified by a different $dnsrewrite rule).CNAME
is a special case that redirects bothA
andAAAA
queries to the specified domain. If there's a rule with theCNAME
type matching the domain name, all other rules will be ignored.Multiple rules matching a single request
$dnsrewrite
rules matching a single request, we will apply each of them.|example.org|$dnsrewrite=127.0.0.1
and|example.org|$dnsrewrite=127.0.0.2
will result in a DNS response with twoA
records:127.0.0.1
and127.0.0.2
.|example.org|$dnsrewrite=REFUSED
and|example.org|$dnsrewrite=127.0.0.1
will result in a DNS response with no resource records and statusREFUSED
.Disabling
$dnsrewrite
rules@@||example.org^$dnsrewrite
will disable all$dnsrewrite
rules matching||example.org^
pattern.@@||example.org^$dnsrewrite=127.0.0.1
will disable rules that are trying to rewrite||example.org^
to127.0.0.1
Examples
Simple rewrite
|example.org|$dnsrewrite=127.0.0.1
— rewritesexample.org
(exact match) to127.0.0.1
. Note that this rule is actually the same as|example.org|$dnsrewrite=NOERROR;A;127.0.0.1
as the typeA
is inferred automatically. Therefore,dig -t a example.org
will return127.0.0.1
, other queries will return an emptyNOERROR
CNAME
rewrite — this is a special case that rewrites bothA
andAAAA
queries.|example.org|$dnsrewrite=example.net
— rewritesexample.org
toexample.net
. Note that theCNAME
type is inferred sinceresponse_content
is a valid domain name and not an IP address.Block
HTTPS
queries||*^$dnstype=HTTPS,dnsrewrite=REFUSED
— responds with an emptyREFUSED
response to allHTTPS
queries. Note, that we specified thednstype
modifier in this rule. Without it, we would've broken all queries (as they would've been rewritten to an emptyREFUSED
response).Multiple
A
andAAAA
records|example.org|$dnsrewrite=127.0.0.1
|example.org|$dnsrewrite=127.0.0.2
|example.org|$dnsrewrite=::1
|example.org|$dnsrewrite=::2
The text was updated successfully, but these errors were encountered: