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

Strip EDNS(0) Client Subnet / MAC information #1240

Closed
wants to merge 2 commits into from

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Nov 1, 2021

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10


Strip EDNS(0) Client Subnet / MAC information if --strip-subnet or --strip-mac is set. If both the add and strip options are set, incoming EDNS0 options are replaced. This ensures we do not unintentionally forward client information somewhere upstream when ECS is used in lower DNS layers in our local network.

Note This PR is just here to remind us to work on and send this patch upstream to the main dnsmasq project once Simon Kelley is back.

@yubiuser
Copy link
Member

yubiuser commented Nov 2, 2021

Confirm working.

Standard behavior

Bildschirmfoto zu 2021-11-02 08-52-15

After stripping enabled
Bildschirmfoto zu 2021-11-02 08-55-03

…strip-mac is set. If both the add and strip options are set, incoming EDNS0 options are replaced. This ensures we do not unintentionally forward client information somewhere upstream when ECS is used in lower DNS layers in our local network.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER changed the title Strip EDNS(0) Client Subnet information if --strip-subnet is set Strip EDNS(0) Client Subnet / MAC information Nov 2, 2021
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/ecs-client-subnet-is-being-sent-to-upstream-server/50790/15

@yubiuser
Copy link
Member

yubiuser commented Nov 2, 2021

Confirm stripping MAC does also work. Additionally, replacing MAC/subnet by having strip-mac/subnet together with add-mac/subnet does also work.

@DL6ER
Copy link
Member Author

DL6ER commented Nov 19, 2021

Superseded by pi-hole/dnsmasq#1 to be submitted when Simon Kelley returns.

@DL6ER DL6ER closed this Nov 19, 2021
@DL6ER DL6ER deleted the tweak/strip_EDNS0_ECS branch November 19, 2021 08:37
@DL6ER
Copy link
Member Author

DL6ER commented Jan 15, 2022

@yubiuser The changes have been merged by Simon Kelley. I imported them into update/dnsmasq just a few seconds ago. Simon changed a lot in the EDNS(0) code afterwards and asks us to verify he didn't broke anything. Could you, if you can find the time, assist in doing the exact same tests you did above, again with the update/dnsmasq branch?

@yubiuser
Copy link
Member

I'll do. I wonder if we should set both strip options be default - could increase privacy of users?

@DL6ER
Copy link
Member Author

DL6ER commented Jan 15, 2022

EDNS(0) ECS shouldn't be used by any client by default. If we set it by default we have to construct all the machinery to allow users disabling it if they want to...

@yubiuser
Copy link
Member

we have to construct all the machinery to allow users disabling it if they want to...

...true....

@yubiuser
Copy link
Member

Tried both: stripping and replacing works with current update/dnsmasq

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants