Merge branch 'windows_ctrl_signal_handler' into 'master'

Handle Ctrl+C (and related) signals on Windows for proper disconnect and cleanup

See merge request openconnect/openconnect!323

Update changelog

As requested during merge request review.

Signed-off-by: Tim De Baets <10608063-tdebaets@users.noreply.gitlab.com>

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>

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>

Merge branch 'Windows_10_has_AF_UNIX_socket' into 'master'

dumb_socketpair(): try to use AF_UNIX socketpair on Windows 10 and newer

Closes #228

See merge request openconnect/openconnect!320

Update changelog

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

dumb_socketpair(): Try a whole series of plausible temporary/writable directories for AF_UNIX sockets

This is probably trying too hard.  Discussed in
https://gitlab.com/openconnect/openconnect/-/merge_requests/320#note_800005154

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Latest version of vendored dumb_socketpair()

These are minor changes, except the return value which really is an
integer error status, not a socket.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

.mailmap update

Pick most-used name and email for those with multiple variants.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'wintun-0.10.2-0.13' into 'master'

Update usage of Wintun, avoid disconnections on Windows

Closes #338

See merge request openconnect/openconnect!306

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>

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.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Merge branch 'reject_bogus_OS_names' into 'master'

openconnect_set_reported_os should reject illegal values

See merge request openconnect/openconnect!310

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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).

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

AC_ERROR → AC_MSG_ERROR

Run autopudate on configure.ac.

According to the Autoconf 2.69 manual, AC_ERROR is obsolete:
https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Obsolete-Macros.html

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix grammar/typos in comments and diagnostic messages

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>
Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'force_dpd_even_if_greater_than_server_interval' into 'master'

The option '--force-dpd' should be followed even if the server specifies a lesser DPD interval

See merge request openconnect/openconnect!301

Merge branch 'm4' into 'master'

Update configure.ac

See merge request openconnect/openconnect!267

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>

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

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

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

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

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

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Merge branch 'csd-wrapper-compressed' into 'master'

csd-wrapper: make it work again if binaries are compressed

See merge request openconnect/openconnect!305

Update changelog

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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>

Merge branch 'fix_F5_plain_auth_form' into 'master'

Bugfix F5 'plain' login form

Closes #351

