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

Revisit running openfortivpn as root? #650

Open
DimitriPapadopoulos opened this issue Apr 18, 2020 · 40 comments
Open

Revisit running openfortivpn as root? #650

DimitriPapadopoulos opened this issue Apr 18, 2020 · 40 comments

Comments

@DimitriPapadopoulos
Copy link
Collaborator

DimitriPapadopoulos commented Apr 18, 2020

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: Add dns suffix information to informative message #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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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 #678 (comment).

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented May 5, 2020

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 ff290d1 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
Copy link

zez3 commented May 5, 2020

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

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented May 6, 2020

@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
Copy link
Owner

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
Copy link

zez3 commented May 6, 2020

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
Copy link
Collaborator Author

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

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented May 6, 2020

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:

int run_tunnel(struct vpn_config *config)

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
Copy link
Owner

@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
Copy link
Collaborator

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
Copy link

dwmw2 commented May 11, 2020

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
Copy link

dwmw2 commented May 11, 2020

@dlenski

@dlenski
Copy link

dlenski commented May 11, 2020

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
Copy link

zez3 commented May 12, 2020

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
Copy link
Collaborator Author

DimitriPapadopoulos commented May 12, 2020

@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
Copy link

dwmw2 commented May 12, 2020

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
Copy link
Collaborator Author

DimitriPapadopoulos commented May 12, 2020

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
Copy link

dwmw2 commented May 12, 2020

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
Copy link
Collaborator Author

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:

openfortivpn/src/tunnel.c

Lines 1076 to 1083 in 3638986

if (ret != 1) {
log_error("Could not authenticate to gateway. Please check the password, client certificate, etc.\n");
log_debug("%s %d\n", err_http_str(ret), ret);
ret = 1;
goto err_tunnel;
}
log_info("Authenticated.\n");
log_debug("Cookie: %s\n", tunnel.cookie);

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
Copy link

dwmw2 commented May 12, 2020

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
Copy link

dwmw2 commented May 12, 2020

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
Copy link

dwmw2 commented May 12, 2020

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
Copy link
Collaborator Author

DimitriPapadopoulos commented May 12, 2020

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
Copy link
Collaborator Author

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
Copy link

dwmw2 commented May 12, 2020

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
Copy link

dwmw2 commented May 12, 2020

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
Copy link

Perlovka commented Sep 1, 2020

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

@dlenski
Copy link

dlenski commented Sep 1, 2020

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
Copy link
Collaborator Author

DimitriPapadopoulos commented Sep 2, 2020

@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
Copy link
Collaborator Author

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
Copy link

dlenski commented Dec 15, 2020

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.

😁

@dlenski
Copy link

dlenski commented Feb 8, 2021

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 [feature request] DTLS support #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
Copy link

zez3 commented Feb 9, 2021

* 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
Copy link
Contributor

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=,chal_msg=Please enter your token code
Failed to find or parse web form in login page
Failed to complete authentication

@dlenski
Copy link

dlenski commented Feb 16, 2021

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=,chal_msg=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
Copy link
Contributor

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

@mrbaseman
Copy link
Collaborator

mrbaseman commented Feb 17, 2021

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
Copy link

zez3 commented Apr 15, 2021

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
Copy link
Collaborator Author

Note that merging #1048 would go a long way to permit running openfortivpn as non-root.

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

No branches or pull requests

8 participants