[Openvpn-devel] [M] Change in openvpn[master]: Implement the --tls-export-cert feature

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: cron2, flichtenheld.

Hello cron2, flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/466?usp=email

to look at the new patch set (#10).


Change subject: Implement the --tls-export-cert feature
..

Implement the --tls-export-cert feature

This is a re-implementation of the --tls-export-cert feature. This
was necessary to due to missing approval to re-license the old
(now removed) code. The re-implementation is based on the following
description of the feature provided by David:

  Add an option to export certificate in PEM format of the remote
  peer to a given directory.

  For example: --tls-export-cert /var/tmp

  This option should use a randomised filename, which is provided via a
  "peer_cert" environment variable for the --tls-verify script or the
  OPENVPN_PLUGIN_TLS_VERIFY plug-in hook.

Once the script or plugin call has completed, OpenVPN should delete
this file.

Change-Id: Ia9b3f1813d2d0d492d17c87348b4cebd0bf19ce2
Signed-off-by: Arne Schwabe 
---
M doc/man-sections/script-options.rst
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/ssl_common.h
M src/openvpn/ssl_verify.c
M src/openvpn/ssl_verify_backend.h
M src/openvpn/ssl_verify_mbedtls.c
M src/openvpn/ssl_verify_openssl.c
9 files changed, 174 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/66/466/10

diff --git a/doc/man-sections/script-options.rst 
b/doc/man-sections/script-options.rst
index 6f90e14..53c9f97 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -423,6 +423,15 @@
   See the `Environmental Variables`_ section below for additional
   parameters passed as environmental variables.

+--tls-export-cert dir
+  Adds an environment variable ``peer_cert_{x}`` (and an alias
+  ``peer_cert`` for ``peer_cert_0`` for compatibility)  when calling the
+  ``--tls-verify`` script or executing the OPENVPN_PLUGIN_TLS_VERIFY plugin
+  hook to verify the certificate.
+
+  The environment variable contains the path to a PEM encoded certificate
+  of the current peer certificate in the directory ``dir``.
+
 --up cmd
   Run command ``cmd`` after successful TUN/TAP device open (pre ``--user``
   UID change).
@@ -763,6 +772,15 @@
 modifier is specified, and deleted from the environment after the script
 returns.

+:code:`peer_cert_{n}`
+If the option ``--tls-export-cert`` is enabled, this option contains
+the path to the current peer certificate to be verified in PEM format
+where ``n`` is the verification level.
+
+:code:`peer_cert`
+Identical to `peer_cert_0` for compatibility with older
+versions.
+
 :code:`proto`
 The ``--proto`` parameter. Set on program initiation and reset on
 SIGHUP.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..c5cc154 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3336,6 +3336,7 @@
 to.auth_user_pass_verify_script_via_file = 
options->auth_user_pass_verify_script_via_file;
 to.client_crresponse_script = options->client_crresponse_script;
 to.tmp_dir = options->tmp_dir;
+to.export_peer_cert_dir = options->tls_export_peer_cert_dir;
 if (options->ccd_exclusive)
 {
 to.client_config_dir_exclusive = options->client_config_dir;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e498114..b3b0a5f 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1986,6 +1986,7 @@
 SHOW_STR(cipher_list_tls13);
 SHOW_STR(tls_cert_profile);
 SHOW_STR(tls_verify);
+SHOW_STR(tls_export_peer_cert_dir);
 SHOW_INT(verify_x509_type);
 SHOW_STR(verify_x509_name);
 SHOW_STR_INLINE(crl_file);
@@ -3048,6 +3049,7 @@
 MUST_BE_UNDEF(cipher_list_tls13);
 MUST_BE_UNDEF(tls_cert_profile);
 MUST_BE_UNDEF(tls_verify);
+MUST_BE_UNDEF(tls_export_peer_cert_dir);
 MUST_BE_UNDEF(verify_x509_name);
 MUST_BE_UNDEF(tls_timeout);
 MUST_BE_UNDEF(renegotiate_bytes);
@@ -4053,6 +4055,13 @@
 R_OK, "--crl-verify");
 }

+if (options->tls_export_peer_cert_dir)
+{
+errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
+ options->tls_export_peer_cert_dir,
+ W_OK, "--tls-export-cert");
+}
+
 ASSERT(options->connection_list);
 for (int i = 0; i < options->connection_list->len; ++i)
 {
@@ -8997,6 +9006,11 @@
 string_substitute(p[1], ',', ' ', >gc),
 "tls-verify", true);
 }
+else if (streq(p[0], "tls-export-cert") && p[1] && !p[2])
+{
+VERIFY_PERMISSION(OPT_P_SCRIPT);
+options->tls_export_peer_cert_dir = p[1];
+}
 else if (streq(p[0], 

[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/481?usp=email )

Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..


Patch Set 2:

(2 comments)

File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/481/comment/0aedaf28_cfeef61a :
PS1, Line 891: /* Warn if we can't find a valid temporary directory, 
which should
> M_USAGE is not really "Warn"
Done


File src/openvpn/win32-util.h:

http://gerrit.openvpn.net/c/openvpn/+/481/comment/fc97a5eb_3cd9a134 :
PS1, Line 44: const char *win_get_tempdir(void);
> corresponding removal from win32. […]
yes. Fixed.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 02 Jan 2024 16:52:39 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [M] Change in openvpn[master]: Move get_tmp_dir to win32-util.c and error out on failure

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/481?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld


Change subject: Move get_tmp_dir to win32-util.c and error out on failure
..

Move get_tmp_dir to win32-util.c and error out on failure

Currently we only warn in get_tmp_dir fails and set o->tmp_dir to
a null pointer. This will not be caught by check_file_access_chroot
either since that ignores NULL pointers but other parts of OpenVPN
will assume that tmp_dir is set to a non-NULL string.

Also move get_tmp_dir to ssl-utils.c to use it in unit tests.

Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Signed-off-by: Arne Schwabe 
---
M src/openvpn/options.c
M src/openvpn/win32-util.c
M src/openvpn/win32-util.h
M src/openvpn/win32.c
M src/openvpn/win32.h
5 files changed, 34 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/81/481/2

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e498114..79958db 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -885,7 +885,15 @@
 #ifdef _WIN32
 /* On Windows, find temp dir via environment variables */
 o->tmp_dir = win_get_tempdir();
-#else
+
+if (!o->tmp_dir)
+{
+/* Error out if we can't find a valid temporary directory, which should
+ * be very unlikely. */
+msg(M_USAGE, "Could not find a suitable temporary directory."
+" (GetTempPath() failed).  Consider using --tmp-dir");
+}
+#else  /* ifdef _WIN32 */
 /* Non-windows platforms use $TMPDIR, and if not set, default to '/tmp' */
 o->tmp_dir = getenv("TMPDIR");
 if (!o->tmp_dir)
diff --git a/src/openvpn/win32-util.c b/src/openvpn/win32-util.c
index 81e504a..c5e7505 100644
--- a/src/openvpn/win32-util.c
+++ b/src/openvpn/win32-util.c
@@ -147,4 +147,26 @@
 }
 return true;
 }
