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

Don't drop root privileges #11

Closed
wants to merge 1 commit into from
Closed

Conversation

borgstrom
Copy link

Given that we're already running in a contained environment, requiring the NET_ADMIN capability so that an unprivileged user inside of the container can control the interfaces for the purposes of DHCP makes not dropping privs and just continuing to run as root seem like a reasonable default to me.

What do you think @andyshinn ?

@andyshinn
Copy link
Contributor

andyshinn commented Sep 16, 2017

🤔 Not sure about this. The reason I don't like it is that for users who don't want it, they need to rewrite ENTRYPOINT at runtime or build to remove it rather than users who do want it where they just need to amend it to the CMD at runtime.

Would --user=root also cover the other possible needs for NET_ADMIN per http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2014q2/008540.html?

I think if we were going to do this, we'd maybe wait until Alpine 3.7 which should have dnsmasq 2.77 so we can tag it as a new version 2.77 that would have separate instructions.

I guess another question would be: Is dropping privileges but using NET_ADMIN still the equivalent of root or is it still technically safer to drop privileges and only give NET_ADMIN to limit scope?

@andyshinn
Copy link
Contributor

For the same reason as #10, I'm going to close this as "wontfix". I think it makes sense to add to downstream images and keep this image more as a plain unaltered base with the bare minimum to run (-k for foregrounding). But let me know if you still disagree and if you have a better case where any base image would need it.

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

Successfully merging this pull request may close these issues.

None yet

2 participants