Revisit running openfortivpn as root?

DimitriPapadopoulos commented 4 years ago

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_setroute() / 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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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?

zez3 commented 4 years ago

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

DimitriPapadopoulos commented 4 years ago

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

adrienverge commented 4 years ago

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.

zez3 commented 4 years ago

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

DimitriPapadopoulos commented 4 years ago

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

DimitriPapadopoulos commented 4 years ago

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?

adrienverge commented 4 years ago

@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 :/

mrbaseman commented 4 years ago

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

dwmw2 commented 4 years ago

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 commented 4 years ago

@dlenski

dlenski commented 4 years ago

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

zez3 commented 4 years ago

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!

DimitriPapadopoulos commented 4 years ago

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

dwmw2 commented 4 years ago

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?

DimitriPapadopoulos commented 4 years ago

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.

dwmw2 commented 4 years ago

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.

DimitriPapadopoulos commented 4 years ago

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.

dwmw2 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.

DimitriPapadopoulos commented 4 years ago

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 commented 4 years ago

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?

dwmw2 commented 4 years ago

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 commented 4 years ago

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.

Perlovka commented 3 years ago

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

dlenski commented 3 years ago

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.

DimitriPapadopoulos commented 3 years ago

@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 commented 3 years ago

The patch provided in #801 can hopefully get rid of pppd at some point. That's good news. If it works, only routing and DNS changes will require root privileges.

dlenski commented 3 years ago

If it works, only routing and DNS changes will require root privileges.

Just a reminder that I'd love some helping filling out — or at least testing — authentication for Fortinet VPNs over at OpenConnect. https://gitlab.com/openconnect/openconnect/-/blob/ppp_rebased/fortinet.c#L36-39

The patch provided in #801 can hopefully get rid of pppd at some point.

In addition to "getting rid of pppd", joining forces with OpenConnect will get you all the benefits of instant CLI support for pretty much every OS in existence, plus GUIs for Linux (NetworkManager, KDE conn man) and macOS and Android… and Windows and maybe eventually iOS too.

:grin:

dlenski commented 3 years ago

I now have a fully working Fortinet implementation in OpenConnect, including authentication.

Build from this branch (https://gitlab.com/openconnect/openconnect/commits/ppp_protocols) and run like so…

openconnect --prot=fortinet [-u USERNAME] fortinet.vpn.server[:port] \
    --vvv --dump # for tons of extra logging

The good:

No reliance on pppd whatsoever; OpenConnect contains a completely independent PPP implementation.
Ability to run as non-root
- No need to GET /remote/index or GET /remote/fortisslvpn (openvortivpn does these, but they're unnecessary, and I verified via MITM that a recent Forticlient doesn't do GET /remote/index at all)
Works with GNOME-NetworkManager GUI, or any other OpenConnect GUI that you can rebuild against the library.
Integration with all of the other scripts and tools designed to work with OpenConnect (such as vpn-slice for split-tunnel setup and customization, 2-phase authentication with openconnect --authenticate, etc.)
~I don't have any way to test 2FA/MFA on the Fortinet VPN that I have access to, so that probably doesn't work (https://gitlab.com/openconnect/openconnect/-/issues/225)~ 2FA is now working, thanks to testing by @emelenas
OpenConnect knows how to talk DTLS ~, but we haven't figured out how to initiate it for Fortinet yet.~ and we now have working DTLS for Fortinet. It should generally perform much better than TLS. (cf #473)

The to-do:

I can't figure out how to pause and reconnect a connection with the same SVPNCOOKIE, without reauthenticating. (UPDATE: This seems to be either a design flaw in the protocol, or a limitation of the server that I have access to.)

zez3 commented 3 years ago

* I don't have any way to test 2FA/MFA on the Fortinet VPN that I have access to, so that probably doesn't work

I can test that. Hopefully somewhere next week.

* OpenConnect already knows how to talk DTLS, but we haven't figured out how to initiate it for Fortinet yet. (cf #473)

If that works I am willing and have to make some donations. Hmm, I cannot seem to find a donation address.

emelenas commented 3 years ago

I have tested this code

With no 2FA required, works as expected
With 2FA, openconnect expects to find a "web form in login page", but it finds a token request instead:

_Got HTTP response: HTTP/1.1 200 OK Date: Mon, 15 Feb 2021 19:52:23 GMT Server: xxxxxxxx-xxxxx Set-Cookie: SVPNCOOKIE=; path=/; expires=Sun, 11 Mar 1984 12:00:00 GMT; secure; httponly; Set-Cookie: SVPNNETWORKCOOKIE=; path=/remote/network; expires=Sun, 11 Mar 1984 12:00:00 GMT; secure; httponly Transfer-Encoding: chunked Content-Type: text/plain X-Frame-Options: SAMEORIGIN Content-Security-Policy: frame-ancestors 'self'; object-src 'none'; script-src 'self' https 'unsafe-eval' 'unsafe-inline'; X-XSS-Protection: 1; mode=block X-Content-Type-Options: nosniff Strict-Transport-Security: max-age=31536000 HTTP body chunked (-2) < ret=2,reqid=198233047,polid=1-1-17689520,grp=RR,portal=RR,magic=1-17689520,tokeninfo=,chalmsg=Please enter your token code Failed to find or parse web form in login page Failed to complete authentication

dlenski commented 3 years ago

I have tested this code

With no 2FA required, works as expected

With 2FA, openconnect expects to find a "web form in login page", but it finds a token request instead:

Thanks for testing!

ret=2,reqid=198233047,polid=1-1-17689520,grp=RR,portal=RR,magic=1-17689520,tokeninfo=,chalmsg=Please enter your token code Failed to find or parse web form in login page Failed to complete authentication

This looks like it'll be reasonably easy to parse and submit. Could you open an issue for us at OpenConnect, @emelenas, so I can ping you there when I have something for you to test?

emelenas commented 3 years ago

This is it: https://gitlab.com/openconnect/openconnect/-/issues/225

mrbaseman commented 3 years ago

Hey guys,

this is the openfortivpn repository. We appreciate suggestions from other projects (e.g. ideas on how things can be handled better, example implementations,...), we are open to collaborate, but I think this discussion has moved far away from openfortivpn.

Please handle openconnect issues within that project. It's ok, to link them here, if there is some relation to openfortivpn.

Thank you.

zez3 commented 3 years ago

Perhaps it would help to have a look at SLiRP and the more recent implemnetation https://github.com/rootless-containers/slirp4netns/blob/master/slirp4netns.1.md#description and how they handle all in userspace

DimitriPapadopoulos commented 1 year ago

Note that merging https://github.com/adrienverge/openfortivpn/pull/1048 would go a long way to permit running openfortivpn as non-root.

adrienverge / openfortivpn