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

Bail out if not run as root? #373

Closed
DimitriPapadopoulos opened this issue Sep 19, 2018 · 14 comments · Fixed by #387
Closed

Bail out if not run as root? #373

DimitriPapadopoulos opened this issue Sep 19, 2018 · 14 comments · Fixed by #387

Comments

@DimitriPapadopoulos
Copy link
Collaborator

See latest comments in #362.

@DimitriPapadopoulos
Copy link
Collaborator Author

If root privileges are required for parts of the tunnel setup (as they certainly are), just exit immediately if geteuid() != 0.

@mrbaseman
Copy link
Collaborator

I'm not sure if full root privileges are required on all platforms. Some linux capabilities to set up routes and a suid mechanism to start pppd might be sufficient.

@DimitriPapadopoulos
Copy link
Collaborator Author

According to the README openfortivpn needs elevated privileges at three steps during tunnel set up:

  • when spawning a /usr/sbin/pppd process; ⇒ Is there an alternative to pppd that wouldn't require elevated privileges?
  • when setting IP routes through VPN (when the tunnel is up); ⇒ see ipv4_set_route() / ioctl(), I doubt we can manipulate routes without elevated privileges
  • when adding nameservers to /etc/resolv.conf (when the tunnel is up) ⇒ probably obsolete if we outsource to pppd as is now the case by default

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Sep 19, 2018

It is possible to work around the elevated privileges requirement with an SUID or sudo mechanism. However the result of the SUID or sudo mechanism would probably be that openfortivpn is run as root, in which case geteuid() == 0 would still be the proper test, wouldn't it?

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Sep 19, 2018

If we really want to avoid root:

Setting routes

The CAP_NET_ADMIN capability might be enough for ipv4_set_route() / ioctl():

  • I don't know how easy it is to manage capabilities (probably apply setcap to openfortivpn) and whether Linux distributions are willing to allow/manage capabilities.
  • In any case we could check either for root geteuid() == 0 or the current process capabilities with something like prctl(PR_CAPBSET_READ, CAP_NET_ADMIN).

Spawning pppd

Spawning pppd requires a SUID or sudo mechanism on pppd or a wrapper of pppd. For example it looks like the dip group does the trick on Ubuntu:

-rwsr-xr-- 1 root dip 390888 Jan 29  2016 /usr/sbin/pppd

Perhaps we could either check for geteuid() == 0 or or SUID set on /usr/sbin/pppd stat().

@DimitriPapadopoulos
Copy link
Collaborator Author

@ageric Running openfortivpn without being root on Ubuntu fails when spawning pppd if I'm not part of group dip.

The resulting error message looks good enough to me:

ERROR:  pppd: An immediately fatal error of some kind occurred, such as an essential system call failing, or running out of virtual memory.
INFO:   Terminated pppd.
INFO:   Closed connection to gateway.
INFO:   Logged out.

It would be even better if we could print an error message right after execv() fails:

		execv(pppd_args.data[0], (char *const *)pppd_args.data);
		free(pppd_args.data);
		/*
		 * The following call to fprintf() doesn't work, probably
		 * because of the prior call to forkpty().
		 * TODO: print a meaningful message using strerror(errno)
		 */
		fprintf(stderr, "execvp: %s\n", strerror(errno));

Unfortunately this error messages doesn't print: Any clue how to fix that?

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Sep 20, 2018

@ageric pppd stills fails if I am part of group dip, probably because the noauth option requires root privilege. Indeed when running pppd standalone::

$ /usr/sbin/pppd 38400 :192.0.2.1 noipdefault noaccomp noauth default-asyncmap nopcomp receive-all nodefaultroute nodetach lcp-max-configure 40 mru 1354 usepeerdns
/usr/sbin/pppd: using the noauth option requires root privilege
$ 

The resulting error message makes sense although I agree it's too vague::

ERROR:  pppd: An error was detected in processing the options given, such as two mutually exclusive options being used.
INFO:   Terminated pppd.
INFO:   Closed connection to gateway.
INFO:   Logged out.

But then we're currently limited to the pppd return values to print an error message and that's all these return values tell us.

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Sep 20, 2018

Anyway, it's now clear we need geteuid() == 0 so we could bail out with a nice error message if that's not the case. Unless the pppd error message is inexact and some Linux capability is sufficient (instead of full root privilege)?

@DimitriPapadopoulos
Copy link
Collaborator Author

C program used for testing:

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/prctl.h>
#include <linux/capability.h>


static const char *argv[] = {
    "/usr/sbin/pppd",
    "38400", // speed
    "192.0.2.1:", // <local_IP_address>:<remote_IP_address>
    "noipdefault",
    "noaccomp",
    "noauth",
    "default-asyncmap",
    "nopcomp",
    "receive-all",
    "nodefaultroute",
    "nodetach",
    "lcp-max-configure", "40",
    "mru", "1354"
};


int main()
{
    // Can modify routes?
    int status = prctl(PR_CAPBSET_READ, CAP_NET_ADMIN);
    if (status < 0) {
        fprintf(stderr, "prctl: %s (%d)\n", strerror(errno), errno);
    } else {
        fprintf(stderr, "prctl: success (%d)\n", status);
    }

    // Can spawn PPPD with elevated privileges?
    struct stat buf;

    if (stat(argv[0], &buf)) {
        fprintf(stderr, "stat: failed: %s (%d)\n", strerror(errno), errno);
        exit(EXIT_FAILURE);
    }

    if (geteuid() == 0 || buf.st_mode & S_ISUID) {
        execv(argv[0], (char *const *)argv);
		fprintf(stderr, "execv: cannot exec: %s\n", strerror(errno));
		exit(EXIT_FAILURE);
    } else {
        fprintf(stderr, "cannot elevate privileges\n");
    }
}

@mrbaseman
Copy link
Collaborator

shall this C program be integrated into autoconf, and if so, what should happen depending on the test result?

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Oct 30, 2018

@mrbaseman No, I was just trying to understand where exactly openfortivpn requires to be root. On Linux the main issue seems to be that the noauth option of pppd requires root privileges. The rest could be addressed by being part of group dip for spawning pppd (on Ubuntu at least) and CAP_NET_ADMIN capability for setting routes.

Perhaps we need to explicitly test geteuid() == 0 in openfortivpn and bail out, at least on Linux. I suspect there are similar reasons why openfortivpn needs to be run as root on other systems, but they need to be understood and addressed for each system.

@mrbaseman
Copy link
Collaborator

we already have this check in main.c. So, the suggestion is to not only print out a warning but going directly to exit: and destroy the vpn config, right?

On OSX I have experienced the same problems as described in #362, namely that I just forgot sudo.
I think it's the same situation there, since it uses pppd as well.

On FreeBSD it is also assumed that ppp is run as root, but one can also configure a group that may run ppp.
The route command, however, requires root privileges to manipulate the routing table.

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Nov 7, 2018

Yes, the suggestion is to exit if not run as root.

On FreeBSD the network group seems equivalent to the dip group on Ubuntu - except on Ubuntu the noauth pppd option requires root privileges so being member of dip is not sufficient.

@mrbaseman
Copy link
Collaborator

I'm not sure if the network group is enough. The route man page states:
"The route utility uses a routing socket and the new message types
RTM_ADD, RTM_DELETE, RTM_GET, and RTM_CHANGE. As such, only the super-
user may modify the routing tables."
So, I think we can implement this as a hard exit instead of just writing out a warning.

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 a pull request may close this issue.

2 participants