openfortivpn icon indicating copy to clipboard operation
openfortivpn copied to clipboard

Revisit running openfortivpn as root?

Open DimitriPapadopoulos opened this issue 4 years ago • 40 comments

After #373 openfortivpn must be run with root privilege. There are multiples reasons for that. It would be worth re-investigating whether there are ways around that, or at least whether dropping root privilege after initial setup is a possibility, for example after spawning pppd.

Spawning pppd

Members of the dip group may run pppd on Linux distributions such as Debian or Ubuntu:

$ ls -l /usr/sbin/pppd
-rwsr-xr-- 1 root dip 395144 Feb 11 16:03 /usr/sbin/pppd
$ 

Yet openfortivpn requires root privilege because option noauth is privileged:

$ id -nG
[...] sudo dip plugdev [...]
$ 
$ pppd noauth
pppd: using the noauth option requires root privilege
$ 

Not sure how to work around this is an a generic way - apart from complex solutions such as splitting openfortivpn into multiple pieces of software with root privileges only the one spawning pppd.

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).

Alternatively routes might be handled outside of openfortivpn:

  • Routing can be handled by the calling framework, for example NetworkManager. Use option --set-routes=0/--no-routes.
  • Routing could be handled by openfortivpn call-back scripts. Such scripts would require specific sudo privileges. See Wrapper for ip.

Name resolution

DNS servers and search domains might be handled outside of openfortivpn:

  • DNS can be handled by the calling framework, for example NetworkManager. Use options --set-dns=0/--no-dns and --pppd-use-peerdns=0. Note that NetworkManager-fortisslvpn currently relies on --pppd-use-peerdns=1 to retrieve DNS parameters from openfortivpn, however that is sort of a hack: #636.
  • DNS could be handled by openfortivpn call-back scripts. Such scripts would require specific sudo privileges. See Wrapper for ip.

External links

Online articles of interest:

DimitriPapadopoulos avatar Apr 18 '20 13:04 DimitriPapadopoulos

In any case it is till true that the noauth option requires root privilege, even for members of group dip:

$ groups
user adm cdrom sudo dip plugdev lpadmin lxd sambashare openfortivpn
$ 
$ env LANG=C ls -l /usr/sbin/pppd
-rwsr-xr-- 1 root dip 395144 Feb 11 16:03 /usr/sbin/pppd
$ 
$ /usr/sbin/pppd 38400 :192.0.2.1 noauth
/usr/sbin/pppd: using the noauth option requires root privilege
$ 

DimitriPapadopoulos avatar May 05 '20 17:05 DimitriPapadopoulos

Instead let's try whether modifying /etc/ppp/chap-secrets or /etc/ppp/pap-secrets can be a solution to avoid noauth as suggested in https://github.com/adrienverge/openfortivpn/issues/678#issuecomment-623090211.

DimitriPapadopoulos avatar May 05 '20 17:05 DimitriPapadopoulos

I can confirm that noauth does not seem to be required if we add this line to /etc/ppp/pap-secrets: * * "" * Before:

$ /usr/sbin/pppd 115200 :192.0.2.1 noipdefault noaccomp default-asyncmap nopcomp receive-all nodefaultroute nodetach lcp-max-configure 40 mru 1354
/usr/sbin/pppd: The remote system is required to authenticate itself
/usr/sbin/pppd: but I couldn't find any suitable secret (password) for it to use to do so.
$ 

After:

$ /usr/sbin/pppd 115200 :192.0.2.1 noipdefault noaccomp default-asyncmap nopcomp receive-all nodefaultroute nodetach lcp-max-configure 40 mru 1354
~�}#�!}!}!} }2}!}$}%J}#}$�#}%}&��}"���~

@zez3 On the other hand I don't think we want to follow that path:

  • We probably don't want such a global change to be applied to the PAP secrets file.
  • We should probably restrict the change to a specific hostname. However that would be on end-users and that's too much to ask from them.

We could perhaps revert ff290d1668d8419d255dbdf72afff262dcd3a686 to a mere warning when openfortivpn is not run as root. We would then have this warning:

WARN:   This process was not spawned with root privileges, this will probably not work.

and this error from pppd which lacks precision:

ERROR:  pppd: An error was detected in processing the options given, such as two mutually exclusive options being used.

Then we would also need an option to remove noauth.

