0
0
mirror of https://github.com/OpenVPN/openvpn.git synced 2024-09-20 03:52:28 +02:00
Commit Graph

165 Commits

Author SHA1 Message Date
Selva Nair
0fe3a98774 Add a test for loading certificate and key to ssl context
The test certificate used in test_ssl.c is updated to use 2048 bit
RSA and the matching key is added.

Tests include loading certificate and key as inlined pem as well as
from files. Note that loading the key also checks that it matches
the certificate, providing an indirect test that the latter was loaded
correctly.

Change-Id: Ic6f089896191145f68ce9a11023587d05dcec4d8
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240906103814.36839-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29074.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-09-08 22:45:55 +02:00
Selva Nair
3512e8d3ad Interpret --key and --cert option argument as URI
OpenSSL 3 has providers which can load keys and certificates
from various key stores and HSMs using a provider-specific URI.
While certificates are generally exportable, and some providers
support a PEM file that acts as a proxy for non-exportable private
keys, not all providers are expected to do so. A generic capability
to read keys and certificates from URIs appears useful.

This patch does this by extending the scope of the argument for
"--key" and "--cert" options to include URIs. Many of OpenSSL 3
utilities also work the same way: e.g., the "-in" option for
"openssl pkey" or "openssl x509" could be a filename or URI.
Other applications have started emulating this behaviour:
e.g., pkcs11: URI works as an alternative to a file name for
certificates and keys in apache. Even for files, this has a nice
side effect that non-PEM files get transparently parsed. E.g., a
pkcs12 file could be used in place of a PEM file without needing
any extra options.

This is backward compatible as OpenSSL falls back to treating URIs
with no scheme or unrecognized scheme as file names.

Parsing of inlined keys and certificates is unchanged (those
should be in PEM format).

Specification of URIs that OpenSSL accepts depends on the
providers that support them. Some are standard URIs such as
"file:/path", but providers may support non-standard URIs
with arbitrary scheme names. OpenSSL by itself recognizes
only file URI.  However, the implementation is agnostic to the
URI specification as parsing is done by the provider that supports
the URI. A new URI gets automatically recognized when the provider
that supports it is loaded.

Below are some usage examples:

Relative or absolute path to a file or as a URI "file:/absolute/path":

   --key mykey.pem      (same as what is currently supported)
   --key file:/path/to/mykey.pem
   --cert file:/path/to/mycert.pem

Other file types supported by OpenSSL would also work:

   --key client.p12
   --cert client.p12

pkcs11-provider supports "pkcs11:" URI (RFC 7512):

   --key pkcs11:token=Foo;id=%01
   --cert pkcs11:token=Foo;id=%01

tpm2-provider recognizes a custom URI "handle:<hex>":

   --key handle:0x81000000

These examples assume that required providers, if any, are loaded
and configured.

v2: same as PR 591 but with the fixup commit that addresses review comments is squashed.

Change-Id: I82b32d5ab472926e7889a5f4a90caba14231879a
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240906103734.36633-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29075.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-09-08 22:10:58 +02:00
Selva Nair
12a9c357b6 Protect cached username, password and token on client
Keep the memory segment containing username and password in
"struct user_pass" encrypted. Works only on Windows.

Username and auth-token cached by the server are not covered
here.

v2: Encrypt username and password separately as it looks more
robust. We continue to depend on the username and password buffer
sizes to be a multiple of CRYPTPROTECTMEMORY_BLOCK_SIZE = 16,
which is the case now. An error is logged if this is not the case.

v3: move up ASSERT in auth_token.c

Change-Id: I42e17e09a02f01aedadc2b03f9527967f6e1e8ff
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240906112908.1009-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29079.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-09-08 15:10:32 +02:00
Arne Schwabe
233e10aeec Implement support for AEAD tag at the end
Using the AEAD tag at the end is the standard way of doing AEAD. Several
APIs even only support the tag at the end (e.g. mbed TLS). Having the tag at
the front or end makes no difference for security but allows streaming HW
implementations like NICs to be much more efficient as they do not need to
buffer a whole packet content and encrypt it to finally write the tag but
instead just add the calculated tag at the end of processing.

