testing a new SSL+ESP VPN

[David Woodhouse]

On Thu, 2016-10-13 at 20:55 -0700, Daniel Lenski wrote:
> I've now got things pretty close to 100% working over SSL. The
> authentication is done using libxml, so I can now connect to my GP VPN
> using...
>
>     openconnect --protocol=gp --user Myusername --certificate certwpk.pem gateway.company.com

Nice!

I note you also added --globalprotect. Let's not do that; it was a
mistake with Juniper. Let's just use --protocol.

It would also be quite nice to have it autodetect the protocol when
just given a URL. Just something to bear in mind... 

> The cookie has the following format, which preserves everything that's
> returned by the login request, and is subsequently needed to get the
> configuration, connect to the SSL tunnel, and log the session out:
> 
>     USER=Username;AUTH=deadbeefdeadbeef;PORTAL=GatewayName;DOMAIN=domain

OK. Fairly much like Juniper in theory then, although we never did
establish that we need anything but the DSID cookie there.

> I tried to reuse existing coding style and interfaces from
> openconnect, but there are a few things that don't work very well:
> 
> 1. In order to log the user out, a separate HTTPS request has to be
> issued. However, openconnect_open_https() and do_https_request() don't
> work once got_cancel_cmd is set. Currently I have to trick openconnect
> by temporarily setting got_cancel_cmd=0 during the logout request. Do
> you have any suggestions on how to do this more cleanly?

Hm. The other protocols are *also* supposed to issue a separate call to
logout. That's cstp_bye() and ... hm, is that not implemented for
Juniper? I thought it had been.

For Cisco it does seem to work but perhaps only because we use the
existing connection and just write the BYE packet to it. We never poll
and wait for input, thus we never check the got_cancel_cmd flag.

You have to establish a new connection though, right? I think it's
probably OK to set vpninfo->got_cancel_cmd=0 in the mainloop right
before calling proto->vpn_close_session. If we get cancelled *again*
then we'll give up on that too.

Do try really hard to find a way you can make it write-and-forget like
the Cisco one though :)

> 2. As I understand it, the two-phase design of openconnect is intended
> to do all the messy XML processing in the unprivileged phase, so that
> only the simplest parts of the SSL/DTLS/ESP tunnel setup need to be
> done by root. However, this doesn't work so well with GlobalProtect,
> because all of the routing information comes in an XML file which
> isn't received until after the login and getconfig requests. So I am
> doing some XML processing during the connection process, in
> gpst_parse_config_xml().

No, that's OK. It's not *so* much about avoiding the text processing.
The two-phase thing is mostly about having access to the user's
certificates, and being able to interact with the user for password
input, etc.

What we don't want is for there to be HTTP redirects to a different
host in the second "connect" phase — because that *might* mean we have
to ask the user if we want to accept this new server's invalid
certificate.

And it's also quite important that the connection works with just the
cookie(s) and *not* having access to the user's certificate. Does it?

FWIW if you hack the default protocol in openconnect_vpninfo_new() in
library.c you can test this with NetworkManager too.

> 3. The existing process_split_xxclude() only handled IPv4 routes
> formatted as "10.0.0.0/255.0.0.0", but the GP gateways returns split
> routes formatted as "10.0.0.0/8". It's possible to unambiguously
> distinguish the two (because no IPv4 netmask address can possibly have
> a decimal integer value <=32) so I extended it to handle this case.

OK.

> Still to do:
> 
> 1. Have someone else test it with another GP VPN so we know I'm not crazy. :-D
> 2. Figure out how to do keepalive or DPD properly.

This is most important on the ESP link, and if you have the ESP config
information then you can use wireshark to decode packets.

> 3. Figure out how to request or determine the MTU (I'm pretty sure ESP
> will drop or fragment packets without getting this right).

Some of the code we have for obtaining the kernel's idea of the link
MTU via TCP_INFO ioctls and such, should probably be made a little more
generic. If you know the ESP config you can calculate the MTU on it.
That can probably be shared with Juniper.

> Should I go ahead and submit my patches for review at this point, or
> would that be premature?

If you want to tidy them up so they're a bit less train-of-thought,
that might make them easier to look over. But not the end of the world;
I can just read your final gpst.c without really looking at the
history.

But where you've got fixes to the existing code (e.g. the excessive
url-encoding, the 'const' to buf_append_urlencoded()), I could merge
those now if you want to submit them as separate patches with a Signed-
off-by: tag.

I'm planning to do an OpenConnect 7.08 release soon — it *was* waiting
for the MTU fixes in https://github.com/openssl/openssl/pull/1666 but
now it's also waiting for related fixes in
https://gitlab.com/gnutls/gnutls/issues/140 as well.

It'd be good to merge GP support after 7.08 perhaps.

-- 
dwmw2


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5760 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20161015/8a520958/attachment.bin>