+
+const char *
+win_get_tempdir(void)
+{
+static char tmpdir[MAX_PATH];
+WCHAR wtmpdir[MAX_PATH];
+
+if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
+{
+return NULL;
+}
+
+if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
+{
+msg(M_WARN, "Could not get temporary directory. Path is too long."
+"  Consider using --tmp-dir");
+return NULL;
+}
+
+WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
+return tmpdir;
+}
 #endif /* _WIN32 */
diff --git a/src/openvpn/win32-util.h b/src/openvpn/win32-util.h
index ac37979..98bf74b 100644
--- a/src/openvpn/win32-util.h
+++ b/src/openvpn/win32-util.h
@@ -40,5 +40,8 @@
 /* return true if filename is safe to be used on Windows */
 bool win_safe_filename(const char *fn);

+/* Find temporary directory */
+const char *win_get_tempdir(void);
+
 #endif /* OPENVPN_WIN32_UTIL_H */
 #endif /* ifdef _WIN32 */
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index e998d90..6b7ba5e 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -1137,34 +1137,6 @@
 set_win_sys_path(buf, es);
 }

-
-const char *
-win_get_tempdir(void)
-{
-static char tmpdir[MAX_PATH];
-WCHAR wtmpdir[MAX_PATH];
-
-if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
-{
-/* Warn if we can't find a valid temporary directory, which should
- * be unlikely.
- */
-msg(M_WARN, "Could not find a suitable temporary directory."
-" (GetTempPath() failed).  Consider using --tmp-dir");
-return NULL;
-}
-
-if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > 
sizeof(tmpdir))
-{
-msg(M_WARN, "Could not get temporary directory. Path is too long."
-"  Consider using --tmp-dir");
-return NULL;
-}
-
-WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, 
NULL);
-return tmpdir;
-}
-
 static bool
 win_block_dns_service(bool add, int index, const HANDLE pipe)
 {
diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
index 3605966..aa8513b 100644
--- a/src/openvpn/win32.h
+++ b/src/openvpn/win32.h
@@ -286,9 +286,6 @@
 /* call self in a subprocess */
 void fork_to_self(const char *cmdline);

-/* Find temporary directory */
-const char *win_get_tempdir(void);
-
 bool win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel);

 bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel);

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/481?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I525ccf7872880367b248ebebb0ddc83551498042
Gerrit-Change-Number: 481
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos 

[Openvpn-devel] [M] Change in openvpn[master]: Implement the --tls-export-cert feature

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: cron2, flichtenheld.

Hello cron2, flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/466?usp=email

to look at the new patch set (#9).


Change subject: Implement the --tls-export-cert feature
..

Implement the --tls-export-cert feature

This is a re-implementation of the --tls-export-cert feature. This
was necessary to due to missing approval to re-license the old
(now removed) code. The re-implementation is based on the following
description of the feature provided by David:

  Add an option to export certificate in PEM format of the remote
  peer to a given directory.

  For example: --tls-export-cert /var/tmp

  This option should use a randomised filename, which is provided via a
  "peer_cert" environment variable for the --tls-verify script or the
  OPENVPN_PLUGIN_TLS_VERIFY plug-in hook.

Once the script or plugin call has completed, OpenVPN should delete
this file.

Change-Id: Ia9b3f1813d2d0d492d17c87348b4cebd0bf19ce2
Signed-off-by: Arne Schwabe 
---
M doc/man-sections/script-options.rst
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/ssl_common.h
M src/openvpn/ssl_verify.c
M src/openvpn/ssl_verify_backend.h
M src/openvpn/ssl_verify_mbedtls.c
M src/openvpn/ssl_verify_openssl.c
9 files changed, 174 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/66/466/9

diff --git a/doc/man-sections/script-options.rst 
b/doc/man-sections/script-options.rst
index 6f90e14..53c9f97 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -423,6 +423,15 @@
   See the `Environmental Variables`_ section below for additional
   parameters passed as environmental variables.

+--tls-export-cert dir
+  Adds an environment variable ``peer_cert_{x}`` (and an alias
+  ``peer_cert`` for ``peer_cert_0`` for compatibility)  when calling the
+  ``--tls-verify`` script or executing the OPENVPN_PLUGIN_TLS_VERIFY plugin
+  hook to verify the certificate.
+
+  The environment variable contains the path to a PEM encoded certificate
+  of the current peer certificate in the directory ``dir``.
+
 --up cmd
   Run command ``cmd`` after successful TUN/TAP device open (pre ``--user``
   UID change).
@@ -763,6 +772,15 @@
 modifier is specified, and deleted from the environment after the script
 returns.

+:code:`peer_cert_{n}`
+If the option ``--tls-export-cert`` is enabled, this option contains
+the path to the current peer certificate to be verified in PEM format
+where ``n`` is the verification level.
+
+:code:`peer_cert`
+Identical to `peer_cert_0` for compatibility with older
+versions.
+
 :code:`proto`
 The ``--proto`` parameter. Set on program initiation and reset on
 SIGHUP.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..c5cc154 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3336,6 +3336,7 @@
 to.auth_user_pass_verify_script_via_file = 
options->auth_user_pass_verify_script_via_file;
 to.client_crresponse_script = options->client_crresponse_script;
 to.tmp_dir = options->tmp_dir;
+to.export_peer_cert_dir = options->tls_export_peer_cert_dir;
 if (options->ccd_exclusive)
 {
 to.client_config_dir_exclusive = options->client_config_dir;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e498114..1c0a6bd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1986,6 +1986,7 @@
 SHOW_STR(cipher_list_tls13);
 SHOW_STR(tls_cert_profile);
 SHOW_STR(tls_verify);
+SHOW_STR(tls_export_peer_cert_dir);
 SHOW_INT(verify_x509_type);
 SHOW_STR(verify_x509_name);
 SHOW_STR_INLINE(crl_file);
@@ -3048,6 +3049,7 @@
 MUST_BE_UNDEF(cipher_list_tls13);
 MUST_BE_UNDEF(tls_cert_profile);
 MUST_BE_UNDEF(tls_verify);
+MUST_BE_UNDEF(tls_export_peer_cert_path);
 MUST_BE_UNDEF(verify_x509_name);
 MUST_BE_UNDEF(tls_timeout);
 MUST_BE_UNDEF(renegotiate_bytes);
@@ -4053,6 +4055,13 @@
 R_OK, "--crl-verify");
 }