Change-Id: I00821d75342daf3f813b829812d648fe298bea81
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240214132719.3031492-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28239.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-08-14 20:06:24 +02:00
Selva Nair
dcf735009c test_pkcs11.c: set file offset to 0 after ftruncate
Currently key and cert file fd's are reused after ftruncate()
without setting the offset to zero. This causes subsequent
data to be written at some finite offset with the hole in
the file automatically filled by zeros. Fix it by calling
lseek() to set the offset to zero.

The test works nevertheless because p11tool seem to generously
ignore any junk before the "BEGIN" marker.

Change-Id: Ib0fe15a4ba18d89216b0288e6cd6be66ed377bd4
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240812232158.3776869-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29010.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-08-13 08:27:53 +02:00
Frank Lichtenheld
418463ad27 console_systemd: rename query_user_exec to query_user_systemd
This allows us to override query_user_exec for unit
tests more consistently without having to jump through
weird hoops.

Fixes running test_pkcs11 with --enable-systemd.

While here also fix documentation comments for
query_user_exec*.

Change-Id: I379e1eb6dc57b9fe4bbdaefbd947a14326e7117a
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240726104032.2112-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28983.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-07-26 12:47:48 +02:00
Arne Schwabe
be31325e1d Allow trailing \r and \n in control channel message
Writing a reason from a script will easily end up adding extra \r\n characters
at the end of the reason. Our current code pushes this to the peer. So be more
liberal in accepting these message.

Github: closes OpenVPN/openvpn#568

Change-Id: I47c992b6b73b1475cbff8a28f720cf50dc1fbe3e
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240710140623.172829-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28910.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-07-17 20:55:21 +02:00
Frank Lichtenheld
56355924b4 configure: Add -Wstrict-prototypes and -Wold-style-definition
These are not covered by -Wall (nor -Wextra) but we want
to enforce them.

Change-Id: I6e08920e4cf4762b9f14a7461a29fa77df15255c
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240620144230.19586-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28823.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-06-20 17:01:31 +02:00
Arne Schwabe
414f428fa2 Properly handle null bytes and invalid characters in control messages
This makes OpenVPN more picky in accepting control message in two aspects:
- Characters are checked in the whole buffer and not until the first
  NUL byte
- if the message contains invalid characters, we no longer continue
  evaluating a fixed up version of the message but rather stop
  processing it completely.

Previously it was possible to get invalid characters to end up in log
files or on a terminal.

This also prepares the logic a bit in the direction of having a proper
framing of control messages separated by null bytes instead of relying
on the TLS framing for that. All OpenVPN implementations write the 0
bytes between control commands.

This patch also include several improvement suggestion from Reynir
(thanks!).

CVE: 2024-5594

Reported-By: Reynir Björnsson <reynir@reynir.dk>
Change-Id: I0d926f910637dabc89bf5fa919dc6beef1eb46d9
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <a@unstable.cc>

Message-Id: <20240619103004.56460-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28791.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-06-19 14:04:40 +02:00
Frank Lichtenheld
7dfff75659 test_user_pass: Fix building with --enable-systemd
Need to make sure that ENABLE_SYSTEMD is really disabled.

Change-Id: Ic33c210f06e173a450534aa0969c57f140086655
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240605111012.3023-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28708.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-06-05 13:15:12 +02:00
Arne Schwabe
763b35f652 Remove custom TLS 1.0 PRF implementation only used by LibreSSL/wolfSSL
After the removal of the OpenSSL 1.0.2 support, LibreSSL/wolfSSL are the
only libraries that still needs the custom implementation.

