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

Deprecate REPLY_ADDR4/6 in favor of more fine-grained setting #1293

Merged
merged 6 commits into from
Feb 3, 2022

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Feb 1, 2022

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


This PR addresses #1291 by replacing the multi-purpose config setting REPLY_ADDR4 and REPLY_ADDR6 with more fine-grained settings.
This allows for fine-grained setting of the IP to hand out for blocked queries in IP blocking mode (using BLOCK_IPV4/6) independently of the IP address returned for lookups of pi.hole and the device's hostname (LOCAL_IPV4/6).

If neither BLOCK_IPV4/6 nor LOCAL_IPV4/6 is set but REPLY_ADDR4/6 is found, we use its value for both settings to preserve existing behavior.

This PR also contains a bug fix ensuring overwriting also works for IPv6 over the loopback device (e5f69e1).

DL6ER added 4 commits January 30, 2022 12:24
Signed-off-by: DL6ER <dl6er@dl6er.de>
…its for deciding if the address is unspecified.

Signed-off-by: DL6ER <dl6er@dl6er.de>
… and BLOCK_IPV4/6. If REPLY_ADDR4/6 is set and neither LOCAL_IPV4/6 nor BLOCK_IPV4/6, we use the value for both to preserve current behavior.

Signed-off-by: DL6ER <dl6er@dl6er.de>
…o replace the blocking IP address but not the local one and need to determine the latter.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add/modify a test for the new settings?

src/config.c Outdated Show resolved Hide resolved
@yubiuser yubiuser linked an issue Feb 1, 2022 that may be closed by this pull request
Co-authored-by: yubiuser <ckoenig@posteo.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Copy link
Member Author

DL6ER commented Feb 1, 2022

Tests added

@DL6ER DL6ER requested a review from yubiuser February 1, 2022 17:34
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally tested.

[2022-02-01 20:49:58.946 32148M]    LOCAL_IPV4: Using IPv4 address 4.3.2.1 for pi.hole and hostname
[2022-02-01 20:49:58.946 32148M]    LOCAL_IPV6: Automatic interface-dependent detection of address
[2022-02-01 20:49:58.946 32148M]    BLOCK_IPV4: Using IPv4 address 1.2.3.4 in IP blocking mode
[2022-02-01 20:49:58.946 32148M]    BLOCK_IPV6: Automatic interface-dependent detection of address



rockpi@rockpi-4b:~$ dig flurry.com @127.0.0.1 +short
1.2.3.4
rockpi@rockpi-4b:~$ dig pi.hole @127.0.0.1 +short
4.3.2.1

@PromoFaux PromoFaux merged commit 86f7f2a into development Feb 3, 2022
@PromoFaux PromoFaux deleted the new/force_ip46 branch February 3, 2022 18:16
@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/pi-hole-ftl-v5-14-web-v5-11-and-core-v5-9-released/53529/1

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.

REPLY_ADDR4/6 has multiple purposes
4 participants