pppd: Check CAP_NET_RAW capability on Linux rather than requiring euid 0

[a-andreyev]

Hello! Thank you for the project!
Faced #98 and searched for a solution to check actual properties to access the network. Am I right with my approach to use libcap?

Feel free to criticize/close, would be happy to get feedback about the issue

[a-andreyev]

Note: force-pushed to update the proposal. Fixed typos and provided general geteuid() method for Solaris too.

[paulusmack]

I'm not sure that CAP_NET_RAW is sufficient for everything pppd does. In run_program() there is a setuid(0) call, for instance. Have you tested that scripts such as /etc/ppp/ip-up still get run as root with this change?

There are some minor stylistic things that are a little annoying in this commit:

• In Makefile.linux there is an unrelated whitespace change in the "LIBS += -lcrypt" line.
• The error message in main.c could be confusing, since users might not know what "net capable" means. I suggest making it "must have CAP_NET_RAW or root privilege to run %s" or similar (note "run %s" not "run the %s", since %s is standing for a name, not a description of a class of things which contains one relevant member).
• The headline of the commit message says "Introduce net_capable option", but in fact there is no option called "net_capable" either before or after this commit. I suggest something like "pppd: Check CAP_NET_RAW capability on Linux rather than requiring euid 0".

[Neustradamus]

@a-andreyev: Have you looked for your PR?

[a-andreyev]

@Neustradamus, thank you for the reminder :) @paulusmack, thank you for the review, fixed mentioned problems.

[a-andreyev]

I'm not sure that CAP_NET_RAW is sufficient for everything pppd does. In run_program() there is a setuid(0) call, for instance. Have you tested that scripts such as /etc/ppp/ip-up still get run as root with this change?

Unfortunately, not, I haven't tested the /etc/ppp/ip-up scripts or more :( Not sure I will test it in the near future, I will be grateful for tips or help (I'm also not sure that CAP_NET_RAW is sufficient).

[Neustradamus]

@a-andreyev: Can you look rebase your PR and test it?

At the end of 2020, @paulusmack has done various changes and some mergers.

It will be perfect if your PR can be merged before Debian 11 freeze.
The next Debian will be in 2023.

[a-andreyev]

Thanks for the notification, will sync with upstream today

[Neustradamus]

@a-andreyev: Do not forget it ;)

[pali]

I'm afraid that CAP_NET_RAW would not be sufficient for some ppp scripts. For example script which modifies /etc/resolv.conf with dns ip address from pppd would needs root file system permission and not only CAP_NET_RAW capability. Also e.g. reloading/restarting nscd daemon requires root rights if is called from ppp scripts.

So I think that if there are any checks for capabilities, it can be done only if the case when external scripts are not going to be called.

[a-andreyev]

Sorry for the delay, rebased the current work to upstream. Anyway, I guess @pali is right and CAP_NET_RAW would not be sufficient for some ppp scripts. Not sure yet, what additional checks should be added.

[paulusmack]

I'm not planning to put this in 2.4.9. After that, it could go in if it defaults to off, so the code is there and distros can enable it easily enough if they want to. Until I can understand how the scripts are going to work when pppd is not root, I will leave it off by default.

By the way, please get rid of the usage of __P, since I removed it across the source tree.

[Neustradamus]

@a-andreyev: Have you looked the @paulusmack comment?

[a-andreyev]

thanks for the reminder, removed the __P usage and set the defaults to off (#USE_LIBCAP=y)

[Neustradamus]

@paulusmack: What do you think, it is good?

[mweinelt]

I'd be surprised if pppd didn't need CAP_NET_ADMIN to set up routes. Either way, not running pppd as root would be a nice improvement.

In fact we currently set

AmbientCapabilities=CAP_SYS_TTY_CONFIG CAP_NET_ADMIN CAP_NET_RAW CAP_SYS_ADMIN
CapabilityBoundingSet=CAP_SYS_TTY_CONFIG CAP_NET_ADMIN CAP_NET_RAW CAP_SYS_ADMIN

In fact I believe CAP_SYS_ADMIN was added for BPF reasons, which since Kernel 5.7 can be safely replaced with CAP_BPF.

[Neustradamus]

@enaess: What do you think?

[enaess]

It would make it a lot easier to resolve this if we got the autotools change merged. Looks like it currently has some merge conflicts and it would need some massaging.

[enaess]

I think I'd want to play around with this a bit too. Also, capabilities are inherited right? So, pppd also does fork/exec of other processes like ip-up/down, etc? Also, are we 100% sure pppd doesn't need root access to access or create other directories too, like /var/run/ppp? This may again depend on proper setup by the distro/maintainer. I suppose they would use the setcap command to enable this on the file system and create another ppp user or run this as any user?

[enaess]

As I am reading through the thread, I'd be really curious who is asking for this feature and why?

Considering the security model, and all the ip-up/down scripts. Just being able to create a network device, and to configure e.g. default routes, this may suffice. However, pppd does a lot more than that. If you consider how it is deployed on Ubuntu or many of the other Linux distributions, it isn't restricted by it's capabilities but much more the conventional file permissions. The /etc/resolv.conf is one, but there are others like accessing UNIX sockets, /proc, etc.

While doing e.g. a fallback (getuid() != 0 && !(getcaps() & CAP_NET_ADMIN)) { exit (-1); ... and letting it continue if it does indeed run with the capabilities then you may have it working on a few embedded distributions if you desire. I still think the overall security model needs to be considered.

You can already use network manager in a non-root-mode and configure a VPN. While pppd is being spun off by network-manager daemon, It can be initiated by a non-root user. pppd still runs as root user even in this case.

[Neustradamus]

@enaess: What do you think now?
Your PR has been merged ^^

[enaess]

@Neustradamus what do you mean? I am a bit confused.

[Neustradamus]

"if we got the autotools change merged", it is done :)

[enaess]

Well, okay. The Makefile.linux no longer exist. No risk of clobbering the change now. Still my comment from Jul 22 still stands. We could take a change like this, but it risk setting up ourselves with a slew of fixes to change this and that because something else broke. Which distributions are trying to resolve this and why? I don't think I fully understand their use-case yet. Nobody should be marking pppd with setuid bit marked, also they will need more than just CAP_NET_ADMIN. Likely all the administrative rights, etc. Changing the security model of pppd shouldn't be taken lightly...

[jkroonza]

This is very interesting. Regarding the scripts, not all use cases require them to have root :). Consider pppd as part of a ras service ... this would in most cases set up proxyarp, an ip (and possibly a route or two). No local changes to eg resolvconf.

