-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
adguard home does not respect client configuration overrides #4982
Comments
Video demonstration. Re-opening the client overrides page, Clicking on unblock all button and then saving changes fixes the problem. adguard.home.client.overrides.glitch.mp4 |
Clicking on unblock all and updating client sends a request to
Looks like it's just missing a check for the global blocked services somewhere. |
Here is the problem, AdGuardHome/internal/home/dns.go Line 361 in 2ffea60
This adds client specific blocked services but does not remove the inherited global blocked list.
This is the value of |
@ishanjain28, hello and thanks for the thorough report and for the contribution, but we've already fixed it ourselves in the latest edge build. Could you please check it out? |
hey @EugeneOne1 I can replicate this bug in the build here, https://github.com/AdguardTeam/AdGuardHome/releases/tag/v0.108.0-b.16 |
@ishanjain28, that's rather odd. We aren't being able to reproduce it with the latest commit on several test machines. Are you sure the client's settings haven't changed since the last report, the |
I can consistently replicate this on a linux x64 machine and some rpi 3b+. I have included verbose logs and the config file in this. Verbose Logs
Configuration file
|
In the PR, I have linked to the problematic line. A call to AdGuardHome/internal/filtering/blocked.go Line 437 in 2ffea60
This call is made without checking if the client has opted to ignore global blocklists here. AdGuardHome/internal/home/dns.go Line 338 in 2ffea60
|
@ishanjain28, firstly, it's quite unsecure to post unredacted configuration file of AdGuard Home to the public space, I'd recommend that you at least change your password. There are no issues with applying services' rules according to the log. It actually says that the client with address By the way, to give a feeedback on your PR, I'd say it introduces a bug, which makes AGH to not apply global services to the non-persistent clients' requests. The logic there is a bit convoluted and really requires attention to all the other parts. As far as I can see, you've built the AdGuard Home yourself. Just to ensure, haven't you change any code in comparison with master branch before building? Could please also try it on our signed builds? Those may either be installed with the README.md script: curl -s -S -L https://raw.githubusercontent.com/AdguardTeam/AdGuardHome/master/scripts/install.sh | sh -s -- -c edge -v or downloaded directly from:
(Replace the If it reproduces there? Thanks. |
Hey @EugeneOne1 You are right in that for the first few times, it says it couldn't find 127.0.0.1 in the clients list. This was because I had not added the client when I sent request the first few times. I had also recorded a video of the whole process along with the logs. I have shared the logs above and the video is here, https://www.youtube.com/watch?v=48BUwk2ABDo (I couldn't upload it here because of size constraints and the video was still processing when I posted the logs)
I have tested this in my PR by removing the
Lastly, This was on the most recent commit in the branch. I did not have any un-stashed changes when building the project. |
Merge in DNS/adguard-home from 4983-fix-custom-svcs to master Updates #4945. Updates #4982. * commit '739e0098ec127045197ea697bae78acb2bd2c729': all: imp code, wording home: fix empty svcs bugfix: Fixed bug which was causing clients to continue following global service blocks even when user opted to not do that
@ishanjain28, well, after digging further, I can confirm the bug. It seems editing the client via UI (at runtime) sets inproper blocked services slice. You may have noticed we've merged your PR, but also modified it a bit. The latest edge build (as per HEAD) should now work as intended, could you please check it again? UPD: the latest release version contains the fix as well. |
Hey, It works perfectly for me now. Thank you! |
Prerequisites
I have checked the Wiki and Discussions and found no answer
I have searched other issues and found no duplicates
I want to report a bug and not ask a question
Operating system type
Linux, Other (please mention the version in the description)
CPU architecture
AMD64
Installation
GitHub releases or script from README
Setup
On a router, DHCP is handled by the router
AdGuard Home version
latest beta release(v0.108.0-b.16 Pre-release) and HEAD(2ffea60)
Description
What did you do?
Expected result
should return the correct IP for cloudflare.com. The client in this case is localhost which comes under the client I added initially and for this client, No services should be blocked.
Actual result
It blocks DNS for all globally blocked services and does not respect client specific overrides.
Screenshots (if applicable)
Additional information
This is happen on the most recent commit and the most recent release. It does not happen in,
v0.107.14
The text was updated successfully, but these errors were encountered: