Commit graph

3528 commits

Author SHA1 Message Date
Antonio Quartulli
c62edebe49
Update issue templates 2023-02-10 15:17:16 +01:00
Arne Schwabe
589cca1563 Fix LibreSSL not building in Github Actions
During the build of LibreSSL portable it pulls in a branch from OpenBSD
upstream. Unfortunately they use master there instead of a fixed branch.
So we work around this issue.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230209163115.465548-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20230209163115.465548-1-arne@rfc2549.org
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-09 18:06:13 +01:00
Frank Lichtenheld
2dc2d16559 Windows: fix unused variable in win32_get_arch
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230203191440.136050-5-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26141.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-07 17:57:41 +01:00
Frank Lichtenheld
a95705be85 Windows: fix wrong printf format in x_check_status
Relevant defines/typedefs:
typedef UINT_PTR        SOCKET;
if defined(_WIN64)
 typedef unsigned __int64 UINT_PTR;
else
 typedef unsigned int UINT_PTR;
endif
ifdef _WIN64
 define PRIuPTR  PRIu64
else
 define PRIuPTR  PRIu32
endif

Remove duplicated include of inttypes.h

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230207134333.52221-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26166.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-07 17:49:54 +01:00
Frank Lichtenheld
48495ce3cd Windows: fix unused variables in delete_route_ipv6
At this point it might be easier to create a
dedicated function for Windows...

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230203191440.136050-3-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26140.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-07 17:22:50 +01:00
Frank Lichtenheld
8aeec3aa36 Windows: fix unused function setenv_foreign_option
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230203191440.136050-2-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26145.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-07 17:17:24 +01:00
Selva Nair
b761cb9bc9 block-dns using iservice: fix a potential double free
- An item added to undo-list was not removed on error, causing
  attempt to free again in Undo().
  Also fix a memory leak possibility in the same context.

Github: fixes OpenVPN/openvpn#232

v2: Split add and delete functions and reuse the delete
function for cleanup.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230201170735.2266851-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26130.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-02 09:37:44 +01:00
Frank Lichtenheld
b2e49465e6 Changes.rst: document removal of --keysize
When reviweing OpenVPN/openvpn#231 I noticed this was
missing from Changes.rst.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230201135221.36135-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26121.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-01 17:34:13 +01:00
Arne Schwabe
98f2950043 Add printing USAN stack trace on github actions
This allows identifying the source of undefined behaviour more easily
from the github action logs.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230130172936.3444840-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26102.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-01 17:25:33 +01:00
Arne Schwabe
dc8f1f3963 Update LibreSSL to 3.7.0 in Github actions
The version 3.5.3 triggers undefined behaviour with the usan sanatizer.
Updating LibreSSSL to 3.7.0 does unfortunately does not fix the issue but
at least we are now using a current version.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230130172936.3444840-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26105.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-01 17:24:27 +01:00
Arne Schwabe
f6ccff6d7e Fix unaligned access in auth-token
The undefined behaviour USAN clang checker found this. The optimiser
of clang/gcc will optimise the memcpy away in the auth_token case and
output excactly the same assembly on amd64/arm64 but it is still better
to not rely on undefined behaviour.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230130172936.3444840-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26103.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-01 15:05:48 +01:00
Matthias Andree
ffcf20ca70 make dist: Ship ovpn_dco_freebsd.h, too
This file was missing from src/openvpn/Makefile.am.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230127203208.305638-1-matthias.andree@gmx.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26085.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-28 19:47:29 +01:00
Antonio Quartulli
537cde6b8f dco_linux: update license for ovpn_dco_linux.h
The linux userspace API header has acquired the MIT license (check the
ovpn-dco repository for the related change), therefore we simply bring
this change in our local copy to ensure compliancy.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230125095321.23063-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26077.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-25 10:58:55 +01:00
Lev Stipakov
7a23a7dda2 openvpnmsica: fix adapters discovery logic for DCO
Custom action "FindSystemInfo" finds adapters with certain hwid and
assigns found adapters' guids to a certain property. Later another custom
action "EvaluateTUNTAPAdapters" schedules adapter creation if the
abovementioned property is not set - which means no adapters exist
with given hwid.

I think this logic is needed to prevent duplicate adapter creation
if adapter was renamed and then new version is installed.

As one can see, there is a typo in property name ("OVPNDCOAPTERS"). As
a result of this typo, installer will always try to create DCO adapter
no matter if there are existing adapters. It however won't do anything
if adapter with the name "OpenVPN Data Channel Offload" already exists,
this is handled in schedule_adapter_create() function.

Because of that typo, following scenario works fine:

 1) Upcoming release of OpenVPN Connect is installed, which creates
adapter named "OpenVPN Connect DCO Adapter"

 2) OpenVPN-GUI is installed. Because of typo, it ignores adapter created
by Connect and creates own "OpenVPN Data Channel Offload" adapter

 3) OpenVPN Connect is uninstalled and it removes
"OpenVPN Connect DCO Adapter".

 4) OpenVPN-GUI still has its "OpenVPN Data Channel Offload" adapter

If we just fix a typo, OpenVPN-GUI won't create a adapter on step 2 and
after Connect removal on step 3 there won't be DCO adapters anymore
for OpenVPN-GUI to use.

The ultimate solution to this would be moving adapter creation to MSM,
a shared component which adds/removes the DCO driver. However this change
is not trivial and requires a lot of work. For the time being we apply
this band-aid by excluding Connect-created adapters from enumerations in
"FindSystemInfo" custom action. This makes sure that OpenVPN-GUI won't
rely on adapter created by Connnect (which is deleted on Connect uninstall)
and ensures that additional DCO adapters won't be created on upgrade
if user decides to rename adapter.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230124142316.441-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26072.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-25 09:59:32 +01:00
Lev Stipakov
6effd9197c openvpnmsica: remove unused declarations
That code has been moved to MSM by commit 640c4d82
("openvpnmsica: remove dco installer custom actions")

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230124091441.397-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26070.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-24 11:49:20 +01:00
Selva Nair
00fac39c58 Fix one more 'existing route may get deleted' case
- Ensure net_route_v4/v6_add/del() functions using iproute2 return
  error when route addition fails. Return value follows the same logic
  as corresponding functions using netlink though all failure reasons
  get the same error code of -1.

TODO: Preserve any preexisting direct route to VPN and optionally the
IPv6 connected net route.

v2: Following review, removed the poorly coded RL_DID_LOCAL-related chunks.
That part needs a better fix.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230121194226.2081637-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26067.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-24 08:33:39 +01:00
Selva Nair
a45c201e2e Cleanup route error and debug logging on Windows
Use a unified logging format for various route-methods

- Route add/delete errors are always logged with M_WARN, so
  log only additional information (succeed/exists) with D_ROUTE.

- Non-windows platforms log route errors with a prefix "ERROR:" and
  debug info with "ROUTE:". Do the same on Windows. Do not log
  errors or success multiple times.

- In add_route_ipv6, log the interface id instead of device name
  as the latter always point to the tun/tap adapter name on Windows.

Log lines prefixed with a PACKAGE_NAME "ROUTE" are unchanged.
They appear to use the same format on all platforms.

v2: rebase to master

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230120094100.2063883-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26058.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-20 18:28:48 +01:00
Selva Nair
abad04fc8e Warn when pkcs11-id or pkcs11-id-management options are ignored
- If there are no pkcs11-providers either directly specified or
  through p11-kit-proxy made available through a build-time detection,
  these options are ignored. Log a warning in such cases.

  Especially important on Windows where automatic loading of p11-kit
  is not enabled in our release builds.

- Document this behaviour.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230120021841.2048791-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26056.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-20 17:58:47 +01:00
Lev Stipakov
7217c7137e openvpnmsica: remove dco installer custom actions
Those have been moved into MSM to be reused by openvpn-gui and Connect.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230119085959.157-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26053.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-19 16:13:42 +01:00
Arne Schwabe
f84a9fc5d4 Workaround: make ovpn-dco more reliable
This workaround avoids the kernel trigger ENOBUFS when the kernel
internal queue is overrun with events of disconnectingh clients or
similar. This is a workaround until we come up with a more permanent
solution.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20230112163737.1240059-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25988.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-19 09:39:52 +01:00
Timo Rothenpieler
d8523119b9 Don't clear capability bounding set on capng_change_id
The bounding set being empty will overpower the likes of su/sudo
and will make it impossible for any child processes to ever gain
additional privileges again.

