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

Sync changes from the upstream repo #1

Open
wants to merge 148 commits into
base: globalprotect
Choose a base branch
from

Conversation

TysonAndre
Copy link

The wrapper scripts using this repo continue to work (including 2fa), and the global protect VPN continues to work.

It's probably easier to rebase the globalprotect branch against upstream and close this PR.

dwmw2 and others added 30 commits June 22, 2017 15:09
Signed-off-by: David Woodhouse <[email protected]>
Small mod to esp.c to support FreeBSD.
Suppress the InsecureRequestWarning messages in globalprotect-challenge-login.py when using the --no-verify flag.
Suppress warnings from noverify in challenge-login
In my case, XML had "6" in 13th argument instead of expected -1;
removing the check didn't seem to break anything.
Remove supposedly excessive check to fix dlenski#50
Signed-off-by: David Woodhouse <[email protected]>
.... even when conversion fails. Otherwise we end up trying to free a
member of argv[], which never works well.

Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
We need it to make the charset testing work.

Signed-off-by: David Woodhouse <[email protected]>
This was filed against Ľubomír Carik's github project for openconnect-gui;
openconnect/openconnect-gui#101

It isn't perfect, as the ANSI code page on Windows can be different
from the OEM code page used for the console, so fgetws() is likely
to do the wrong thing — which is why we force-opened the console and
used ReadConsoleW() in the first place. But perfect is the enemy of
good in this case, as reading from something other than stdin is
*definitely* wrong. We still use ReadConsoleW() when stdin does happen
to be the console, so that part shouldn't regress.

I hate Windows...

Signed-off-by: David Woodhouse <[email protected]>
It's not worth the effort to keep it building for <3.2 any more; nobody
cares... or noticess when we accidentally break it. So kill it; we've
been threatening to for ages.

Use 3.2.10 as the base because 3.2.x before that was broken on Windows.

Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
We don't support any verions of GnuTLS that can't do these, any more.

Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Mike Miller points out that LC_ALL has precedence over LC_CTYPE, so the
test fails when LC_ALL is set to something different.

Signed-off-by: David Woodhouse <[email protected]>
These will be used in GlobalProtect protocol support, so it makes sense
to factor them out into shared utility functions rather than use slight
variants for each protocol.

Signed-off-by: Daniel Lenski <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
This fixes the "Unexpected response of size 3 after hostname packet" or "Invalid packet waiting for KMP 301" errors
which I get intermittently when connecting to an old Juniper NC server:

    $ openconnect --prot=nc -vvvv
    ...
    NCP-Version: 2
    ...
    > 0000: 18 00 00 04 00 00 00 0c 00 64 65 61 64 62 65 65
    > 0010: 66 2d 31 32 33 bb 01 00 00 00 00
    Read 3 bytes of SSL record
    < 0000: d2 01 00
    Read 465 bytes of SSL record

Here's what is going on: this server is (sometimes) concatenating the 3-byte
response packet together with the longer IP-configuration packet that
follows.  When they are concatenated together, the server sends only a
single 2-byte length prefix for both (0x01d2 = 466).

Signed-off-by: Daniel Lenski <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
…tect ESP

If a protocol wishes to have dtls_state set to DTLS_SLEEPING after closing
UDP, then it must now do so explicitly, because the mainloop will no longer
set it.  This patch make both existing protocols set dtls_state explicitly
after closing the UDP connection.  (The nc protocol already did so
explicitly, but the anyconnect protocol didn't.)

The previous behavior, wherein dtls_state was *always* set to DTLS_SLEEPING
after closing UDP, was incompatible with the GlobalProtect VPN.
Disconnecting and reconnecting GlobalProtect VPN doesn't just require
require reconnecting the UDP socket and resending probes; it actually
invalidates any previously-obtained ESP secret.

Signed-off-by: Daniel Lenski <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
…in preparation for supporting GlobalProtect ESP

The existing Juniper ESP code can be almost entirely reused for
GlobalProtect ESP, except for the Juniper-specific code for sending and
recognizing the probe packets used for ESP initiation and DPD.

The Juniper-specific code is moved into functions names esp_send_probes
(sends Juniper probe packets) and esp_catch_probe (recognizes Juniper probe
packet responses), which are called via vpn_proto member functions.

Signed-off-by: Daniel Lenski <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
…ng GlobalProtect ESP

The existing ESP key setup code can be almost entirely reused for
GlobalProtect ESP, except for the fact that esp_setup_keys() always
overwrites the secret keys with new random keys.

Since GlobalProtect ESP always uses keys provided by the server, a new
argument is added to esp_setup_keys() to make this behavior optional.
The Juniper-specific code in oncp.c calls it with new_keys=1 in order
to explicitly request it.

Signed-off-by: Daniel Lenski <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
dlenski and others added 30 commits April 21, 2018 16:44
When going from the "normal" username/password login form to the username/challenge form,
the username should be reused, but placed in a hidden field so that the user isn't
prompted to re-enter it.
Signed-off-by: andor-pierdelacabeza <[email protected]>
Signed-off-by: andor-pierdelacabeza <[email protected]>
Signed-off-by: andor-pierdelacabeza <[email protected]>
Signed-off-by: andor-pierdelacabeza <[email protected]>
Signed-off-by: andor-pierdelacabeza <[email protected]>
Signed-off-by: andor-pierdelacabeza <[email protected]>
Signed-off-by: andor-pierdelacabeza <[email protected]>
Signed-off-by: andor-pierdelacabeza <[email protected]>
Signed-off-by: andor-pierdelacabeza <[email protected]>
In July 2016, the "Fixed regression with CSTP MTU handling" patch
(http://git.infradead.org/users/dwmw2/openconnect.git/commitdiff/90e1555494dbc1cf462552679f9aa3d30451d123)
allowed openconnect to gracefully handle uncompressed CSTP packets larger
than the negotiated MTU.

This patch extends that approach to tolerate compressed packets which are
larger than the negotiated MTU after decompression.
…ts across protocols

We've now implemented mechanisms to tolerate larger-than-expected packets for:

  - Uncompressed CSTP packets ("Fixed regression with CSTP MTU handling"
    patch in July 2016)

  - Uncompressed oNCP packets ("Do not drop vpn connection if packet arrived
    is larger than MTU" patch in May 2017)

  - Uncompressed GPST packets (in original merge from March 2018; this is a
    virtual necessity for GlobalProtect because it has no functional
    mechanism for negotiating the MTU)

  - Uncompressed ESP packets ("check for oversize ESP packets, with 256
    bytes of headroom above calculated" in March 2018; GlobalProtect requires
    this for the aforementioned reason)

  - Compressed CSTP packets (preceding patch in this series)

Since this is a requiring issue across protocols, it's useful to align the
naming, commenting, and packet sizing-tolerance across the source files.

  1) Use receive_mtu everywhere as the name for the maximum tolerated size of an
     incoming packet.
  2) Insert similar comments explaining its purpose everywhere it's used.
  3) Use receive_mtu = MAX(16384, vpninfo->ip_info.mtu) for all TLS-based
     tunnels, because 16384 is the maximum TLS record size.
  4) Use receive_mtu = MAX(2048, vpninfo->vpninfo->ip_info.mtu + 256) for
     all UDP-based tunnels, because the MTU of IP datagrams on the public
     internet is effectively ~1500.
