]> www.infradead.org Git - users/dwmw2/openconnect.git/log
users/dwmw2/openconnect.git
5 years agoResync translations with sources
David Woodhouse [Mon, 30 Dec 2019 00:34:09 +0000 (00:34 +0000)]
Resync translations with sources

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix potential NULL dereference in openconnect_get_peer_cert_chain()
David Woodhouse [Mon, 30 Dec 2019 00:11:46 +0000 (00:11 +0000)]
Fix potential NULL dereference in openconnect_get_peer_cert_chain()

Using 'ctx' before checking it for NULL.

Fixes: #93
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agopulse: Attempt to handle EAP-TTLS fragmentation
David Woodhouse [Sun, 29 Dec 2019 13:58:10 +0000 (13:58 +0000)]
pulse: Attempt to handle EAP-TTLS fragmentation

This seems entirely gratuitous but has been seen in the wild, with a
server bizarrely fragmenting a TTLS message of only 3637 bytes, into a
first fragment of 3550 bytes.

We limit the individual IF-T/TLS fragments to 16KiB which is the TLS
record length, because we've never seen IF-T/TLS messages which aren't
precisely aligned in a TLS record (although NC did do horrible things
there).

Each call to pulse_eap_ttls_recv() will receive data from only a single
fragment; another call will be needed to send the Acknowledge frame and
then receive data from the next fragment.

Also implement outbound fragmentation with a fragment limit of 8KiB,
in case it's ever needed.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoAllow custom stoken rcfile to be specfied
David Woodhouse [Tue, 17 Dec 2019 14:28:54 +0000 (14:28 +0000)]
Allow custom stoken rcfile to be specfied

Fixes: #71
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoMove read_file_into_string() to ssl.c and rename it
David Woodhouse [Tue, 17 Dec 2019 14:00:57 +0000 (14:00 +0000)]
Move read_file_into_string() to ssl.c and rename it

We'll want to use this from stoken.c too.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoConsolidate almost-identical set_[ht]otp_mode() functions
David Woodhouse [Tue, 17 Dec 2019 13:18:21 +0000 (13:18 +0000)]
Consolidate almost-identical set_[ht]otp_mode() functions

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoSend Coverity reports to $GITLAB_USER_EMAIL instead of hard-coding it.
David Woodhouse [Tue, 17 Dec 2019 13:17:34 +0000 (13:17 +0000)]
Send Coverity reports to $GITLAB_USER_EMAIL instead of hard-coding it.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoDetect closed HTTPS socket when sending requests
David Woodhouse [Mon, 16 Dec 2019 13:15:43 +0000 (13:15 +0000)]
Detect closed HTTPS socket when sending requests

Fixes: #87
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoAdd separate list-taps.exe "test"
David Woodhouse [Tue, 17 Dec 2019 17:09:06 +0000 (17:09 +0000)]
Add separate list-taps.exe "test"

This should help debug user reports of tailing to find tap devices,
like https://gitlab.com/openconnect/openconnect/issues/88

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoMake win32 search_taps() function work standalone, add debugging
David Woodhouse [Tue, 17 Dec 2019 17:07:58 +0000 (17:07 +0000)]
Make win32 search_taps() function work standalone, add debugging

Would like to create a separate list-taps.exe binary.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoAdd password change support
David Woodhouse [Thu, 12 Dec 2019 14:43:43 +0000 (14:43 +0000)]
Add password change support

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agotun-win32: Attempt to open all adapters, don't bail if the first is in use
David Woodhouse [Tue, 10 Dec 2019 09:44:21 +0000 (09:44 +0000)]
tun-win32: Attempt to open all adapters, don't bail if the first is in use

https://gitlab.com/openconnect/openconnect-gui/issues/278

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoUpdate translations from GNOME
David Woodhouse [Fri, 29 Nov 2019 10:58:33 +0000 (10:58 +0000)]
Update translations from GNOME

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix build error in select fix
David Woodhouse [Mon, 14 Oct 2019 14:16:00 +0000 (15:16 +0100)]
Fix build error in select fix

Bad dwmw2. No biscuit.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoWhen select() returns with errno == EINTR, that isn't an error
David Woodhouse [Mon, 14 Oct 2019 14:08:53 +0000 (15:08 +0100)]
When select() returns with errno == EINTR, that isn't an error

