Daniel Lenski [Thu, 21 Dec 2017 06:56:58 +0000 (22:56 -0800)]
Protocols should try explicitly request the same IP addresses on reconnect, since they will abort if new addresses are sent by the server.
* GlobalProtect:
- Supported and used by official clients (POST /ssl-vpn/getconfig.esp with preferred-ip form field).
- GlobalProtect servers often give different IP addresses on reconnect if this mechanism is *not* used,
so this mechanism is necessary.
- Same mechanism appears to exist for IPv6 (preferred-ipv6) and was added to OpenConnect in d6db0ec03394234d41fbec7ffc794ceeb486a8f0, even though IPv6 support is not yet complete.
* AnyConnect:
- Not (yet) supported by ocserv
- It appears that *some* AnyConnect server will try to provide the IP address provided in the X-CSTP-Address
*request* header along with the CONNECT request, but other servers appear not to
- This patch reproduces the behavior of GPST: attempt to request same IPv4 and IPv6 addresses on reconnect,
via CONNECT headers.
* Juniper:
- There does not appear to be any way to provide this using the Juniper NC protocol.
- No known reports of Juniper servers giving out different IP address on reconnect.
Sergei Trofimovich [Fri, 8 May 2020 14:39:41 +0000 (10:39 -0400)]
gnutls: prevent buffer overflow in get_cert_name
The test suite for ocserv calls openconnect with a certificate that has
a name that is 84 bytes in length. The buffer passed to get_cert_name is
currently 80 bytes.
The gnutls_x509_crt_get_dn_by_oid function will update the buffer size
parameter if the buffer is too small.
RETURNS
GNUTLS_E_SHORT_MEMORY_BUFFER if the provided buffer is not long
enough, and in that case the buf_size will be updated with the
required size. GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE if there are no
data in the current index. On success 0 is returned.
Use a temporary variable to avoid clobbering the namelen variable that is
passed to get_cert_name.
Bug: https://bugs.gentoo.org/721570 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> Signed-off-by: Mike Gilbert <floppym@gentoo.org>
David Woodhouse [Mon, 4 May 2020 10:56:36 +0000 (11:56 +0100)]
Use shorter pathname for COPR RPM build
If the path of SOCKET_WRAPPER_DIR is too long, it doesn't fit in the
sun_path field of the sockaddr_un, and libsocket_wrapper gets very
unhappy, reporting 'Too many unix sockets'. Despite actually only ever
trying *one* path over and over again 1024 times due to truncation.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Fri, 1 May 2020 18:49:25 +0000 (11:49 -0700)]
stop asking users to report unexpected GP login argument arg[19]="4"
We still don't know what this one means (my wild guess is that it's telling the client to prefer IPv4), but newer GP servers always send it and it's basically uninteresting.
David Woodhouse [Thu, 30 Apr 2020 16:41:20 +0000 (17:41 +0100)]
Attempt to fix EPEL8 build
Use --without-gnutls-version-check; as if EPEL8 *does* get the fix for
the zero-client-random bug it will probably come without a version bump.
This also partially reverts commit 68641c0393e which disabled the use of
--with-default-gnutls-priority on *all* EPEL versions, but since I wasn't
building for EPEL8 at that point I don't think it was done for EPEL8
specifically, and can probably be restored.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Wed, 29 Apr 2020 21:34:43 +0000 (14:34 -0700)]
add and fix a few changelog entries
One significant user-facing entries left out of v8.09 changelog:
* modernized Juniper TNCC script
Two were labeled as being in v8.08 when in fact they weren't merged until v8.09:
* GlobalProtect MRs (!90, !93, !95)
* disabling of Nagle's algorithm for TLS sockets
Use OpenSSL X509_check_host() and X509_check_ip() correctly.
These functions return 1 for a successful match, 0 for a failed match,
-1 for an internal error, or -2 if the certificate is malformed.
OpenConnect has been treating any value other than zero as a success,
meaning that an attacker who could get a trusted CA to issue an invalid
certificate (on which the ASN.1 decoder fails, for example), could use
that to assume *any* identity.
Daniel Lenski [Thu, 23 Apr 2020 17:30:40 +0000 (10:30 -0700)]
fix IPv4 split-{in,ex}clude routes with misspecified host bits
Some VPN platforms (GlobalProtect, apparently) allow administrators to input
such non-canonical IPv4 routes, and some routing configuration utilities
(apparently *not* iproute2) simply do not accept such non-canonical IPv4
routes.
An example of the confusion this can cause:
https://lists.infradead.org/pipermail/openconnect-devel/2020-April/005665.html
The robustness principle suggests that the best thing to do here is to fix
these routes, but complain about them while we're at it.
David Woodhouse [Sat, 25 Apr 2020 08:54:28 +0000 (09:54 +0100)]
Fix dependencies and tests/configs/server-cert.prm to dist
Strictly, *break* the dependencies. We don't want server-cert.pem being
gratuitously rebuilt. It's breaking the CI because the file isn't pristine
when 'make tmp-distdir' runs.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Wed, 22 Apr 2020 14:30:11 +0000 (15:30 +0100)]
Update SoftHSM token import scripting and reimport
The slot numbers get reassigned now.
The RSA key modulus had been imported with a leading zero bytes,
confusing SoftHSM when it tried to perform CKM_RSA_PKCS or
CKM_RSA_PKCS_PSS signatures.
Daniel Lenski [Tue, 21 Apr 2020 20:03:42 +0000 (13:03 -0700)]
set TCP_NODELAY unconditionally on TCP/TLS sockets
This replaces 67162301, where I tried to only set `TCP_NODELAY` when using
TLS for the tunnel transport.
See https://gitlab.com/openconnect/openconnect/-/merge_requests/89#note_328398311
for why setting it unconditionally is probably the best choice for openconnect.
Daniel Lenski [Sun, 19 Apr 2020 01:13:39 +0000 (18:13 -0700)]
URL-decode GlobalProtect login response fields
The usage of URL encoding in the fields sent by GP servers here is
inconsistent, but in particular the value "%28empty_domain%29" keeps popping up
in places where the server expects "(empty_domain)" (like the stupidly redundant
logout operation). So we do this to be safe and to ensure logout succeeds.
Daniel Lenski [Tue, 21 Apr 2020 01:51:41 +0000 (18:51 -0700)]
periodic HIP fix: ping /ssl-vpn/hipreportcheck.esp at specified interval no matter what
This is a fix for the very subtle regression between v8.05 and v8.08 described in this thread: https://lists.infradead.org/pipermail/openconnect-devel/2020-April/005609.html
- Server never asks for HIP report submission, and no HIP script specified
with `--csd-wrapper`
- v8.05 successfully rekeys 1 minute before server-specified rekey interval,
and continues successfully
- v8.08 appears to successfully rekey, but then gets forced off one minute
later
- Only apparent difference between the two is that v8.05 re-pings
/ssl-vpn/hipreportcheck.esp every time it gets the config
(/ssl-vpn/getconfig.esp), whereas v8.08 only pings it exactly once.
The bottom line is that _even if_ we have no mechanism to actually submit a
HIP report (no `--csd-wrapper`) and _even if_ we think the server will say
"no HIP report needed" every time we check, in order to keep the server from
kicking the client off, we should still…
* ping /ssl-vpn/hipreportcheck.esp at the interval (specified by the portal or `--force-trojan` or 1 hour default)
* ping /ssl-vpn/hipreportcheck.esp every time we re-fetch the config (/ssl-vpn/getconfig.esp) in order to rekey or reconnect
Daniel Lenski [Mon, 20 Apr 2020 17:14:50 +0000 (10:14 -0700)]
Fix print_supported_protocols and print_supported_protocols_usage
These were broken in 7cb8996e21b442c4ec60ce25c87e8a69516fac17, when the
empty sentinel value at the end of the array was removed, without changing
the way these functions iterate over that array.
For some reason, this continues to work on Linux (probably due to `calloc`
allocating more zeroed bytes than we request, in
`openconnect_get_supported_protocols`), but is causing the expected SIGSEGV on
Solaris:
https://lists.infradead.org/pipermail/openconnect-devel/2020-April/005640.html
Fix:
- Modify `print_supported_protocols` and `print_supported_protocols_usage` to
rely on the length returned by `openconnect_get_supported_protocols`.
- Restore the sentinel value at the end of the array returned by
`openconnect_get_supported_protocols`, to preserve ABI compatibility for
other users who may depend on this sentinel.
Daniel Lenski [Thu, 9 Apr 2020 00:57:26 +0000 (17:57 -0700)]
better heuristic for determining where to fill in a token in GP forms
GlobalProtect's prelogin doesn't give us much information to determine where
a token code might be filled in.
Current behavior:
1. Use the password file in the first form as a token field.
2. Ignores the fact that a second "challenge" form might be coming.
New heuristic:
1. If the label for the password field in the first form has a non-default
value (not empty or “Password”), then treat that as the token field.
2. Otherwise, assume a second form ("challenge") is coming, and treat the
password field in the first form as a normal password, then treat the
password field in the second form as the token field.
Daniel Lenski [Thu, 9 Apr 2020 00:35:48 +0000 (17:35 -0700)]
GP auth: give challenge/2FA forms a constant auth_id/name of "_challenge"
Until now, we've been using the `inputStr` value (hex token that has to
accompany challenge form submission) as the `auth_id` for challenge forms,
but it appears these values aren't fixed from run-to-run, which makes it
impossible to use `--form-entry` to fill them out.
This patch makes all challenge forms have `auth_id=_challenge`, so they can
be filled with `--form-entry=_challenge:passwd=VALUE`. The `inputStr` value
will now be shoehorned into `form->action`.
Unless we find a GP VPN that uses multiple independent challenges (3FA?),
this should work better.
Daniel Lenski [Wed, 8 Apr 2020 06:10:14 +0000 (23:10 -0700)]
periodic TNCC
- Factor out oncp_send_tncc_command function
- Reuse csd_token and csd_starturl for TNCC state (just like GP already does)
- Teach `tncc-emulate.py` to send the TNCC interval *back* to OpenConnect
- Apply `--force-trojan` (vpninfo->trojan_interval) to TNCC as well
Daniel Lenski [Thu, 2 Apr 2020 05:05:54 +0000 (22:05 -0700)]
pass TNCC_SHA256 and TNCC_HOSTNAME environment variables to wrapper script (just like for CSD)
TNCC_SHA256 will allow a future version to validate the server certificate
fingerprint (like csd-post.sh already does).
TNCC_HOSTNAME passes along the *local* hostname override from OpenConnect
(set with `--local-hostname` or `openconnect_set_localname`) to the TNCC
wrapper script.
projects / users / dwmw2 / openconnect.git / log