+if (options->tls_export_peer_cert_dir)
+{
+errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
+ options->tls_export_peer_cert_dir,
+ W_OK, "--tls-export-cert");
+}
+
 ASSERT(options->connection_list);
 for (int i = 0; i < options->connection_list->len; ++i)
 {
@@ -8997,6 +9006,11 @@
 string_substitute(p[1], ',', ' ', >gc),
 "tls-verify", true);
 }
+else if (streq(p[0], "tls-export-cert") && p[1] && !p[2])
+{
+VERIFY_PERMISSION(OPT_P_SCRIPT);
+options->tls_export_peer_cert_dir = p[1];
+}
 else if (streq(p[0], 

[Openvpn-devel] [M] Change in openvpn[master]: Implement the --tls-export-cert feature

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: cron2, plaisthos.

Hello cron2, flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/466?usp=email

to look at the new patch set (#8).


Change subject: Implement the --tls-export-cert feature
..

Implement the --tls-export-cert feature

This is a re-implementation of the --tls-export-cert feature. This
was necessary to due to missing approval to re-license the old
(now removed) code. The re-implementation is based on the following
description of the feature provided by David:

  Add an option to export certificate in PEM format of the remote
  peer to a given directory.

  For example: --tls-export-cert /var/tmp

  This option should use a randomised filename, which is provided via a
  "peer_cert" environment variable for the --tls-verify script or the
  OPENVPN_PLUGIN_TLS_VERIFY plug-in hook.

Once the script or plugin call has completed, OpenVPN should delete
this file.

Change-Id: Ia9b3f1813d2d0d492d17c87348b4cebd0bf19ce2
Signed-off-by: Arne Schwabe 
---
M doc/man-sections/script-options.rst
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/ssl_common.h
M src/openvpn/ssl_verify.c
M src/openvpn/ssl_verify_backend.h
M src/openvpn/ssl_verify_mbedtls.c
M src/openvpn/ssl_verify_openssl.c
9 files changed, 174 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/66/466/8

diff --git a/doc/man-sections/script-options.rst 
b/doc/man-sections/script-options.rst
index 6f90e14..53c9f97 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -423,6 +423,15 @@
   See the `Environmental Variables`_ section below for additional
   parameters passed as environmental variables.

+--tls-export-cert dir
+  Adds an environment variable ``peer_cert_{x}`` (and an alias
+  ``peer_cert`` for ``peer_cert_0`` for compatibility)  when calling the
+  ``--tls-verify`` script or executing the OPENVPN_PLUGIN_TLS_VERIFY plugin
+  hook to verify the certificate.
+
+  The environment variable contains the path to a PEM encoded certificate
+  of the current peer certificate in the directory ``dir``.
+
 --up cmd
   Run command ``cmd`` after successful TUN/TAP device open (pre ``--user``
   UID change).
@@ -763,6 +772,15 @@
 modifier is specified, and deleted from the environment after the script
 returns.

+:code:`peer_cert_{n}`
+If the option ``--tls-export-cert`` is enabled, this option contains
+the path to the current peer certificate to be verified in PEM format
+where ``n`` is the verification level.
+
+:code:`peer_cert`
+Identical to `peer_cert_0` for compatibility with older
+versions.
+
 :code:`proto`
 The ``--proto`` parameter. Set on program initiation and reset on
 SIGHUP.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..c5cc154 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3336,6 +3336,7 @@
 to.auth_user_pass_verify_script_via_file = 
options->auth_user_pass_verify_script_via_file;
 to.client_crresponse_script = options->client_crresponse_script;
 to.tmp_dir = options->tmp_dir;
+to.export_peer_cert_dir = options->tls_export_peer_cert_dir;
 if (options->ccd_exclusive)
 {
 to.client_config_dir_exclusive = options->client_config_dir;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e498114..ecbc63e 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1986,6 +1986,7 @@
 SHOW_STR(cipher_list_tls13);
 SHOW_STR(tls_cert_profile);
 SHOW_STR(tls_verify);
+SHOW_STR(tls_export_peer_cert_path);
 SHOW_INT(verify_x509_type);
 SHOW_STR(verify_x509_name);
 SHOW_STR_INLINE(crl_file);
@@ -3048,6 +3049,7 @@
 MUST_BE_UNDEF(cipher_list_tls13);
 MUST_BE_UNDEF(tls_cert_profile);
 MUST_BE_UNDEF(tls_verify);
+MUST_BE_UNDEF(tls_export_peer_cert_path);
 MUST_BE_UNDEF(verify_x509_name);
 MUST_BE_UNDEF(tls_timeout);
 MUST_BE_UNDEF(renegotiate_bytes);
@@ -4053,6 +4055,13 @@
 R_OK, "--crl-verify");
 }

+if (options->tls_export_peer_cert_dir)
+{
+errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
+ options->tls_export_peer_cert_dir,
+ W_OK, "--tls-export-cert");
+}
+
 ASSERT(options->connection_list);
 for (int i = 0; i < options->connection_list->len; ++i)
 {
@@ -8997,6 +9006,11 @@
 string_substitute(p[1], ',', ' ', >gc),
 "tls-verify", true);
 }