Github: fixes OpenVPN/openvpn#220

Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230118142428.162-1-timo@rothenpieler.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26048.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-19 08:39:03 +01:00
Gert Doering
adc54f483b Repair special-casing of EEXIST for Linux/SITNL route install
The code in sitnl_route_set() used to treat "route can not be installed
because it already exists" (EEXIST) as "not an error".

This is arguably a reasonable approach, but needs to handled higher
up - if the low level add_route() function say "no error", we will try
to remove that route later on in delete_route(), possibly removing
someone else's "already existing" route then.

So:
 - remove special case in sitnl_route_set()
 - do not pass NLM_F_REPLACE flag to sitnl_route_set() call - this would
   cause netlink to just replace existing routes, never return EEXIST
   (see "man netlink(7)")
 - add detailed return code handling to add_route(), assign "2" on
"-EEXIST"
   (and log appropriate message).

(Note: sitnl_route_set() is a common function for sitnl route add and
delete, but EEXIST can not happen on delete - so this change has no
impact for the "delete" case)

v2: use RTA_ macros, also adjust add_route_ipv6()

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230118074633.27586-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26046.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-19 08:20:05 +01:00
Selva Nair
328cc40c83 Define and use macros for route addition status code
- Instead of 0, 1, 2 use RTA_ERROR, RTA_SUCCESS, RTA_EEXIST
  as the return code of route addition functions.

- Also fix a logging error: status -> (status == RTA_SUCCESS)

v2: fold long lines
    use "bool ret = .." pattern for android too
    fix two more lines where status was directly assigned to bool

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230115164818.1973210-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26041.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-16 09:58:42 +01:00
Gert Doering
cf545d603e Fix OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT breakage on FreeBSD+DCO
commit 67c4eebdae introduces a new peer disconnect reason (transport
disconnected, aka "TCP session closed") which breaks compilation on
FreeBSD - OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT not part of the
enum in freebsd_dco.h, and no kernel support for TCP anyway.

This patch is an intermediate bandaid, making the offending code in
multi.c "linux only" while a better solution is discussed.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20230113080745.82783-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20230113080745.82783-1-gert@greenie.muc.de
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-13 14:42:04 +01:00
Antonio Quartulli
67c4eebdae dco: print proper message in case of transport disconnection
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230111235052.24855-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25977.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-12 13:27:04 +01:00
Arne Schwabe
2104ea6243 Deprecate OCC checking
- Move OCC warnings to debug level. This moves the only useful OCC message
  of compress-migrate to D_PUSH
- remove configure option --enable-strict-options
- ignore disable-occ in TLS mode as it is logged under debug now only
  disable-occ is now strictly a non-TLS option
- mark opt-verify and disable-occ as deprecated.

Patch v2: change one missed M_WARN to D_OCC

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230111134439.1107915-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25970.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-11 15:41:36 +01:00
Frank Lichtenheld
ee0a6026af documentation: update 'unsupported options' section
We listed those in Changes, but did not update the documentation.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230111125242.21025-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25968.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-11 14:52:24 +01:00
Frank Lichtenheld
af25448ee1 check_engine_keys: make pass with OpenSSL 3
Not enabled by default with OpenSSL 3, so we don't
see this in our builds.
While here add missing entries to .gitignore (which
is what made me look at engine-key test in the first
place).

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230110170257.113527-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25949.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-11 13:54:27 +01:00
Frank Lichtenheld
ff7d7989e0 options: Always define options->management_flags
That makes it possible to remove several preprocessor
directives which is a good thing. The cost should be
negligible.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221127142506.41986-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25554.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-11 13:38:22 +01:00
Selva Nair
eafbedc583 Include CE_DISABLED status of remote in "remote-entry-get" response
- The response to the management command "remote-entry-get" is
  amended to include the status of the remote entry. The status
  reads "disabled" if (ce->flag & DISABLED) is true, "enabled"
  otherwise.

