No longer need to run knockd with sudo #3472
Conversation
|
This approach looks fine. What I'm now wondering is with the setcap applied to the executable, maybe it doesn't even need to be sudo'ed Can you research that a bit? |
|
...in fact, I think I'll merge this now and we can follow up with the other research outcome. |
scripts/start
Outdated
| @@ -48,6 +48,11 @@ if ! isTrue "${SKIP_SUDO:-false}" && [ "$(id -u)" = 0 ]; then | |||
| echo 'hosts: files dns' > /etc/nsswitch.conf | |||
| fi | |||
|
|
|||
| if [[ ${ENABLE_AUTOPAUSE^^} == TRUE ]]; then | |||
| log "Autopause is enabled, setting up permissions" | |||
| echo "minecraft ALL=(ALL) NOPASSWD:/usr/local/sbin/knockd" >> /etc/sudoers.d/sudoers-mc | |||
There was a problem hiding this comment.
I just realized, this will probably keep re-appending the row on startup when autopause is enabled.
Ah good point! Let me look into that. If it is still required to sudo I'll fix the constant appending on every start - good catch! |
|
@itzg you were correct, with the capabilities set, it isn't necessary to sudo start knockd. The testing I've done:
I haven't tested every distro (alpine/ubuntu/ol), let me know if more thorough testing is necessary |
|
Awesome! I'm not sure why that didn't dawn on me when the setcap change was first contributed, but glad to leverage it now. 😃 That sounds like sufficient testing and can follow up with other distros later if needed. |
|
Thanks for the quick reviews! |
itzg
changed the title
If you are hosting this image for unknown users, sudo access to knockd can be used for privilege escalation within the container.
There needs to be a lot of surrounding functionality for this to be a security risk, and for self-hosted minecraft servers this isn't a concern. I'm not 100% certain this is the correct approach, but I'm happy to make changes as necessary.
If you think this is the correct approach I'm happy to update (with the help of Peri123214) the docs outlining the risk once this has been patched.
Big thanks to Peri123214 for finding this.