+else if (streq(p[0], "tls-export-cert") && p[1] && !p[2])
+{
+VERIFY_PERMISSION(OPT_P_SCRIPT);
+options->tls_export_peer_cert_dir = p[1];
+}
 else if (streq(p[0], 

[Openvpn-devel] [M] Change in openvpn[master]: Implement the --tls-export-cert feature

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: cron2, flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/466?usp=email )

Change subject: Implement the --tls-export-cert feature
..


Patch Set 7:

(4 comments)

Patchset:

PS6:
So we have to decide how to go about this. The current patch only So just 
exporting and providing


File doc/man-sections/script-options.rst:

http://gerrit.openvpn.net/c/openvpn/+/466/comment/f8e03d25_01b36152 :
PS6, Line 426: --tls-export-cert-path dir
> the manpage calls the option "tls-export-cert-path", while options. […]
Ooops missed that one.


File src/openvpn/init.c:

http://gerrit.openvpn.net/c/openvpn/+/466/comment/e88dae53_952dc331 :
PS6, Line 3339: to.export_peer_cert_dir = 
options->tls_export_peer_cert_path;
> why call this "_dir" in the to, and "_path" in options-> ?
Fixed and now using always _dir


File src/openvpn/ssl_verify.c:

http://gerrit.openvpn.net/c/openvpn/+/466/comment/6cb0e20f_1acc6fec :
PS6, Line 734: if (opt->export_peer_cert_dir)
> So I tried to understand why it seemed to work in my testing. […]
This version of the patch now removes the environment variable together with 
the file. That is not as intrusive and should give at least backwards 
compatibility for now. The better solution is more complicated and requires 
modification to env handling (or even lot bigger refactoring) and is moved to 
follow up patches.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/466?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia9b3f1813d2d0d492d17c87348b4cebd0bf19ce2
Gerrit-Change-Number: 466
Gerrit-PatchSet: 7
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: cron2 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 02 Jan 2024 16:45:08 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 
Comment-In-Reply-To: flichtenheld 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [M] Change in openvpn[master]: Allow the TLS session to send out TLS alerts

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: plaisthos.

Hello flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/449?usp=email

to look at the new patch set (#6).


Change subject: Allow the TLS session to send out TLS alerts
..

Allow the TLS session to send out TLS alerts

Previous OpenVPN versions shut down the TLS control channel immediately
when encountering an error. This also meant that we would not send out
TLS alerts to notify a client about potential problems like mismatching
TLS versions or having no common cipher.

This commit adds a new key_state S_ERROR_PRE which still allows to
send out the remaining TLS packets of the control session which are
typically the alert message and then going to S_ERROR. We do not
wait for retries. So this is more a one-shot notify but that is
acceptable in this situation.

Sending out alerts is a slight compromise in security as alerts give
out a bit of information that otherwise is not given
out. But since all other consumers TLS implementations are already doing this
and TLS implementations (nowadays) are very careful not to leak (sensitive)
information by alerts and since the user experience is much better with
alerts, this compromise is worth it.

Change-Id: I0ad48915004ddee587e97c8ed190ba8ee989e48d
Signed-off-by: Arne Schwabe 
---
M Changes.rst
M src/openvpn/ssl.c
M src/openvpn/ssl_backend.h
M src/openvpn/ssl_common.h
M src/openvpn/ssl_mbedtls.c
M src/openvpn/ssl_openssl.c
6 files changed, 87 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/49/449/6

diff --git a/Changes.rst b/Changes.rst
index 69c811d..52ff8db 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -1,5 +1,14 @@
 Overview of changes in 2.7
 ==
+New features
+
+TLS alerts
+OpenVPN 2.7 will send out TLS alerts to peers informing them if the TLS
+session shuts down or when the TLS implementation informs the peer about
+an error in the TLS session (e.g. mismatching TLS versions). This improves
+the user experience as the client shows an error instead of running into
+a timeout when the server just stops responding completely.
+
 Deprecated features
 ---
 ``secret`` support has been removed by default.
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 33c8670..f287e4d 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -690,6 +690,9 @@
 case S_ERROR:
 return "S_ERROR";

+case S_ERROR_PRE:
+return "S_ERROR_PRE";
+
 case S_GENERATED_KEYS:
 return "S_GENERATED_KEYS";

@@ -2683,6 +2686,25 @@
 }

 static bool
+check_outgoing_ciphertext(struct key_state *ks, struct tls_session *session,
+  bool *continue_tls_process)
+{
+/* Outgoing Ciphertext to reliable buffer */
+if (ks->state >= S_START)
+{
+struct buffer *buf = 
reliable_get_buf_output_sequenced(ks->send_reliable);
+if (buf)
+{
+if (!write_outgoing_tls_ciphertext(session, continue_tls_process))
+{
+return false;
+}
+}
+}
+return true;
+}
+
+static bool
 tls_process_state(struct tls_multi *multi,
   struct tls_session *session,
   struct buffer *to_link,
@@ -2706,7 +2728,7 @@
 }

 /* Are we timed out on receive? */
-if (now >= ks->must_negotiate && ks->state < S_ACTIVE)
+if (now >= ks->must_negotiate && ks->state >= S_UNDEF && ks->state < 
S_ACTIVE)
 {
 msg(D_TLS_ERRORS,
 "TLS Error: TLS key negotiation failed to occur within %d seconds 
(check your network connectivity)",
@@ -2760,6 +2782,16 @@
 return false;
 }

+if (ks->state == S_ERROR_PRE)
+{
+/* When we end up here, we had one last chance to send an outstanding
+ * packet that contained an alert. We do not ensure that this packet
+ * has been successfully delivered  (ie wait for the ACK etc)
+ * but rather stop processing now */
+ks->state = S_ERROR;
+return false;
+}
+
 /* Write incoming ciphertext to TLS object */
 struct reliable_entry *entry = 
reliable_get_entry_sequenced(ks->rec_reliable);
 if (entry)
@@ -2840,29 +2872,31 @@
 dmsg(D_TLS_DEBUG, "Outgoing Plaintext -> TLS");
 }
 }