Since our LibreSSL/wolfSSL support is always best effort, we can afford to
limit LibreSSL support in this way. If they want to support this, they
should expose the functionality as well.

Change-Id: I5bfa3630ad4dff2807705658bc877c4a429a39ce
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240515100115.11056-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28672.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-05-15 13:10:49 +02:00
Arne Schwabe
130548fe4d Remove openvpn_snprintf and similar functions
Old Microsoft versions did strange behaviour but according to the
newly added unit test and
https://stackoverflow.com/questions/7706936/is-snprintf-always-null-terminating
this is now standard conforming and we can use the normal snprintf
method.

Microsoft own documentation to swprintf also says you nowadays need to
define _CRT_NON_CONFORMING_SWPRINTFS to get to non-standard behaviour.

Change-Id: I07096977e3b562bcb5d2c6f11673a4175b8e12ac
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240506102710.8976-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28617.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-05-06 16:56:24 +02:00
Frank Lichtenheld
b25c6d7e86 Update Copyright statements to 2024
Change-Id: Ic377958d303b1dcfa9d877d3a63ecf39bdff7aef
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240315170054.2368254-1-frank@lichtenheld.com>
URL: https://sourceforge.net/p/openvpn/mailman/message/58749316/
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-03-18 18:46:26 +01:00
Frank Lichtenheld
0c7cf0694e t_client.sh: Allow to skip tests
Individual tests can define a script to run to test
whether they should be skipped.

Included in this commit is an example check which
checks whether we can do NTLM checks. This fails
e.g. on recent versions of Fedora with mbedTLS
(tested with Fedora 39) or when NTLM support is not
compiled in.

v2:
 - ntlm_support:
   - support OpenSSL 3
   - allow to build without cmocka
v3:
 - add example to t_client.rc-sample
 - t_client.sh code style
 - use syshead.h in error.h
v5:
 - rename SKIP_x to CHECK_SKIP_x

Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240308102818.9249-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20240308102818.9249-1-gert@greenie.muc.de
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-03-08 12:27:05 +01:00
Juliusz Sosinowicz
54475711eb Change include order for tests
Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The openvpn/src directory needs to be included before include/wolfssl. include/wolfssl needs to be included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL headers without changing the paths.

src/openvpn/Makefile.am does not need to be modified because AM_CPPFLAGS is placed before AM_CFLAGS in the output Makefile.

Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20240212132522.125903-1-juliusz@wolfssl.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28229.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-02-12 16:59:35 +01:00
Arne Schwabe
91b057a2b5 Turn dead list test code into unit test
Change-Id: I7511bc43cd6a0bcb89476f27d5822ab4a78d0d21
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240209105902.14506-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28201.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-02-10 11:38:39 +01:00
Arne Schwabe
70b39f2bea Add unit test for encrypting/decrypting data channel
This test is reusing code from --test-crypto but is modified to not rely
on the static key functionality and also only tests the most common
algorithm. So it does not yet completely replace --test-crypto

Change-Id: Ifa5ae96165d17b3cae4afc53e844bb34d1610e58
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240208085749.869-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28195.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-02-08 11:01:03 +01:00
Frank Lichtenheld
ca122f990c test_user_pass: add basic tests for static/dynamic challenges
Change-Id: I8b5570f6314e917f92dce072279efe415d79b22a
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20240207171239.86730-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28191.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-02-08 09:04:36 +01:00
Arne Schwabe
bb0849db20 Allow unit tests to fall back to hard coded location
Settings the environment variable required for running unit tests is
tiresome in my IDE (Clion). So allow unit tests to fall back to a hard
coded location in case the environment variable is not set.

Change-Id: Ide72b81f497088dd0fd2cdcfff83cbce5b48f145
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240201144817.188884-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28161.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-02-01 18:22:02 +01:00
Frank Lichtenheld
55418bf62e test_user_pass: Add UTs for character filtering
For simplicity I implemented them only with the
inline method, but they actually apply to all methods.

