David Woodhouse [Wed, 14 Jun 2023 08:20:53 +0000 (09:20 +0100)]
Fix TPMv2 ECDSA signature ASN.1
I lifted this code to use it elsewhere and found that 'openssl dgst -verify'
didn't like the resulting signatures.
So ensure we have a definite lengh for the overall SEQUENCE and that we
don't have gratuitous zeroes at the start of each INTEGER. Even 'openssl
asn1parse' whines about the latter, calling it a :BAD INTEGER:.
I can't find any documentation which mandates DER, and I don't see the
point since there's a randomly generated salt so there's no 'canonical'
signature result anyway. But it doesn't hurt, and this matches what
GnuTLS does in 3.6.0 onwards where it *does* provide this function.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 19 May 2023 13:54:26 +0000 (14:54 +0100)]
Clean up ifreq_set_ifname() and use it from bsd_open_tun() too
Currently, if we set a name with --interface which is too long to fit in
ifr->ifr_name, it gets silently truncated with strncpy(). This in itself
is not immediately broken, although the FreeBSD build does complain:
tun.c:262:17: warning: 'strncpy' output may be truncated copying 15 bytes from a string of length 74 [-Wstringop-truncation]
262 | strncpy(ifr.ifr_name, tun_name + 5, sizeof(ifr.ifr_name) - 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
It's not *immediately* broken, and there are no string overflows; the NUL
termination is there anyway. But it *is* broken eventually, because we'll
spawn vpnc-script with the *originally* intended name, and it won't find
the device with that name.
So fix it up to check the length and then return an error if the requested
name is too long, and just use memcpy() to put the string into ifr_name,
which was pre-zeroed anyway.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 19 May 2023 13:16:53 +0000 (14:16 +0100)]
Fix unaligned accesses in ESP checksum calculation
The FreeBSD 14 build complains:
gpst.c: In function 'gpst_esp_send_probes':
gpst.c:1512:57: warning: taking address of packed member of 'struct ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
1512 | uint32_t sum = csum_partial((uint16_t *)&iph->ip6_src, 8); /* 8 uint16_t */
| ^~~~~~~~~~~~~
gpst.c:1513:49: warning: taking address of packed member of 'struct ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
1513 | sum += csum_partial((uint16_t *)&iph->ip6_dst, 8); /* 8 uint16_t */
| ^~~~~~~~~~~~~
gpst.c:1525:17: warning: converting a packed 'struct icmp6_hdr' pointer (alignment 1) to a 'uint16_t' {aka 'short unsigned int'} pointer (alignment 2) may result in an unaligned pointer value [-Waddress-of-packed-member]
1525 | sum += csum_partial((uint16_t *)icmph, icmplen / 2);
| ^~~
Rather than loading a potentially unaligned uint16_t directly, use
load_be16() instead. And pass (void *) pointers around instead of
(uint16_t *).
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 19 May 2023 11:06:28 +0000 (12:06 +0100)]
Fix time_t handling in parsing F5 session timeout
We can't assume that time_t is 'long'. When building for win64 we get:
../f5.c: In function 'f5_configure':
../f5.c:690:63: warning: format '%ld' expects argument of type 'long int *', but argument 6 has type 'time_t *' {aka 'long long int *'} [-Wformat=]
690 | if (sscanf(cookie->value, "%dz%dz%dz%ldz%ld%c", &junk, &junk, &junk, &start, &dur, &c) >= 5
| ~~^ ~~~~~~
| | |
| long int * time_t * {aka long long int *}
| %lld
../f5.c:690:67: warning: format '%ld' expects argument of type 'long int *', but argument 7 has type 'time_t *' {aka 'long long int *'} [-Wformat=]
690 | if (sscanf(cookie->value, "%dz%dz%dz%ldz%ld%c", &junk, &junk, &junk, &start, &dur, &c) >= 5
| ~~^ ~~~~
| | |
| long int * time_t * {aka long long int *}
| %lld
Make it explicitly 'unsigned long long' instead.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Dimitri Papadopoulos [Sun, 8 Jan 2023 12:09:01 +0000 (13:09 +0100)]
Ignore non-sensical NBNS/WINS server address
A VPN server sent the non-sensical NBNS/WINS server IP address 0.0.0.0.
I assume this is the default value in the VPN configuration. If so, it
could happen again. Do not pass this invalid default value to the script.
Mike Gilbert [Thu, 18 May 2023 19:04:17 +0000 (15:04 -0400)]
Move JSON_CFLAGS before LIBPROXY_CFLAGS
Depending on build options, libproxy-1.0.pc depends indirectly
on json-c.pc:
libproxy-1.0 -> gio-2.0 -> mount -> libcryptsetup -> json-c
This causes "pkg-config --cflags libproxy-1.0" to emit
"-I/usr/include/json-c".
json-c installs a "json.h" file that conflicts with the one provided by
json-parser. If json-c comes before json-parser on the compiler command,
we get a build failure:
openconnect-internal.h:1654:59: error: unknown type name 'json_value'
[ dwmw2: This is a combination of at *least* three different bugs in
three different packages conspiring to be my problem. See
https://gitlab.com/openconnect/openconnect/-/merge_requests/476#note_1397129468
But still, working around it does no harm for now.
Ironically, if the presence of json-c on the include path
wasn't *entirely* gratuitous then hiding it by putting it
last wouldn't actually work because then something would
fail to include the json-c version of <json.h> instead. ]
Bug: https://bugs.gentoo.org/906662 Signed-off-by: Mike Gilbert <floppym@gentoo.org> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Thu, 18 May 2023 15:49:29 +0000 (16:49 +0100)]
Move openconnect_set_sni() to API v5.9
We retrospectively added openconnect_set_sni() with the @OPENCONNECT_5_8
symbol version, *long* after API v5.8 was set in stone with the v9.00
release in April 2022.
Fix that by retconning it into a @OPENCONNECT_5_9 version which will be
part of the *next* release.
We have a unit test to prevent us from doing it again, and this commit
is the exception to the general rule that we should *never* commit to
libopenconnect5.symbols except as a side-effect of 'make tag' creating
a new release.
Fixes: 494edf49e628 ("Add openconnect_set_sni API function and Java setSNI() wrapper") Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Thu, 18 May 2023 13:07:33 +0000 (14:07 +0100)]
Add dpkg-gensymbols template file and test for ABI violations
Symbol versioning is hard.
Add some sed magic to build a symbols file of the form consumed by
dpkg-gensymbols, which maps symbols+versions to the first version
of the package in which they appeared.
This serves two purposes.
Firstly it allows us to have a unit test which helps prevent us from
retrospectively adding symbols to a given version after it is first
released — as we did for example when we added openconnect_set_sni() to
OPENCONNECT_5_8 in the 9.10 release.
Secondly, it helps the Debian packaging to get dependencies right. In
RPM distributions, symbol versions map automatically to RPM dependencies
and everyhing Just Works. The package with the library gets a virtual
Provides: of e.g. 'libopenconnect.so.5(OPENCONNECT_5_8)(64bit)', any
package which *uses* symbols from the library will get a corresponding
virtual Requires: — for the symbols it's actually *using* — and it all
works out perfectly. Debian packages, on the other hand, appear to be
held together with duct tape and tears, and need the developer or the
packager to manually curate a file with the mapping of symbol versions
to the first version of the package in which they appeared.
Look Ma! I can sed!
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Wed, 17 May 2023 08:51:48 +0000 (09:51 +0100)]
Rebuild all test certificates
The CA has expired. Rebuild it (and remove the old GnuTLS CA from the
ca-key.pem file where it was just noise).
Rebuild all other certificates while we're at it, but leave the keys
as they were. Extend the validity to 10000 days which should expire
in 2050, by which time it probably won't be my problem.
Dan seems young and healthy; maybe he can thank me then for pedantially
scripting it all instead of doing it manually. Or maybe it'll have
bitrotted so much by then that it won't help.
Most of it worked out of the box this time, but I re-imported the certs
into SoftHSM manually because I didn't want to start from scratch using
the softhsm-setupX make targets. I think some of the behaviour of the
GnuTLS tools (not importing pubkeys, etc) has changed since I did this.
Arguably we should rewrite those rules to import things the same way
into each token and then explicitly tweak them, deleting the public
keys and explicitly marking objects public or private as needed for
each token.
The SoftHSM modifications also had to be done with an older version
of SoftHSM (I used 2.2.0 on Ubuntu 18.04) because doing it with a
newer version meant the newly-imported certs weren't visible in the
Ubuntu 18.04 or CentOS 9 test runs.
Fixes: #609 Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Tue, 16 May 2023 16:08:45 +0000 (17:08 +0100)]
Fix order-only rule dependency variables
When I made the cert rules order-only to prevent all the certs from being
rebuilt unnecessarily, I forgot to switch $< to $| in referencing the
names of the dependencies.
Fixes: e24ef965a96a ("Make all cert rules order-only") Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Tue, 16 May 2023 22:54:12 +0000 (15:54 -0700)]
Fix broken ESP config parsing for GlobalProtect
This was broken in
https://gitlab.com/openconnect/openconnect/-/commit/e2bbc2a1f#efecf80fa476ca5abf1502940e60d7984c6d1df9_426_430
As the comment below that change notes, "We ignore the Legacy IP tag
(<gw-address>) if we've already gotten the IPv6 (<gw-address-v6) tag."
We do indeed want to prioritize ESP-over-UDP-over-IPv6 over
ESP-over-UDP-over-IPv4.
However, this change broke things by making it so that effectively, "We
ignore either tag, unless we've already received a tag."
Thanks to nemo44@gmail.com for bringing this to our attention in
https://gitlab.com/openconnect/openconnect/-/merge_requests/475. I've
modified that patch slightly to make it a bit easier to read and more
idiot-proof in the future (while giving the idiot in question a cold hard
stare in the mirror.)
[Incidentally, a misordered `#endif` / `}` pair also made it so that
https://gitlab.com/openconnect/openconnect/-/commit/e2bbc2a1f and later
wouldn't even compile unless `HAVE_ESP` was `#define`d. Probably no one
is building without ESP support… why would they?]
David Woodhouse [Thu, 11 May 2023 14:55:41 +0000 (15:55 +0100)]
Consolidate browser spawn functions
These were almost identical except that the one in main.c would allow the
browser to be overridden. Combine them, as it's only going to end up with
more duplication if we manage to add Windows support.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Wed, 10 May 2023 11:22:20 +0000 (12:22 +0100)]
Fix stray (null) in URL path after Pulse authentication
When using 'openconnect --authenticate' with a Pulse server, if the urlpath
is empty we append '(null)' to the URL instead of appending nothing as we
should. This also affects NetworkManager-openconnect, since it started to
use openconnect_get_connect_url() in v1.2.8 (commit 911151fc966790c).
Fixes: ec6c0caed28e ("Add openconnect_get_connect_url(), use it in --authenticate output") Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Dimitri Papadopoulos [Sat, 6 May 2023 16:18:19 +0000 (18:18 +0200)]
Document that OpenConnect calculates TOTP/HOTP codes on its own
OpenConnect has calculated TOTP/HOTP token codes without liboath since 554454bf;
we should document that.
Alo:
- Remove the unnecessary downloading and building of liboath from 'android/Makefile'.
- Remove obsolete references to liboath in comments and error messages
- Fix man page formatting surrounding token mode
Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
Daniel Lenski [Fri, 17 Jun 2022 16:59:03 +0000 (09:59 -0700)]
Add os-tcp-mtu utility
Makes a host connection to an arbitrary TCP/IP host:port, and checks the
estimates of the MTU/MSS provided by various getsockopt() calls, just as
OpenConnect uses in calculate_mtu().
TODO:
1. Implement a working os-tcp-mtu for Windows, and build that too.
2. Use https://github.com/morristech/android-ifaddrs as
as a drop-in replacement for `getifaddrs(3)` on Android
David Woodhouse [Thu, 4 May 2023 17:58:14 +0000 (18:58 +0100)]
Fix use-after-free in realloc_inplace()
In file included from auth-globalprotect.c:20:
auth-globalprotect.c: In function 'parse_prelogin_xml':
openconnect-internal.h:1180:17: warning: pointer '__realloc_old_176' may be used after 'realloc' [-Wuse-after-free]
1180 | free(__realloc_old); \
| ^~~~~~~~~~~~~~~~~~~
openconnect-internal.h:1178:13: note: call to 'realloc' here
1178 | p = realloc(p, size); \
| ^~~~~~~~~~~~~~~~
This is a true warning. The second argument to the realloc_inplace()
macro includes a strlen() of the first. Evaluate it first, before the
attempt to realloc().
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 28 Apr 2023 07:22:19 +0000 (09:22 +0200)]
Rework ESP probe retries
We weren't attempting to resend ESP probes at all, except at the retry
interval of about a minute. In a lossy network, or perhaps when the
server is slow to configure its end and start accepting ESP probes,
this meant that users sometimes saw the ESP failing to establish for
a whole minute (or multiple thereof).
Drop the loops in the protocol-specific udp_send_probes() functions
which were a primitive attempt to handle packet loss, and instead
deliberately send one probe a second for five seconds, before giving
up for the remainder of the dtls_attempt_period.
Fix up the reconnect handling with vpninfo->dtls_need_reconnect while
we're at it; it looks like that would just cause us to keep sending
probes and the flag would never be cleared.
Fixes: #601 Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Dimitri Papadopoulos [Fri, 17 Mar 2023 06:27:25 +0000 (07:27 +0100)]
pulsesecure.net → ivanti.com
We should also rename Pulse Connect Secure to Ivanti Connect Secure
at some point. For now, even the Ivanti web site uses both, perhaps
we should wait before we switch Pulse to Ivanti.
Alex Samorukov [Fri, 28 Apr 2023 17:53:29 +0000 (17:53 +0000)]
Add MacOS support to the hipreport
DRL tweaked:
1. Fixed indentation
2. Populated datemon/dateday/dateyear attributes
3. If reporting MacOS, but not actually running on MacOS, then fall back to
reporting OS version 10.16.0 (see https://ss64.com/osx/sw_vers.html)
Signed-off-by: Alex Samorukov <samm@os2.kiev.ua> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
David Woodhouse [Wed, 19 Apr 2023 11:49:42 +0000 (12:49 +0100)]
Attempt to handle Legacy IP frames in the middle of oNCP config
Is there no end to the stupidity of these proprietary protocols?
So... TCP provides you with a byte stream. Forget IP packets underneath;
TCP hides all that for you, and all there is is a hosepipe of bytes.
On top of that, TLS provides 'records' of up to 16KiB, encrypting each
record and sending it down the TCP pipe, then collecting the whole record
on the receiving end, decrypting it and providing it to the user. Mostly,
nobody cares much about TLS records, and they choose to treat TLS just as
a byte stream too.
The Juniper oNCP protocol adds its own layers on top of that TLS byte
stream. First there's the thing which OpenConnect has been calling the
'SSL record', which has a little-endian 16-bit length followed by that
many bytes of data. Then another 16-bit length and that much data, etc.
Those "SSL records" may be correlated with the TLS records, or maybe I
just chose a really stupid name back in 2015 when I first did this. Who
knows, who cares, since we mostly ignore TLS records anyway.
On top of *this* there is an actual packet-based protocol which may
perhaps be called something like KMP. There's a 20-byte header, most of
which we don't understand but it has a type field and a length that we
do understand. This one is big-endian, unlike the layer below it The
type 300 is data, while type 301 is the configuration we get when we
first connect.
Within the KMP300 packets there can be multiple actual IP packets; we
need to look at the length field of the IP packet and split them apart
for delivering them to the tun device. There is no apparent relationship
between the KMP300 packets and the SSL record below; you can have many
KMP300 packets within an SSL record, and the SSL record boundary can
fall *within* a KMP300 packet. So the SSL records have, thus far, just
been a pointless nuisance which means we occasionally have to drop a
2-byte length field from what would otherwise just be treated as a
simple byte stream of data which packetises itself.
Thus far.
If you have a large enough KMP301 config frame (for example, if you have
lots of split include routes), it might get sent in more than one SSL
record. So OpenConnect has code to keep reading those records until the
KMP301 frame is complete. And it *did* assume that the SSL record would
end at the end of the KMP301 frame, since we haven't even finished the
negotiation at this point; we have to send a KMP303 frame back to the
server with our idea of the MTU.
Today we have learned that the server might throw a KMP300 packet with
an IP frame into the *middle* between the two consecutive records that
comprise the KMP301 config frame:
https://gitlab.com/openconnect/openconnect/-/issues/562#note_1358466466
It's not clear if this will only ever be *one* IP packet, or if it can
end up being lots of packets and split across multiple SSL records. In
the example I've seen, it was a *unicast* packet for the VPN client
address.
Attempt to work around the case we've already seen, by discarding a
continuation that looks like a KMP300 with precisely one Legacy IP
packet in it. We *are* also assuming that we at least get the *start*
of the KMP301 config frame first.
Perhaps we'll end up having to keep a list of the records we receive,
then attempt to piece them together more flexibly. But this should
suffice for now.
Fixes: #562 Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Tue, 18 Apr 2023 21:37:05 +0000 (22:37 +0100)]
Fix EINTR handling for select() on cmd_fd
KDE plasma-nm was failing with external browser authentication, reporting
'Socket connect cancelled' immediately after spawning the browser command.
This was caused by the SIGCHLD which kded handles (instead of it being
ignored as in the standalone openconnect process). Thus, the select()
call returns with EINTR, and the fd sets are not changed.
So when check_cmd_fd(vpninfo, &rd_set) is called, it looks like the cmd_fd
was readable. And since plasma-nm uses the legacy cancel_fd setup, the
mere fact of it being *readable* is enough to set vpninfo->got_cancel_cmd.
Instead of forging ahead and interpreting the fd_sets after select()
returns with EINTR, loop and go straight back into select() instead.
For signals like SIGINT from Ctrl-C, we *handle* those and deliver the
cancel command anyway, so we don't depend on the EINTR return in that
case.
Daniel Lenski [Tue, 18 Apr 2023 18:27:34 +0000 (11:27 -0700)]
More specific error message with proposed workaround for Pulse EAP-TLS requests
See https://gitlab.com/openconnect/openconnect/-/merge_requests/414#note_1149887354
for discussion of the apparent layering insanity that's going on in this
corner case.
Daniel Lenski [Sat, 30 Jul 2022 22:59:14 +0000 (15:59 -0700)]
Add --no-external-auth option, and follow it for Cisco protocol
This option is intended to prevent OpenConnect from advertising to the
server that it supports any kind of authentication mode that requires an
external browser. Some servers will force the client to use such an
authentication mode if the client advertises it, but fallback to a
"scriptable" authentication mode if the client doesn't appear to support it.
See https://gitlab.com/openconnect/openconnect/-/issues/470#note_1028595620
for an example.
This option is only implemented here for the Cisco protocol, in which case
it causes OpenConnect not to advertise the 'single-sign-on-v2' or
'single-sign-on-external-browser' auth-methods.
I have verified on one Cisco VPN that this works as intended, as has one
other user (see
https://gitlab.com/openconnect/openconnect/-/issues/470#note_1045509425).
David Woodhouse [Wed, 12 Apr 2023 20:10:34 +0000 (21:10 +0100)]
Fix --server vs. positional argument handling
If we have a --server argument, we shouldn't expect to find a positional
argument at argv[optind]. We mostly got this right, except that we still
called config_lookup_host() with argv[optind] even when --server was
given.
The only thing that saved us from dying with a strcmp() on NULL was
the fact that the loop over the XML elements is using the fact that
vpninfo->hostname gets set as its terminating condition, so it never
got that far.
Fix that, because it's icky. And make the --server argument work for
XML config lookups too.
Fixes: a2fd6f4f2e8a ("New option to define server name in config file") Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Wed, 12 Apr 2023 15:28:50 +0000 (16:28 +0100)]
Set SOCK_CLOEXEC on listening socket for Cisco external browser support
Not entirely sure, but it seems to be accused of causing a hang in
https://gitlab.gnome.org/GNOME/NetworkManager-openconnect/-/merge_requests/49#note_1720281
and regardless of whether that's the case or not, we should be consistent
about using {SOCK,O}_CLOEXEC whenever we can.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Tue, 11 Apr 2023 22:54:33 +0000 (23:54 +0100)]
Fix installer suffix handling
Generate $(INSTALLER_PREFIX) automatically instead of having to pass it
in explicitly, and then the COPR builds should probably work again.
It's kind of awful that we can't get the real version string in the
Makefile so we don't actually know what the true target name is, but
we can live with it.
Fixes: a2dbef082512 ("Unique names for each variant openconnect-installer.exe") Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Tue, 11 Apr 2023 21:54:58 +0000 (22:54 +0100)]
Fix F5 build with json-parser 1.1.0
The json_object_entry structure doesn't exist in json-parser 1.1.0, which
is the latest release and what's shipped by distributions. It's an
anonymous struct as part of the union there, so reference its 'name'
and 'value' as separate pointers.
This should fix the COPR package builds which have been failing.
Fixes: 514cacaff59f ("Parse JSON login forms for F5") Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Tue, 11 Apr 2023 13:39:35 +0000 (14:39 +0100)]
Redirect stdout to stderr when spawning external browser
When running in --authenticate mode we don't want to pollute stdout. So
just redirect it to stderr instead.
Chrome is known to be noisy to stdout when it's reusing an existing
session, and we've already patched NetworkManager to do the same because
it also suffers from stdout pollution.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 7 Apr 2023 15:33:21 +0000 (16:33 +0100)]
Bump default queue length to 32
Some users are reporting that transfer speeds with the default queue
length of 10 packets are poor. Increasing to 32 shouldn't be causing
too much bufferbloat, and appears to resolve the issue.
There's more to be understood here; OpenConnect is in the middle of
multiple other queues for the inbound and outbound traffic paths, and
we should never be starving any of them. And for a bunch of protocols
OpenConnect isn't even honouring the queue length. For *incoming* as
a VPN client, that's probably a bad idea anyway; if packets have made
it all the way across the Internet and the wet piece of string that
connects our client, then we should make sure we accept them and don't
let them build up in the UDP socket receive buffers to the point where
the kernel drops them.
My previous testing of this was in 2008, and was focused on performance
across a local 1GbE connection, which will behave differently.
This change will enable vhost-net by default. That does also help, but
isn't the only factor. And it doesn't help much until the queue length
is higher anyway.
See https://gitlab.com/openconnect/openconnect/-/issues/582 for
further discussion.
Signed-off-by: David Woodhouse <dwmw2@infradead.org> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
David Woodhouse [Fri, 7 Apr 2023 15:11:33 +0000 (16:11 +0100)]
Update translations from GNOME
[ DL: re-fixed inconsistent line endings in po/ug.po; see
https://gitlab.com/openconnect/openconnect/-/commit/682553ecbdd9d159e358e190f6f6009f2e2c9864
for where these were previously fixed, and
https://gitlab.gnome.org/GNOME/NetworkManager-openconnect/-/merge_requests/54
for MR to fix upstream ]
Signed-off-by: David Woodhouse <dwmw2@infradead.org> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
Daniel Lenski [Sat, 4 Mar 2023 06:01:41 +0000 (22:01 -0800)]
Bugfix Y2038 for F5 authentication timestamp
This good recent article on Y2038
(https://www.thkukuk.de/blog/Y2038_glibc_utmp_64bit) reminded me to look for
Unix-epoch integer timestamps that are <64 bits in OpenConnect.
Daniel Lenski [Wed, 1 Mar 2023 01:25:59 +0000 (17:25 -0800)]
Tell Apple users not to use '-i tunX', but '-i utunX' instead.
Per discussion in https://gitlab.com/openconnect/openconnect/-/issues/18#note_953145553,
the BSD "tun" devices have been obsolete for a long time on Mac OS; "utun"
needs to be used instead on modern versions.
However, many users still find suggestions to use `--interface tunX`
floating around the web, and try them. The resulting error message from
OpenConnect is somewhat confusing:
Cannot open '/dev/tunX': Invalid argument
Set up tun device failed
Perhaps we could improve the logic to precisely detect whether or not the OS
wants us to use "tun" or "utun", but that would require a contribution by
someone who understands and cares about Mac OS. In the absence of that, we
can simply add a warning to Mac OS users who attempt to use "tun", telling them
that it's probably wrong.
Daniel Lenski [Sat, 25 Feb 2023 22:27:11 +0000 (14:27 -0800)]
Document the potential need for an EAP-TLS-within-EAP-TTLS workaround for Pulse
See https://gitlab.com/openconnect/openconnect/-/merge_requests/414#note_1149887354
for a more thorough description of this problem by dwmw.
He commented about it in the source code very early on in the development of
the Pulse protocol support, specifically in
https://gitlab.com/openconnect/openconnect/commit/3fb7645608e49c875c20f55352990c7c883bbf96.
Daniel Lenski [Mon, 24 Oct 2022 20:50:15 +0000 (13:50 -0700)]
Pulse needs an 'official' version string in IF/T-T establishment to support IPv6
According to end-user testing in
https://gitlab.com/openconnect/openconnect/-/issues/506#note_1145946165 and
https://gitlab.com/openconnect/openconnect/-/issues/506#note_1146848739,
recent Pulse servers will not send correct IPv6 settings, or simply will not
enable IPv6 traffic, unless the client version string sent in the IF/T
session establishment begins with "Pulse-Secure/" followed by a digit >=8.
From Ivanti/Pulse documentation at
https://help.ivanti.com/ps/help/en_US/PCS/9.1R14/ag/network_n_host_admin.htm#network_and_host_administration_1399867268_681155:
> Only the Pulse client supports IPv6.
> Ivanti Connect Secure release 8.0 and later supports Pulse client access
> to the IPv6 corporate network using VPN Tunneling Connection Profile
> features.
In order to enable IPv6 support, while not misinforming the server about its
identity to an unnecessary disagree, OpenConnect should therefore include an
official-looking prefix ("Pulse-Secure/9.0.3.1667 " for now) in front of its
default user-agent string.
Daniel Lenski [Sun, 23 Oct 2022 18:17:28 +0000 (11:17 -0700)]
Newer Pulse servers can disable their ESP protocol layering malpractice
See b4f50f8bd5da7e6ac926ddd5095501edbc204cd0 for the way that the Pulse ESP
tunnel is mangled. In brief, if the Pulse ESP tunnel is running over IPvX,
then it will only transport tunnelled packets of address family IPvX;
tunnelled packets of IPvY must go over the TLS/TCP tunnel.
In addition to being a complete inversion of the abstraction of a protocol
with independent layers:
- This results in poor performance for tunnelled IPvY packets (TCP-over-TCP)
- Our original implementation of this caused a regression for the
Juniper/NC ESP tunnel (fixed in 3d1ec6e0a126d4b9fdd18473558cf816d2045b76).
As reported in http://lists.infradead.org/pipermail/openconnect-devel/2020-October/004934.html
and https://gitlab.com/openconnect/openconnect/-/issues/506, newer Pulse
servers starting with 9.1R9 can apparently use the ESP tunnel in a sane way,
if the administrator sets a flag labelled "Use ESP tunnel for 6in4 and 4in6
traffic".
OpenConnect should try to coax the server to use this saner tunnel setup
if possible.
Daniel Lenski [Sun, 15 Jan 2023 19:16:53 +0000 (11:16 -0800)]
Add FTM-push token mode for Fortinet
If the server sends a "tokeninfo-style" 2FA request (`ret=N,…,tokeninfo=…`)
with the special value `tokeninfo=ftm_push`, then it means that it
can (optionally!) use mobile-device "push"-based authentication instead of
having the user enter a token code.
If (and only if) the user leaves the token code blank, we must (1) remove
`magic` from the response and (2) add `ftmpush=1`, in order to trigger the
correct response.
This also adds a "type_2fa=ftmpush" mode to our fake Fortinet server, and
simulates this option in our tests.
Thanks to Ivan Futivić for the very useful and detailed report in
https://gitlab.com/openconnect/openconnect/-/issues/555#note_1239886436.
Andy Teijelo [Fri, 10 Feb 2023 14:37:23 +0000 (14:37 +0000)]
Use the timeout command in csd-wrapper.sh
The timeout command (from coreutils) can replace the more complex
job-control-based approach which we current use to limit the execution time
of the Trojan binary (`cstub`).
Signed-off-by: Andy Teijelo Pérez <ateijelo@gmail.com> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
Daniel Lenski [Mon, 20 Feb 2023 20:35:36 +0000 (12:35 -0800)]
Bundled Cisco CSD wrapper script only works on GNU/Linux on Intel x86/x86_64 processors
See discussion at
https://gitlab.com/openconnect/openconnect/-/merge_requests/455#note_1275204407.
It appears that wrapper scripts for MacOS (aka Darwin) have been written,
but would need testing in order to integrate into our repository. See
https://gist.github.com/asarkar/fb4452a4abdf9e4a9752a7d55d2cdc93.
Dimitri Papadopoulos [Fri, 24 Feb 2023 05:36:12 +0000 (21:36 -0800)]
Cherry-pick several one-line cleanup MRs
Closes https://gitlab.com/openconnect/openconnect/-/merge_requests/444,
https://gitlab.com/openconnect/openconnect/-/merge_requests/447, and
https://gitlab.com/openconnect/openconnect/-/merge_requests/454
Dimitri Papadopoulos [Mon, 14 Nov 2022 17:41:29 +0000 (18:41 +0100)]
Make it clearer that the preferred driver is Wintun
We continue to load TAP-Windows preferentially, if it is installed.
However, we always install Wintun as part of the OpenConnect installer, and
do *not* always install TAP-Windows. If users want to use TAP-Windows with
OpenConnect, they need to install it manually.
Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
Daniel Lenski [Sat, 21 Jan 2023 00:55:54 +0000 (16:55 -0800)]
GnuTLS: Add UNSAFE_RENEGOTIATION to allow-insecure-crypto
When `--allow-insecure-crypto` is specified, we should explicitly enable
unsafe/legacy TLS renegotiation.
Recent distributions and library versions disable unsafe/legacy TLS
renegotiation by default, because it is vulnerable to a MITM-like attack
(see http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2009-3555).
However, many older Cisco servers don't appear to be able to cope *without*
unsafe/legacy TLS renegotiation (see
https://gitlab.com/openconnect/openconnect/-/issues/558 for an example).
We already made the equivalent change for OpenSSL in 2021, back in
https://gitlab.com/openconnect/openconnect/commit/c8dcf10cb9bd63c3148922c42b9c47392c89fe9d.
Because of that partial change, OpenConnect+OpenSSL has been able to connect
to Cisco servers that OpenConnect+GnuTLS *can't* connect to.
projects / users / dwmw2 / openconnect.git / log