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

Linux capabilities #81

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

pfactum
Copy link

@pfactum pfactum commented Oct 17, 2022

Address #80.

It's a preparatory commit to reflect upcoming changes in how
capabilities are checked under Linux in presence of libcap-ng.

No functional change.

Signed-off-by: Oleksandr Natalenko <[email protected]>
@yarrick
Copy link
Owner

yarrick commented Oct 26, 2022

This feels very Linux-specific. What do you think about adding a new argument flag that skips the check, and then it is up to the user to make it work anyway

@yarrick
Copy link
Owner

yarrick commented Oct 26, 2022

If we do it like this I think it makes sense to exit only after all checks are done, so that the user does not keep getting new errors after fixing the first one.

capng_get_caps_process failure should fall back to the old uid 0 check also I think

Under Linux, a process may not be run under root, yet it may have a permission
to do what a superuser may do given specific capabilities are granted.

This commit makes iodine not depend on EUID being 0 in order to run
properly. Instead, in presence of libcap-ng, the following capabilities
are being checked:

* `CAP_NET_BIND_SERVICES` for server to bind to a port, lower than
  `/proc/sys/net/ipv4/ip_unprivileged_port_start`
* `CAP_NET_ADMIN` to operate on a TUN device
* `CAP_SETUID` and `CAP_SETGID` in case server is configured to change
  the user it runs on behalf of

This change is handy if iodine is being run under a non-root user, provided
`AmbientCapabilities=` and `CapabilityBoundingSet=` of systemd are employed
in the first place.

Fixes: yarrick#80
Signed-off-by: Oleksandr Natalenko <[email protected]>
@pfactum
Copy link
Author

pfactum commented Nov 2, 2022

This feels very Linux-specific. What do you think about adding a new argument flag that skips the check, and then it is up to the user to make it work anyway

That can be an additional argument that does not intersect with proposed changes. Proposed changes allow more granular capabilities checking and reporting.

@pfactum
Copy link
Author

pfactum commented Nov 2, 2022

If we do it like this I think it makes sense to exit only after all checks are done, so that the user does not keep getting new errors after fixing the first one.

Thanks for the suggestion, forced-pushed with this change, please check.

capng_get_caps_process failure should fall back to the old uid 0 check also I think

Nope. Having UID 0 doesn't guarantee having all the required capabilities. On Linux root (UID 0) can also be restricted.

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.

2 participants