- Update and correct the description of this option in
  management-notes.txt

  Example responses:
  In response to "remote-entry-get 0"

  0,vpn.example.com,udp,enabled
  END

  Or, in response to "remote-entry-get all"

  0,vpn.example.org,udp,enabled
  1,vpn.example.com,udp,enabled
  2,vpn.example.net,tcp-client,disabled
  END

This helps the management client to show only enabled remotes
to the user.
An alternative would require the  UI/GUI to have knowledge of
what makes the daemon set CE_DISABLED (--proto-force,
--htttp-proxy-override etc.).

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230111062910.1846688-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/search?l=mid&q=20230111062910.1846688-1-selva.nair@gmail.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-11 08:31:30 +01:00
Frank Lichtenheld
202b34da38 xkey_pkcs11h_sign: fix dangling pointer
Warning by GCC 12:
pkcs11_openssl.c:237:22: warning:
dangling pointer ‘tbs’ to ‘enc’ may be used [-Wdangling-pointer=]

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230110131947.59552-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25942.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-10 19:59:51 +01:00
Frank Lichtenheld
ccf9d57249 Update copyright year to 2023
Manually excluded ovpn_dco_win.h because it is an
imported file. ovpn_dco_linux.h is already excluded
because it still says 2021.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230110160531.81010-1-frank@lichtenheld.com>
URL: https://patchwork.openvpn.net/project/openvpn2/patch/20230110160531.81010-1-frank@lichtenheld.com/
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-10 17:24:37 +01:00
Arne Schwabe
533c170fb6 Log peer-id if loglevel is D_DCO_DEBUG and dco is enabled
This enables logging the peer id in p2mp mode if dco is enabled
and the log level is high enough

Patch v2: use check_debug_level to check current log level

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230110151901.998479-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25946.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-10 16:44:17 +01:00
Gert Doering
85e0df6b49 Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode
p2p --tls-server with no active client/peer logs once per second

  "dco_update_keys: peer_id=-1"

which does exactly nothing, except fill the disk.  So skip the call to
dco_update_keys() if peer_id == -1.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230109200011.2525342-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25935.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-10 15:03:13 +01:00
Selva Nair
e04c253618 Propagate route error to initialization_completed()
Makes it possible to report management state as CONNECTED,ROUTE_ERROR
instead of CONNECTED,SUCCESS in case of routing errors.

This depends on treating "route already exists" as not
an error which right now works when using netlink on Linux
and IPAPI or iservice on Windows.

For route set via command line there is no easy way to get this
information and current behaviour is unchanged: i.e., the management
state continues to be reported as CONNECTED,SUCCESS.

Status notification to systemd is not affected.

To test on Linux, build with netlink and use a --route option with
an unreachable gateway like:
"--route 192.168.122.0 255.255.255.0 1.1.1.1"

Notes:
On windows, if the route method is "exe", setting a route
that exists *may* get logged as error and this patch will lead to
a slightly misleading CONNECTED,ROUTE_ERROR state message. This is
considered tolerable as no one should be using "exe" (i.e. route.exe)
as the route method.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230105022718.1641751-3-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25884.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-10 12:28:45 +01:00
Arne Schwabe
b520c68c67 Add connect-freq-initial option to limit initial connection responses
This limits the number of packets OpenVPN will respond to. This avoids
OpenVPN servers being abused for refelection attacks in a large scale
as we gotten a lot more efficient with the cookie approach in our
initial connection handling.

The defaults of 100 attempts per 10s should work for most people,
esepcially since completed three way handshakes are not counted. So
the default will throttle connection attempts on server with high packet
loss or that are actually under a DOS.

The 100 per 10s are similar in size to the old 2.5 and earlier behaviour
where every initial connection attempt would take up a slot of the
max-clients sessions and those would only expire after the TLS timeout.
This roughly translates to 1024 connection attempts in 60s on an
empty server.