See merge request openconnect/openconnect!304

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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`.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'repeat' into 'master'

Remove repeated words from documentation

See merge request openconnect/openconnect!302

Remove repeated words from documentation

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Update documentation of --force-dpd to reflect its new behavior

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Add changelog entry

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

If oNCP negotiation response is a redirect, cookie is invalid

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Fix missing protocol flag for Juniper NC

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'revert_to_using_TAPWindows_by_default' into 'master'

Switch back to TAP-Windows as default driver on Windows

See merge request openconnect/openconnect!300

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Avoid code duplication in www/html.py

Ping #342, closes !279.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>
Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'vpn_progress_n' into 'master'

vpn_progress(), vpn_perror() and '\n'

See merge request openconnect/openconnect!287

Merge branch 'discourage_use_of_csd-wrapper.sh' into 'master'

Encourage use of csd-post.sh, and discourage use of csd-wrapper.sh

See merge request openconnect/openconnect!231

Merge branch 'python3' into 'master'

These are Python 3 scripts

See merge request openconnect/openconnect!291

Merge branch 'Fortinet_HTML_form_based_MFA' into 'master'

Add support for Fortinet's HTML-type multi-factor authentication

Closes #332

See merge request openconnect/openconnect!295

Test both tokeninfo- and HTML-based MFA challenges for Fortinet

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

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.

OpenConnect has supported "tokeninfo-type 2FA" (1) since #225 and
426fc3d434ae614b7e10999aff84c52dcffd047a.

This adds support for "HTML-type 2FA" (2), based on the example logs
from #332, and mentions its support in the documentation.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Add flag to allow do_http_request() to return the server response body even on error

Also, rename the argument 'fetch_redirect' to 'flags', and make it a
bitfield with explicit #define'd values.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Update documentation on state of Fortinet reconnects

Inadvertently omitted this from !292.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'refine_Fortinet_reconnect_and_DPD' into 'master'

refine Fortinet reconnect and DPD

See merge request openconnect/openconnect!292

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.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Do request "ancient HTML config" in order to distinguish truly-ancient Fortinet servers from some reconnection problems

This should at least partially address #298.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Attempt to determine whether Fortinet server really supports reconnect-after-drop (without reauth)

If the server doesn't actually support reconnection, then DPD is probably
useless or counterproductive, and we shouldn't enable it.

See discussion here: https://gitlab.com/openconnect/openconnect/-/issues/297#note_664686767

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Add links to latest Windows builds to www/packages.html and README.md

Fixes #286.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Dump initial oNCP negotiation request if --dump-http-traffic is specified

Joachim Kuebart noted the absence of this output in
https://gitlab.com/openconnect/openconnect/-/merge_requests/271#note_654871352

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

These are Python 3 scripts

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Resync translations with sources

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Remove extra '\n' from a vpn_perror() message

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Add missing '\n' to vpn_progress() messages

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

bugfix openconnect__strchrnul function in compat.c

In e6ce815e112415e8a00cf838bcaf9121be3503de, I claimed…

> GNU strchrnul() is trivial to implement

Nevertheless, I got it wrong: there was an off-by-one error in the case
where the sought character is indeed found.

The strchrnul() function is used in protocol-specific code for both GP and
Fortinet. Fixes #318.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'wintun-0.13' into 'master'

Upgrade to Wintun 0.13?

See merge request openconnect/openconnect!264

Annotate vpnc-script-win.js with a header documenting its exact source revision

By annotating the vpnc-script-win.js included in the Windows installers
with information about the exact source revision, we will ease reporting
and reduce confusion about problems resulting from interactions between
OpenConnect and vpnc-script.

Example of the header added to vpnc-script-win.js:

    // This script matches the version found at https://gitlab.com/openconnect/vpnc-scripts/-/blob/b749c2cadc2f32e2efffa69302861f9a7d4a4e5f/vpnc-script-win.js
    // Updated on 2021-09-24 by Daniel Lenski <dlenski@gmail.com> ("Ensure that vpnc-script-win.js works even if INTERNAL_IP4_{NETADDR,NETMASK} are unset")
    //

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Fix missing newline in Windows error message

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'deepsource' into 'master'

Fix DeepSource alerts

See merge request openconnect/openconnect!284

Fix DeepSource alert

Useless inheritance from `object`

The class is inheriting from `object`, which is implicit under Python 3 ,
hence can be safely removed from bases.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix DeepSource alert

Logging is not lazy

The logging statement has the call of the form
logging.(format_string % (format_args...))
For such calls, it is recommended to leave string interpolation to the
logging method itself and be written as
logging.(format_string, format_args...)
so that the program may avoid incurring the cost of the interpolation
in those cases in which no message will be logged. For more details, see
PEP 282.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix DeepSource alert

Consider decorating method with `@staticmethod`

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix DeepSource alert

Function/method with an empty body

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix DeepSource alert

Dangerous default argument

Do not use a mutable like `list` or `dictionary` as a default value to an
argument. Python’s default arguments are evaluated once when the function
is defined. Using a mutable default argument and mutating it will mutate
that object for all future calls to the function as well.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

This is a Python 3 script

Remove Python2 test, this will at the same time remove a DeepSource
false positive about `reload` being an undefined name.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix DeepSource alert

Consider using literal syntax to create the data structure

Using the literal syntax can give minor performance bumps compared to
using function calls to create `dict`, `list` and `tuple`.

This is because here, the name `dict` must be looked up in the global
scope in case it has been rebound. Same goes for the other two types
`list()` and `tuple()`.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix DeepSource alert

Built-in function `len` used as condition

Using the `len` function to check if a sequence is empty is not idiomatic
and can be less performant than checking the truthiness of the object.

`len` doesn't know the context in which it is called, so if computing the
length means traversing the entire sequence, it must; it doesn't know
that the result is just being compared to 0. Computing the boolean value
can stop after it sees the first element, regardless of how long the
sequence actually is.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix DeepSource alert

Useless `return` detected

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Fix DeepSource alert

`print` statement has no effect

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Wintun 0.10.2 (2021-02-16) → 0.13 (2021-08-02)

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Merge branch 'wintun_doc_and_naming_tweaks' into 'master'

Wintun and related improvements

See merge request openconnect/openconnect!178

Nuke tabs in Python

TAB → 4 × SP

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

LGTM recommendations: Except block handles 'BaseException'

Except block directly handles BaseException.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Document --force-trojan as available on _WIN32

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Merge branch 'set_VPNPID_for_vpnc_script' into 'master'

Provide the vpnc-script with our PID (as $VPNPID)

See merge request openconnect/openconnect!278

Provide the vpnc-script with our PID (as $VPNPID)

This will enable a vpnc-script to more easily identify which VPN connection
is calling it, in the case of multiple concurrent or "stacked" VPN
connections.

Because OpenConnect (and vpnc) invoke the vpnc-script via an intermediate
shell process, the vpnc-script would otherwise need to determine its
GRANDparent PID, which is an error-prone, and not easily portable, process.
See https://gitlab.com/openconnect/vpnc-scripts/-/issues/28 and
https://gitlab.com/openconnect/vpnc-scripts/-/merge_requests/36 for issues
with the current approaches.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Update changelog to reflect Wintun and vpnc-script-win.js improvements

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Try to delete-and-reclaim IP addresses from down interfaces

As mentioned previously, Windows will not allow us to set an IP address on
the tunnel interface (using 'netsh' in vpnc-script-win.js) if that address
is assigned to another interface, *even if* that interface is not currently
up.

In order to figure out if our interface's desired IP address(es) are
assigned to any other interface(s), we have to iterate over the results of
`GetAdaptersAddresses`. This retrieves a list of all IP addresses (both
Legacy and IPv6) by interface, along with up/down status thereof.

If we find another interface that is assigned our desired IP address, but is
not up, we should attempt to unassign/delete/reclaim the address from this
interface. In order to do that, we need to use a completely separate API
(`GetUnicastIpAddressTable`) to get a list of all assigned IP addresses in a
form that can be used to delete them (with `DeleteUnicastIpAddressEntry`).

The simplest way to structure the code to do this is to use two nested
loops:

- The outer loop iterates over the adapters and addresses returned by
  `GetAdaptersAddresses`
- The inner loop — which runs only in the case of a conflicting address —
  iterates over the addresses as returned by `GetUnicastIpAddressTables`.
- Perhaps this could be optimized, but because this runs only once per VPN
  connection, and a typical client system will have <<N=100 adapters and IP
  addresses… it doesn't seem worth it.

NB: I had previously tried avoiding the use of `GetUnicastIpAddressTable`
    altogether by creating a fake/synthetic `MIB_UNICASTIPADDRESS_ROW` with
    most of the fields filled in using information returned by
    `GetAdaptersAddresses`, and passing this directly to
    `DeleteUnicastIpAddressEntry`.  It simply did not work, perhaps due to
    the absence of one or two seemingly-unimportant fields whose values
    couldn't be determined. (See 4acc773c69d8b06da2983fd5fe70cca357822503)

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Add check_address_conflicts() to tun-win32.c

If other adapters have conflicting IPv4/IPv6 addresses, vpnc-script-win.js
will fail to configure the tunnel addresses correctly.

For now, check_address_conflicts() only checks and fails; it doesn't try to
delete/reclaim the conflicting addresses.

[ Yay, Windows, for persisting a value we don't need or want persisted, and
  not allowing us to override this with 'netsh interface ipvX set address'.
  This appears to be a limitation of the Windows 'netsh' configuration
  utility. ]

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Don't set Legacy IP address on Windows tunnel interface within OpenConnect itself

The pre-existing Windows routing connection script (vpnc-script-win.js) was
not able to reliably set the Legacy IP address of the tunnel interface, so
we've been setting it in OpenConnect itself since
60d1f092e35f05217f1c96823c4f1b86c7915bbd.

As of https://gitlab.com/openconnect/vpnc-scripts/-/merge_requests/26, we've
got vpnc-script-win.js setting the Legacy IP address correctly, and IPv6 as
well. We should leave this as the script's responsibility for
cross-platform consistency.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Check vpnc-script exit status on all platforms including Windows

See https://gitlab.com/openconnect/vpnc-scripts/-/merge_requests/26, in
which I modified vpnc-script-win.js to return a usable exit status.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Distinguish ERROR_ACCESS return value from create_wintun()

The error 'Is the driver installed?' is misleading in this case; the actual
problem is likely that the user isn't running as SYSTEM/Administrator.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Remove TAP-Windows driver from installer, and update docs to reference Wintun's default inclusion

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Use hostname as Wintun ifname (if ifname not specified)

Without this change, OpenConnect will *only* attempt to use the Wintun
driver if `-i InterfaceName`, and will require TAP-Windows driver otherwise.

That seems like a surprising and hard-to-discover behavior.

Instead, we should use the VPN server's hostname as a sane default interface
name with Wintun, and only attempt to use TAP-Windows as a fallback in the
case where Wintun can't be initialized.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

Merge branch 'flake8' into 'master'

Flake8 errors and warnings

See merge request openconnect/openconnect!277

Flake8 errors and warnings

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Merge branch 'codespell' into 'master'

Typos caught by codespell

See merge request openconnect/openconnect!275

Typos caught by codespell

More typos caught by the latest version of codespell.

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Merge branch 'ERR_GET_FUNC_OpenSSL_3.0' into 'master'

Build with OpenSSL 3.0 beta 2 Release Candidate

Closes #289

See merge request openconnect/openconnect!269

Build with OpenSSL 3.0 beta 2 Release Candidate

I had forgotten this ERR_GET_FUNC() call in my previous patch !262.

Again, removing calls to ERR_GET_FUNC() will not change anything:

PKCS11_F_PKCS11_LOGIN / ERR_LIB_PKCS11

We check the function code is PKCS11_F_PKCS11_LOGIN right after
calling PKCS11_login().

Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>

Merge branch 'lgtm' into 'master'

Fix LGTM alerts in Python code

See merge request openconnect/openconnect!266