David Woodhouse [Mon, 29 Mar 2021 16:44:21 +0000 (17:44 +0100)]
Fix installer deps
Just including .openconnect.exe.d from the main Makefile means it always
has to be up to date. Which means we always have to *build* openconnect,
even if it was just invoked for 'make clean'. That's bogus.
So shift the DLL dependency tracking out into its own Makefile, to be
invoked recursively. Much as I hate recursive make in the general case,
this is one of the few cases where it makes sense.
The automatic dependencies weren't working very well anyway, as it was
happy to 'discover' openconnect.exe and libopenconnect-5.dll in the MinGW
sysroot instead of using the locally built ones, *and* libtool messes up
the filenames so you can't just depend on .libs/openconnect.exe anyway.
Fix up $(DISTCLEANFILES) to be a bit more complete while we're at it.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Mon, 29 Mar 2021 16:09:32 +0000 (17:09 +0100)]
Fix handling of downloaded files
We want the MinGW package builds to have the downloads provided in advance
as part of the source RPM, instead of requiring network access and fetching
them once for each of the win32 and win64 builds.
So let make find the file in its VPATH and then use the full pathname that
it puts into $< for the recipe. And use that recipe to validate the hash
of the file too.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Mon, 29 Mar 2021 10:17:21 +0000 (11:17 +0100)]
Turn off -Wdeclaration-after-statement and allow C99
I think this far into the 21st century we can allow declarations to be
in the middle of code blocks. Even MSVC supports that, even if it doesn't
do other parts of C99 like named struct initialisers.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Mon, 29 Mar 2021 02:55:18 +0000 (19:55 -0700)]
ppp-over-tls tests: fix PPP-over-IPv6 tests on Ubuntu
For reasons that are unclear, but probably also unimportant, IPv6 is disabled by default on this CI
image (verified in https://gitlab.com/openconnect/openconnect/-/jobs/1135199323#L335), and this will
cause PPP tests using IPv6 to fail.
Explicitly enabling IPv6 with sysctl resolves this.
Daniel Lenski [Fri, 26 Feb 2021 02:39:45 +0000 (18:39 -0800)]
ppp-over-tls tests: give up on CentOS 6
We should be able to --enable-ppp-tests on CentOS 6, but they simply aren't working.
For reasons that are not clear, OpenConnect fails to handshake a TLS
connection to socat 1.7.2 (from CentOS 6 EPEL), and I don't want to install
an 11-year-old distribution on a VM just to figure out why.
Some CI is still failing because pppd can't successfully execute the
/etc/ppp/* scripts after configuring the interfaces. Let's just move these
out of the way so that pppd won't try to execute them.
Side rant: pppd is the most appallingly bad program in terms of separation
of concerns.
Daniel Lenski [Mon, 8 Feb 2021 19:38:19 +0000 (11:38 -0800)]
factor out internal_split_cookies from auth-juniper.c
This is useful for other protocols that use HTTP cookies for authentication, and may
need a way to handoff >1 cookie from the authentication phase to the connection phase.
Improve it slightly by allowing it to set a "default" HTTP cookie if the
authcookie string doesn't contain '=', since most protocols really only NEED
one cookie for the connection phase to work.
This allows shortcuts (like `openconnect --protocol=nc -C 'foobar'` → 'Cookie: DSID=foobar'
or `openconnect --protocol=fortinet -C 'ABCD123456=='` → 'Cookie: SVPNCOOKIE=ABCD123456==') without limiting
the ability to store multiple cookies if/when useful.
Daniel Lenski [Wed, 24 Feb 2021 05:03:47 +0000 (21:03 -0800)]
ppp-over-tls tests: try to keep CentOS 6 CI working, and improve flaky startup of pppd
Even with EPEL, CentOS has an old version of socat which doesn't support the
'rawer' option, so let's use the older 'raw,echo=0' combination to keep it
limping along.
More carefully try to verify that socat and pppd start up and connect to each other:
- Wait for socat to create PTY in 1-second increments, and keep going until PTY
actually exists (up to 15 seconds).
- Wait for ppp to connect to PTY in 1-second increments, and keep going until pppd
creates a "UUCP-style lockfile" for the PTY.
- Log how long it takes for the above process to complete (socat and pppd combined
startup) in the test output.
Daniel Lenski [Tue, 9 Feb 2021 00:08:17 +0000 (16:08 -0800)]
CI: re-enable PPP tests for CentOS7, Fedora, and Ubuntu
Still to-do:
1) Get socat+pppd working in CentOS8 and CentOS6 CI
2) Figure out why PPP tests are so slow (added log retention for 1 week, even on success, in Ubuntu18.04/GnuTLS build)
Daniel Lenski [Tue, 9 Feb 2021 01:07:10 +0000 (17:07 -0800)]
improve ppp-over-tls tests
- Cleanup ugly pppd syntax
- Always set 'nodefaultroute' and negotiate link-local IPv4 addresses
- Don't rely on non-root to cause OpenConnect to terminate
- More tests with HDLC, only one without
Daniel Lenski [Fri, 22 May 2020 03:47:59 +0000 (20:47 -0700)]
add ppp-over-tls tests (with pppd as the reference peer implementation)
These test OpenConnect's ability to communicate with the standard pppd using
PPP-over-TLS, with a variety of PPP negotiation options:
± IPv4
± IPv6
± DNS and NBNS server negotiation
± PPP header protocol/address field compression
± Van Jacobson header compression (always rejected by OpenConnect)
These tests use socat to create TLS socket pairs, connecting `openconnect
--protocol=nullppp` to one end and `pppd` to the other.
I tried and failed to combine socat and pppd invocations, but pppd seemingly
cannot handle being wrapped by libsocket_wrapper.so (nor libuid_wrapper.so;
it must run as root).
`pppd sync` (non-HDLC framing) appears to have trouble reacting to incoming
packets in this configuration, so OpenConnect has to invoke retry timers during
negotiation. This seems to be a bug in pppd, rather than a flaw in OpenConnect's
implementation of PPP. Added logging of the time that each run takes.
Connect OpenConnect to the TLS socket, and watch it negotiate LCP/IPCP/IP6CP with its peer, and reject CCP:
# Add noipv4,noipv6 to cookie to try those
./openconnect --protocol=nullppp --cookie hdlc --servercert=d66b507ae074d03b02eafca40d35f87dd81049d3 --dump localhost:5556
David Woodhouse [Wed, 6 May 2020 21:10:42 +0000 (22:10 +0100)]
add support for PPP-based protocols
This started out as the 'f5' branch, which was then rebased (by DL) onto a
more recent master as 'ppp_rebased'.
This was then squashed down into a single commit, with all of the bits
depending on "real" PPP-based VPN protocols removed, so that only the shell
of generic PPP support remains, including PPP protocol negotation (LCP,
IPCP, IP6CP) and mainloop.
Almost all of this code was done by David Woodhouse and Daniel Lenski in
May 2020, with a few more recent bugfixes by DL in early February 2021.
The remaining shell supports two different encapsulations of PPP:
- PPP_ENCAP_RFC1661: Plain PPP. “Synchronous” in the ’90s-era language,
because the start and end of the frame are known from external means.
In modern instances, this is because they arrive in a complete packet with
a known length from its lower-level encapsulation. (TLS or DTLS records in
our expected use cases.)
This is what `pppd sync` does.
- PPP_ENCAP_RFC1662: “PPP in HDLC-like framing.”
This is what `pppd` without `sync` does.
The following commits will add tests to demonstrate the functionality of
OpenConnect's PPP implementation at the level of this generic encapsulation.
Daniel Lenski [Mon, 11 May 2020 22:34:34 +0000 (15:34 -0700)]
oncp_control_queue → tcp_control_queue
This is a queue for outgoing packets which must be sent over the
TCP-based transport; that is, they cannot be sent over the
UDP-based transport.
This queue was initially used by oNCP protocol for ESP enable/disable
packets, and it is now also used by Pulse. It will likely be used for
control packets by some PPP-based protocols as well.
Renaming it to TCP control queue to emphasize its cross-protocol
nature (cf. https://gitlab.com/openconnect/openconnect/-/merge_requests/151).
David Woodhouse [Sat, 27 Mar 2021 15:00:34 +0000 (15:00 +0000)]
Cast GetVolumeInformationByHandleW to (void *)
The 64-bit Windows build was complaining of pointer type mismatches but
I'm fairly sure it was a false positive:
../ssl.c: In function 'openconnect_passphrase_from_fsid':
../ssl.c:598:9: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'BOOL (*)(void *, WCHAR *, DWORD, DWORD *, DWORD *, DWORD *, WCHAR *, DWORD)' {aka 'int (*)(void *, short unsigned int *, long unsigned int, long unsigned int *, long unsigned int *, long unsigned int *, short unsigned int *, long unsigned int)'} [-Wcast-function-type]
598 | func = (GVIBH)GetProcAddress(kernlib, "GetVolumeInformationByHandleW");
| ^
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 26 Mar 2021 15:54:39 +0000 (15:54 +0000)]
Fix obsolete-server-crypto in the GnuTLS build not the OpenSSL one.
We're clearing OpenSSL capabilities in the GnuTLS tests to work around
a SoftHSM bug, which is what confused me into putting the XFAIL in the
wrong case.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 26 Mar 2021 15:38:16 +0000 (15:38 +0000)]
Add obsolete-server-crypto to XFAIL tests in Fedora package
The Fedora specfile explicitly sets the GnuTLS priority string to
include @OPENCONNECT but the test sets GNUTLS_SYSTEM_PRIORITY_FILE
to point to /dev/null so that stops working. Just XFAIL the test for
now so the builds start working again. More thinking required...
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 26 Mar 2021 15:34:17 +0000 (15:34 +0000)]
Fix up string handling for ciphersuite_config
Sure it isn't C++ and std::string, but we *have* a method for appending
strings to a dynamic buffer. We don't need this snprintf("%s%s%s%s%s")
nonsense.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 26 Mar 2021 14:52:40 +0000 (14:52 +0000)]
Fix pfs test for out-of-tree builds
Originally we generated config files from autoconf, so we had:
• ${srcdir}/tests/configs/test-foo.in
• ${builddir}/tests/configs/test-foo
Later, we wanted to generate files more dynamically at runtime with
different contents, so we added the update_config() function in
tests/common.sh which did its own substitution to a temporary file,
from
• $(srcdir}/tests/configs/test-foo
The pfs and obsolete-server-crypto tests appear to use a broken hybrid
of the two, first creating ${builddir}/tests/configs/test-foo from
autoconf and then attempting to use ${srcdir}/tests/configs/test-foo
at runtime. The latter isn't going to exist if ${srcdir} != ${builddir}.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 26 Mar 2021 14:03:10 +0000 (14:03 +0000)]
Drop web page handling
The web site is handled by a cron job; if we wanted to flip that
over to pull from gitlab instead of git.infradead.org it would
be simple enough to it. Let's keep it in the same place for now.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Tue, 23 Mar 2021 17:55:19 +0000 (10:55 -0700)]
MingW32 builds: generate NSIS installers for Windows
nsiswrapper is a rather dodgy Perl script (looks like it hasn't actually
been updated since ~2009) with one indispensable function: it can
automatically find required DLLs and bundle them into the installer.
1. The `PATH` variable must be set to include DLL locations (why `PATH`?️)
2. It mixes up its stdout and stderr, so `--verbose` can't be used without
`--run`. (Argh…)
3. It doesn't try to normalize the paths of the bundled files, so
`./x` and `$PWD/x` result in different directory structures.
Daniel Lenski [Tue, 16 Mar 2021 21:40:18 +0000 (14:40 -0700)]
cstp: don't send X-AnyConnect-Platform header
1. Cisco AnyConnect 4.8+ no longer sends it, and some newer servers reject
any client which sends it (see #101)… including older versions of Cisco's
own client. (Great job, Cisco 🤦🏻♂️.)
2. We can't find any evidence of older Cisco servers which *do* require this
header to be present in order to authenticate the client.
3. It's redundant. Any server that wants to know the client's platform as
soon as it receives the initial XML POST already has it. (It's in the
<device-id> tag in addition to the header.)
If there actually are any servers that *do* require this header to identify
and authenticate the client/platform, then the `--local-id` mechanism of
!103 is probably the right way to ensure that it is sent.
Daniel Lenski [Mon, 22 Feb 2021 03:17:42 +0000 (19:17 -0800)]
return EPERM, not EINVAL, when GP gateways reject the cookie upon get-config or GET-tunnel
Tested against 2 real GlobalProtect gateway servers, which both exhibit
these error behaviors:
1. 'GET /ssl-vpn/config' with bogus 'portal' field in cookie
=> XML error "Portal name not found"
2. 'GET /ssl-vpn/config' with 'user', 'authcookie', or 'portal' fields missing from cookie
=> HTTP/1.1 200, plaintext "errors getting SSL/VPN config"
3. 'GET /ssl-vpn-tunnel' with bogus 'user' or 'authcookie'
=> HTTP/1.1 504 (Gateway Timeout)
Daniel Lenski [Thu, 13 Jun 2019 15:53:27 +0000 (10:53 -0500)]
ensure that openconnect_disable_{dtls,ipv6} do nothing if vpninfo has ever been connected
Both openconnect_disable_dtls() and openconnect_disable_ipv6() now return
int (0 or -EINVAL), and are used in CLI argument processing.
Ideally, we would allow these to be used when IPv6 or DTLS/ESP are not currently
in use, but we do not have sufficiently reliable cross-protocol indicators of these.
The main use case for both of these functions is disable a problematic feature
prior to initial connection.
Daniel Lenski [Wed, 5 Jun 2019 03:30:30 +0000 (20:30 -0700)]
add openconnect_disable_dtls() API function
This also adds the API function to the Java bindings.
The immediate motivation is that there are a lot of Android users with
MTU-related issues (https://github.com/cernekee/ics-openconnect), and
disabling UDP/DTLS/ESP is a good temporary band-aid.
Daniel Lenski [Sat, 20 Feb 2021 00:05:09 +0000 (00:05 +0000)]
Try to generate static website using GitLab pages
Based on the example at https://gitlab.com/pages/plain-html, and ocserv's configuration (https://gitlab.com/openconnect/ocserv/blob/master/.gitlab-ci.yml)
Daniel Lenski [Mon, 25 Jan 2021 07:39:39 +0000 (23:39 -0800)]
allow specification of multiple certificate fingerprints on command-line via --servercert
Server certificates will be accepted if they match *any* of the provided fingerprints.
Behavior with `--servercert` is otherwise unchanged; it still disables system trust
stores, meaning that _only_ certificates matching the provided fingerprints will be
accepted if it is specified one or more times.
This will allow the use of `--servercert` to non-interactively connect to a server which
has a non-trusted certificate and redirects to one or more other servers with non-trusted
certificates. (See #25 for a real case.)