David Woodhouse [Wed, 9 Jan 2019 12:02:57 +0000 (12:02 +0000)]
OpenSSL: Loop over DTLS ciphersuites looking for the one we asked for.
As of OpenSSL 1.1.1, the trick of using SSL_CTX_set_cipher_list() and then
expecting only the one ciphersuite to be present in what we get back from
SSL_get_ciphers(), is no longer working. It now always returns the TLSv1.3
ciphers, even though we don't have DTLSv1.3 yet.
Reported as https://github.com/openssl/openssl/issues/8004 but probably
not going to change; the most likely outcome there is that I'm told that
I'm Doing It Wrong™ and a different approach is suggested.
In the meantime, just loop over the results and pick the one that we
actually asked for.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David GEIGER [Sat, 5 Jan 2019 22:27:24 +0000 (22:27 +0000)]
Fix Mageia TSS2_ESYS build
On Mageia Cauldron latest openconnect 8.00 and 8.01 fais to build with
TSS2_ESYS support due to a missing header in gnutls_tpm2_esys.c file, so
adding #include <errno.h> in gnutls_tpm2_esys.c fixes build.
Signed-off-by: David Geiger (Mageia Linux Team Packager) <geiger.david68210@gmail.com> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 4 Jan 2019 23:51:47 +0000 (23:51 +0000)]
Explicitly reference python2 in shebang for tncc-wrapper.py
The RPM build complains:
BUILDSTDERR: *** ERROR: ambiguous python shebang in /usr/libexec/openconnect/tncc-wrapper.py: #!/usr/bin/python. Change it to python3 (or python2) explicitly.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 4 Jan 2019 14:37:14 +0000 (14:37 +0000)]
Clean up TNCC error handling
As suggested by Daniel Lenski, create the oc_text_buf for the request
only once the TNCC wrapper has been spawned, to make the error handling
a bit saner. And remember to close the socketpair if fork() fails, too.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 4 Jan 2019 12:44:33 +0000 (12:44 +0000)]
Use cancellable_gets() for TNCC communication
Just keep things simple. This avoids SOCK_SEQPACKET which doesn't work on
OSX, and stops assuming that TNCC will send the whole response in a single
send() call.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Thu, 3 Jan 2019 21:39:08 +0000 (21:39 +0000)]
Encrypt digests being signed with IBM TSS2.
The digest itself will end up on the wire. But the computed hash including
the secrets should probably be obsecured. For the TPM that's an input
parameter, which it must decrypt. Hence TPMA_SESSION_DECRYPT.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 21 Dec 2018 15:45:42 +0000 (15:45 +0000)]
Fix re-prompting for empty parent key password with TCG TSS2
It's odd, but persistent keys can be generated with empty password yet
still without the NODA flag.
It's OK to prompt the user for the (empty) password in that case, but
not to do it more than once, after already authenticating successfully
the first time.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 21 Dec 2018 12:12:46 +0000 (12:12 +0000)]
Clear full buffer in buf_truncate() and buf_free()
This reduces the chances of passwords and other secrets lying around in
memory when we're done. Arguably if anyone can just read memory of the
VPN client while it's running, the game is already lost... but still,
this is easy enough to do, and it's been reported as CVE-2018-20319.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Sun, 25 Nov 2018 15:53:37 +0000 (10:53 -0500)]
GlobalProtect: apparently, the parameter `clientos=Linux` value is not just allowed, but necessary, for some VPNs.
* Previously, I had received at least two reports of servers where
`clientos=Windows` was required for the VPN to work correctly.
* Per https://github.com/dlenski/openconnect/issues/126, there is at least
one report where *not* setting `clientos=Windows` was required for the VPN
to work.
* The truly maddening part is not only the pointless and inconsistent
behavior of the GlobalProtect servers, but also the fact that the servers
give such misleading and irrelevant error messages ("Incorrect username or
password" or "Unable to assign private IP address", etc.) rather than
something that makes sense like "Unknown clientos value."
This patch makes `clientos=Linux` the default behavior when
`vpninfo->platname` is `linux-64` or `android`, while still allowing it to
be overridden with `--os=win` etc.
Ralph Schmieder [Sat, 8 Sep 2018 12:57:29 +0000 (14:57 +0200)]
chg: add --version-string
I've included a patch that provides better compatibility with CSD on
ASA head ends. E.g. it allows to specify the version string that is
presented to the ASA. Previous to this patch, OC presents its own
version e.g. 0.7.8 but that could cause rejection on the head end if
it looks for a matching AC version string.
[dwmw2: All the library ABI support for the new function]
Signed-off-by: Ralph Schmieder <ralph.schmieder@gmail.com> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Nikos Mavrogiannopoulos [Sat, 6 Oct 2018 05:44:12 +0000 (07:44 +0200)]
Use the client hello session identifier to transmit the client identifier
Currently the openconnect (protocol) client uses a custom extension to provide
information to the server on which session it was previously associated with.
However, a private extension cannot be defined in IETF without going through
a tedious standardization process involving the TLS working group. To avoid
that process we should provide the client identifier on the DTLS session using
alternative methods.
In TLS 1.3 (and DTLS) the session ID field was made obsolete, and as such we can
use it to place the client identifier instead of an extension field. We can do it
safely because (1) there is no session resumption -in the dtls1.2 or earlier sense-
and (2) ocserv is already checking this field for that value due to the old protocol
format.
Resolves #5
Signed-off-by: Nikos Mavrogiannopoulos <nmav@gnutls.org> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Mon, 8 Oct 2018 20:13:30 +0000 (21:13 +0100)]
Add openconnect_set_key_password()
For auto-provisioning via NetworkManager it's actually quite useful to be
able to set vpninfo->cert_password and have that special case handled,
instead of having to inject the password into the user's keyring somehow.
It's either that or revise the FSID hack...
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Mon, 8 Oct 2018 18:03:45 +0000 (11:03 -0700)]
Fix GlobalProtect authgroup handling
When connecting to a GlobalProtect server via the portal interface, then
`vpninfo->authgroup` needs to be set to the URL of one of the allowed
gateways.
The problem here is that if the user actually wanted to select the _first_
gateway in the dropdown list, it was already pre-selected, and thus clicking
"continue"/"login" on the form wouldn't trigger `OC_FORM_RESULT_NEWGROUP`.
This would prevent `vpninfo->authgroup` from getting set correctly, and the
gateway redirect would be skipped entirely. Thus it was effectively
impossible to select the first option in the gateway dropdown.
Daniel Lenski [Mon, 8 Oct 2018 17:39:54 +0000 (10:39 -0700)]
Fix issue causing front-ends/GUIs to be insensitive to changes in the Juniper realm dropdown
This has been a persistent, puzzling issue
(http://lists.infradead.org/pipermail/openconnect-devel/2018-July/004926.html,
http://lists.infradead.org/pipermail/openconnect-devel/2017-November/004558.html,
etc.). When connecting to a Juniper VPN from a front-end (e.g.
NM-OpenConnect, OpenConnect-GUI for Windows, OpenConnect for Android),
changing the selected realm/`authgroup` in the drop-down box causes the form
to immediately reload *without* saving the change.
This turned out to be a rather subtle issue…
The meaning and usage of `vpninfo->authgroup` differs across the different
protocols, which made this hard to isolate.
* With AnyConnect, changing the authgroup value in the form is supposed to
trigger an immediate reload of the form, since the form contents can
differ from one authgroup to another. Hence a `process_auth_form`
callback should immediately return `OC_FORM_RESULT_NEWGROUP` when the form
value changes.
* With Juniper, the authgroup dropdown don't *actually* need to trigger a reloading
of the form, since the form won't change if the authgroup field changes.
(At least not on any Juniper VPN I have access to.) However, it doesn't
hurt anything either, and setting the dropdown as `form->authgroup_opt`
allows CLI users to specify the desired setting with `--authgroup`, which
is very convenient.
* With GlobalProtect, the authgroup has been repurposed to represent the desired
*gateway* to connect to, in the cases where the user is connecting via the
*portal* interface. The authgroup selection always appears in a form by
itself, currently. This similarly allows CLI users to pick the desired
gateway with `--authgroup`.
Long story short, the problem here was that `form->authgroup_selection`
needed to be set to a specific index (within `form->authgroup_opt->choices[]`)
of the currently selected value, in order
for the GUI to show the right value as selected. If this wasn't set, then
every time the selection was changed (causing the form handler to return
`OC_FORM_RESULT_NEWGROUP`), the selected index would revert to `0` on the
next iteration of the form.
For AnyConnect, the `form->authgroup_selection` was already set correctly;
for Juniper and GlobalProtect, it wasn't. It seems to me that the most
robust fix here is to ensure that `process_auth_form` itself always sets
`form->authgroup_selection` to the index of the value matching
`vpninfo->authgroup` _before_ handing the form off `process_auth_form_cb`.
Tested that this change makes Juniper realm dropdowns work correctly in the
NM-OpenConnect and Android front-ends.
David Woodhouse [Mon, 1 Oct 2018 11:01:00 +0000 (12:01 +0100)]
Shift PC/SC context out of generic vpninfo
TPM2 support wants its own BOOL definition which conflicts with the
"standard" Windows one from libpcsc. Let's just isolate things so that
we only need to include PC/SC header files from within yubikey.c.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Sat, 8 Sep 2018 05:22:40 +0000 (22:22 -0700)]
GlobalProtect: query and parse prelogin.esp and use it to build auth forms, including preliminary SAML support
Until recently, I've believed the prelogin.esp to be useless, because the
initial GlobalProtect login form always contains the same two fields:
username and password.
However, the prelogin response is also important for signalling when SAML
login is required. When the VPN uses SAML login, the official GP clients
redirect the user to a web-based authentication flow (e.g. Okta,
https://github.com/dlenski/openconnect/issues/116).
That auth flow eventually sends the official client back to the GP VPN,
armed with a special cookie field, `portal-userauthcookie` or
`prelogin-cookie`, that needs to be submitted in place of the password
(already supported by openconnect as of 8b2bc5f22dda).
This preliminary SAML support simply includes the SAML method and URL in the
form banner, and fails with an error message if the cookie field name was
not specified (since it cannot be autodetected).