Change-Id: Ie8d2d5f6f58679baaf5eb817a7e2ca1afcb8c4db
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240129105358.11161-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20240129105358.11161-1-gert@greenie.muc.de
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-01-29 14:23:49 +01:00
Frank Lichtenheld
b9696ff387 test_user_pass: new UT for get_user_pass
UTs for basic functionality, without management functions.

v2:
 - add CMake support
 - add GHA support for both MSVC and mingw
v3:
 - fix distcheck by adding input/ directory to dist

Change-Id: I193aef06912f01426dd4ac298aadfab97dd75a35
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20240127200716.10255-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28138.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-01-29 09:31:21 +01:00
Arne Schwabe
7869617a0f Ensure that all unit tests use unbuffered stdout and stderr
stderr is normally always unbuffered but stdout can be buffered. Especially,
when stdout is redirected it will become buffered while it is normally
unbuffered when connected to a terminal. This mean that if the unit exits
prematurely, the output in the buffered output will be lost.

As the unit test x_msg mock implementation prints even fatal on stdout
we ensure with this setup method that stdout is also unbuffered.

Change-Id: I5c06dc13e9d8ab73997f79b13c30ee8949e5e993
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240123104358.495517-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28122.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-01-23 16:34:50 +01:00
Arne Schwabe
dc4fde8052 Fix ssl unit tests on OpenSSL 1.0.2
OpenSSL 1.1.1 will initialise itself using clever linker magic. For
OpenSSL 1.0.2 we need to manually initialise the library. For other
unit tests just doing the OpenSSL_add_all_algorithms is enough but
this unit test needs a more complete initialisation.

Change-Id: I378081f391ad755d0a6fd5613de5c2a8bacc389a
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240122130909.10706-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28112.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-01-22 14:12:23 +01:00
Arne Schwabe
cedbac710c Add test_ssl unit test and test export of PEM to file
This introduces a number of mock function to be able to compile
ssl_verify_*.c and ssl_mbedtls.c/ssl_openssl.c into a unit and adds
quite a number of files to that unit. But it allows similar unit tests
(in term of dependencies) to be added in the future.

Change-Id: Ie248d35d063bb6878f3dd42840c77ba0d6fa3381
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240116214152.27316-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28028.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2024-01-16 22:44:47 +01:00
Frank Lichtenheld
21910ebc2e Remove support for NTLM v1 proxy authentication
Due to the limitation of the protocol it is not
considered secure. Better to use basic auth instead
of a false sense of security. NTLM v2 remains
supported for now.

Change-Id: I0dcb2dac4136f194da7050a8ea8495e9faba9dd9
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20231230143733.4426-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27862.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-12-30 17:06:14 +01:00
Frank Lichtenheld
55d73959cc tests: disable automake serial_tests
Serial mode is the old one and offers much less options for
running the tests. Generally our tests seem to work fine
with the newer parallel mode. The only reason we stuck with
serial_tests seems to be that we didn't like that it doesn't
output the test output by default. We could fix that with a
custom test driver. But will put that into a separate commit.

Change-Id: Ic7265d89142637b0963a6847c6beb06d9163bbb1
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20231214111635.237429-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27812.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-12-18 19:01:38 +01:00
Frank Lichtenheld
975ef50b9c buffer: add documentation for string_mod and extend related UT
Since I was confused what exactly string_mod does, I
added documentation and additional UTs to make it
clearer.

Change-Id: I911fb5c5fa4b41f1fc1a30c6bf8b314245f64a6e
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20231211170214.85417-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27761.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-12-12 07:58:53 +01:00
Frank Lichtenheld
9b4ed6d801 unit_tests: remove includes for mock_msg.h
Not actually used.

