# Configure fake server for SAML on the portal interface
$ curl -sk https://localhost:8080/CONFIGURE -d portal_saml=portal-userauthcookie -d portal_cookie=portal-userauthcookie
# Use gp-saml-gui to authenticate to it
$ gp-saml-gui --no-verify localhost:8080
...
... pops up window
... fills out login form
...
HOST=https://localhost:8080/global-protect/getconfig.esp:portal-userauthcookie
USER=nobody
COOKIE=FAKE_username_nobody_password_whatever
OS=linux-64
The goal of this is to have a SAML-supporting GP server to test against
while modifying openconnect to directly call the GP SAML webview handler
itself (see https://github.com/dlenski/gp-saml-gui/issues/45).
Daniel Lenski [Thu, 12 May 2022 20:58:04 +0000 (13:58 -0700)]
Rework GP fake server to have a persistent configuration
Until now, all of our Flask-based servers for testing protocol
authentication flows have "cheated" a bit.
They're configurable (e.g. should the authentication require a 2FA token,
or should it not?), but this configuration has not actually been persisted
by the server.
Instead, the server sends the client a session cookie which has the desired
*server* configuration glommed into it, and relies on the client sending
this cookie back on every request (which OpenConnect obliges, for
authentication requests) so that the server can "remember" what it's
configured to do.
This is very confusing to read and understand, and it's also untenable to
continue using it for fake servers that simulate external authentication
(SAML/SSO). That's because external authentication uses a separate
browser/handler for the authentication flow, so it won't share the session
cookies.
This modifies the fake GP server to store a persistent in-memory
configuration which can easily be set and inspected with cURL:
$ ./fake-gp-server localhost 8080 certs/server-{cert,key}.pem 2>&1 >/dev/null &
$ curl -sk https://localhost:8080/CONFIGURE -d gw_2fa=1
$ curl -sk https://localhost:8080/CONFIGURE
Current configuration of fake GP server configuration:
TestConfiguration(gateways=('Default gateway',), portal_2fa=False, gw_2fa=True, portal_cookie=None, portal_saml=None, gateway_saml=None)
$ openconnect --protocol=gp localhost:8080
Please login to this fake GP VPN portal
Username: fakeusername
Password: *******
...
2FA challenge from gateway
Challenge:
Daniel Lenski [Sat, 28 May 2022 21:37:44 +0000 (14:37 -0700)]
Clearer error for list-system-keys on Unix-like platforms
It appears that the `gnutls_system_key*` functions are only implemented on
Windows currently. Lots of people are likely to test this executable on
Unix-y systems, so we should give a clearer error message.
Daniel Lenski [Thu, 2 Jun 2022 21:54:54 +0000 (21:54 +0000)]
Clearer error message when GlobalProtect portal configuration contains no gateways at all
The GP portal config parser is intended to print a clear error message when the list
of gateway servers is empty.
However, when the `<gateways><external><list>` tag is entirely absent, this
message is skipped, resulting in a vague "Failed to parser server response"
as seen in https://gitlab.com/openconnect/openconnect/-/issues/444.
David Woodhouse [Sat, 21 May 2022 22:12:12 +0000 (23:12 +0100)]
Don't install list-system-keys
It breaks the Linux COPR builds since the native RPM doesn't list it as
one of the files that are expected to be installed. And in fact we don't
*want* it installed for Linux as GnuTLS system keys aren't supported on
Linux anyway. So just leave it uninstalled for now. It's in the Windows
installer.
We *will* want to make it available for the GUI installer to use, but
I'm still not sure how to handle that. Since the GUI changes less often
perhaps I should actually pull in the openconnect-gui MinGW package as
a dependency of the openconnect build. Which is backwards really, but
it would allow us to spit out an installer including OpenConnect-GUI
for every libopenconnect build. Not today though...
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Sat, 21 May 2022 20:39:24 +0000 (21:39 +0100)]
Clean up NSIS installation a bit
Move list-system-keys.exe to the top-level directory as it isn't really
a test, and install it as part of the RPM.
Clean up the DLL dependency handling to allow for multiple .exe files as
'roots', and also add a hack to make it possible to include extra files
like openconnect-gui.exe
That required supporting Qt and its plugin DLLs, which isn't strictly
needed in OpenConnect itself but I'll probably lift the same mechanisms
to use them in the mingw-openconnect-gui build, and it's best for them
to remain in sync.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Thu, 12 May 2022 21:16:20 +0000 (14:16 -0700)]
Bugfix fake-gp-server.py: <saml-request> uses the 'standard' base64 alphabet, not the 'URL-safe' one
We know that both openconnect_base64_decode() and
https://github.com/dlenski/gp-saml-gui successfully decode the contents of
the <saml-request> tag fromn real GP servers that require SAML auth, so
their expectation of the 'standard' base64 alphabet here must be correct.
This may fix some of the intermittent failures in tests/gp-auth-and-config.
As enforced by autopudate, see 4.2 Dealing with Autoconf versions:
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/html_node/Versioning.html
CentOS 6 has reached End of Life, Maintenance support ended in November 2020.
RHEL 6 has entered Extended Life Phase (ELP), but its scope is limited:
• assistance on existing installs only (not new installs),
• no investigation of the root cause for unknown issues.
Extended Life Cycle Support (ELS) does include some updates, but only
certain critical-impact security fixes and selected urgent priority bug fixes.
It's pointless to provide more support than Red Hat themselves.
Implement a function openconnect_set_useragent to allow external
programme that use libopenconnect to start a VPN (like NetworkManager)
to tune the Useragent of the connection like the option --useragent do.
If they only tune the useragent agent name by the variable passed
through the opeconnect_vpninfo_new function, the version number of
openconnect is automatically added after the string choosed.
AC_TRY_COMPILE is obsolete starting with autoconf 2.70
AC_COMPILE_IFELSE has been around since autoconf 2.50 at least,
according to "18.6.5 AC_ACT_IFELSE vs. AC_TRY_ACT":
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/html_node/AC_005fACT_005fIFELSE-vs-AC_005fTRY_005fACT.html#AC_005fFOO_005fIFELSE-vs-AC_005fTRY_005fFOO
Since Autoconf 2.50, internal codes uses
AC_PREPROC_IFELSE, AC_COMPILE_IFELSE, AC_LINK_IFELSE, and AC_RUN_IFELSE
on one hand and
AC_LANG_SOURCE, and AC_LANG_PROGRAM
on the other hand instead of the deprecated
AC_TRY_CPP, AC_TRY_COMPILE, AC_TRY_LINK, and AC_TRY_RUN.
Fixes this Autoconf warning:
warning: The macro `AC_TRY_COMPILE' is obsolete.
m4/lib-ld.m4
m4/lib-link.m4
m4/lib-prefix.m4
the latest versions from gnulib
acinclude.m4 → m4/ax_jni_include_dir.m4
the latest release 2022.02.11 from the Autoconf Archive
acinclude.m4 → as-compiler-flag.m4
copied as is because the origin of this file is unclear
it is different from the latest version in the Autostar Sandbox
As enforced by autopudate, see 3.1.2 The Autoconf Language:
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/html_node/Autoconf-Language.html
Daniel Lenski [Thu, 20 May 2021 00:01:01 +0000 (00:01 +0000)]
Do not ignore 0.0.0.0/0 specified as a "split"-{in,ex}clude route for oNCP
This addresses https://gitlab.com/openconnect/openconnect/-/issues/245. In the case
presented there, the oNCP server sends a Legacy IP netmask ("default route") of
255.255.255.255, and a "split"-include route of 0.0.0.0/0.0.0.0:
> Received split include route 0.0.0.0/0.0.0.0
> Received netmask 255.255.255.255
We also should not ignore 0.0.0.0/0 if specified as a "split"-exclude route, though
the purpose of such a route is unclear and we have never seen one in the wild.
Next, we should handle this case in the same way that we do for GlobalProtect,
as of https://gitlab.com/openconnect/openconnect/-/merge_requests/118; namely,
by replacing the 255.255.255.255 netmask with the 0.0.0.0/0 send as a "split"-include,
and removing the latter from the list of split-includes.
David Woodhouse [Fri, 22 Apr 2022 16:02:08 +0000 (17:02 +0100)]
Make all STRAP support conditional on HPKE
We really don't care about STRAP; we only did it for the external browser
support. The only case we've seen STRAP failing is when we *did* advertise
it and then didn't really do it, so omitting it completely seems harmless
for now.
And older GnuTLS such as on CentOS doesn't have some of the functions we
are using to obtain the Finished message or export the privkey, so just
disable it all. We *could* support the basic STRAP from GnuTLS 3.4.0 on
but there's not a lot of point.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 22 Apr 2022 15:14:17 +0000 (16:14 +0100)]
Silence static-analyser warning about redundant assignment to 'sep'
I did this for a reason. The *compiler* is clever enough not to bother
actually doing the assignment (not that it would matter anyway, since it
is hardly a fast path). But *developers*, including myself, are much less
likely to spot that it needs to be added in the 'deflate' case if we add
a new case at the end. So now in order to shut the tools up, I have to
turn a non-bug into a latent *actual* bug.
I suppose I could leave it there with a comment, or refactor it into a
loop over tuples of the form { COMPR_LZ4, "oc-lz4" }… but it probably
doesn't matter as we're unlikely to be adding more. Just suck it up.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
The assignment *was* necessary. The point was that the first time 'sep'
was used, it's a space. And *after* that, it's a comma. Using a comma
every time ends up sending headers which look like this:
X-DTLS-Accept-Encoding:,lzs
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 22 Apr 2022 14:39:14 +0000 (15:39 +0100)]
Export STRAP private key with AnyConnect cookie
For STRAP we need to reconnect using the same private key as the auth
process did. Thankfully we already have precedent for this; we can put
multiple 'cookies' into the opaque string that is passed from auth to
connection process, and use internal_split_cookies() to parse them.
So encode the privkey into an 'openconnect_strapkey' cookie which we
handle specially on ingestion.
Fix up a few places where vpninfo->cookie was handled directly, to make
it all work.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Thu, 21 Apr 2022 21:14:00 +0000 (22:14 +0100)]
Attempt to implement AnyConnect Session Token Re-use Anchor Protocol (STRAP)
In order to implement the external-browser SAML support we had to send
the X-AnyConnect-STRAP-Pubkey: header, even though we didn't really know
what that was.
That turns out to cause a regression for some user (issue #410), as the
server then rejects us we don't include a valid X-AnyConnect-STRAP-Verify:
header in our CONNECT request.
That header is supposed to contain our Finished message from the TLS
handshake, hashed and signed with our STRAP-Pubkey. Or if we rekey, it's
a signed hash of the Finished message concatenated with the (DER) public
key that we also send in a new X-AnyConnect-STRAP-Pubkey: header.
Lightly tested, as we can't actually work out how to make *our* servers
reject the connections for this offence, and Cisco's documentation is
very sparse. But it shouldn't make things worse for anyone.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Fri, 11 Feb 2022 18:13:46 +0000 (10:13 -0800)]
GP: add 'internal=no' flag to the login and configuration requests
Without these flags, one user reports consistently hitting the "Matching
client config not found" error in response to the /ssl-vpn/getconfig.esp
request. See https://gitlab.com/openconnect/openconnect/-/issues/246
I *suspect* that 'internal=no' is the implicit default if unspecified
(https://gitlab.com/openconnect/openconnect/-/issues/246#note_836128670),
but we should get more testing on other GlobalProtect VPNs to confirm that
this works fine with them. If there is variation in whether this parameter
is allowed/expected, then we need a way to automatically detect the correct
value.
Daniel Lenski [Wed, 20 Apr 2022 03:43:27 +0000 (20:43 -0700)]
Fix initial client request XML structure when announcing multicert capability
Having a separate 'announce_multicert_capability' function wasn't ideal:
1. Building the initial '<config-auth>' XML was more spread out and complex
than necessary due to an extra layer of functions.
2. When multicert auth is offered by the client, the resultant XML actually
contained *two* nested capabilities/auth-method tags:
What Cisco clients appear to send is a *single* '<capabilities>' tag
containing multiple '<auth-method>', as shown in the MITM capture in the
comments of f51ecb36bedcd370086586295978627daeabade4 ("Converse the multiple certificate authentication (multicert) protocol."),
which introduced this structural mistake.
With the non-repeated '<capabilities>' tag, we can also fix the multicert
auth tests, where the check that the client was offering multicert was
disabled in 45da3b07dfc8f808e7b0d0bf80fbf7e73b1b0721 ("fake-cisco-server.py: Disable check for `multiple-cert` support"),
probably due to confusion over the XML structure and xmltodict's handling
of it.
DWORD is unsigned, so PRIu32 would be the proper format specifier for
DWORD, not PRId32. Except DWORD is defined as 'unsigned long int', while
PRIu32 is defined as u, and "%u" is the format specifier for 'unsigned int'.
What a mess! It looks like the only viable format specifier is "%lu".
projects / users / dwmw2 / openconnect.git / log