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

DNS response case preservation #3194

Closed
mpcaddy opened this issue May 27, 2021 · 4 comments
Closed

DNS response case preservation #3194

mpcaddy opened this issue May 27, 2021 · 4 comments

Comments

@mpcaddy
Copy link

mpcaddy commented May 27, 2021

Issue Details

  • Version of AdGuard Home server:
    • v0.106.3
  • How did you install AdGuard Home:
    • Homeassistant Addon Docker Contaoner
  • How did you setup DNS configuration:
    • Router DHCP
  • If it's a router or IoT, please write device model:
    • EdgeRouter
  • CPU architecture:
    • x64
  • Operating system and version:
    • Ubuntu 20.04.1

Expected Behavior

RFC4343 for dns states that the reply must match the request. The case for the reply must match the case for the request.

Actual Behavior

The reply for the dns query is now returning lowercase domain name.

Additional Information

With the changes made in PR3115 the DNS reply is now invalid as it is lowercasing the domain. Which whilst this is fine and great for filtering the reply should maintain the original case.

This problem caused one of my devices to go offline as it could no longer resolve the address it was sending data to and after an exchange with their support and running a tcpdump, found the dns reply and request was different. The IoT device has very limited configuration options and no static configuration it gets its settings over dhcp so as a workaround I've set it to return the router as a dns server which looks up via adguard as the router is returning a valid response even though the response it gets from adguard is invalid (which suggests the routers dns client is implemented incorrectly but that's another matter).

From looking at the code I believe it is line 26 in filter.go that is the cause of this as if I've read it correctly that replaces the original name with a lowercase one.

@ainar-g ainar-g changed the title DNS replies are now invalid DNS response case preservation May 27, 2021
@ainar-g ainar-g self-assigned this May 27, 2021
@ainar-g ainar-g added this to the v0.107.0 milestone May 27, 2021
@ainar-g
Copy link
Contributor

ainar-g commented May 27, 2021

Thanks for the report. We'll try to return the case preservation in the next few days.

adguard pushed a commit that referenced this issue May 31, 2021
Updates #3194.

Squashed commit of the following:

commit 42a363c
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon May 31 15:15:59 2021 +0300

    dnsforward: preserve domain name case
@ainar-g
Copy link
Contributor

ainar-g commented May 31, 2021

@mpcaddy, this should be fixed as of edge snapshot c95acf7. Can you please check if the case is now retained for you? Thanks!

@mpcaddy
Copy link
Author

mpcaddy commented Jun 2, 2021

Hi thanks, from looking at the code I think that looks fine.
I'm running it using the Home Assistant addon and don't see a way to get it to run the edge version from there.

@ainar-g
Copy link
Contributor

ainar-g commented Jun 3, 2021

I see. I don't know if Home Assistant has a way to switch to an edge revision, as it is an unofficial plugin, if I recall correctly. I'll close this issue, of you don't mind. Feel free after the final v0.107.0 release if the issue persists.

@ainar-g ainar-g closed this as completed Jun 3, 2021
@ainar-g ainar-g mentioned this issue Aug 27, 2021
3 tasks
heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Updates AdguardTeam#3194.

Squashed commit of the following:

commit 42a363c
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon May 31 15:15:59 2021 +0300

    dnsforward: preserve domain name case
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

2 participants