docker-pi-hole

docker-pi-hole copied to clipboard

Description

This option controls the permit all origins flag[0] - setting this to local only does not make logical sense for a container image build. While the web UI can be used to toggle this option, one of the strengths of running pihole in a container is that users can use it in a fully automated, toil free setup, which this option impedes. The local-subnets option may be very useful when run in a container either (depending on the definition of a local subnet) so I'm proposing that we change it to all.

[0] https://docs.pi-hole.net/ftldns/interfaces/#all

Motivation and Context

Docker/podman will by default, run containers in bridge mode, which means that the container itself is not very useful without fiddling with the same origin policy in the web UI. This breaks automated deployments.

It can be worked around by setting this env variable in the container run args but I feel like having it by default is a good idea and is not any bigger of a security hole than someone fiddling with the option in the web UI.

How Has This Been Tested?

Tested by setting this variable and running the container.

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

Did you try to add this variable using a compose file?

Just to note, the readme states:

Use the Pi-hole web UI to change the DNS settings Interface listening behavior to "Listen on all interfaces, permit all origins", if using Docker's default bridge network setting

I think a better solution that setting this as the default (overriding the "allow only local" default) would be to update this section of the readme to mention setting the environment variable as ana alternative to using the web UI

@PromoFaux I agree that the readme should be updated, but I'm struggling to think of why this would be a sensible default for a container. Expecting users to have different container networking settings for this container to work out of the box is weird, and this container not working out of the box on default settings is weirder still. It does not give any additional security benefits, because users have to 1) manually map 53 onto their public interface 2) open up 53 using iptables for this to be an open resolver. I do not advocate for this to be the default option for non container builds.

Expecting users to have different container networking settings for this container to work out of the box is weird...

Using different network types (host, macvlan, etc.) for containers is very common.