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

Allow pppd to start as non-root. #98

Closed

Conversation

micah-morton
Copy link

This patch adds #ifndef macros in 2 spots in order to allow pppd to be
spawned as a non-root user with only runtime capabilities (e.g.
CAP_NET_{RAW/ADMIN}) instead of giving pppd full root privileges. This
is helpful if pppd is itself spawned by a non-root user and the use of
file permissions (e.g. setuid-root) on the pppd binary is not a
desirable solution.

This patch adds #ifndef macros in 2 spots in order to allow pppd to be
spawned as a non-root user with only runtime capabilities (e.g.
CAP_NET_{RAW/ADMIN}) instead of giving pppd full root privileges. This
is helpful if pppd is itself spawned by a non-root user and the use of
file permissions (e.g. setuid-root) on the pppd binary is not a
desirable solution.
@paulusmack
Copy link
Collaborator

The basic idea seems fine, and the commit message is OK, though lacking a signed-off-by line.

With the ifndef in pppd/main.c, I'd prefer to see an explicit check that we have the capabilities we need if we're not running with euid=0, rather than simply not checking.

The ifndef in pppd/options.c doesn't look to be necessary. That code is about preventing a non-root user from overriding options set by the system administrator, it has basically nothing to do with the euid.

@paulusmack paulusmack closed this Jun 23, 2018
@micah-morton
Copy link
Author

Any idea regarding the set of runtime capabilities that pppd uses (and hence which capabilities we should check for in pppd/main.c)? The only capability I know for sure pppd needs is CAP_NET_ADMIN, but we are currently giving it CAP_NET_RAW and CAP_NET_BIND_SERVICE as well as part of a tree of processes. Then again I'm not familiar with the different functionalities of pppd to know if there are any other capabilities it may need in other use cases that I haven't mentioned here.

@Neustradamus
Copy link
Member

@micah-morton: Have you looked the PR from @a-andreyev?

What do you think?

@paulusmack: Can you look the update from today too?

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

Successfully merging this pull request may close these issues.

3 participants