OpenVPN will announce once per period when starting to drop packets and
ultimatively how many packets it dropped:

    Connection Attempt Note: --connect-freq-initial 100 10 rate limit
    exceeded, dropping initial handshake packets for the next 10 seconds

    Connection Attempt Dropped 217 initial handshake packets due to
    --connect-freq-initial 100 10

to inform an admin about the consequences of this feature.

Patch v2: use strtol instead of atoi to be able to differentiate between
          an error parsing and parsing 0. Use int64_t instead int to
          avoid overflow errors.

Patch v3: Add message when we start dropping. Add a few fixes to the logic.
          improve docs

Patch v4: missing missing return statement.
Patch v5: add build files for msvc build

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230110015901.933522-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25938.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-10 08:04:05 +01:00
Gert Doering
16d7f2cd4d Undo FreeBSD 12.x workaround on IPv6 ifconfig for 12.4 and up
commit 5e19cc2c1b introduced a workaround for a race condition
that showed itself on IPv6 ifconfig on FreeBSD 12.x - sometimes breaking
IPv6 connectivity on tun/tap interfaces.

This was fixed on the FreeBSD side in 12.4, 13.1 and up, and 13.0 is
no longer supported.  So conditionalize the workaround on "12.0..12.3",
to be fully removed later when 12.3 is also running out of support.

v2: fix version number comparison

Trac: 1226

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230107162558.59659-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25911.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-09 17:01:02 +01:00
Selva Nair
9c6d72c783 Distinguish route addition errors from route already exists
When possible, functions that add a route now return 1 on success,
or 2 if route already exists or 0 on other errors instead of true/false.

Note:
net_route_v4/v6_add using netlink filters out EEXIST before returning
this looks like a bug as add_route() and add_route_ipv6() should set
RT_ADDED only if route was really added.

v2: "succeeded/skipped" --> "succeeded" in log.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230106150412.1667492-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25903.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-09 12:53:45 +01:00
Lev Stipakov
a0eb1f764d tun: move print_windows_driver() out of tun.h
We got warnings from MinGW about function being defined
but not used when compiling modules which include tun.h.

This function is not defined as inline, so its definition
should not be in header. Since this is not a performance
critical, no need to make it inline.

Leave declaration in tun.h and move definition to tun.c.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230109113046.1678-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25923.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-09 12:37:07 +01:00
Selva Nair
3a72579257 Assign and honour signal priority order
Signals are ordered as SIGUSR2, SIGUSR1, SIGHUP, SIGTERM, SIGINT
in increasing priority. Lower priority signals are not allowed to
overwrite higher ones.

This should fix Trac #311, #639 -- SIGTER/SIGINT lost during dns
resolution (except for the Windows-specific bug handled in previous commit).

On sending SIGTERM during dns resolution, it still takes several seconds
to terminate as the signal will get processed only after getaddrinfo times
out twice (in phase1 and phase2 inits).

Github: fixes OpenVPN/openvpn#205
Trac: #311, #639

Note: one has to still wait for address resolution to time out as
getaddrinfo() is no interruptible. But a single ctrl-C (and some
patience) is enough.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230101215109.1521549-4-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25871.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-08 17:47:43 +01:00
Selva Nair
22977577ed Fix signal handling on Windows
- In win32_signal_get() re-order the check so that Windows
  signals are picked up even if signal_received is non-zero

- When management is not active, management_sleep() becomes sleep()
  but it is not interruptible by signals on Windows. Fix this by
  periodically checking for signal.

Trac: #311 #639 (windows specific part)
Github: Fixes OpenVPN/openvpn#205 (windows specific part)

Note: if stuck in address resolution, press ctrl-C and wait for
getaddrinfo() to timeout.

v2: WIN32 --> _WIN32
    add a chunk in management_sleep that was missed by sloppy
    conflict-resolution

v3: following review by Lev Stipakov <lstipakov@gmail.com>
  win32_sleep()
    - Early fallback to Sleep() if no wait handles -- less indentation
    - Check signal only if wait-object triggered
    - Exit the while loop if not safe to continue
  Behaviour of win32_sleep(0) checking signal is retained though may be
  redundant

