Daniel Lenski [Thu, 3 Feb 2022 22:12:17 +0000 (14:12 -0800)]
In dumb_socketpair(), delete Unix-domain socket path once no longer needed
Small follow-up improvement to
https://gitlab.com/openconnect/openconnect/-/merge_requests/320, which made
dumb_socketpair() able to use Unix-domain sockets, on those Windows versions
that support them albeit only with named paths.
This was suggested as a way to prevent the Windows dumb_socketpair()
implementation from leaving behind size-0 files, even if normally only in
temporary directories.
See original suggestion at
https://github.com/microsoft/WSL/issues/4240#issuecomment-1027607891.
Luca Boccassi [Wed, 18 Mar 2020 16:36:25 +0000 (12:36 -0400)]
libopenconnect: add public interface stubs for SAML support
The SAML support is still work in progress and not merged yet.
Start adding the new public API to libopenconnect, so that
distributions can choose to ship with out-of-tree support to
let their users have the feature without breaking ABI
compatibility.
Signed-off-by: Luca Boccassi <luca.boccassi@microsoft.com> Co-authored-by: Steven Walter <steven@stevenwalter.org>
Daniel Lenski [Wed, 1 Sep 2021 00:49:27 +0000 (17:49 -0700)]
Clarify Fortinet no-valid-cookie error paths
When requesting connection options in XML format…
1. A redirect indicates invalid cookie only if it is to /remote/login
2. A 403 followed by a fetch of the HTML format connection options
indicates an ancient FortiOS version only if the HTML fetch is actually
*successful* (200)
Daniel Lenski [Tue, 31 Aug 2021 23:44:52 +0000 (16:44 -0700)]
Print warning if Fortinet server doesn't indicate support/no-support for reconnect-after-drop
FortiGate v6.2.1 and newer appear to support reconnect-after-drop without
reauth, but only if the tag and attribute
'<auth-ses tun-connect-without-reauth="1">' are present in the config. As
of https://gitlab.com/openconnect/openconnect/-/merge_requests/292, we print
and act on this information.
We should also request feedback from users of Fortinet VPNs which don't
explicitly advertise either allowing or disallowing it.
See discussion at
https://gitlab.com/openconnect/openconnect/-/issues/297#note_664686767
Also, print the 'mr_num' field which is apparently part of some newer
Fortinet servers' version information.
Daniel Lenski [Mon, 30 Aug 2021 04:41:12 +0000 (21:41 -0700)]
Print Pulse server's IPv6 internal gateway address (in addition to Legacy IP)
The existence of this attribute was noted in
https://gitlab.com/openconnect/openconnect/-/issues/254#note_595455571. As
with its Legacy IP equivalent, an IPv6 "gateway" address is superfluous and
unnecessary for a tunnel connection.
Known Pulse servers consistently send an IPv6 internal gateway address which
falls in the fc00::/7 range of "unique local addresses"
(https://en.wikipedia.org/wiki/Unique_local_address), as in this example:
$ openconnect -vv pulse.vpn.com
...
Received internal Legacy IP address 10.200.200.2
Received netmask 255.255.255.255
Received internal gateway address 10.200.200.200 (Legacy IP)
Received internal IPv6 address 2001:abc:123:4::567/128
Received internal gateway IPv6 address fd00::ac8:c8c8 (IPv6)
Received IPv6 split include ::/0
This patch also *renames* the 'cstp_options' entry for the Legacy IP gateway
address (was 'ipaddr', changed to 'gateway') so that any front-end which
uses this list won't confuse it with the VPN interface's assigned Legacy IP
address (which is named 'ipaddr' for all currently-supported protocols).
Dimitri Papadopoulos [Sat, 25 Dec 2021 21:28:54 +0000 (22:28 +0100)]
Print detailed error information when opening cmd pipe/socketpair fails
Retrieve and print detailed information using GetLastError() and
strerror().
This should be more useful to end-users than the current message (simply
“Error opening cmd pipe”) and might have helped us to solve
https://gitlab.com/openconnect/openconnect/-/issues/228 more quickly.
Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
David Overton [Fri, 28 Jan 2022 23:35:49 +0000 (23:35 +0000)]
Pulse: handle 0x2e20f000 main configuration packet
This packet type was received upon attempting to connect to a
Pulseb 9.1R14 server (with IPv6 enabled, though this may not
be relevant).
Upon receiving this packet we previously bailed out and failed
back to the user with:
Unexpected IF-T/TLS packet when expecting configuration
The "new" config packet packs what appears to be a second
attributes section in front of the legacy routing block. It
is not yet clear what the single example attribute seen so
far (0x4025) is for (perhaps it is to indicate the presence
or absence of the legacy routing block?).
We now have two other reports that this fixes
https://gitlab.com/openconnect/openconnect/-/issues/379, allowing other
users to connect to Pulse 9.1R14 servers as well:
Daniel Lenski [Mon, 7 Feb 2022 02:01:41 +0000 (18:01 -0800)]
Fix memory leak in pulse.c
This issue was hidden by the oversight of Legacy IPv4 split routes in 3d845bc9b, which was subsequently fixed in
https://gitlab.com/openconnect/openconnect/-/merge_requests/330 / 52d1c674.
David Overton [Fri, 28 Jan 2022 22:49:46 +0000 (22:49 +0000)]
Bugfix Legacy IP split include/exclude routes for Pulse
In 3d845bc9b, routing configuration was modified to use the `new_ip_info`
and `install_vpn_opts()`. Pulse IPv6 split include/exclude handling were
modified accordingly in that commit, but Legacy IP split include/exclude
routes were overlooked.
Since `install_vpn_opts()` clobbers the split include/exclude routes, this
means Legacy IP split routes for Pulse have been ignored since then.
Dimitri Papadopoulos [Sat, 29 Jan 2022 16:35:27 +0000 (17:35 +0100)]
Add jq as a build dependency to fix COPR builds
OpenConnect is cross-compiled directly from Fedora in COPR builds, so
this build dependency is a native Fedora package.
The dependency on 'jq' was introduced in b8f79ce9, and the resulting
brokenness of the COPR builds was noted in
https://gitlab.com/openconnect/openconnect/-/issues/286#note_825611200
Daniel Lenski [Sat, 22 Jan 2022 23:36:20 +0000 (15:36 -0800)]
Add new documentation on how to observe/MITM VPN clients
Put this in a new subsection of "Contributing". This is based on
https://gitlab.com/openconnect/openconnect/-/issues/246#note_811153868, and
other recent requests for help MITM'ing VPN clients.
Daniel Lenski [Sun, 23 Jan 2022 02:02:38 +0000 (18:02 -0800)]
The GitLab repo is more than an "experiment" at this point
We're using it for everything from bug reports, to new code contributions,
to CI for automated building of Windows installers. Make the docs reflect
that!
Daniel Lenski [Sat, 22 Jan 2022 23:08:07 +0000 (15:08 -0800)]
Update "Contributing" docs
1. Testing: We understand GlobalProtect IPv6 very well now. No immediate need for
more testers.
2. New protocols: Reference the MR with working code for at least some
modes of CheckPoint.
3. Help needed: We *do* need a bunch of help with external auth/SAML/SSO.
1. Nontext files, those where '-text' is explicitly set in .gitattributes
2. LICENSE/COPYING files (we never modify these)
3. '.po' files (not hand-edited)
Daniel Lenski [Fri, 21 Jan 2022 16:52:57 +0000 (08:52 -0800)]
Change library ordering when testing for library availability with autoconf
As discussed in #371, the order in which libraries must be listed on the
compiler command-line has changed in recent versions of GCC, in order for
linking to succeed. This is because the `--as-needed` option has become
enabled by default:
https://sigquit.wordpress.com/2011/02/16/why-asneeded-doesnt-work-as-expected-for-your-libraries-on-your-autotools-project
A succinct explanation of the required changes in library-ordering from
https://wiki.ubuntu.com/NattyNarwhal/ToolchainTransition by way of
https://stackoverflow.com/a/69795683:
> "The --as-needed option also makes the linker sensitive to the ordering
> of libraries on the command-line. You may need to move some libraries
> later in the command-line, so they come after other libraries or files
> that require symbols from them." [3]
It appears that both Debian-based and Fedora-based distributions are moving
towards this as the new default behavior:
Daniel Lenski [Wed, 19 Jan 2022 02:17:58 +0000 (18:17 -0800)]
Fix changelog links/labels
- A few merge-request links (`!123`) and issue links (`#456`) were confused,
in that they pointed to the wrong URL, or had the wrong label.
- Reference specific commits, or ranges of commits, where there was no
MR or issue.
- Consistently use the human-friendly searchable term Juniper, rather than
the jargon-y oNCP/NC, in the changelog.
Dimitri Papadopoulos [Thu, 23 Dec 2021 09:58:20 +0000 (10:58 +0100)]
Option --version prints default script location
Add a function to print the default location of the VPNC-compatible
script. Print this default location also with option --version, not
only with option --help.
Tim De Baets [Wed, 12 Jan 2022 17:20:09 +0000 (18:20 +0100)]
Issue OC_CMD_DETACH instead of OC_CMD_CANCEL on Ctrl+Break
If the user terminates OpenConnect with Ctrl+C, the session will be
logged off (OC_CMD_CANCEL), and with Ctrl+Break the session will NOT
be logged off (OC_CMD_DETACH). Either way, the vpnc-script
will still be invoked with reason=disconnect.
Signed-off-by: Tim De Baets <10608063-tdebaets@users.noreply.gitlab.com>
Tim De Baets [Tue, 11 Jan 2022 17:40:28 +0000 (18:40 +0100)]
Install a custom signal handler on Windows using SetConsoleCtrlHandler()
This fixes the longstanding bug
https://gitlab.com/openconnect/openconnect/-/issues/362, wherein the
vpnc-script never gets called to do any routing cleanup on Windows.
Also added checking for the number of characters returned by ReadConsole() so
that we still exit when receiving a control signal while waiting for user
input.
Signed-off-by: Tim De Baets <10608063-tdebaets@users.noreply.gitlab.com>
Daniel Lenski [Mon, 3 Jan 2022 16:23:18 +0000 (11:23 -0500)]
dumb_socketpair(): fallback from AF_UNIX to AF_INET if AF_UNIX fails
1) If bind() fails with an AF_UNIX socket, we should retry with
AF_INET socket
Because we have to used named paths for AF_UNIX sockets on Windows, a
likely point of failure is that the temporary directory in
nonexistent/non-writable, or even that we somehow have a collision in the
filename.
2) If any of the other AF_UNIX operations (listen, socket, connect, accept)
fail, we might as well also retry with AF_INET.
We don't know of any expected points-of-failure, but all indications are
that AF_UNIX support in Windows is incomplete, undocumented, and
potentially buggy.
Daniel Lenski [Sat, 1 Jan 2022 03:22:54 +0000 (22:22 -0500)]
dumb_socketpair(): generate named socket path more carefully
Windows forces us to use named-path Unix sockets. Generating a path in the
temporary directory, combining current high-res time and PID, seems like a
less-bad option.
On GitHub, a commenter
[suggested](https://github.com/microsoft/WSL/issues/4240#issuecomment-1010545442)
that it would be better to use
[GetTempFileName](https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettempfilenamea)
here. However, that function:
- Only adds 16 bits of time-based random bits,
- Will currently fail if there aren't 14 characters available for the filename,
- Might conceivably generate paths longer than UNIX_PATH_MAX, and
- Offers no other apparent offsetting advantages
Daniel Lenski [Fri, 31 Dec 2021 17:25:43 +0000 (12:25 -0500)]
dumb_socketpair(): try to use AF_UNIX socketpair on Windows 10 and newer
As a workaround for the lack of socketpair() on Windows, we use
dumb_socketpair() from https://github.com/ncm/selectable-socketpair/blob/master/socketpair.c,
which uses a pair of IPv4 local sockets (listening on 127.0.0.1).
Unfortunately, and maddeningly, it's possible for the local IPv4 routes
(127.0.0.0/8) to be deleted on Windows; this will prevent dumb_socketpair()
from working in its current form.
See https://gitlab.com/openconnect/openconnect/-/issues/228 and
https://gitlab.com/openconnect/openconnect/-/issues/361 for examples of how
to trigger this condition. The simplest way to do it is with `route /f`.
Fortunately, Windows 10+ supports AF_UNIX sockets, which we should be able
to use to sidestep this issue.
This feature was announced in December 2017 in
https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows.
It is evidently incomplete, and also buggy:
1) "Abstract" sockets don't actually seem to work, and this is probably why
the socketpair() function still isn't implemented, even though AF_UNIX
support would naturally enable it:
https://github.com/microsoft/WSL/issues/4240#issuecomment-506437851
2) Actual MSDN documentation for this feature is seemingly nonexistent.
3) MinGW lacks the expected <afunix.h> header, but other FLOSS projects show
how to embed the needed `struct sockaddr_un` definition:
- https://github.com/MisterDA/ocaml/commit/5855ce5ffd931a2802d5b9a5b2987ab0b276fd0a
- https://github.com/curl/curl/blob/curl-7_74_0/lib/config-win32.h#L725-L734
Nevertheless, it works well enough that we can use it in OpenConnect. The
modified version of dumb_socketpair() in this patch tries to create an AF_UNIX
socketpair, and only uses IPv4 local sockets as a fallback. With this modified
version, I confirm that I can do the following on Windows 10:
1) Nuke routes with `route /f`.
2) Reconnect network adapter via GUI.
3) Confirm that IPv4 local route (127.0.0.0/8) still hasn't been recreated.
4) Run OpenConnect and successfully create the cmd pipe.
So this appears to fix https://gitlab.com/openconnect/openconnect/-/issues/228 and
https://gitlab.com/openconnect/openconnect/-/issues/361, at least on Windows 10
and newer.
Dimitri Papadopoulos [Tue, 23 Nov 2021 09:56:32 +0000 (10:56 +0100)]
Windows: fix instability with Wintun as tunnel device driver
In https://gitlab.com/openconnect/openconnect/-/issues/338, multiple users
reported that connections using Wintun as the tunnel device driver become
non-functional after 20-30 minutes of operation, without
any message from OpenConnect at all.
We analyzed this issue as follows:
1. There was an off-by-one error in the check of outgoing packet size
against the tunnel device's MTU (`tun_len < pkt->len`). Outgoing packets
of exactly the MTU size would be considered errors and silently discarded
by OpenConnect.
2. However, OpenConnect failed to instruct the driver to release these
discarded packets. They would accumulate in the Wintun driver buffer and
probably cause an out-of-memory condition, eventually freezing the
driver.
We fixed the issued as follows:
1. Fix the off-by-one error, changing to `tun_len <= pkt->len`.
2. Always release outgoing packets, even if discarded.
3. Print extended debugging messages when receiving/sending packets. Such
messages would have helped us diagnose the error much sooner.
Developers and users have confirmed that, with these changes, Wintun
connections run stably for at least 60 minutes. (See
https://gitlab.com/openconnect/openconnect/-/merge_requests/306#note_745393731).
Also, in
https://gitlab.com/openconnect/openconnect/-/merge_requests/300?commit_id=b5ff6f3fb1b8d06cf56426b13c7af96e25cd922b,
we reverted to TAP-Windows as the default driver on Windows due to the
aforementioned stability issues. Although Wintun connections now appear to
be stable, we are not quite ready yet to un/re-revert, and make Wintun the
default driver again.
Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
Dimitri Papadopoulos [Tue, 23 Nov 2021 16:08:37 +0000 (17:08 +0100)]
Follow Wintun example to the letter (versions 0.10.2 or 0.13)
* Call WintunFreeAdapter() after WintunDeleteAdapter()
* Optional argument of WintunFreeAdapter() argument can be NULL
* Call WintunGetRunningDriverVersion() before WintunStartSession()
This does not actually fix anything. However, by following the Wintun
example to the letter, we make sure there are no other hidden isues.
Daniel Lenski [Wed, 15 Dec 2021 01:06:50 +0000 (17:06 -0800)]
When running on Windows, the default OS value should be 'win'
This makes the behavior on Windows consistent with other platforms.
Also attempt to detect iOS, and set the default OS value to 'apple-ios'
there. As far as we know, no one has built LibOpenConnect on iOS (see
https://gitlab.com/openconnect/openconnect/-/issues/163), but this should
help future-proof it.
Daniel Lenski [Wed, 15 Dec 2021 00:54:34 +0000 (16:54 -0800)]
openconnect_set_reported_os should reject illegal values
The OS values ('vpninfo->platname') that OpenConnect accepts are
historically derived from the Cisco AnyConnect protocol:
linux, linux-64, win, mac-intel, android, apple-ios.
In the Cisco AnyConnect protocol, the platname was sometimes sent verbatim
on the wire (see auth.c), and could perhaps be thought of as a potentially
free-form string.
However:
(a) The platname value is now used in other protocols, requiring lookups to
convert to the correct forms for those protocols (e.g., see
auth-globalprotect.c).
(b) Even in the Cisco AnyConnect protocol, the platname value has
has to be handled with switch/if statements which can only correctly
handle the limited set of known values.
Since only a limited set of values are actually *understood* by OpenConnect,
allowing arbitrary values to be provided—but silently ignored or
mishandled—leads to confusing errors, such as in
https://lists.infradead.org/pipermail/openconnect-devel/2021-December/005079.html.
In that case, a user specified '--os=windows' (incorrect) instead of
'--os=win' (correct). This likely led to incorrect server behavior, but
OpenConnect silently accepted the incorrect value.
This patch modifies openconnect_set_reported_os to return -EINVAL if
any OS name other than one of the 6 legal values is specified.
Future improvements:
1. Replace the numerous repetitions of the literal OS values with something
saner, like an enum type.
2. Consider retiring the Cisco-specific values altogether as part of the
"Great Renaming"
(https://gitlab.com/openconnect/openconnect/-/merge_requests/151).
Dimitri Papadopoulos [Wed, 8 Dec 2021 08:21:06 +0000 (09:21 +0100)]
Load wintun.dll from the application directory only
Do not attempt to load it from the System32 directory.
Different versions of `wintun.dll` or `wintun.sys` float around in system
directories. In my case, a `C:\Windows\System32\wintun.sys` had been left
behind for some reason, and was being loaded at startup, taking precedence
over the `wintun.dll` bundled with OpenConnect. Unfortunately, different
versions are not compatible, at least not entirely, while OpenConnect is
being tested with the bundled `wintun.dll` only.
To avoid this DLL hell, we shall load exclusively the bundled version of
`wintun.dll` from the application directory, and disregard any `wintun.dll`
or `wintun.sys` installed in system directories by other software.
Dimitri Papadopoulos [Wed, 1 Dec 2021 16:43:55 +0000 (17:43 +0100)]
Fix Windows installer so that it uninstalls cleanly
The "Online Documentation.url" web-link was added in
https://gitlab.com/openconnect/openconnect/-/commit/44c8038707c06897588aa75ef02a38793387b86e.
It needs to be deleted upon uninstallation.
Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com> Signed-off-by: Daniel Lenski <dlenski@gmail.com>
Dimitri Papadopoulos [Sat, 28 Aug 2021 06:08:08 +0000 (09:08 +0300)]
AC_PROG_LIBTOOL → LT_INIT
Run autopudate on configure.ac.
According to the Libtool manual, AC_PROG_LIBTOOL and AM_PROG_LIBTOOL are
deprecated names for older versions of LT_INIT:
https://www.gnu.org/software/libtool/manual/html_node/LT_005fINIT.html
Dimitri Papadopoulos [Sun, 8 Aug 2021 09:45:06 +0000 (11:45 +0200)]
AC_LANG_C → AC_LANG([C])
Run autopudate on configure.ac.
According to the Autoconf 2.69 manual, AC_LANG_C is obsolete:
https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Obsolete-Macros.html
Dimitri Papadopoulos [Sun, 8 Aug 2021 08:03:51 +0000 (10:03 +0200)]
AC_CONFIG_MACRO_DIRS
According to the Automake manual, we should declare the 'm4' directory
explicitly:
https://www.gnu.org/software/automake/manual/html_node/Local-Macros.html
André Draszik [Tue, 23 Nov 2021 12:16:43 +0000 (12:16 +0000)]
csd-wrapper: make it work again if binaries are compressed
CSD binaries can be uncompressed or .gz compressed, and the
csd-wrapper.sh script here tries to support both cases,
but actually doesn't work in the compressed case.
It just ends up specifying incorrect URLs and destination
file names, making this miserably fail:
+ curl -s -k https://xx.xxx.xxx.xx/CACHE/sdesktop/hostscan/linux_x64/cscan -o /home/xxx/.cisco/hostscan/bin/cscan.tmp
+ [[ ! -f /home/xxx/.cisco/hostscan/bin/cscan.tmp ]]
+ [[ ! -s /home/xxx/.cisco/hostscan/bin/cscan.tmp ]]
+ [[ ! -s /home/xxx/.cisco/hostscan/bin/cscan.tmp ]]
+ rm /home/xxx/.cisco/hostscan/bin/cscan.tmp
+ echo 'Failure on cscan, trying gz'
Failure on cscan, trying gz
+ FILE_GZ=/home/xxx/.cisco/hostscan/bin/cscan.tmp.gz
+ curl -s -k --pinnedpubkey sha256//vI158z4H4BLBZKv927uWmvsJbFZzGEilTkI36lKv5BM= https://xx.xxx.xxx.xx/CACHE/sdesktop/hostscan/linux_x64//home/xxx/.cisco/hostscan/bin/cscan.tmp.gz -o /home/xxx/.cisco/hostscan/bin/cscan.tmp.gz
+ gunzip --verbose --decompress /home/xxx/.cisco/hostscan/bin/cscan.tmp.gz
gzip: /home/xxx/.cisco/hostscan/bin/cscan.tmp.gz: unexpected end of file
As can be seen the 2nd curl call has my local path appended to the
URL, which of course can not work ($TMPFILE contains the full local
path).
Fixes: 9da32db08ba4 ("Clean up csd-wrapper.sh") Signed-off-by: André Draszik <git@andred.net>
Daniel Lenski [Sat, 20 Nov 2021 21:12:34 +0000 (13:12 -0800)]
Refuse to handle forms without ->auth_id (but do it in the right place, and noisily)
In 0b47ea1882346fdedfcd8a315f51aeb39e13459e ("Refuse to handle forms without
->auth_id"), the process_auth_form_cb for the OpenConnect CLI was modified
to silently reject forms with auth_id unset.
Issues with this:
1. If a form doesn't have its auth_id set, it'll fail *silently*, which
makes it confusingly difficult to identify the root cause. (See #351.)
2. As that commit message says, GUIs/front-ends need the auth_id to be set,
but it didn't do anything to enforce it other than for the CLI.
The solution is to reject forms with auth_id unset in process_auth_form()
itself, rather than expecting the front-ends’ callback functions to check
this, and to do so with an error message explaining that this is a bug in
OpenConnect.
Daniel Lenski [Fri, 19 Nov 2021 19:26:59 +0000 (11:26 -0800)]
Bugfix F5 'plain' login form
Fixes #351.
It turns out that the "plain auth form" for F5 has been broken since 0b47ea1882346fdedfcd8a315f51aeb39e13459e. (This form is used as a stand-in
when the server fails to provide a real HTML login form, and only gives a
JavaScript mess that dynamically creates a form.)
DW probably didn't notice this when making that commit, because a07bbbcd711c6fc4ef20fd9719e0e5da40f141f9 had previously fixed the other
places where a form with no `auth_id` could be created.
This ensures that the "plain auth form" has its `auth_id` set, and adds test
coverage of this form to `f5-auth-and-config`.
Daniel Lenski [Mon, 15 Nov 2021 05:27:12 +0000 (21:27 -0800)]
The option '--force-dpd' should be followed even if the server specifies a lesser DPD interval
The `openconnect_set_dpd()` API function, and the command-line option
`--force-dpd=SEC`, specify an exact DPD (Dead Peer Detection) interval for
the client, which is intended to override the server-specified value.
However, several protocols
would simply *ignore* the specified interval, and accept the
server's interval, if the server's interval was *less* than the one
specified via API or command-line.
- AnyConnect/ocserv has behaved this way since the `--force-dpd` option was
first added in 2012 (see 54784befa190a670a7c33661e505415eaaab8dd1)
- ESP support, added for oNCP/Pulse, copied this behavior
- Fortinet copied this behavior
- Array copied this behavior
This behavior can cause problems for protocols where DPD, or reconnection in
general, is broken, and where a user wishes to specify a long interval (e.g.
`--force-dpd=99999`) to effectively disable DPD. Specifically, Fortinet is
affected by this; see
https://gitlab.com/openconnect/openconnect/-/issues/334#note_732774445.
The straightforward solution here is just to make the `--force-dpd` option
do what it says, and set the client's DPD interval unconditionally.
Daniel Lenski [Wed, 13 Oct 2021 01:25:40 +0000 (18:25 -0700)]
Juniper/NC ESP rekey fix
This should fix https://gitlab.com/openconnect/openconnect/-/issues/322.
This was an immensely tricky issue to identify and solve. Many thanks to
@john508 for his patient logging, testing, and bisecting.
This came down to a subtle combination of 2 problems, described further in
https://gitlab.com/openconnect/openconnect/-/issues/322#note_702122197.
First, a bug:
The code for Juniper/Pulse 'dontsend' added in
https://gitlab.com/openconnect/openconnect/-/commit/b4f50f8bd5da7e6ac926ddd5095501edbc204cd0
caused IPv6 packets to get sent over the Juniper oNCP/TLS channel in a
completely mangled form, thereby confusing the server and making it not
respond to subsequent ESP rekey packets sent over the oNCP/TLS channel.
The fix is to use this code path *only* for the Pulse protocol, not for the
Juniper protocol. Juniper cannot do ESP-over-IPv6 at all, and cannot send
tunneled IPv6 packets at all (neither via ESP-over-IPv4, nor via oNCP/TLS).
Second, we introduced a subtle regression against a Juniper server behavior
which we weren't previously aware of:
Juniper servers apparently don't always resend the IP address in a
configuration packet, if that configuration packet is for
rekey/reconnection only.
To address this, we need to add a special case for Juniper to the
install_vpn_opts() function, which was added in
https://gitlab.com/openconnect/openconnect/-/commit/3d845bc9bf62b3e816e0d2fd72970ef33964a191.
Daniel Lenski [Sat, 13 Nov 2021 18:35:36 +0000 (10:35 -0800)]
Fix/update comments in fake-*-server.py scripts
Also adds a stub to make sure that 'fake-juniper-server.py' rejects attempts
to start speaking the Junos/Pulse protocol, rather than the Juniper/oNCP
protocol.
Daniel Lenski [Fri, 12 Nov 2021 01:16:01 +0000 (17:16 -0800)]
Re-add TAP-Windows driver to installer, and update docs to reference its inclusion
This mostly reverts commit 8c1d2de09829d0930d1a7642b0729cd6cbf5ab67 ("Remove
TAP-Windows driver from installer, and update docs to reference Wintun's
default inclusion").
It appears that Wintun is not fully stable yet for OpenConnect VPN connections.
See #338 for two reports of instability (with #324 as a possible third), including
one where switching to TAP-Windows clearly resolved the issue.
Per https://gitlab.com/openconnect/openconnect/-/issues/338#note_731175712,
we should revert to installing and using TAP-Windows by default until we can figure out why.
OpenConnect will continue to use Wintun if TAP-Windows is unavailable, but will
issue a loud warning to the log.
Daniel Lenski [Thu, 21 Oct 2021 00:48:12 +0000 (17:48 -0700)]
Add support for Fortinet's HTML-type multi-factor authentication
Fortinet is known to support *two* different types of MFA/2FA challenges
(see details in
https://gitlab.com/openconnect/openconnect/-/issues/332#note_709413011):
1. HTTP 200 response to initial login with plain-text body
(`ret=...,reqid=...,polid=...,...tokeninfo=,chal_msg=...`) containing
prompt and fields to be parroted back.
2. HTTP 401 response to initial login with HTML body containing a "normal"
HTML form to be filled out and sent back.
Daniel Lenski [Mon, 18 Oct 2021 03:03:23 +0000 (20:03 -0700)]
Enable Fortinet DPD even if server doesn't say that reconnect-after-drop is allowed
Even if we can't automatically reconnect, disabling DPD doesn't seem like a
good idea. This will just recapitulate the poor behavior of official
Fortinet clients, which don't provide useful signals about dropped
connection.
Instead, let's just tell users explicitly that reconnect probably won't work
in the logging messages.
projects / users / dwmw2 / openconnect.git / log