AdguardTeam / AdguardForAndroid

Open bug tracker for Android version of AdGuard.
https://adguard.com/
1.23k stars 86 forks source link

DNS filtering is invalid in v3.1.28η. #2798

Closed lancelot-moon closed 4 years ago

lancelot-moon commented 5 years ago

Test v3.1.28η.

  1. The domains in the filters for DNS filtering aren't shown as blocked in the filtering log.

  2. DNS filtering is invalid. Ads appear.

  3. Remark the supported formats information of filter list. ex: Basic adblock rule syntax, domains, hosts files... etc.

SergeiShir commented 5 years ago

Exactly the same symptoms. The Filtering Log just shows "Processed", although explicit User Rules exist to block / allow the DNS packets. In addition, when you try to create a new rule for a DNS packet, there is no option anymore to select $app=com.adguard.dns - there is nothing under the rule string. Example screenshots are below. 1st log: 1-log 1st rule: 1-rule 2nd log: 2-log 2nd rule: 2-rule 3rd log: 3-log 3rd rule: 3-rule 4th log: 4-log 4th rule: 4-rule

Not possible to configure a new rule for DNS: 5-log

TPS commented 5 years ago

@ameshkov @TheHasagi @admitrevskiy: I've precisely same symptoms as all of the above w/ 1 additional datum: I tried toggling the new Advanced pref.dns.blocking.nxdomain, but that had no effect. DNS blocking seems completely broken. ☹️

ameshkov commented 5 years ago

Regarding user filter, I guess the current approach is super confusing.

The only way to resolve it is to have a different section for DNS-level rules. I'll file a separate task about it.

ameshkov commented 5 years ago

The bug is caused by proguard obfuscation in the production build, it seems that it strips something important to DNS filtering. We'll fix it tomorrow.

TPS commented 5 years ago

Thanks, @ameshkov @TheHasagi @admitrevskiy &c: All's well w/ DNS-block again. 🙇🏾‍♂️

Curious questions: What is the advantage of changing default from responding NXDOMAIN to 0.0.0.0 (or IPv6 equivalent)? What circumstances do y'all forsee us needing pref.dns.blocking.nxdomain to revert that?

ameshkov commented 5 years ago

Curious questions: What is the advantage of changing default from responding NXDOMAIN to 0.0.0.0 (or IPv6 equivalent)? What circumstances do y'all forsee us needing pref.dns.blocking.nxdomain to revert that?

Frankly, for the local DNS filtering, there's no difference.

However, for the case when you're running a DOH server, using NXDOMAIN prevents caching of the result by the client -> leads to more DNS queries.

SergeiShir commented 5 years ago

The v3.1.37 uses the existing DNS filtering rules. BUT you still can not create new rules - when you try to create a new rule for a DNS packet, there is no option anymore to select $app=com.adguard.dns - there is nothing under the rule string.

Revertron commented 5 years ago

@SergeiShir, this is how new system works. You don't need that part of rule anymore, it will be added in a special section in DNS filtering.

TPS commented 5 years ago

this is how new system works. You don't need that part of rule anymore, it will be added in a special section in DNS filtering.

So, this means 1 can no longer make a general rule from such a log entry, only a DNS rule⁈ That seems less than optimal, as I myself have generated mostly dual-use rules, which need to match both DNS & URLs.

(While I'm kvetching, I'd still really like the options to both create a white-/blacklist rule, or edit my original black-/whitelist rule, not just directly delete my original rule. The reasoning to do so then, IMHO, was naive, & isn't better now.)

SergeiShir commented 5 years ago

For example, I always want to allow DNS packet for facebook.com, but I want to block the actual connections to Facebook for some specific URLs or apps. I need the behavior described in Issue 2769

ameshkov commented 5 years ago

@SergeiShir you're about to get it.

We will introduce separate "custom rules" settings for DNS filtering in one of the next nightlies.

TPS commented 5 years ago

This seems partially broken again since 3.1.37η: Certain general user rules don't process for DNS-blocking anymore, which is noticeable, because everything's slower this way.

It turns out to be very specific: all my rules that are simple strings (e.g. adobedtm) no longer DNS-block. But if they're delimited in any fashion (e.g. ||app-measur), then they work just as before.

TPS commented 5 years ago

Another glitch: When using a hosts subscription for DNS-block, pref.dns.blocking.nxdomain (tried because of discussion in #1582) seems to be ineffective for domains set to ::, 127.0.0.1, or 0.0.0.0 — those don't get NXDOMAIN response per AG log.

admitrevskiy commented 5 years ago

@TPS, hi! Looks like there is a mistake in description for pref.dns.blocking.nxdomain. NXDomain should be generated for regular filtering rules, not host rules. Thank you for the feedback, I'll fix this phrase.

ameshkov commented 5 years ago

Hosts subscriptions already indicate the IP address that needs to be returned

TPS commented 5 years ago

That makes sense when using hosts as originally intended, but adblockers don't do that. I suppose if y'all want to support the former (as rare as that might be), that'd be fine, but isn't the major use by AG purely adblock? Then wouldn't it make sense to be consistent in the DNS-based blocking behavior, especially for an advanced toggle which (currently) isn't the default behavior?

ameshkov commented 5 years ago

I guess this needs to be a separate feature request.

TPS commented 5 years ago

@admitrevskiy, @ameshkov: Opened & quoted y'all in #2847 to handle pref.dns.blocking.nxdomain issue.