-
-/* Outgoing Ciphertext to reliable buffer */
-if (ks->state >= S_START)
+if (!check_outgoing_ciphertext(ks, session, _tls_process))
 {
-buf = reliable_get_buf_output_sequenced(ks->send_reliable);
-if (buf)
-{
-if (!write_outgoing_tls_ciphertext(session, _tls_process))
-{
-goto error;
-}
-}
+goto error;
 }

 return continue_tls_process;
 error:
 tls_clear_error();
-ks->state = S_ERROR;
+
+  

[Openvpn-devel] [M] Change in openvpn[master]: Keep exported certificate files for following calls

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld.

Hello flichtenheld,

I'd like you to do a code review.
Please visit

http://gerrit.openvpn.net/c/openvpn/+/487?usp=email

to review the following change.


Change subject: Keep exported certificate files for following calls
..

Keep exported certificate files for following calls

Since the lifetime of environment variables is quite different, we
need to tie the lifetime of these files to their environment variables
which in turn requires a special function to be called on the removal
of these env variables.

Change-Id: Ic494d43c835220ae71f10e3afbe53db918887370
Signed-off-by: Arne Schwabe 
---
M src/openvpn/ssl_verify.c
1 file changed, 31 insertions(+), 37 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/87/487/1

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 35d3377..ff1a932 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -39,6 +39,7 @@
 #include "run_command.h"
 #include "ssl_verify.h"
 #include "ssl_verify_backend.h"
+#include "platform.h"

 #ifdef ENABLE_CRYPTO_OPENSSL
 #include "ssl_verify_openssl.h"
@@ -459,53 +460,51 @@
 }

 /**
+ * Unlinks a file specified by in the env item that has the form
+ * key=filename.
+ */
+static void
+unlink_file_env(struct env_item *env)
+{
+/* values in env are always x=y */
+const char *filename = strchr(env->string, '=');
+ASSERT(filename);
+
+/* Move just past the = */
+filename += 1;
+
+platform_unlink((const char *) filename);
+}
+
+/**
  * Exports the certificate in \c peer_cert into the environment and adds
  * the filname
  */
 static bool
-verify_cert_cert_export_env(struct env_set *es, openvpn_x509_cert_t *peer_cert,
-int cert_depth, const char *pem_export_fname)
+verify_cert_cert_export_env(const struct tls_options *opt,
+openvpn_x509_cert_t *peer_cert, int cert_depth)
 {
-char envname[64];
-/* Make copy of the filename to manage that copy by the gc_arena */
-char *pem_export_filename = strdup(pem_export_fname);
-
-if (!pem_export_filename)
-{
-return false;
-}
+struct gc_arena gc = gc_new();
+const char *pem_export_filename = 
platform_create_temp_file(opt->export_peer_cert_dir,
+"pef", );
+char envstr[128];

 /* export the path to the certificate in pem file format */
-openvpn_snprintf(envname, sizeof(envname), "peer_cert_%d", cert_depth);
-setenv_str(es, envname, pem_export_filename);
+openvpn_snprintf(envstr, sizeof(envstr), "peer_cert_%d=%s", cert_depth,
+ pem_export_filename);
+setenv_str(opt->es, envstr, pem_export_filename);
+env_set_add_specialfree(opt->es, envstr, _file_env);

 /* compatibility with older scripts/plugins that expect peer_cert without
  * suffix */
 if (cert_depth == 0)
 {
-setenv_str(es, "peer_cert", pem_export_filename);
+setenv_str(opt->es, "peer_cert", pem_export_filename);
 }

 return backend_x509_write_pem(peer_cert, pem_export_filename) == SUCCESS;
 }

-static void
-verify_cert_cert_delete_env(struct env_set *es, int cert_depth,
-const char *pem_export_fname)
-{
-char envname[64];
-openvpn_snprintf(envname, sizeof(envname), "peer_cert_%d", cert_depth);
-env_set_del(es, envname);
-
-/* compatibility with older scripts/plugins that expect peer_cert without
- * suffix */
-if (cert_depth == 0)
-{
-env_set_del(es, "peer_cert");
-}
-unlink(pem_export_fname);
-}
-
 /*
  * call --tls-verify plug-in(s)
  */
@@ -625,7 +624,6 @@
  * them defined */
 result_t ret = FAILURE;
 struct gc_arena gc = gc_new();
-const char *pem_export_fname = NULL;

 const struct tls_options *opt = session->opt;
 ASSERT(opt);
