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

Use the response RR type when filtering #4238

Closed
ainar-g opened this issue Feb 1, 2022 · 2 comments
Closed

Use the response RR type when filtering #4238

ainar-g opened this issue Feb 1, 2022 · 2 comments
Assignees
Milestone

Comments

@ainar-g
Copy link
Contributor

ainar-g commented Feb 1, 2022

(Split from #4230.)

Currently, when we filter a DNS response, we use the request resource record type in the filtering request. That creates issues with e.g. CNAME responses to A/AAAA queries. @ameshkov proposes using the type of the response RRs rather than the request ones.

I feel like there could be issues with misbehaving upstreams though. Like an upstream that responds with AAAA records on an A request, for example. We'll see how much of an issue that would be.

@ameshkov
Copy link
Member

ameshkov commented Feb 1, 2022

@ainar-g this is actually a simple improvement, but worthy of implementing in a patch and not waiting for v0.108.0 release (since we need the same change for AdGuard DNS).

@ainar-g
Copy link
Contributor Author

ainar-g commented Feb 1, 2022

@ameshkov, I thought about that, but I feel like this is a change in behaviour that isn't necessarily a bug fix, and I prefer to leave such changes to “major” (minor in SV) versions. In any case, nothing prevents us completing it on the main branch and then cherry-picking it into the release branch if there is an actual need for it.

I'll take AGD into account.

@adguard adguard closed this as completed in e783564 Feb 3, 2022
@ainar-g ainar-g modified the milestones: v0.108.0, v0.107.7 Jun 2, 2022
heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Merge in DNS/adguard-home from 4238-response-filtering to master

Closes AdguardTeam#4238.

Squashed commit of the following:

commit 2113f83
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Feb 3 21:04:25 2022 +0300

    dnsforward: restore a bit

commit f78607e
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Feb 3 20:52:45 2022 +0300

    all: imp code, docs

commit 646074c
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Feb 3 20:37:05 2022 +0300

    all: log changes

commit 94556d8
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Feb 3 20:30:57 2022 +0300

    all: imp test upstream, cover resp filtering

commit 63e7721
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Feb 1 21:58:08 2022 +0300

    all: filter response by rrtype
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

3 participants