v4: Avoid Sleep(0) and never loop back to wait again if wait-failed

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230106005438.1664046-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25895.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-08 17:28:09 +01:00
Selva Nair
dd66958198 Use IPAPI for setting ipv6 routes when iservice not available
Currently we use netsh for this. The new code closely follows
what interactive service does.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230105022718.1641751-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25886.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-08 16:50:54 +01:00
Antonio Quartulli
b20daf2743 dco: improve comment about hidden debug message
While at it also improve the debug message itself
to be more self-explanatory.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230103202330.1835-3-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25883.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-07 18:53:28 +01:00
Antonio Quartulli
ea6ee7635f dco: bail out when no peer-specific message is delivered
multi_process_incoming_dco() is currently partly processing
messages that were actually discarded. This results in a bogus
message being printed:

  "Received packet for peer-id unknown to OpenVPN: -1, type 0, reason 2"

Change the flow so that we bail out immediately when we know that no
message was truly delivered by DCO.
Currently this can be verified by checking that the peer_is is greater
than -1.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230103202330.1835-2-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25882.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-07 18:02:45 +01:00
Antonio Quartulli
e9889016fb dco: properly re-initialize dco_del_peer_reason
After processing a message, all fields of the dco object should be
re-initialized so that future processings are not affected by stale
values.

This includes dco_del_peer_reason.

Since its values can start at 0, re-initialize it with -1.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230103202330.1835-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25881.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-07 17:57:18 +01:00
Selva Nair
eff95d5004 Refactor signal handling in openvpn_getaddrinfo
Pass in sig_info struct to use register signal instead of
modifying signal_received.

No functional changes though some may be warranted.
Questions:
  - Why are we overwriting SIGUSR1 in this function?
  - Why the special interrupted syscall treatment for getaddrinfo?
    Its not a syscall, is it?

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230101215109.1521549-3-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25872.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-05 15:40:43 +01:00
Selva Nair
05715485b4 Preparing for better signal handling: some code refactoring
- Do not directly update signal_received: always use register_signal()
  throw_signal() or signal_reset().
  To facilitate this, register_signal() now takes c->sig as an argument
  instead of the context c itself, and sig_info struct is passed-in to
  functions that need to set a signal.

- openvpn_getaddrinfo() is updated in a following commit as it
  could benefit from some logic changes that we may or may not want
  to do.

No functional changes.

TODO:
(i)   update signal handling in openvpn_getaddrinfo
(ii)  enforce signal priority
(iii) fix signal handling on Windows
for 2.7?
(iv)  replace system-V signal with POSIX sigaction

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230101215109.1521549-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25874.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-01-05 11:36:13 +01:00
Selva Nair
a10564c716 Cleanup: Close duplicated handles in interactive service
Several handles from openvpn.exe are duplicated in the
service for registering ring buffer memory maps with the
driver. These handles are not required after registration,
as all access is through handles in openvpn.exe. Only the
map base address (send_ring, rceive_ring) need be retained
for later unmapping.

Use local variables for duplicated handles and close them
soon after use.

The struct ring_buffer_handles_t is renamed to ring_buffer_maps_t
as there are no handles in there any longer.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20221229182739.1477336-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25863.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2022-12-31 10:16:14 +01:00
Selva Nair
6ea9cf8146 Use undo_lists for saving ring-buffer handles in interactive service
HandleRegisterRingBuffers() in interactive.c did not follow the
the original API of HandleMessage(): a new argument was added
to HandleMessage to pass-in prer-process ring-buffer handles. The
existing undo lists argument is meant for such use.

Rewrite following the original design.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20221229182739.1477336-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25864.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2022-12-31 10:12:35 +01:00
Selva Nair
64f8833e11 Properly unmap ring buffer file-map in interactive service
The return value of MapViewOfFile must be passed to UnmapViewofFile,
instead of the file handle.

Github: Fixes OpenVPN/openvpn#206

v2: move *ring = NULL inside if {}

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20221229134729.1474034-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25859.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2022-12-29 16:15:33 +01:00