This stopped us from actually sending the BYE packet and closing the
session cleanly on exit.

Fixes: a07183b79f ("Check select() return value in main loop") et al.
This is why we don't blindly "fix warnings" reported by tools like
Coverity, and should sometimes be a little more reticent, and only
actually fix *bugs* that are highlighted.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix pulse session kill request
David Woodhouse [Mon, 14 Oct 2019 13:07:11 +0000 (14:07 +0100)]
Fix pulse session kill request

Fixing leaks is good. Fixing them by freeing a string we were about to
use and then setting it to NULL, thus triggering a later check to report
-ENOMEM, less good.

Stupid dwmw2, no biscuit.

Fixes: 097586fe ("Fix leaks in Pulse duplicate session handling")
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix build with libressl 2.7.x/2.9.x
John Spencer [Wed, 9 Oct 2019 15:41:23 +0000 (16:41 +0100)]
Fix build with libressl 2.7.x/2.9.x

rather than hardcoding version numbers with ifdefs, we simply check
whether the functionality is available or not.

[dwmw2: Use #ifndef instead of #if !HAVE_SSL_CIPHER_FIND]
Signed-off-by: John Spencer <maillist-openconnect@barfooze.de>
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoGlobalProtect: Ensure timeout is less than DPD when DTLS connecting
Corey Wright [Mon, 23 Sep 2019 05:37:53 +0000 (00:37 -0500)]
GlobalProtect: Ensure timeout is less than DPD when DTLS connecting

When transitioning from DTLS_CONNECTING to DTLS_CONNECTED ensure that
the current timeout is less than or equal to 10-second DTLS DPD
otherwise timeout might be greater than 2x DPD, eg set to 60-second
DTLS attempt period from the ESP main loop where we were "connecting",
and we might sleep right through the DTLS DPD period and falsely
detect a dead peer and needlessly fall back to HTTPS.

This is only relevant to reconnects because during the initial
connection the timeout is artificially set low, ie 1 second, by the
OpenConnect mainloop because the TUN device is not yet up.

Signed-off-by: Corey Wright <cwright@digitalocean.com>
5 years agoMerge branch 'clarify_cafile_and_no_system_trust' of gitlab.com:dlenski/openconnect
David Woodhouse [Mon, 14 Oct 2019 12:06:30 +0000 (13:06 +0100)]
Merge branch 'clarify_cafile_and_no_system_trust' of gitlab.com:dlenski/openconnect

5 years agoMerge branch 'correct_mimetype_and_charset_for_XML_post' of gitlab.com:dlenski/openco...
David Woodhouse [Mon, 14 Oct 2019 12:04:42 +0000 (13:04 +0100)]
Merge branch 'correct_mimetype_and_charset_for_XML_post' of gitlab.com:dlenski/openconnect

5 years agopulse: A value of 0xF for AVP 0xd73 means 'prompt for password only'
David Woodhouse [Mon, 14 Oct 2019 11:03:08 +0000 (12:03 +0100)]
pulse: A value of 0xF for AVP 0xd73 means 'prompt for password only'

I wish I knew how to interpret this sanely.

https://gitlab.com/openconnect/openconnect/issues/82#note_229748355

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoReally pull in tpm2-tss for EPEL8 builds
David Woodhouse [Mon, 14 Oct 2019 11:01:20 +0000 (12:01 +0100)]
Really pull in tpm2-tss for EPEL8 builds

It should be checking for %{?rhel} >= 8, not %{?epel}.

Not that it'll build yet because ocserv isn't there for EPEL8 yet
(https://bugzilla.redhat.com/show_bug.cgi?id=1761396)

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agopulse: Be more forgiving about unknown AVP 0xd73 values
David Woodhouse [Mon, 14 Oct 2019 08:04:57 +0000 (09:04 +0100)]
pulse: Be more forgiving about unknown AVP 0xd73 values

If we don't know which of username/password to prompt for, prompt for
both. The server ignores the ones it doesn't want anyway.

One of the 'Unhandled authentication packet' issues in #82.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoCorrect handling of empty responses to HTTP requests
David Woodhouse [Fri, 11 Oct 2019 15:32:29 +0000 (16:32 +0100)]
Correct handling of empty responses to HTTP requests

If there's nothing to return, read_http_header() doesn't append anything
at all to hdrbuf, and it ends up with hdrbuf->data still NULL.

As long as *something* has once been added to it, there is a guarantee
that it's OK to use hdrbuf->data as a NUL-terminated string, as we do
when looking for HTTP/1.x responses.

Spotted when I just hit 'enter' when testing with openssl s_server.

Fixes: 093de80507 ("Rework HTTP header fetching to receive into oc_text_buf")
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoCheck setsockopt() return value when setting SO_SNDBUF
David Woodhouse [Thu, 10 Oct 2019 13:54:22 +0000 (14:54 +0100)]
Check setsockopt() return value when setting SO_SNDBUF

Coverity whines unless we do.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoCheck select() return value in ssl.c
David Woodhouse [Thu, 10 Oct 2019 13:52:09 +0000 (14:52 +0100)]
Check select() return value in ssl.c

Should probably never go wrong, but keeps Coverity happy.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoCheck select() return value in main loop
David Woodhouse [Thu, 10 Oct 2019 13:47:15 +0000 (14:47 +0100)]
Check select() return value in main loop

Coverity doesn't like it if we don't.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoCheck return value from select() in GnuTLS code
David Woodhouse [Thu, 10 Oct 2019 13:05:53 +0000 (14:05 +0100)]
Check return value from select() in GnuTLS code

Coverity doesn't like it if we don't.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoPrint warning if sending to cookie to TNCC script fails
David Woodhouse [Thu, 10 Oct 2019 12:53:26 +0000 (13:53 +0100)]
Print warning if sending to cookie to TNCC script fails

Coverity didn't like us just ignoring it.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoPrint debug warning if sending ESP probe fails
David Woodhouse [Thu, 10 Oct 2019 12:17:44 +0000 (13:17 +0100)]
Print debug warning if sending ESP probe fails

Coverity was whining that we ignored the result of send().

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoSilence Coverity warnings about set_fd_cloexec() return values
David Woodhouse [Thu, 10 Oct 2019 12:36:24 +0000 (13:36 +0100)]
Silence Coverity warnings about set_fd_cloexec() return values

We just don't care.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoAlways check return value of set_sock_nonblock()
David Woodhouse [Thu, 10 Oct 2019 12:00:59 +0000 (13:00 +0100)]
Always check return value of set_sock_nonblock()

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoSilence Coverity warning in oncp_obtain_cookie()
David Woodhouse [Thu, 10 Oct 2019 11:20:17 +0000 (12:20 +0100)]
Silence Coverity warning in oncp_obtain_cookie()

For some reason Coverity doesn't realise that if buf_alloc() fails that's
only because it didn't allocate.

I suppose in that case, perhaps I should only have checked for !resp_buf
when checking for error, instead of using buf_error(resp_buf). Doing that
would probably make Coverity shut up too, but instead just let it use the
existing 'goto out' error handling.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoClean up openconnect_protos handling
David Woodhouse [Thu, 10 Oct 2019 10:49:46 +0000 (11:49 +0100)]
Clean up openconnect_protos handling

Do it with a bit less pointer arithmetic and it won't need a sentinel
at the end of the array.

It's a little bit cleaner, and hopefully Coverity will shut up about
leaks from openconnect_get_supported_protocols() potentially returning
non-negative after allocating the returned pointer.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoTry harder to make Coverity shut up about read_file_into_string()
David Woodhouse [Thu, 10 Oct 2019 10:09:02 +0000 (11:09 +0100)]
Try harder to make Coverity shut up about read_file_into_string()

Maybe it thinks st.st_size can be negative? Previously checking for
st.st_size >= INT_MAX wasn't sufficient.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix leak on error path in pulse_request_session_kill()
David Woodhouse [Thu, 10 Oct 2019 00:10:43 +0000 (01:10 +0100)]
Fix leak on error path in pulse_request_session_kill()

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix potential leak of saml_path in GlobalProtect parse_prelogin_xml()
David Woodhouse [Thu, 10 Oct 2019 00:03:22 +0000 (01:03 +0100)]
Fix potential leak of saml_path in GlobalProtect parse_prelogin_xml()

Coverity spotted the leak if we see two saml-request nodes. As far as
I can tell, it didn't spot the bogus realloc() usage which would also
have caused a leak and a crash.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix potential leak in do_https_request()
David Woodhouse [Wed, 9 Oct 2019 23:56:30 +0000 (00:56 +0100)]
Fix potential leak in do_https_request()

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix leak on error path vin read_stdin()
David Woodhouse [Wed, 9 Oct 2019 23:51:25 +0000 (00:51 +0100)]
Fix leak on error path vin read_stdin()

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix leaks in Pulse duplicate session handling
David Woodhouse [Wed, 9 Oct 2019 22:59:25 +0000 (23:59 +0100)]
Fix leaks in Pulse duplicate session handling

Clean up handling of from/sessid strings in processing session lists.

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix leak on error path in connect_https_socket()
David Woodhouse [Wed, 9 Oct 2019 23:07:17 +0000 (00:07 +0100)]
Fix leak on error path in connect_https_socket()

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix leak on error path in Juniper parse_select_node()
David Woodhouse [Wed, 9 Oct 2019 23:06:53 +0000 (00:06 +0100)]
Fix leak on error path in Juniper parse_select_node()

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix leak in Juniper parse_input_node()
David Woodhouse [Wed, 9 Oct 2019 22:56:35 +0000 (23:56 +0100)]
Fix leak in Juniper parse_input_node()

If it drops a duplicate form entry, it doesn't free it (and doesn't
return an error code so it's not freed on the way out either).

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix NULL dereference when checking IP addresses after reconnect
David Woodhouse [Wed, 9 Oct 2019 22:51:06 +0000 (23:51 +0100)]
Fix NULL dereference when checking IP addresses after reconnect

If we don't get a new address but we had one before, we'll end up
calling strcmp with a NULL argument. Don't do that.

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix potential NULL dereference in GlobalProtect parse_login_xml()
David Woodhouse [Wed, 9 Oct 2019 22:43:11 +0000 (23:43 +0100)]
Fix potential NULL dereference in GlobalProtect parse_login_xml()

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix potential NULL dereference in GlobalProtect XML parsing
David Woodhouse [Wed, 9 Oct 2019 22:36:03 +0000 (23:36 +0100)]
Fix potential NULL dereference in GlobalProtect XML parsing

Spotted by Coverity.

Perhaps the intent in reusing xml_node here for the inner loop was that
by leaving it NULL we'd automatically break out of the outer loop too,
which is reasonable. Except it wouldn't have done that because it'd
have died on trying to move to xmlnode->next. Break out of the outer
loop explicitly, instead.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoMake free_opt(NULL) work like free(NULL)
David Woodhouse [Wed, 9 Oct 2019 22:29:50 +0000 (23:29 +0100)]
Make free_opt(NULL) work like free(NULL)

Coverity spotted such a call in the error path of parse_select_node()
in the Juniper auth code, after allocating 'opt' fails.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agojni: Consistently check for getctx() failing
David Woodhouse [Wed, 9 Oct 2019 22:19:36 +0000 (23:19 +0100)]
jni: Consistently check for getctx() failing

Spotted as a potential NULL dereference by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix use-after-free in GPST debug checks
David Woodhouse [Wed, 9 Oct 2019 22:08:30 +0000 (23:08 +0100)]
Fix use-after-free in GPST debug checks

Don't print vpninfo->cstp_pkt right after queueing it and setting it to NULL.

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix leak of PKCS#11 object ID on URI parse failure
David Woodhouse [Wed, 9 Oct 2019 21:29:28 +0000 (22:29 +0100)]
Fix leak of PKCS#11 object ID on URI parse failure

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix potential leak in XML config handling
David Woodhouse [Wed, 9 Oct 2019 15:48:46 +0000 (16:48 +0100)]
Fix potential leak in XML config handling

If the file is of zero length a buffer of one byte is allocated and not
freed. Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix potential leak on error path in xmlpost_initial_req()
David Woodhouse [Wed, 9 Oct 2019 15:35:31 +0000 (16:35 +0100)]
Fix potential leak on error path in xmlpost_initial_req()

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoCheck for BYE packet being correctly written in cstp_bye()
David Woodhouse [Wed, 9 Oct 2019 15:29:50 +0000 (16:29 +0100)]
Check for BYE packet being correctly written in cstp_bye()

Spotted as an unchecked return value by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix strerror() calls with negative values
David Woodhouse [Wed, 9 Oct 2019 15:11:48 +0000 (16:11 +0100)]
Fix strerror() calls with negative values

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agocstp: Tweak DTLS header matching
David Woodhouse [Wed, 9 Oct 2019 14:27:17 +0000 (15:27 +0100)]
cstp: Tweak DTLS header matching

Coverity doesn't like the (i = 7) && … construct and thinks it's an
"assign instead of compare" bug. Which it isn't; the extra brackets
around it were enough to stop the compiler from making the same complaint.

Try doing it differently, purely for Coverity's benefit.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agooncp: Tear down ESP if receiving new config fails
David Woodhouse [Wed, 9 Oct 2019 14:21:01 +0000 (15:21 +0100)]
oncp: Tear down ESP if receiving new config fails

Spotted by Coverity as an 'unused value' because the return value from
oncp_receive_espkeys() was ignored.

Use oncp_esp_close() to do the shutdown, and fix it only to send the
'ESP off' message to the server if an 'ESP on' message has previously
been sent (i.e. if vpninfo->dtls_state >= DTLS_CONNECTING).

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix undefined shift in decode_base32() for invalid input
David Woodhouse [Wed, 9 Oct 2019 11:59:22 +0000 (12:59 +0100)]
Fix undefined shift in decode_base32() for invalid input

In the case where fewer than 2 characters were present in a base32 group,
Coverity complained of an undefined shift. Return -EINVAL early for this
case instead of triggering the undefined behaviour.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix error return from process_http_response()
David Woodhouse [Tue, 8 Oct 2019 15:26:04 +0000 (16:26 +0100)]
Fix error return from process_http_response()

In reworking the error returns from process_http_response() to go via
the 'err' label, I forgot to actually return the correct variable.

Spotted by Coverity.

Fixes: 093de8050 ("Rework HTTP header fetching to receive into oc_text_buf")
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoSilence Coverity warning in cancellable_connect()
David Woodhouse [Tue, 8 Oct 2019 14:56:36 +0000 (15:56 +0100)]
Silence Coverity warning in cancellable_connect()

It's whining that we are reading a single byte into an int variable.
Which would be wrong if we were doing anything with it, and is often
a sign of little-endian-itis. As it happens it's harmless here because
it's only an attempt to capture the correct error condition by reading
from a defunct socket. But fix it anyway. There was no need for the
mismatch.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoAvoid strcpy() in Esys install_tpm_passphrase
David Woodhouse [Tue, 8 Oct 2019 14:12:54 +0000 (15:12 +0100)]
Avoid strcpy() in Esys install_tpm_passphrase

Coverity didn't like it, because it couldn't see that we had actually
checked the length manually. By doing it like this we get to use
free_pass() on the full initial password instead of just the truncated
part, so it's not just a bogus "warning fix".

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix double-free in xmlpost_initial_req()
David Woodhouse [Tue, 8 Oct 2019 13:55:19 +0000 (14:55 +0100)]
Fix double-free in xmlpost_initial_req()

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix potential uninitialised data in LZS decompression
David Woodhouse [Tue, 8 Oct 2019 13:17:56 +0000 (14:17 +0100)]
Fix potential uninitialised data in LZS decompression

A compressed sequence with an offset of zero is encodeable via an 11-bit
offset, as 1000000000000. But would end up copying dst[outlen] to itself.

Spotted by Coverity.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoClean up GnuTLS token info more
David Woodhouse [Tue, 8 Oct 2019 12:11:56 +0000 (13:11 +0100)]
Clean up GnuTLS token info more

Coverity is still whining. I don't think a zero-length memset is actually
a problem, but again it does no harm to be careful, and we might as well
consolidate it all into one function instead of duplicating the code.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoCoverity cosmetics
David Woodhouse [Tue, 8 Oct 2019 10:42:49 +0000 (11:42 +0100)]
Coverity cosmetics

Put the branch/version information into the description so it's visible
in the Coverity snapshot information, and put Java first so it doesn't
mess up the pathnames.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoAdd Java to Coverity
David Woodhouse [Tue, 8 Oct 2019 09:42:40 +0000 (10:42 +0100)]
Add Java to Coverity

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoExplicitly check for PKCS#11 token info overrun
David Woodhouse [Tue, 8 Oct 2019 09:35:55 +0000 (10:35 +0100)]
Explicitly check for PKCS#11 token info overrun

Could probably hever have happened but Coverity doesn't like it, and it
doesn't hurt to err on the side of caution. We're already carefully
checking for weird behaviour from old versions of GnuTLS anyway.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoopenssl: Fix certificate load failure harder
David Woodhouse [Mon, 7 Oct 2019 17:39:32 +0000 (18:39 +0100)]
openssl: Fix certificate load failure harder

Note to self: Don't do half-arsed fixes just to test the Coverity/CI
integration while on a conference call, without actually paying full
attention.

We needed to actually return -EINVAL here, not just set ret = -EINVAL
which *still* left us using 'f' after closing it, and introduced a
*new* Coverity 'Unused value' defect.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoopenssl: Check for SSL_CTX_use_PrivateKey() failure in PKCS#12
David Woodhouse [Mon, 7 Oct 2019 17:26:22 +0000 (18:26 +0100)]
openssl: Check for SSL_CTX_use_PrivateKey() failure in PKCS#12

Coverity spotted that we check for failure in 5 out of 6 invocations.
It'll be a rare error but Coverity was right to complain.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoopenssl: Fix error path when loading certificate fails
David Woodhouse [Mon, 7 Oct 2019 17:24:15 +0000 (18:24 +0100)]
openssl: Fix error path when loading certificate fails

We were supposed to return here. Spotted by Coverity as a use-after-close
error when we fail to do so.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoTurn libp11 CI off again
David Woodhouse [Mon, 7 Oct 2019 16:58:52 +0000 (17:58 +0100)]
Turn libp11 CI off again

It's broken: https://github.com/OpenSC/libp11/issues/312

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoAdd Coverity CI support
David Woodhouse [Mon, 7 Oct 2019 16:46:09 +0000 (17:46 +0100)]
Add Coverity CI support

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoUpdate Gitlab CI to include TPM2, GSSAPI and libp11 support
David Woodhouse [Mon, 7 Oct 2019 16:44:56 +0000 (17:44 +0100)]
Update Gitlab CI to include TPM2, GSSAPI and libp11 support

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix EPEL builds in COPR
David Woodhouse [Thu, 3 Oct 2019 12:52:24 +0000 (13:52 +0100)]
Fix EPEL builds in COPR

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoMerge branch 'globalprotect_auth' of gitlab.com:dlenski/openconnect
David Woodhouse [Thu, 3 Oct 2019 11:58:52 +0000 (12:58 +0100)]
Merge branch 'globalprotect_auth' of gitlab.com:dlenski/openconnect

5 years agoMerge branch 'fix_issue_78_crash_and_stop_trying_to_reconnect' of gitlab.com:dlenski...
David Woodhouse [Thu, 3 Oct 2019 11:57:06 +0000 (12:57 +0100)]
Merge branch 'fix_issue_78_crash_and_stop_trying_to_reconnect' of gitlab.com:dlenski/openconnect

Fixes: #78
5 years agoUse GNUTLS_PK_ECC instead of GNUTLS_PK_ECDSA for compatibility
David Woodhouse [Thu, 3 Oct 2019 11:52:05 +0000 (12:52 +0100)]
Use GNUTLS_PK_ECC instead of GNUTLS_PK_ECDSA for compatibility

Fixes: #76
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoclarify in manual how to use --cafile with --no-system-trust (ping issue #80)
Daniel Lenski [Wed, 2 Oct 2019 22:48:30 +0000 (15:48 -0700)]
clarify in manual how to use --cafile with --no-system-trust (ping issue #80)

5 years agoanother GP error string that tells the client to stop trying to reconnect
Daniel Lenski [Sun, 29 Sep 2019 20:29:26 +0000 (13:29 -0700)]
another GP error string that tells the client to stop trying to reconnect

As shown in https://gitlab.com/openconnect/openconnect/issues/78, the
message "Allow Automatic Restoration of SSL VPN is disabled" in a
GlobalProtect error response indicates that the server will not accept the
previously-valid auth cookie, so the client should give up retrying.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
5 years agoFix double-free when client repeatedly fails to pull GlobalProtect client config
Daniel Lenski [Sun, 29 Sep 2019 20:36:44 +0000 (13:36 -0700)]
Fix double-free when client repeatedly fails to pull GlobalProtect client config

When openconnect attempts to rebuild the GP connection, upon rekey or
loss-of-connectivity, it re-requests the client configuration XML
(/ssl-vpn/getconfig.esp).  It saves the old `cstp_options` prior to querying
the new ones, and then free()'s them after verifying that the IP addresses
and netmasks haven't changed.

If the config request fails to return valid XML twice in a row, the old
`cstp_options` would be double-freed, causing the crash described in
https://gitlab.com/openconnect/openconnect/issues/78.

The fix is to ensure that the old `cstp_options` are set to NULL as soon as
they're copied into `old_cstp_options`.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
5 years agoRework HTTP header fetching to receive into oc_text_buf
David Woodhouse [Fri, 13 Sep 2019 12:51:37 +0000 (13:51 +0100)]
Rework HTTP header fetching to receive into oc_text_buf

Also handles continuations.

Fixes: #65
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoUse localtime_s where available to fix MSYS2 build
David Woodhouse [Fri, 13 Sep 2019 09:43:44 +0000 (10:43 +0100)]
Use localtime_s where available to fix MSYS2 build

Fixes: #74
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoExplicitly link libtss2-mu for tss2-esys build
David Woodhouse [Fri, 13 Sep 2019 07:35:43 +0000 (08:35 +0100)]
Explicitly link libtss2-mu for tss2-esys build

We use the marshalling functions directly and mustn't rely on transitive
linking.

Fixes: #73
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoTag version 8.05 v8.05
David Woodhouse [Wed, 11 Sep 2019 23:31:04 +0000 (00:31 +0100)]
Tag version 8.05

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoFix buffer overflow with chunked HTTP handling (CVE-2019-16239)
David Woodhouse [Tue, 10 Sep 2019 16:30:12 +0000 (17:30 +0100)]
Fix buffer overflow with chunked HTTP handling (CVE-2019-16239)

Over a decade ago, I was vocally sad about the fact that I needed to
implement HTTP client code for myself because none of the available
options at the time gave me sufficient control over the underlying
TLS connection.

This is why. A malicious HTTP server (after we have accepted its
identity certificate) can provide bogus chunk lengths for chunked
HTTP encoding and cause a heap overflow.

Reported by Lukas Kupczyk of the Advanced Research Team at CrowdStrike
Intelligence.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoUpdate translations from GNOME
David Woodhouse [Wed, 11 Sep 2019 13:51:36 +0000 (14:51 +0100)]
Update translations from GNOME

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoClose HTTPS connection on failure returns from process_http_response()
David Woodhouse [Tue, 10 Sep 2019 16:10:23 +0000 (17:10 +0100)]
Close HTTPS connection on failure returns from process_http_response()

If we've failed to process the response, don't leave the connection open.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoDon't crash if gnutls_x509_crt_list_import() fails
Omar Sandoval [Tue, 27 Aug 2019 16:27:51 +0000 (09:27 -0700)]
Don't crash if gnutls_x509_crt_list_import() fails

On error, gnutls_x509_crt_list_import() deinitializes any certificates
that it loaded (this isn't documented, of course, but see [1]). However,
we're also deinitializing them in the error handling case, resulting in
a double-free. Set nr_extra_certs to zero in that case so that we don't
crash.

1: https://gitlab.com/gnutls/gnutls/blob/gnutls_3_6_9/lib/x509/x509.c#L3864

Signed-off-by: Omar Sandoval <osandov@osandov.com>
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoexplain GlobalProtect portals vs. gateways in the docs
Daniel Lenski [Wed, 28 Aug 2019 23:43:27 +0000 (16:43 -0700)]
explain GlobalProtect portals vs. gateways in the docs

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
5 years agoGlobalProtect: try to connect to portal interface before gateway
Daniel Lenski [Wed, 28 Aug 2019 23:21:03 +0000 (16:21 -0700)]
GlobalProtect: try to connect to portal interface before gateway

This makes OpenConnect behave more like the official GP clients, which
should make more sense to new users especially when troublesheeting, without
removing the useful ability to connect directly to a gateway.
(See https://gitlab.com/openconnect/openconnect/merge_requests/56#note_209428777)

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
5 years agoGlobalProtect login bugfix: reversed condition where portal form should be retried...
Daniel Lenski [Wed, 28 Aug 2019 23:18:39 +0000 (16:18 -0700)]
GlobalProtect login bugfix: reversed condition where portal form should be retried on gateway

I broke this in 3e91f7bf

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
5 years agoUpdate changelog for GP ESP fix
David Woodhouse [Mon, 12 Aug 2019 08:39:35 +0000 (10:39 +0200)]
Update changelog for GP ESP fix

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoOnly add packet to oNCP control queue with nc and pulse protocols
Corey Wright [Sun, 11 Aug 2019 10:00:04 +0000 (05:00 -0500)]
Only add packet to oNCP control queue with nc and pulse protocols

Don't add packets to the oNCP control queue if not using Juniper
Network Connect or Pulse Connect Secure protocols otherwise a number
of packets equal to the maximum queue length can be queued and disable
reading from the TUN device for the duration of the VPN connection
because the packets will never get dequeued except when using those
two protocols.

Commit b4f50f8 broke OpenConnect transmitting across the GlobalProtect
protocol with ESP packets when:
1. The tun device has an IPv6 address (eg link local).
2. IPv6 packets (eg router solicitation) are transmitted in quantity
   equal to maximum queue length.

[dwmw2: Check the udp_send_probes function insted of two string compares]
Signed-off-by: Corey Wright <cwright@digitalocean.com>
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoTag version 8.04 v8.04
David Woodhouse [Fri, 9 Aug 2019 15:08:45 +0000 (16:08 +0100)]
Tag version 8.04

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
5 years agoRemove hipreport-android.sh from COPR RPM build
David Woodhouse [Tue, 6 Aug 2019 11:57:29 +0000 (12:57 +0100)]
Remove hipreport-android.sh from COPR RPM build

It causes bogus dependencies on /system/bin/sh

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
6 years agoSimplify openconnect_set_http_proxy() and report errors
David Woodhouse [Fri, 2 Aug 2019 21:52:22 +0000 (14:52 -0700)]
Simplify openconnect_set_http_proxy() and report errors

There was a failure mode where openconnect would exit silently if given
a --proxy= argument it didn't like. Make it print errors in all cases,
and eliminate an -ENOMEM case which seems entirely gratuitous; I don't
think internal_parse_url() needs the 'const char *url' it's passed to
be hackishly writeable, so there is no need to allocate our own copy.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
6 years agoFix proxy username and password parsing.
David Woodhouse [Fri, 2 Aug 2019 21:05:08 +0000 (14:05 -0700)]
Fix proxy username and password parsing.

We are supposed to take the *last* (unescaped) @ sign as the separation
between user:pass and hostname, not the first. So use strrchr() instead
of strchr().

Conversely, the first colon is the separation between user and pass so
strchr is still correct there.

Also actually support unescaping the resulting username and password.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
6 years agoImplicitly enable basic auth for SOCKS if creds are provided.
David Woodhouse [Fri, 2 Aug 2019 17:51:09 +0000 (10:51 -0700)]
Implicitly enable basic auth for SOCKS if creds are provided.

Forcing the user to add --proxy-auth=basic on the command line as well as
providing the creds in the proxy URL is horrid. It took me a long time to
work out why it wasn't working.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
6 years agoUpdate translatons from GNOME
David Woodhouse [Thu, 1 Aug 2019 22:33:58 +0000 (15:33 -0700)]
Update translatons from GNOME

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
6 years agoKill bogus 'no GSSAPI' warning when it isn't true
David Woodhouse [Thu, 1 Aug 2019 22:32:40 +0000 (15:32 -0700)]
Kill bogus 'no GSSAPI' warning when it isn't true

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
6 years agoIncrease buffer size for oNCP configuration
David Woodhouse [Wed, 24 Jul 2019 11:02:44 +0000 (12:02 +0100)]
Increase buffer size for oNCP configuration

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1729693

Signed-off-by: David Woodhouse <dwmw2@infradead.org>