Am I missing something?

DimitriPapadopoulos avatar May 05 '20 18:05 DimitriPapadopoulos

I agree, this root permission does not bother me much at this time.

zez3 avatar May 05 '20 20:05 zez3

@adrienverge What was the rationale behind using the pppd command instead of directly using a PPP library? Did any C PPP libraries exist in the first place? If they did exist, were they not complete enough for openfortivpn? Is kernel PPP support required? On Linux perhaps it doesn't make any sense because you need the PPP support in the kernel anyway. I really don't know.

I have found some PPP C code, although I have no clue at this point if it can replace pppd or not:

  • the Linux pppd and FreeBSD ppp of course
  • contiki
  • lwIP

Some of these projects might be small enough to be added as a dependency to openfortivpn. Perhaps we can get rid of the authentication part without too much work. It looks like PPP support in C is ~ 1000 lines of code.

DimitriPapadopoulos avatar May 06 '20 08:05 DimitriPapadopoulos

What was the rationale behind using the pppd command instead of directly using a PPP library? Did any C PPP libraries exist in the first place?

Either they didn't exist, or I wasn't aware of them. If they now exist and work, they could be a solution.

adrienverge avatar May 06 '20 08:05 adrienverge

But why do we need it in the first place? L2 frame HDLC encapsulation is just non-sense for me. It must be a good reason for all this...Perhaps NAT-traverse or something like that? " PPP frames are encapsulated in a lower-layer protocol that provides framing and may provide other functions such as a checksum to detect transmission errors. PPP on serial links is usually encapsulated in a framing similar to HDLC, described by IETF RFC 1662. " all of that is already provided by ssl tcp We establish an SSL connection with our Server and then we get an ip and some routes from it Can we not point those received routes behind a TUN interface or virtual dummy/loopback interface ? http://tuntaposx.sourceforge.net/index.xhtml it can even have an L2 mac address if needed by the user but this is a L3 tunnel anyway so no mac will go through. Please let me know what you think of this

zez3 avatar May 06 '20 14:05 zez3

@zez3 I really don't know. Isn't PPP encapsulation enforced by the SSL-VPN portal? Do we really have a choice here?

DimitriPapadopoulos avatar May 06 '20 14:05 DimitriPapadopoulos

I see that we open an SSL connection, some HTTP negotiation happens, we ask the gateway to start tunnelling, and I/O between pppd and the gateway is initiated: https://github.com/adrienverge/openfortivpn/blob/5d0a8ab423ce9c53d2512e1843a8b14d6dfd03f2/src/tunnel.c#L1036 I haven't seen code anywhere that would initiate PPP on our side. PPP just happens or so it seems to me.

@zez3 What I mean to say is that I am unable find an alternative myself, not that an alternative doesn't exist. I'm not good at networking and I am open to suggestions.

@adrienverge Do you recall whether PPP is enforced by the VPN gateway? Are there any alternatives to PPP, such as the TUN interface suggested by @zez3?

DimitriPapadopoulos avatar May 06 '20 15:05 DimitriPapadopoulos

@adrienverge Do you recall whether PPP is enforced by the VPN gateway? Are there any alternatives to PPP, such as the TUN interface suggested by @zez3?

I'm sorry, I don't know :/

adrienverge avatar May 06 '20 15:05 adrienverge

I believe we don't have a choice here. ppp with hdlc encoding is just how it is implemented on the server side, at least that's my understanding, but I may be wrong

mrbaseman avatar May 06 '20 18:05 mrbaseman

FWIW in OpenConnect we've been looking at the F5 BIG-IP SSL-VPN support. That's PPP-based too, and always used to have HDLC but now has an option for running without HDLC.

We're adding PPP support to OpenConnect — I was a little dubious but since it we only need really basic LCP and IPCP/IP6CP without all the bells and whistles that pppd supports, I think it's OK.

https://gitlab.com/dlenski/openconnect/commits/f5

It means we can run the client completely as a non-root user. For all the other protocols, NetworkManager already runs it like that; even setting up the tun device for it in advance so it doesn't even need privs to do that. http://www.infradead.org/openconnect/nonroot.html

It'd be really interesting to look at expanding to support FortiVPN in OpenConnect too. @adrienverge, @DimitriPapadopoulos how would you feel about maintaining that?

