Brahmajit Das [Mon, 29 Jan 2024 17:58:53 +0000 (23:28 +0530)]
Fix implicit declaration of function 'malloc'
First observed on Gentoo Linux with GCC 14. This is due to GCC 14
enabling -Werror=implicit-function-declaration by default.
Thus resulting in errors such as:
openconnect-internal.h: In function 'alloc_pkt':
openconnect-internal.h:911:27: error: implicit declaration of function 'malloc' [-Werror=implicit-function-declaration]
911 | struct pkt *pkt = malloc(alloc_len);
| ^~~~~~
Plese refer gentoo bug: https://bugs.gentoo.org/923173 Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
../mtucalc.c: In function 'calculate_mtu':
../mtucalc.c:75:33: warning: passing argument 4 of 'getsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
75 | &mss, &mss_size)) {
| ^~~~
| |
| int *
In file included from ../openconnect-internal.h:31,
from ../mtucalc.c:20:
C:/msys64/mingw64/include/winsock2.h:1010:82: note: expected 'char *' but argument is of type 'int *'
1010 | WINSOCK_API_LINKAGE int WSAAPI getsockopt(SOCKET s,int level,int optname,char *optval,int *optlen);
| ~~~~~~^~~~~~
CC libopenconnect_la-lzo.lo
../cstp.c: In function 'calculate_dtls_mtu':
../cstp.c:134:33: warning: passing argument 4 of 'getsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
134 | &mss, &mss_size)) {
| ^~~~
| |
| int *
In file included from ../openconnect-internal.h:31,
from ../cstp.c:21:
C:/msys64/mingw64/include/winsock2.h:1010:82: note: expected 'char *' but argument is of type 'int *'
1010 | WINSOCK_API_LINKAGE int WSAAPI getsockopt(SOCKET s,int level,int optname,char *optval,int *optlen);
| ~~~~~~^~~~~~
They add spaces (BWS) at the end of chunk-size, even in the absence of chunk-ext.
Be lenient when parsing chunk:
1. Accept bogus chunk-ext, with ";" not followed by chunk-ext-name.
2. Discard leading/trailing spaces in chunk-size, strtol() will do that for us.
Nikos Mavrogiannopoulos [Wed, 10 Jan 2024 19:51:37 +0000 (20:51 +0100)]
nsis: create self-contained nsi file
Including from a relative path is interpreted differently
depending on where the caller is started. This allows running
nsis on the output nsi even if not located at the build directory.
warning 9100: Generating version information for language
"1033-English" without standard key "FileVersion"
warning 9100: Generating version information for language
"1033-English" without standard key "FileDescription"
<libxml/tree.h> used to be included both by "openconnect-internal.h"
and from *.c source files. We don't need both. Let's settle on including
from "openconnect-internal.h" only.
Rahul Rameshbabu [Thu, 21 Dec 2023 20:46:08 +0000 (12:46 -0800)]
cstp: Check if uri is NULL in sso_detect_done
Passing a NULL value to strcmp is undefined behavior. Some web engines
might have events where cookies are enumerated, but the event does not
contain a uri enumeration. An example is QtWebEngine where it has discrete
signals, QWebEngineView::urlChanged and QWebEngineCookieStore::cookieAdded.
Add a check similar to the one found in gpst_sso_detect_done for the uri
member of struct oc_webview_result.
Because we know the code in `main.c` is executed in a single-threaded
environment, we don't need to modify non-reentant functions in this file,
unless some linter complains in the future:
* localtime()
* getpwnam()
The only remaining non-entrant function is:
* getpwuid()
Using constant 2049 instead of sysconf(_SC_GETPW_R_SIZE_MAX) might not
be the best idea. I want to avoid dynamic allocation. On Ubuntu 18.04,
sysconf(_SC_GETPW_R_SIZE_MAX) is 1024, so 2049 "ought to be enough".
From the POSIX documentation of ctime:
The ctime() function shall convert the time pointed to
by clock [...] to local time in the form of a string.
It shall be equivalent to:
asctime(localtime(clock))
From the POSIX documentation of asctime:
The asctime() function shall convert the broken-down time
in the structure pointed to by timeptr into a string in the
form:
Sun Sep 16 01:03:52 1973\n\0
We need to get rid of that new line otherwise it appears in the log.
The POSIX documentation goes on:
These functions are included only for compatibility with older
implementations. They have undefined behavior if the resulting
string would be too long, so the use of these functions should
be discouraged. On implementations that do not detect output
string length overflow, it is possible to overflow the output
buffers in such a way as to cause applications to fail, or
possible system security violations. Also, these functions do
not support localized date and time formats. To avoid these
problems, applications should use strftime() to generate
strings from broken-down times.
Because we have already been using strftime() with gmtime() elsewhere,
using strftime() with locatime() here makes sense.
The i1On mechanisme we currently use to print dates is non-sensical:
we force the format string to "%a, %d %b %Y %H:%M:%S" which might not
make sense in some locales. We shall fix i10n in a different merge
request or commit.
Daniel Lenski [Tue, 26 Sep 2023 22:29:48 +0000 (15:29 -0700)]
Change default user-agent string to be compatible with newer Cisco servers
See https://gitlab.com/openconnect/openconnect/-/issues/665 for a summary of
this issue.
This implements the simplest reasonable solution to the problem: Just Change
The Default™ UA string.
Short summary: Cisco did something stupidly backwards-incompatible in their
authentication flow. It's hard to tell if it was due to incompetence or due
to malice towards unofficial clients
(https://gitlab.com/openconnect/openconnect/-/issues/635#note_1451782874)
but it doesn't really matter.
If merged, this should fix #544, #593, #602, #618, #635, #657, #662,
and #665.
Daniel Lenski [Sat, 30 Sep 2023 05:02:33 +0000 (22:02 -0700)]
Bugfix GP XML config: always include portal
Ever since 8e7efd51f, the GlobalProtect *portal* has been included in the
newly-written XML config (`<ServerList>`) only if the portal config XML
contained a `<portal-name>` tag.
We should include the portal even if it doesn't have a name for itself.
Daniel Lenski [Fri, 22 Sep 2023 16:54:11 +0000 (09:54 -0700)]
GlobalProtect SAML completion pages sometimes have the SAML fields only in comments
This modifies the fake GP server to have a 'saml_comments_only' option. If
set, the SAML completion fields ('saml-username', 'prelogin-cookie', etc.)
will be sent to the client *only* in a blob of XML wrapped in HTML comments,
and *not* in HTTP headers.
Some real GP servers are known to behave like this, and authentication
handlers like 'gp-saml-gui' need to be able to handle this case correctly
(see https://github.com/dlenski/gp-saml-gui/issues/51 and
https://github.com/dlenski/gp-saml-gui/pull/59).
Some GlobalProtect servers complain about old versions of the client
software connecting to them.
In the case of a connection via the GlobalProtect "portal" interface,
we capture the preferred software version from the portal and parrot it back,
as of https://gitlab.com/openconnect/openconnect/-/commit/c0d2daeaa85f69ed2f89330a53d97ae7eafdffb1?merge_request_iid=333.
However, we should update the GlobalProtect software version used as a fallback
in the case of a direct connection to the "gateway" interface.
Signed-off-by: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Signed-off-by: Daniel Lenski <dlenski@amazon.com>
Rahul Rameshbabu [Sat, 19 Aug 2023 22:39:12 +0000 (15:39 -0700)]
Support --external-browser flag on _WIN32 systems
When external browser support for AnyConnect was added to _WIN32 platforms
in commit d4fc4b084748, the relevant flag in the CLI was not enabled for
the platform.
Daniel Lenski [Tue, 22 Aug 2023 19:02:19 +0000 (12:02 -0700)]
Shim for renaming of GNUTLS_NO_EXTENSIONS in GnuTLS v3.8.1
The constant `GNUTLS_NO_EXTENSIONS` was renamed in
https://gitlab.com/gnutls/gnutls/-/commit/a7c4a04e (released in v3.8.1), and
then a backwards-compatibility shim was belatedly added in
https://gitlab.com/gnutls/gnutls/-/commit/abfa8634, which has not yet been
released.
We need to re-add the constant ourselves in order to build correctly with
GnuTLS v3.8.1. This should fix
https://gitlab.com/openconnect/openconnect/-/issues/650.
Audric Schiltknecht [Wed, 2 Aug 2023 15:15:50 +0000 (15:15 +0000)]
Fix invalid reset of URL variable in csd-wrapper
The URL variable is constructed from the CSD_HOSTNAME at the beginning of
the script. However, prior to parsing the command line, it was reset to
an empty value.
[DRL: This bug has existed since
https://gitlab.com/openconnect/openconnect/-/commit/cb83e535213ff2132643d2a68c50abc294b43b82,
when I modified the `csd-wrapper.sh` script to parse its `-url` command-line
argument, but forgot to remove the subsequent line `URL=`.]
Daniel Lenski [Wed, 26 Jul 2023 20:41:15 +0000 (16:41 -0400)]
Request help with the interpretation of F5 URIs in the docs
Some F5 VPNs use these to complete authentication and handoff to the
proprietary client, and we currently don't know how to interpret them in a
way that would allow OpenConnect to be used instead.
See https://gitlab.com/openconnect/openconnect/-/issues/639 and
https://lists.infradead.org/pipermail/openconnect-devel/2021-August/005035.html
for further discussion.
David Woodhouse [Tue, 25 Jul 2023 22:13:03 +0000 (23:13 +0100)]
Fix changelog entry for Pulse OS reporting
This was added under v9.12 instead of the HEAD section. Next person to do
that gets to implement a CI test for it :)
Perhaps we should have a policy of adding in reverse chronological order
so that newly-added lines are always immediately below the 'HEAD' title,
which would mean that merging older PRs would *conflict* instead of
silently merging into the older changelog?
Fixes: ff86be7281 ("update changelog") Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Sun, 23 Jul 2023 17:18:09 +0000 (13:18 -0400)]
Fix juniper-auth test
In 57160c9f2673adbbe468db137b28da4187549061, I updated
fake-juniper-server.py to use a "persistent" configuration (as already done
for fake GlobalProtect, Fortinet, F5 servers), but thne I somehow forgot to
update the actual juniper-auth test script accordingly.
Daniel Lenski [Wed, 19 Jul 2023 14:41:16 +0000 (07:41 -0700)]
Replace broken link with Wayback Machine link
The article "Why TCP Over TCP Is A Bad Idea" is very useful for explaining
why VPNs perform better when using UDP-based transport (DTLS or ESP) rather
than TCP-based transport (TLS), but unfortunately the original site is no
longer available.
Replace it with a link to the Internet Archive's Waback Machine, specifically
https://web.archive.org/web/20230228035749/http://sites.inka.de/~W1011/devel/tcp-tcp.html
Dimitri Papadopoulos [Sat, 20 May 2023 12:10:39 +0000 (14:10 +0200)]
Update supported protocols
* Standardise on Array Networks, not Array Networks AG
From https://arraynetworks.com/ssl-vpn/:
> Array SSL VPN gateways provide secure remote access to
> applications, desktops, file shares, networks, and Web
> sites from a broad range of remote and mobile devices.
> Deployed at the network perimeter or in front of
> business-critical resources, the AG provides secure
> remote access for employees, guests, partners, and
> other communities of interest. SSL VPNs are ideal for
> simplifying the user experience while reducing potential
> attack vectors.
>
> Every AG SSL VPN provides a complete secure access
> feature set, including TLS encrypted connectivity,
> device validation, endpoint and server-side security,
> advanced AAA, and granular policy controls. Available
> as physical or virtual appliances, or on your choice
> of public cloud, the AG Series is ideal for businesses
> needing enterprise-wide remote access, and for cloud
> service providers needing flexible remote access to
> meet broad ranging customer requirements.
I think AG refers to the gateway series that support SSL VPN,
not to the protocol.
* PAN → Palo Alto Networks
End-users may not know of this abbreviation, which is not
used in the documentation and marketing material.
* Add Ivanti to Pulse Connect Secure
* List these protocols separately:
- Juniper Network Connect
- Pulse/Ivanti Connect Secure
Daniel Lenski [Tue, 13 Jun 2023 19:10:33 +0000 (12:10 -0700)]
OpenConnect should report the client operating system to Pulse servers
We already know from a MITM capture on Windows how and where this is
reported by the official clients.
As seen with other protocols, some Pulse VPN servers may rely on the
presence of OS information in order to respond with a complete and correct
main configuration packet (see possible cases of this requirement in
https://gitlab.com/openconnect/openconnect/-/issues/459).
Daniel Lenski [Fri, 26 May 2023 19:39:33 +0000 (12:39 -0700)]
Handle Pulse main config packets up to 1 MiB
Our implementation has assumed that the entirety of the main Pulse
configuration “packet” will fit in one TLS record; however,
https://gitlab.com/openconnect/openconnect/-/issues/617 demonstrates that it
can in fact exceed 16 KiB if it includes e.g. a large proxy configuration.
In order to handle this, we need to dynamically allocate the space to hold
this packet, and read it in a loop.
(See https://gitlab.com/openconnect/openconnect/-/commit/2d77040a870851a625de16938fcdda6a5494d7ed
for a previous case where a configuration packet unexpectedly exceeded the
limits of a single TLS record.)
Daniel Lenski [Fri, 2 Jun 2023 21:48:32 +0000 (14:48 -0700)]
Log attributes for proxy auto-config (PAC) in Pulse configuration
Per https://gitlab.com/openconnect/openconnect/-/issues/617#note_1413539553,
Pulse servers may send proxy auto-config information
(https://en.wikipedia.org/wiki/Proxy_auto-config) in two forms
in the main configuration packet:
- attr 0x4023 contains a URL where the PAC file can be downloaded
- attr 0x4009 contains the full contents of the PAC file (may
be very large)
Daniel Lenski [Fri, 30 Jun 2023 20:50:33 +0000 (13:50 -0700)]
CI: Allow Android jobs to fail (error → warning)
Until we figure out how to make these reliable, they're preventing automatic
merging of several MRs. Android is decidedly a third- or fourth-class
platform in terms of OpenConnect developers' ability and willingness to
support it.
Daniel Lenski [Thu, 20 Apr 2023 21:18:25 +0000 (14:18 -0700)]
Stricter chunked-encoding error detection
The only acceptable inputs for an HTTP chunk length/header line are
non-negative hexadecimal integers followed immediately by EOL, or followed
by `;`, then followed by chunk extensions which we ignore.
We should prevent anything other than these from being tacitly accepted as
equivalent to a length of 0, which indicates the last chunk.
Improvements in the error handling of chunked Transfer-Encoding responses
were discussed in https://gitlab.com/openconnect/openconnect/-/issues/597.
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>
projects / users / dwmw2 / openconnect.git / log