Change-Id: I5e394bb73702d87562ed354100eaff9b41f5389e
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20231208173529.95023-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27727.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-12-09 10:59:03 +01:00
Arne Schwabe
0a27c98a61 Replace character_class_debug with proper unit test
Change-Id: Ib2aa85b9c34d0a0b8b1dfb9f477f56c9a6b705d0
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20231201112243.15541-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27628.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-12-02 16:23:22 +01:00
Arne Schwabe
e7427bcbb9 Remove openssl engine method for loading the key
This is a contribution for loading engine key. OpenSSL engine is
deprecated since OpenSSL 3.0 and James Bottomley has not agreed to
the proposed license chagne. He is also okay with removing the
feature from the current code base as it is obsolete with OpenSSL 3.0.

The original commit ID was a0a8d801dd0d84e0ec844b9ca4c225df7 (plus
subsequent fixes).

Change-Id: I2d353a0cea0a62f289b8c1060244df66dd7a14cb
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20231006111910.3541180-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27133.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-10-18 15:24:10 +02:00
Selva Nair
2671dcb698 Log OpenSSL errors on failure to set certificate
Currently we log a bogus error message saying private key password
verification failed when SSL_CTX_use_cert_and_key() fails in
pkcs11_openssl.c. Instead print OpenSSL error queue and exit promptly.

Also log OpenSSL errors when SSL_CTX_use_certiifcate() fails in
cryptoapi.c and elsewhere. Such logging could be useful especially when
the ceritficate is rejected by OpenSSL due to stricter security
restrictions in recent versions of the library.

Change-Id: Ic7ec25ac0503a91d5869b8da966d0065f264af22
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20231001174920.54154-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27122.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-10-02 10:08:56 +02:00
Frank Lichtenheld
6d76218dd6 Remove --no-replay option
Officially deprecated since v2.4.
We have warned about using this forever.
It is time to pull the plug.

Change-Id: I58706019add6d348483ba222dd74e1466ff6c709
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Heiko Hund <heiko@openvpn.net>
Message-Id: <20230922103830.37151-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27059.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-09-22 14:20:05 +02:00
Frank Lichtenheld
95cc5faa16 CMake: various small non-functional improvements
These are based on review comments for the 2.6 backport.
But since they apply to the original master implementation
as well, I address them in this separate patch.