dwmw2 avatar May 11 '20 08:05 dwmw2

@dlenski

dwmw2 avatar May 11 '20 15:05 dwmw2

What @dwmw2!

Integrating into OpenConnect has so many advantages that reduce the tedium of dealing with boilerplate issues in VPN clients, as I wrote about here recently (in regards to the PPP-based protocol which we're currently trying to implement): https://gitlab.com/openconnect/openconnect/-/issues/107#note_336889894

dlenski avatar May 11 '20 20:05 dlenski

It would be nice if someone would be able to demo speed compare the LCP HDLC PPP to the TLS implementations in OpenConnect before or after you actually implement it. I was thinking to do the same with the openfortivpn but I am still thinking at how can I achieve that. (Perhaps I just need to use the Forti Web-Mode Portal) What I believe at the moment is that all that PPP stuff is a non-sense since this comes from the era of serial modems and the most recent TLS implementations already provide all the error reliability integrity checking and connection (re-)negotiation part. I think that we don't need to fit all this stuff in a HDLC L2 frame and repeat all the above checking. If someone could explain why and enlighten me to come to the senses Please DO!

zez3 avatar May 12 '20 08:05 zez3

@zez3 As far as I can see the FortiGate portal uses PPP and HDLC. It's not our choice. It doesn't make sense to compare TLS vs. PPP and HDLC, openfortivpn does use TLS.

DimitriPapadopoulos avatar May 12 '20 08:05 DimitriPapadopoulos

We have the HDLC mode working in OpenConnect now, doing PPP over HDLC over TLS. It's all in OpenConnect itself without using external pppd. It's still in the 'f5' branch for now; will look at making it merge-ready and CPU-optimising it.

This has been tested with the F5 BIG-IP SSL VPN, but we've tried to keep the PPP bits separate from the F5 specifics, so adding a new PPP-based protocol like Fortinet's should be fairly trivial.

You'll note that the f5_obtain_cookie() login function is empty for now and we rely on authenticating externally then just invoking openconnect with the -C (cookie) option. That's only because authentication is the boring part and we'll do that last.

I'm looking at auth_request_vpn_allocation() — am I right in thinking that it doesn't care about the response content at all, only that it gets a 2xx response? In OpenConnect we split the authentication phase (which runs as the user, using user's certs, etc.) from the connection phase (which can run as a separate unprivileged user, perhaps spawned from NetworkManager or something like that, having been given the auth cookie). It looks like those two GET requests to /remote/index and /remote/fortisslvpn should be done as part of the auth phase? While getting the config (auth_get_config()) would be part of the connection phase?

dwmw2 avatar May 12 '20 09:05 dwmw2

About auth_request_vpn_allocation(), as far as I can see we don't even check the status code of the HTTP response - we just need a response. Perhaps that could be improved but it hasn't been an issue so far.

I had a look at OpenConenct. Just wondering, why don't you re-use existing reference code? For example for HDLC we re-use the reference code directly from RFC 1662, as is. For HTTP and XML parsing I was thinking of using 3rd party code, fast and with a small footprint, that can be included as a mere C file/header (#347), for example PicoHTTPParser for HTTP and Yxml for XML.

DimitriPapadopoulos avatar May 12 '20 10:05 DimitriPapadopoulos

Is there a FortiGate server anywhere I could have a test account on?

I can throw up a shell of a 'fortinet' protocol in OpenConnect, cloned from the F5 one, and it really shouldn't take long to make it functional now we have PPP+HDLC working. It'd be really useful to tease out any accidental F5-isms in our PPP code.

dwmw2 avatar May 12 '20 10:05 dwmw2

As for the separation between authentication and connection, the sequence of operations can be seen in run_tunnel(). I believe the separation is right here: https://github.com/adrienverge/openfortivpn/blob/363898661f842c0cab6449aa6c206531c015d4d2/src/tunnel.c#L1076-L1083 What happens before is authentication (ssl_connect() and auth_log_in()) and what happens after is connection (auth_request_vpn_allocation(), ssl_connect() and the rest). Does this make any sense?

That said I don't think all of the connection phase can or should be outsourced to NetworkManager. Besides note that NetworkManager is not always available, typically on servers. Setting up the TLS/PPP tunnel should not require special privileges as far as I can see (right now it does because of pppd). Only the name resolution and routing changes require privileges and are best handled by NetworkManager, systemd-networkd or whatever is being used to manage networking. Yet I can see why you want to be able to run the tunnel itself under a different account, separately from authentication.

DimitriPapadopoulos avatar May 12 '20 10:05 DimitriPapadopoulos

Reference code... I try to where I can. I was very annoyed in the early days when none of the existing HTTP libraries really let me have enough control of the underlying SSL connection, so I ended up having to do that part myself. I note ocserv is using the http-parser library... but I also note that that is a constant source of breakage as http-parser breaks its ABI and ocserv stops working. (OK, it's happened twice so maybe constant is an overstatement but that's twice too many)

We do use libxml for XML, and other libraries where appropriate. I don't like reinventing the wheel and I was even baulking at PPP for a long time, before I realised that what we're doing here is really only a subset of PPP and it doesn't fit the pppd model particularly well either.

dwmw2 avatar May 12 '20 10:05 dwmw2

Authentication happens once. It results in the cookie.

Connection can happen more than once. If the underlying network changes (you undock your laptop and move to wireless, or suspend/resume) while the cookie is still valid, or even if there's just a brief network outage or a NAT failure causing the existing connection to stall, then the client can reconnect using the same cookie, and expects to have the same IP addresses and configuration on the VPN tunnel when it does.

So the important distinction for me with the auth_request_vpn_allocation() call was whether it happens once or whether it happens each time.

dwmw2 avatar May 12 '20 10:05 dwmw2

NetworkManager... right, NM isn't always available, and is never mandatory. It only really sits in the middle, optionally, between the authentication and the connection. Run OpenConnect from the command line, and it can do all at once (but probably needs to be run as root so it can do the network setup).

If you run from NetworkManager, or ConnMan, or anything else like that, then the two-phase design lets you run the authentication in the user's session, interactively and with access to their SSL keys etc., and then hand the actual connection over to an unprivileged process, with NetworkManager playing the middle-man, creating the actual tun device for the unprivileged process to use, and actually setting up the IP config on it when it gets the results back (via D-Bus) from openconnect.

But yes, it's all optional. Running directly from the command line also works.

dwmw2 avatar May 12 '20 10:05 dwmw2

So the important distinction for me with the auth_request_vpn_allocation() call was whether it happens once or whether it happens each time.

I'm afraid don't know. I've always used run_tunnel() as is.

DimitriPapadopoulos avatar May 12 '20 10:05 DimitriPapadopoulos

Reference code... I try to where I can.

Specifically for HDLC you might want to use the code from RFC 1662 - unless you have found performance issues?

DimitriPapadopoulos avatar May 12 '20 10:05 DimitriPapadopoulos

Yeah, that's what we'll use for the FCS but we haven't actually done that yet as F5 doesn't care. It doesn't have the actual HDLC bit-stuffing, does it?

dwmw2 avatar May 12 '20 11:05 dwmw2

I've pulled in the code from RFC1662 and implemented FCS support. Can't push to gitlab as @dlenski has been forgetting to add signed-off-by, so it's at http://git.infradead.org/users/dwmw2/openconnect.git/commitdiff/8b122363

On the performance front, I suspect calculating the FCS as we pass through the data doing the HDLC (un)escaping is probably a better use of the data cache than doing a second pass just to do the FCS, so I've done that.

dwmw2 avatar May 12 '20 20:05 dwmw2

Maybe it will be more simple to run daemon as root and provide control interface for users? Like wpa_supplicant does?

Perlovka avatar Sep 01 '20 18:09 Perlovka

Either they didn't exist, or I wasn't aware of them. If they now exist and work, they could be a solution.

OpenConnect itself now contains an LGPL-licensed, event-driven, rather-complete implementation of PPP (either with or without HDLC framing) that does not depend on pppd in any way.

I also wrote some tests to make OpenConnect's PPP implementation talk to pppd and verify that they can successfully negotiate a connection with a variety of parameters.

dlenski avatar Sep 01 '20 20:09 dlenski

@Perlovka You're right, however the daemon itself shouldn't need to run as root either, at least for creating the tunnel. Only routing and DNS server changes at the system level require root privileges (as well as running pppd of course).

DimitriPapadopoulos avatar Sep 02 '20 05:09 DimitriPapadopoulos