@@ -758,12 +756,9 @@

 if (opt->export_peer_cert_dir)
 {
-pem_export_fname = platform_create_temp_file(opt->export_peer_cert_dir,
- "pef", );

-if (!pem_export_fname
-|| !verify_cert_cert_export_env(opt->es, cert, cert_depth,
-pem_export_fname))
+
+if (!verify_cert_cert_export_env(opt, cert, cert_depth))
 {
 msg(D_TLS_ERRORS, "TLS Error: Failed to export certificate for "
 "--tls-export-cert in %s", opt->export_peer_cert_dir);
@@ -821,7 +816,6 @@
 ret = SUCCESS;

 cleanup:
-verify_cert_cert_delete_env(opt->es, cert_depth, pem_export_fname);
 if (ret != SUCCESS)
 {
 tls_clear_error(); /* always? */

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/487?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn

[Openvpn-devel] [M] Change in openvpn[master]: Implement the --tls-export-cert feature

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: cron2, plaisthos.

Hello cron2, flichtenheld,

I'd like you to reexamine a change. Please visit

http://gerrit.openvpn.net/c/openvpn/+/466?usp=email

to look at the new patch set (#7).


Change subject: Implement the --tls-export-cert feature
..

Implement the --tls-export-cert feature

This is a re-implementation of the --tls-export-cert feature. This
was necessary to due to missing approval to re-license the old
(now removed) code. The re-implementation is based on the following
description of the feature provided by David:

  Add an option to export certificate in PEM format of the remote
  peer to a given directory.

  For example: --tls-export-cert /var/tmp

  This option should use a randomised filename, which is provided via a
  "peer_cert" environment variable for the --tls-verify script or the
  OPENVPN_PLUGIN_TLS_VERIFY plug-in hook.

Once the script or plugin call has completed, OpenVPN should delete
this file.

Change-Id: Ia9b3f1813d2d0d492d17c87348b4cebd0bf19ce2
Signed-off-by: Arne Schwabe 
---
M doc/man-sections/script-options.rst
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/ssl_common.h
M src/openvpn/ssl_verify.c
M src/openvpn/ssl_verify_backend.h
M src/openvpn/ssl_verify_mbedtls.c
M src/openvpn/ssl_verify_openssl.c
9 files changed, 174 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/66/466/7

diff --git a/doc/man-sections/script-options.rst 
b/doc/man-sections/script-options.rst
index 6f90e14..0e60ab5 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -423,6 +423,15 @@
   See the `Environmental Variables`_ section below for additional
   parameters passed as environmental variables.

+--tls-export-cert-path dir
+  Adds an environment variable ``peer_cert_{x}`` (and an alias
+  ``peer_cert`` for ``peer_cert_0`` for compatibility)  when calling the
+  ``--tls-verify`` script or executing the OPENVPN_PLUGIN_TLS_VERIFY plugin
+  hook to verify the certificate.
+
+  The environment variable contains the path to a PEM encoded certificate
+  of the current peer certificate in the directory ``dir``.
+
 --up cmd
   Run command ``cmd`` after successful TUN/TAP device open (pre ``--user``
   UID change).
@@ -763,6 +772,15 @@
 modifier is specified, and deleted from the environment after the script
 returns.

+:code:`peer_cert_{n}`
+If the option ``--tls-export-cert`` is enabled, this option contains
+the path to the current peer certificate to be verified in PEM format
+where ``n`` is the verification level.
+
+:code:`peer_cert`
+Identical to `peer_cert_0` for compatibility with older
+versions.
+
 :code:`proto`
 The ``--proto`` parameter. Set on program initiation and reset on
 SIGHUP.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9e2b3845..917ae33 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3336,6 +3336,7 @@
 to.auth_user_pass_verify_script_via_file = 
options->auth_user_pass_verify_script_via_file;
 to.client_crresponse_script = options->client_crresponse_script;
 to.tmp_dir = options->tmp_dir;
+to.export_peer_cert_dir = options->tls_export_peer_cert_path;
 if (options->ccd_exclusive)
 {
 to.client_config_dir_exclusive = options->client_config_dir;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e498114..714a578 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1986,6 +1986,7 @@
 SHOW_STR(cipher_list_tls13);
 SHOW_STR(tls_cert_profile);
 SHOW_STR(tls_verify);
+SHOW_STR(tls_export_peer_cert_path);
 SHOW_INT(verify_x509_type);
 SHOW_STR(verify_x509_name);
 SHOW_STR_INLINE(crl_file);
@@ -3048,6 +3049,7 @@
 MUST_BE_UNDEF(cipher_list_tls13);
 MUST_BE_UNDEF(tls_cert_profile);
 MUST_BE_UNDEF(tls_verify);
+MUST_BE_UNDEF(tls_export_peer_cert_path);
 MUST_BE_UNDEF(verify_x509_name);
 MUST_BE_UNDEF(tls_timeout);
 MUST_BE_UNDEF(renegotiate_bytes);
@@ -4053,6 +4055,13 @@
 R_OK, "--crl-verify");
 }

+if (options->tls_export_peer_cert_path)
+{
+errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
+ options->tls_export_peer_cert_path,
+ W_OK, "--tls-export-cert");
+}
+
 ASSERT(options->connection_list);
 for (int i = 0; i < options->connection_list->len; ++i)
 {
@@ -8997,6 +9006,11 @@
 string_substitute(p[1], ',', ' ', >gc),
 "tls-verify", true);
 }
+else if (streq(p[0], "tls-export-cert") && p[1] && !p[2])
+{
+VERIFY_PERMISSION(OPT_P_SCRIPT);
+options->tls_export_peer_cert_path = p[1];
+}
 else if (streq(p[0], 

[Openvpn-devel] [M] Change in openvpn[master]: Allow having an extra function that is called when an env is freed

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld.

Hello flichtenheld,

I'd like you to do a code review.
Please visit

http://gerrit.openvpn.net/c/openvpn/+/486?usp=email

to review the following change.


Change subject: Allow having an extra function that is called when an env is 
freed
..

Allow having an extra function that is called when an env is freed

Change-Id: I6899c1af65fbd670d434cdb17f81c82b900bfbd4
Signed-off-by: Arne Schwabe 
---
M CMakeLists.txt
M src/openvpn/env_set.c
M src/openvpn/env_set.h
M tests/unit_tests/openvpn/Makefile.am
M tests/unit_tests/openvpn/test_buffer.c
5 files changed, 106 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/86/486/1

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 96328a1..04e9e34 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -685,6 +685,7 @@

 target_sources(test_buffer PRIVATE
 tests/unit_tests/openvpn/mock_get_random.c
+src/openvpn/env_set.c
 )

 target_sources(test_crypto PRIVATE
diff --git a/src/openvpn/env_set.c b/src/openvpn/env_set.c
index 97b011f..fb7cd98 100644
--- a/src/openvpn/env_set.c
+++ b/src/openvpn/env_set.c
@@ -112,6 +112,10 @@
 }
 if (do_free)
 {
+if (current->free_function)
+{
+current->free_function(current);
+}
 secure_memzero(current->string, strlen(current->string));
 free(current->string);
 free(current);
@@ -124,15 +128,37 @@
 }

 static void
-add_env_item(char *str, const bool do_alloc, struct env_item **list, struct 
gc_arena *gc)
+env_gc_freefunction_wrapper(void *arg)
+{
+struct env_item *ei = arg;
+ei->free_function(ei);
+free(ei->string);
+free(ei);
+}
+
+static void
+add_env_item(char *str, void (*free_function)(struct env_item *),
+ struct env_item **list, struct gc_arena *gc)
 {
 struct env_item *item;

 ASSERT(str);
 ASSERT(list);

-ALLOC_OBJ_GC(item, struct env_item, gc);
-item->string = do_alloc ? string_alloc(str, gc) : str;
+if (gc && free_function)
+{
+/* alloc items manually without gc to allow the gc_wrap_free
+ * function to free them all in the right order */
+ALLOC_OBJ(item, struct env_item);
+item->string = string_alloc(str, NULL);
+gc_addspecial(item, env_gc_freefunction_wrapper, gc);
+}
+else
+{
+ALLOC_OBJ_GC(item, struct env_item, gc);
+item->string = string_alloc(str, gc);
+}
+item->free_function = free_function;
 item->next = *list;
 *list = item;
 }
@@ -146,10 +172,11 @@
 }

 static void
-env_set_add_nolock(struct env_set *es, const char *str)
+env_set_add_nolock(struct env_set *es, const char *str,
+   void (*free_function)(struct env_item *))
 {
 remove_env_item(str, es->gc == NULL, >list);
-add_env_item((char *)str, true, >list, es->gc);
+add_env_item((char *)str, free_function, >list, es->gc);
 }

 struct env_set *
@@ -171,6 +198,10 @@
 while (e)
 {
 struct env_item *next = e->next;
+if (e->free_function)
+{
+e->free_function(e);
+}
 free(e->string);
 free(e);
 e = next;
@@ -194,7 +225,16 @@
 {
 ASSERT(es);
 ASSERT(str);
-env_set_add_nolock(es, str);
+env_set_add_nolock(es, str, NULL);
+}
+
+void
+env_set_add_specialfree(struct env_set *es, const char *str,
+void (*free_function)(struct env_item *))
+{
+ASSERT(es);
+ASSERT(str);
+env_set_add_nolock(es, str, free_function);
 }

 const char *
@@ -246,7 +286,7 @@
 e = src->list;
 while (e)
 {
-env_set_add_nolock(es, e->string);
+env_set_add_nolock(es, e->string, e->free_function);
 e = e->next;
 }
 }
diff --git a/src/openvpn/env_set.h b/src/openvpn/env_set.h
index 4599977..27bc4dd 100644
--- a/src/openvpn/env_set.h
+++ b/src/openvpn/env_set.h
@@ -36,6 +36,7 @@

 struct env_item {
 char *string;
+void (*free_function)(struct env_item *);
 struct env_item *next;
 };

@@ -87,6 +88,11 @@

 void env_set_add(struct env_set *es, const char *str);

+void
+env_set_add_specialfree(struct env_set *es, const char *str,
+void (*free_function)(struct env_item *));
+
+
 const char *env_set_get(const struct env_set *es, const char *name);

 void env_set_print(int msglevel, const struct env_set *es);
diff --git a/tests/unit_tests/openvpn/Makefile.am 
b/tests/unit_tests/openvpn/Makefile.am
index 6667626..c5098a9 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -52,6 +52,7 @@
 buffer_testdriver_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn 
-Wl,--wrap=parse_line
 

[Openvpn-devel] [PATCH v8] Check PRF availability on initialisation and add --force-tls-key-material-export

2024-01-02 Thread Gert Doering
From: Arne Schwabe 

We now warn a user if the TLS 1.0 PRF is not supported by the cryptographic
library of the system. Also add the option --force-tls-key-material-export
that automatically rejects clients that do not support TLS Keying Material
Export and automatically enable it when TLS 1.0 PRF support is not available.

Change-Id: I04f8c7c413e7cb62c726262feee6ca89c7e86c70
Signed-off-by: Arne Schwabe 
Acked-by: Gert Doering 
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/460
This mail reflects revision 8 of this Change.
Acked-by according to Gerrit (reflected above):
Gert Doering 


diff --git a/doc/man-sections/protocol-options.rst 
b/doc/man-sections/protocol-options.rst
index 948c0c8..8b061d2 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -242,3 +242,11 @@
   a key renegotiation begins (default :code:`3600` seconds). This feature
   allows for a graceful transition from old to new key, and removes the key
   renegotiation sequence from the critical path of tunnel data forwarding.
+
+--force-tls-key-material-export
+  This option is only available in --mode server and forces to use
+  Keying Material Exporters (RFC 5705) for clients. This can be used to
+  simulate an environment where the cryptographic library does not support
+  the older method to generate data channel keys anymore. This option is
+  intended to be a test option and might be removed in a future OpenVPN
+  version without notice.
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index e4452d7..8c17f2a 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1789,3 +1789,22 @@
 gc_free();
 return ret;
 }
+
+bool
+check_tls_prf_working(void)
+{
+/* Modern TLS libraries might no longer support the TLS 1.0 PRF with
+ * MD5+SHA1. This allows us to establish connections only
+ * with other 2.6.0+ OpenVPN peers.
+ * Do a simple dummy test here to see if it works. */
+const char *seed = "tls1-prf-test";
+const char *secret = "tls1-prf-test-secret";
+uint8_t out[8];
+uint8_t expected_out[] = { 0xe0, 0x5f, 0x1f, 1, 0, 0, 0, 0};
+
+int ret = ssl_tls1_PRF((uint8_t *)seed, (int) strlen(seed),
+   (uint8_t *)secret, (int) strlen(secret),
+   out, sizeof(out));
+
+return (ret && memcmp(out, expected_out, sizeof(out)) != 0);
+}
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 9255d38..4201524 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -593,4 +593,12 @@
 return kt;
 }
 