- Add documentation to contrib/cmake/*.py
- Fix grammar in README.cmake.md
- Update a TODO in CMakeLists.txt to better reflect
  the status quo
- Fix indentation in unit_tests' Makefile.am

Change-Id: I4e16767ee221e1aefdd18d13b3411c27d8dd844a
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/342
Message-Id: <20230919155635.708557-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27043.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-09-21 22:43:54 +02:00
Arne Schwabe
ede590e57c Mock openvpn_exece on win32 also for test_tls_crypt
This function is needed to commpile on win32 as run_command.c defines it
on Unix Linux but on windows it is defined in win32.c which pulls in too
many other unresolvable symbols.

Patch v2: Also add mock_win32_execve.c to automake files

Change-Id: I8c8fe298eb30e211279f3fc010584b9d3bc14b4a
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230712095412.570106-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26849.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-07-17 19:04:14 +02:00
Frank Lichtenheld
e87e44f7bc test_tls_crypt: Improve mock() usage to be more portable
Use the casting variants of mock(). Using the mock_ptr_type
fixes an existing bug where test_tls_crypt.c couldn't
build in MinGW 32bit:

test_tls_crypt.c:127:27: error:
cast to pointer from integer of different size
[-Werror=int-to-pointer-cast]
  127 |     const char *pem_str = (const char *) mock();

Change-Id: I6c03313b8677fa07c07e718b1f85f7efd3c4dea8
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230630123908.82588-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26796.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-07-01 22:07:47 +02:00
Frank Lichtenheld
7fbb948411 Remove all traces of the previous MSVC build system
Completely replaced by the CMake build system now.

v3:
 - rebase on top of my dist fixes

Change-Id: I807cffa40f18faa1adec4e15e84c032877a2b92e
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230620135310.94455-4-frank@lichtenheld.com>
URL: https://www.mail-archive.com/search?l=mid&q=20230620135310.94455-4-frank@lichtenheld.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-06-27 21:58:03 +02:00
Frank Lichtenheld
97223cb057 unit_tests: Add missing cert_data.h to source list for unit tests
Document the dependency. Also fixes cert_data.h missing from
distribution.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230619132934.76085-4-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26750.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-06-21 14:19:08 +02:00
Frank Lichtenheld
eecf9e9d70 test_buffer: add tests for buf_catrunc and its caller format_hex_ex
Just some very basic tests.

v2:
 - fix off-by-one

Change-Id: I73fc893136387d1da05f4aea98cb37b02d6c3230
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230517084422.70547-2-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26680.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-05-17 18:50:54 +02:00
Selva Nair
846951665a Make cert_data.h and test_cryptoapi/pkcs11.c MSVC compliant
- Do not use non-literal initializers for static objects
- Replace empty initializer {} by {0}

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Co-authored-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230327114937.28246-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26525.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-29 11:30:32 +02:00
Selva Nair
3013fde1c8 Unit tests: Test for PKCS#11 using a softhsm2 token
- Load some test certificate/key pairs into a temporary softhsm2 token
  and enumerate available objects through pkcs11-helper interface

- For each object, load it into SSL_CTX and test sign (if using OpenSSL 3)
  or check the certificate and public-key match (if using OpenSSl 1.1.1.).
  The pkcs11-id for each object is specified directly or
  through a mocked management callback to test pkcs11-id-management

Limitations:
  Depends on libsofthsm2.so and p11tool (install softhsm2 and gnutls-bin
  packages). Mbed-TLS/pkcs11-helper combination is not tested.

  If locations of these binaries are not auto-detected or need to be
  overridden, use -DSOFTHSM2_UTIL=<path> -DP11TOOL=<path> to configure.
  Location of SOFTHSM2_MODULE is not auto-detected and defaults to
  /usr/lib/softhsm/libsofthsm2.so. It may be changed by passing
  -DSOFTHSM2_MODULE=/some-path/libsofthsm2.so to configure.
  Also see "configure --help".

  The test is enabled only if --enable-pkcs11 is in use, and SOFTHSM2_UTIL
  & P11TOOL are found in path or manually defined during configuring.

Changes relative to github PR
  - Explicitly disable building the test on Windows: need to port mkstemp,
    mkdtemp, setenv etc., before enabling this on Windows.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230322221456.1660425-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26483.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-29 10:39:13 +02:00
Selva Nair
3bc071a8e3 Move digest_sign_verify out of test_cryptoapi.c
- This function will be reused for testing pkcs11

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230322221456.1660425-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26484.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-29 10:12:29 +02:00
Selva Nair
85da9de524 Unit tests: add test for SSL_CTX_use_Cryptoapi_certificate()
- This is the only remaining function in cryptoapi.c that has no
  direct or indirect test.

  This test confirms that an SSL_CTX context gets a certificate and
  private key loaded into it and the public key in the certificate
  matches the private key. As signing with certificate/key pairs
  fetched from the store is independently tested by the 'cryptoapi_sign'
  test, signing is not re-tested here.

  The functions "setup_/teardown_cryptoapi_sign()" are renamed to
  "setup_/teardown_xkey_provider()" to better reflect their purpose.
  These are also reused for the new test.

  While touching this context, also fix a memory leak in
  test_cryptoapi_sign: X509_get_pubkey() -> X509_get0_pubkey()

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230318144325.1316320-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26438.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-20 18:23:50 +01:00
Selva Nair
0267649a21 Add a test for signing with certificates in Windows store
- For each sample certificate/key pair imported into the store,
  load the key into xkey-provider and sign a test message.
  As the key is "provided", signing will use appropriate
  backend (Windows CNG in this case).

  The signature is then verified using OpenSSL.

Change-Id: I520b34ba51e8c6d0247a82edc52bde181ab5a717
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315013516.1256700-5-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26416.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-16 10:54:14 +01:00
Selva Nair
b538a33428 Add tests for finding certificates in Windows cert store
- find_certificate_in_store tested using 'SUBJ:', 'THUMB:'
  and 'ISSUER:' select strings. Uses test certificates
  imported into the store during the import test.

Change-Id: Ib5138465e6228538af592ca98b3d877277355f59
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315013516.1256700-3-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26415.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-16 10:28:08 +01:00
Selva Nair
d6cf0239e8 Import some sample certificates into Windows store for testing
- A few sample certificates are defined and imported into
  Windows certificate store (user store).
  This only tests the import process. Use of these certs to test the
  core functionality of 'cryptoapicert' are in following commits.

Change-Id: Ida5fc12c5bad5fde202da0bf0e8cdc71efe548c2
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315013516.1256700-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26417.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-15 22:29:55 +01:00
Arne Schwabe
e8ecaadd2a Fix memory leaks in HMAC initial packet generation
The HMAC leaks are just forgotten frees/deinitialisations.

tls_wrap_control() will sometimes return the original buffer (non
tls-crypt) and sometimes tls_wrap.work, so handling this buffer lifetime
is a bit more complicated.  Instead of further complicating that code
just give our work buffer the same lifetime as the other one inside
tls_wrap.work (put it into per-session gc_arena) as that is also more
consistent.

Second, packet_id_init() allocates a buffer with malloc and not using a
gc_arena, so we need to also manually free it.

Patch v2: add missing deallocations in unit tests of the new workbuf
Patch v3: remove useless allocation of 0 size buffer in
          tls_auth_standalone_init

Found-By: clang with asan
Change-Id: I0cff44f79ee7e3bcf7b5981fc94f469c15f21af3
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315195512.323070-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-15 21:45:14 +01:00
Frank Lichtenheld
8384741459 tests/unit_tests: Fix 'make distcheck' with subdir-objects enabled
Commit 7f72abcf8a enabled subdir-objects
when using automake 1.16+.

There is an issue with the handling of .deps directories with this option.
While automake 1.16 fixed subdir-objects to work at all when _SOURCES
contains "unexpanded references" and it did fix subdir-objects to work
with out-of-tree build for "source files specified with an explicit
'$(srcdir)'" those fixes are not transitive. "unexpanded references"
still break out-of-tree builds when enforcing a read-only source dir
like 'make distcheck' does. When using *explicit* references to
srcdir and top_srcdir it works correctly.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230308150704.128797-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26352.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-09 20:18:31 +01:00
Arne Schwabe
6a05768a71 Dynamic tls-crypt for secure soft_reset/session renegotiation
Currently we have only one slot for renegotiation of the session/keys.
If a replayed/faked packet is inserted by a malicous attacker, the
legimate peer cannot renegotiate anymore.

This commit introduces dynamic tls-crypt. When both peer support this
feature, both peer create a dynamic tls-crypt key using TLS EKM (export
key material) and will enforce using that key and tls-crypt for all
renegotiations. This also add an additional protection layer for
renegotiations to be taken over by an illegimate client, binding the
renegotiations tightly to the original session. Especially when 2FA,
webauth or similar authentication is used, many third party setup ignore
the need to secure renegotiation with an auth-token.

Since one of tls-crypt/tls-crypt-v2 purposes is to provide poor man's post
quantum crypto guarantees, we have to ensure that the dynamic key tls-crypt
key that replace the original tls-crypt key is as strong as the orginal key
to avoid problems if there is a weak RNG or TLS EKM produces weak keys. We
ensure this but XORing the original key with the key from TLS EKM. If
tls-crypt/tls-cryptv2 is not active, we use just the key generated by
TLS EKM. We also do not use hashing or anything else on the original key
before XOR to avoid any potential of a structure in the key or something
else that might weaken post-quantum use cases.

OpenVPN 2.x reserves the TM_ACTIVE session for renegotiations. When a
SOFT_RESET_V1 packet is received, the active TLS session is moved from
KS_PRIMARY to KS_SECONDARY. Here an attacker could theorectically send a
faked/replayed SOFT_RESET_V1 and first packet containing the TLS client
hello. If this happens, the session is blocked until the TLS
renegotiation attempt times out, blocking the legimitate client.

Using a dynamic tls-crypt key here blocks any SOFT_RESET_V1 (and following
packets) as replay and fake packets will not have a matching
authentication/encryption and will be discarded.

HARD_RESET packets that are from a reconnecting peer are instead put in the
TM_UNTRUSTED/KS_PRIMARY slot until they are sufficiently verified, so the
dynamic tls-crypt key is not used here.  Replay/fake packets also do not
block the legimitate client.

This commit delays the purging of the original tls-crypt key data from
directly after passing it to crypto library to tls_wrap_free. We do this
to allow us mixing the new exported key with the original key.
To be able to generate the dynamic tls-cryptn key, we need the original
key, so deleting the key is not an option if we need it later again to
generate another key. Even when the client does not support secure
renegotiation, deleting the key is not an option since when the
reconnecting client or (especially in p2p mode with float) another client
does the reconnect, we might need to generate a dynamic tls-crypt key
again. Delaying the deletion of the key has also little effect as the
key is still present in the OpenSSL/mbed TLS structures in the tls_wrap
structure, so only the number of times the keys is in memory would be
reduced.

Patch v2: fix spellings of reneg and renegotiations.
Patch v3: expand comment to original_tlscrypt_keydata and commit message,
          add Changes.rst
Patch v4: improve commit message, Changes.rst
Patch v5: fix spelling/grammar mistakes. Add more comments.
Patch v6: consistently calld this feature dynamic tls-crypt crypt. Note
          this changes the export label and makes it incompatible with
          previous patches.
Patch v7: also xor tls-auth key data into the dynamic tls-crypt key like
          tls-crypt key data
Patch v8: Avoid triggering ASSERT added in v7 by properly setting keys.n = 2
          when loading tls crypt v2 client keys. Add dyn-tls-crypt to
          protocol options printout.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
Message-Id: <20230307150233.3551436-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26341.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-03-07 16:13:48 +01:00
Selva Nair
8aff5655a5 Add a unit test for functions in cryptoapi.c
- Though named cryptoapi_testdriver, right now this only tests
  parsing of thumbprint specified as a selector for --cryptioapicert
  option. More tests coming..

v2: a line that belongs here was mistakenly included in the previous
commit. Corrected.
v3: add to list of tests run in github actions
v4: - correct comment above invalid strings (copy paste error)
    - make invalid strings differ from correct value only in the
      explicitly introduced invalid characters/separators (one had
      two distinct errors which is not a robust test).

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230214200804.600405-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26268.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-25 17:09:27 +01:00
Selva Nair
e3ad1fc423 Build unit tests in mingw Windows build
- Minor changes to the build system to include some
  dependencies for Windows build

- test_tls_crypt not built as it will pull in win32.c and
  its dependencies

- If cross-compiling, "make check" will only build the tests but not
  run any. Copy to Windows and run manually. Executables are in
  <buid-dir>/tests/unit_tests/openvpn/.libs/ and these depend on
  cmocka.dll in addition to openssl libs that some tests link to.

  Building with mingw on Windows should run the tests (untested).

v2: networking_testdriver was mistakenly enabled to run, while
originally it was only set to build. Corrected.

v3: exclude check_engine_keys.sh when cross-compiling
As suggested by Arne Schwabe <arne@rfc2549.org>

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230208005925.393200-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26188.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2023-02-10 22:02:18 +01:00