And I think with the network manager system most distro's tend towards nowadays, non-root users have the ability to via network manager make many of these changes anyway (disclaimer: don't use network manager personally, so statement is based on deductions from what I've seen). So if that's the case there is no reason why nmcli could not potentially be used as a non-root user to set these up.

[Neustradamus]

@a-andreyev: Have you seen comments on your PR?

[a-andreyev]

Thanks for a reminder @Neustradamus! I haven't seen it for a while, will take a look 👍

[Neustradamus]

@a-andreyev: Did you take the time to look?

[Neustradamus]

@a-andreyev: Did you take the time to look?

It is important because @paulusmack has merged several PRs before 2.5.1 release build...

[jkroonza]

Perhaps keep this check first, as it's going to be the most likely case anyway?

[jkroonza]

If already ok, why run the second check?

For that matter, why not simply return 1 immediately when things check out?

[CircuitChaos]

Hi.

I just came across your PR and one thing is not clear to me.

In kernel's code we can see that /dev/ppp can be opened only if CAP_NET_ADMIN (not CAP_NET_RAW) is set:

https://github.com/torvalds/linux/blob/master/drivers/net/ppp/ppp_generic.c#L391

It's consistent with my tests – I set /dev/ppp permissions to be readable and writable by the current (non-root) user, removed the root check from the pppd, and it still needs CAP_NET_ADMIN to access /dev/ppp. It fails with „Operation not permitted” error when trying to open /dev/ppp without it (so maybe even a capability check isn't needed in pppd, because it will always fail with this error?).

Once I give it CAP_NET_ADMIN (and remove the need to be root to use the noauth option), it works. I was able to successfully create two PPP interfaces on one machine this way and connect them with netcat (pty "nc …" option):

pppd noauth nodetach 1.1.1.1:1.1.1.2 pty 'nc -l 5555'

pppd noauth nodetach pty 'nc 0 5555'

But you're testing for CAP_NET_RAW, not CAP_NET_ADMIN. Why?

[enaess]

I wasn't the original author of the change so I don't really know the reason to why CAP_NET_RAW check was added, but if what you say is true; it should probably be CAP_NET_ADMIN. From what I recall, the CAP_NET_RAW allows us to open a RAW socket and write RAW packets (maybe this CAP_NET_RAW is also needed for e.g. PPPoE or some strange other protocol?), but to create a ppp device or to add / remove routes, should require CAP_NET_ADMIN as you administer the network on the host ppp is opened on.

@CircuitChaos I was under the impression you'd have to use file capabilities to grant /sbin/pppd the capabilities to run as non-root user and still be able to run w/CAP_NET_ADMIN|RAW, is this no longer true? Much similar to setting suid bit on the executable, but more secure?

You are saying it is sufficient to set file permissions on /dev/ppp (e.g. something similar to chown && chmod +rwg +rwu /dev/ppp?). Also, this would require a change on the OS you are planning to use this with, i.e. are you planning to submit upstream patches to Arch, Debian or RedHat to make this work, or are you doing a custom Linux distribution somehow?

[CircuitChaos]

@enaess What you're saying is true. We need both rw file permissions on /dev/ppp (otherwise we'll get „Permission denied” when trying to open it) and CAP_NET_ADMIN capabilities on the pppd binary (otherwise we'll get „Operation not permitted”).

I didn't test it with PPPoE, I just did a basic test with a pseudo-tty (but on my target system it will be sufficient).

Generally my (our) solution already works, but pppd is suid root and I don't like it. I'm looking for a way to use capabilities instead, for more fine-grained control of permissions.

Right know it seems that simply removing a root check from pppd/main.c, without even adding libcap dependency, should be sufficient, as the next function called is ppp_available(), which tries to open /dev/ppp and fails if it can't (so permissions and capabilities checks are done by the kernel).

I just want to know why @a-andreyev checks for CAP_NET_RAW and not CAP_NET_ADMIN before I blindly use the latter. Often there's a reason for why things are the way they are, even if it's not obvious at first…

When it comes to the OS, it's a custom one, so fortunately no upstream patches will be needed.

[paulusmack]

I think, before I would merge anything like this I would like to see a written-down analysis of what things pppd does, under what circumstances it does them, and what capabilities those things need. That way we can rationally check for the capabilities we need and refuse options that would require capabilities we don't have.