- Include both the TCP- and UDP-based protocols' compression details
- The UDP-based protocol really can't be connected by the time this
  prints, since the mainloop hasn't had enough time to receive the
  connection confirmation packets; show it as "in progress"

Before (with default verbosity):

    Connected as 10.0.0.3 + dead::be:ef, using SSL + deflate
    Established DTLS connection (using GnuTLS). Ciphersuite (DTLS1.2)-(RSA)-(AES-128-GCM).

After:

    Connected as 10.0.0.3 + dead::be:ef, using SSL + Deflate, with DTLS + LZS in progress
    Established DTLS connection (using GnuTLS). Ciphersuite (DTLS1.2)-(RSA)-(AES-128-GCM).

Signed-off-by: Daniel Lenski <[email protected]>
…port in the docs

Also clarifies the command-line options regarding compression

Signed-off-by: Daniel Lenski <[email protected]>
The current oNCP (Juniper) protocol support sets "Connection: close" in all
HTTP requests.  This is not ideal because it requires many TLS handshakes
and round-trips, making the connection very slow to start when the latency
of the connection to the gateway is high, especially if the number of
authentication forms and redirects is large.

Simply removing the "Connection: close" header causes the oNCP connection
to fail; the server doesn't interpret the first packet sent over the oNCP
tunnel correctly (the vestigial authentication packet).

However, it appears that the "Connection: close" header *only* needs to be
specified for this final HTTP request, and not for any of the prior ones.
The presence of this header seems to signal to the gateway that it should
stop treating this as an HTTP connection, and start treating it as an
oNCP tunnel.

Tested on two different Juniper gateways, one which returns
"NCP-Version: 2" and one which returns "NCP-Version: 3" in response to
the oNCP negotiation requests.
The current oNCP (Juniper) protocol support issues two separate
oNCP negotiation requests.

1) POST /dana/js?prot=1&svc=1 HTTP/1.1
   <ignore response body>
   <teardown and restart TLS connection>

2) POST /dana/js?prot=1&svc=4 HTTP/1.1
   <continue using open TLS connection for oNCP tunnel>

The first of these two requests appears to be totally unnecessary, based on
testing with two different Juniper gateways, one of which returns
"NCP-Version: 2" and one which returns "NCP-Version: 3" in response to the
oNCP negotiation requests.

Removing the first request saves an additional TLS negotiation (2-3
roundtrips with TLS 1.0) and allows the connection to start faster.
The GlobalProtect "cookie" is an overstuffed monstrosity, due to the
requirement to retain a few random, non-secret values in order to logout
successfully (see gpst_bye):

    authcookie=d41d8cd98f00b204e9800998ecf8427e&portal=Gateway-X&user=user.name&domain=big-corp

Until now, I've avoided including the computer field in this cookie, on the assumption that it
can reproduced at any time using vpninfo->localname. However, it appears that this value can't always
be reproduced correctly when running under NetworkManager:

    dlenski/network-manager-openconnect#7

In order to be more robust, this patch therefore also includes the local hostname in the cookie:

    authcookie=d41d8cd98f00b204e9800998ecf8427e&portal=Gateway-X&user=user.name&domain=big-corp&computer=hostname
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 this pull request may close these issues.