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>
Tom Carroll [Fri, 24 Apr 2020 06:15:59 +0000 (23:15 -0700)]
Converse the multicert auth protocol
Implement the multiple certificate-based authentication protocol. The
XML-message protocol may require three rounds of conversations (instead
of the existing two): HTTP redirect, announce capabilities and receive
multicert challenge, and respond to the multicert challenge and obtain
cookie.
Signed-off-by: Tom Carroll <incentivedesign@gmail.com>
Tom Carroll [Sat, 25 Apr 2020 06:47:33 +0000 (23:47 -0700)]
Add password to pin callback
There are (at least) two passwords, one for TLS session and another for
the multiple certificate-based authentication challenge. Add password
parameter to the pin callback.
Signed-off-by: Tom Carroll <incentivedesign@gmail.com>
Tom Carroll [Fri, 24 Apr 2020 06:15:59 +0000 (23:15 -0700)]
GnuTLS challenge signing
This is the GnuTLS implementation for signing the challenge. Variables
added to store the paths for the second set of credentials.
Challenge response comprises two elements: an identity and signature.
The identity is the user certificate, represented as a PEM-encoded PKCS7
certificate chain. The challenge is signed with the user certificate
and submitted back to the server as a PEM-encoded signature.
Signed-off-by: Tom Carroll <incentivedesign@gmail.com>
Tom Carroll [Fri, 24 Apr 2020 06:15:59 +0000 (23:15 -0700)]
Factor out keycert loading
The objective is to have load_keycert fetch credentials for both
establishing the TLS session and responding to the multicert auth
challenge. The main changes are: load_keycert that is parameterized by
key_path, cert_path, and password; have secondary functions
parameterized by password; load_certificate stub to call load_keycert
and match caller impedance, including maintaining
gnutls_certificate_credentials and gnutls_session dependencies.
Signed-off-by: Tom Carroll <incentivedesign@gmail.com>
Tom Carroll [Wed, 6 May 2020 05:57:48 +0000 (22:57 -0700)]
A simple referenced-counted string
An implementation of a simple reference-counted string. The purpose of
this is to simplify password password and handling, and reduce the
number of password copies.
Signed-off-by: Tom Carroll <incentivedesign@gmail.com>
Tom Carroll [Fri, 24 Apr 2020 06:15:59 +0000 (23:15 -0700)]
Factor out trust and cafile loading
Simplifies subsequent load_keycert rework.
Previously, a separate path for android existed. Noted that it
duplicated much of the existing functionality of load_datum and cafile.
Functionality was factored into a separate function, removing
duplicative code path for Android.
Signed-off-by: Tom Carroll <incentivedesign@gmail.com>
Tom Carroll [Fri, 24 Apr 2020 06:15:59 +0000 (23:15 -0700)]
Factored out key+cert match mechanism
Previously there were two alternates to check that the key matched the
certificate. Alternative one was employed if the key was a x509_privkey;
alternative two was used if the key was an abstract privkey. By
preferring the abstract privkey, the code can be deduplicated and
simplified. The adoption was modeled after GnuTLS 3.6.13
_gnutls_check_key_cert_match, which GnuTLS uses to verify that key and
cert are a pair. Match was factored out of load_certificate and put into
a separate function.
Signed-off-by: Tom Carroll <incentivedesign@gmail.com>
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.
projects / users / dwmw2 / openconnect.git / log