+/**
+ * Checks if the current TLS library supports the TLS 1.0 PRF with MD5+SHA1
+ * that OpenVPN uses when TLS Keying Material Export is not available.
+ *
+ * @return  true if supported, false otherwise.
+ */
+bool check_tls_prf_working(void);
+
 #endif /* CRYPTO_H */
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 8b490ed..35e8707 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1830,6 +1830,16 @@
 {
 o->imported_protocol_flags |= CO_USE_TLS_KEY_MATERIAL_EXPORT;
 }
+else if (o->force_key_material_export)
+{
+msg(M_INFO, "PUSH: client does not support TLS key material export"
+"but --force-tls-key-material-export is enabled.");
+auth_set_client_reason(tls_multi, "Client incompatible with this "
+   "server. Keying Material Exporters (RFC 5705) "
+   "support missing. Upgrade to a client that "
+   "supports this feature (OpenVPN 2.6.0+).");
+return false;
+}
 if (proto & IV_PROTO_DYN_TLS_CRYPT)
 {
 o->imported_protocol_flags |= CO_USE_DYNAMIC_TLS_CRYPT;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e498114..1b28a19 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1561,6 +1561,7 @@
 SHOW_STR(auth_user_pass_verify_script);
 SHOW_BOOL(auth_user_pass_verify_script_via_file);
 SHOW_BOOL(auth_token_generate);
+SHOW_BOOL(force_key_material_export);
 SHOW_INT(auth_token_lifetime);
 SHOW_STR_INLINE(auth_token_secret_file);
 #if PORT_SHARE
@@ -2802,6 +2803,11 @@
 {
 msg(M_USAGE, "--vlan-tagging requires --mode server");
 }
+
+if (options->force_key_material_export)
+{
+msg(M_USAGE, "--force-tls-key-material-export requires --mode 
server");
+}
 }
 
 /*
@@ -3634,6 +3640,30 @@
 }
 
 static void
+options_process_mutate_prf(struct options *o)
+{
+if (!check_tls_prf_working())
+{
+msg(D_TLS_ERRORS, "Warning: TLS 1.0 PRF with MD5+SHA1 PRF is not "
+"supported by the TLS library. Your system does not support this "
+"calculation anymore or your security policy (e.g. FIPS 140-2) "
+"forbids it. Connections 

[Openvpn-devel] [M] Change in openvpn[master]: Check PRF availability on initialisation and add --force-tls-key-mate...

2024-01-02 Thread cron2 (Code Review)
Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/460?usp=email )

Change subject: Check PRF availability on initialisation and add 
--force-tls-key-material-export
..


Patch Set 8: Code-Review+2

(1 comment)

Patchset:

PS8:
Looks good now.  Will proceed to send mail and subject this patch to more 
testing.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/460?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I04f8c7c413e7cb62c726262feee6ca89c7e86c70
Gerrit-Change-Number: 460
Gerrit-PatchSet: 8
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 02 Jan 2024 12:51:26 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [XS] Change in openvpn[master]: forked-test-driver: Show test output always

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/479?usp=email )

Change subject: forked-test-driver: Show test output always
..


Patch Set 5: Code-Review-1

(1 comment)

Patchset:

PS5:
The patch does not do what it promises to do. I cherry-picked this patch and 
the other forked-test-driver patch on my branch and in github actions still 
only reports that a unit test has failed without any indication of why:

PASS: argv_testdriver
FAIL: buffer_testdriver
PASS: crypto_testdriver

https://github.com/schwabe/openvpn/actions/runs/7385546211/job/20090514521



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/479?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I11e0091482d9acee89ca018374cb8d96d22f8514
Gerrit-Change-Number: 479
Gerrit-PatchSet: 5
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Tue, 02 Jan 2024 12:41:22 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [L] Change in openvpn[master]: Add test_ssl unit test and test export of PEM to file

2024-01-02 Thread plaisthos (Code Review)
Attention is currently required from: cron2.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/471?usp=email )

Change subject: Add test_ssl unit test and test export of PEM to file
..


Patch Set 3:

(1 comment)

Patchset:

PS3:
> this fails the GHA mingw builds with […]
That is really strange. test_ssl.c includes ssl_verify_backend.h that defines 
that function. I need to see if I can reproduce this.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/471?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ie248d35d063bb6878f3dd42840c77ba0d6fa3381
Gerrit-Change-Number: 471
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: cron2 
Gerrit-Comment-Date: Tue, 02 Jan 2024 11:35:14 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel