[Openvpn-devel] [PATCH] Improve data key id not found error message

[Arne Schwabe]

With delayed data key generation now with deferred auth, NCP and similar
mechanism the "TLS Error: local/remote TLS keys are out of sync" is shown
much too frequent and confuses a lot of people.

This also removes the dead code of printing multi not ready keys and
replace it with an assert.

Factor out printing of error messages into an extra function to make
the code easier to understand and also to only call into that function
in the case that a key is not found and avoid the overhead.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl.c | 74 +++
 1 file changed, 56 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 33e145b3f..6a3473944 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3202,11 +3202,61 @@ nohard:
 return (tas == TLS_AUTHENTICATION_FAILED) ? TLSMP_KILL : active;
 }
 
-/*
- * Pre and post-process the encryption & decryption buffers in order
- * to implement a multiplexed TLS channel over the TCP/UDP port.
+/**
+ * We have not found a matching key to decrypt data channel packet,
+ * try to generate a sensible error message and print it
  */
+static void
+print_key_id_not_found_reason(struct tls_multi *multi,
+  const struct link_socket_actual *from, int 
key_id)
+{
+struct gc_arena gc = gc_new();
+const char *source = print_link_socket_actual(from, &gc);
+
+
+for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+{
+struct key_state *ks = get_key_scan(multi, i);
+/*
+ * Our key state has been progressed far enough to be part of an valid
+ * session but has not generated keys. Since there can
+ * be multiple valid/semi valid key states with key id 0 (especially in
+ * p2p mode when a client reconnects), we still need
+ * to loop through all keys and only remember here that there is at
+ * least one key that is not ready yet*/
+if (ks->state >= S_INITIAL && key_id < S_GENERATED_KEYS)
+{
+msg(D_MULTI_DROPPED,
+"Key %s [%d] not initialized (yet), dropping packet.",
+source, key_id);
+gc_free(&gc);
+return;
+}
+if (ks->state >= S_ACTIVE && ks->authenticated == KS_AUTH_FALSE)
+{
+msg(D_MULTI_DROPPED,
+"Key %s [%d] no longer authorized (yet), dropping packet.",
+source, key_id);
+gc_free(&gc);
+return;
+}
+}
 
+msg(D_TLS_ERRORS,
+"TLS Error: local/remote TLS keys are out of sync: %s "
+"(received key id: %d, known key ids: %s)",
+source, key_id,
+print_key_id(multi, &gc));
+gc_free(&gc);
+}
+
+/**
+ * Check the keyid of the an incoming data channel packet and
+ * return the matching crypto parameters in \c opt if found.
+ * Also move the \c buf to the start of the encrypted data, skipping
+ * the opcode and peer id header and setting also set \c ad_start for
+ * AEAD ciphers to the start of the authenticated data.
+ */
 static inline void
 handle_data_channel_packet(struct tls_multi *multi,
const struct link_socket_actual *from,
@@ -3221,7 +3271,6 @@ handle_data_channel_packet(struct tls_multi *multi,
 int op = c >> P_OPCODE_SHIFT;
 int key_id = c & P_KEY_ID_MASK;
 
-/* data channel packet */
 for (int i = 0; i < KEY_SCAN_SIZE; ++i)
 {
 struct key_state *ks = get_key_scan(multi, i);
@@ -3243,14 +3292,7 @@ handle_data_channel_packet(struct tls_multi *multi,
 && ks->authenticated == KS_AUTH_TRUE
 && (floated || link_socket_actual_match(from, &ks->remote_addr)))
 {
-if (!ks->crypto_options.key_ctx_bi.initialized)
-{
-msg(D_MULTI_DROPPED,
-"Key %s [%d] not initialized (yet), dropping packet.",
-print_link_socket_actual(from, &gc), key_id);
-goto done;
-}
-
+ASSERT(ks->crypto_options.key_ctx_bi.initialized);
 /* return appropriate data channel decrypt key in opt */
 *opt = &ks->crypto_options;
 if (op == P_DATA_V2)
@@ -3284,17 +3326,13 @@ handle_data_channel_packet(struct tls_multi *multi,
 }
 }
 
-msg(D_TLS_ERRORS,
-"TLS Error: local/remote TLS keys are out of sync: %s "
-"(received key id: %d, known key ids: %s)",
-print_link_socket_actual(from, &gc), key_id,
-print_key_id(multi, &gc));
+print_key_id_not_found_reason(multi, from, key_id);
 
 done:
+gc_free(&gc);
 tls_clear_error();
 buf->len = 0;
 *opt = NULL;
-gc_free(&gc);
 }
 
 /*
-- 
2.32.1 (Apple Git-133)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Improve data key id not found error message

[Frank Lichtenheld]

On Wed, Aug 24, 2022 at 11:37:23AM +0200, Arne Schwabe wrote:
> With delayed data key generation now with deferred auth, NCP and similar
> mechanism the "TLS Error: local/remote TLS keys are out of sync" is shown
> much too frequent and confuses a lot of people.
> 
> This also removes the dead code of printing multi not ready keys and
> replace it with an assert.
> 
> Factor out printing of error messages into an extra function to make
> the code easier to understand and also to only call into that function
> in the case that a key is not found and avoid the overhead.

Okay, I have to say that this patch confuses me. Might be my limited
understanding of the involved structures, though.

> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 33e145b3f..6a3473944 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -3202,11 +3202,61 @@ nohard:
>  return (tas == TLS_AUTHENTICATION_FAILED) ? TLSMP_KILL : active;
>  }
>  
> -/*
> - * Pre and post-process the encryption & decryption buffers in order
> - * to implement a multiplexed TLS channel over the TCP/UDP port.
> +/**
> + * We have not found a matching key to decrypt data channel packet,
> + * try to generate a sensible error message and print it
>   */
> +static void
> +print_key_id_not_found_reason(struct tls_multi *multi,
> +  const struct link_socket_actual *from, int 
> key_id)
> +{
> +struct gc_arena gc = gc_new();
> +const char *source = print_link_socket_actual(from, &gc);
> +
> +
> +for (int i = 0; i < KEY_SCAN_SIZE; ++i)
> +{
> +struct key_state *ks = get_key_scan(multi, i);
> +/*
> + * Our key state has been progressed far enough to be part of an 
> valid
> + * session but has not generated keys. Since there can
> + * be multiple valid/semi valid key states with key id 0 (especially 
> in
> + * p2p mode when a client reconnects), we still need
> + * to loop through all keys and only remember here that there is at
> + * least one key that is not ready yet*/
> +if (ks->state >= S_INITIAL && key_id < S_GENERATED_KEYS)

Why are you comparing key_id to a state value? Shouldn't you compare
ks->state to S_GENERATED_KEYS and ks->key_id to the key_id?

> +{
> +msg(D_MULTI_DROPPED,
> +"Key %s [%d] not initialized (yet), dropping packet.",
> +source, key_id);
> +gc_free(&gc);
> +return;
> +}
> +if (ks->state >= S_ACTIVE && ks->authenticated == KS_AUTH_FALSE)

While here we do check key_id at all?

> +{
> +msg(D_MULTI_DROPPED,
> +"Key %s [%d] no longer authorized (yet), dropping packet.",
> +source, key_id);
> +gc_free(&gc);
> +return;
> +}
> +}


Regards,
-- 
  Frank Lichtenheld


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Improve data key id not found error message

[Arne Schwabe]

With delayed data key generation now with deferred auth, NCP and similar
mechanism the "TLS Error: local/remote TLS keys are out of sync" is shown
much too frequent and confuses a lot of people.

This also removes the dead code of printing multi not ready keys and
replace it with an assert.

Factor out printing of error messages into an extra function to make
the code easier to understand and also to only call into that function
in the case that a key is not found and avoid the overhead.

Patch v2: fix comparing key_id to state value, improve message

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl.c | 75 +++
 1 file changed, 57 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 33e145b3f..8c95a945f 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3202,11 +3202,62 @@ nohard:
 return (tas == TLS_AUTHENTICATION_FAILED) ? TLSMP_KILL : active;
 }
 
-/*
- * Pre and post-process the encryption & decryption buffers in order
- * to implement a multiplexed TLS channel over the TCP/UDP port.
+/**
+ * We have not found a matching key to decrypt data channel packet,
+ * try to generate a sensible error message and print it
  */
+static void
+print_key_id_not_found_reason(struct tls_multi *multi,
+  const struct link_socket_actual *from, int 
key_id)
+{
+struct gc_arena gc = gc_new();
+const char *source = print_link_socket_actual(from, &gc);
+
+
+for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+{
+struct key_state *ks = get_key_scan(multi, i);
+/*
+ * Our key state has been progressed far enough to be part of an valid
+ * session but has not generated keys. Since there can
+ * be multiple valid/semi valid key states with key id 0 (especially in
+ * p2p mode when a client reconnects), we still need
+ * to loop through all keys and only remember here that there is at
+ * least one key that is not ready yet*/
+if (ks->state >= S_INITIAL && ks->state < S_GENERATED_KEYS)
+{
+msg(D_MULTI_DROPPED,
+"Key %s [%d] not initialized (yet), dropping packet.",
+source, key_id);
+gc_free(&gc);
+return;
+}
+if (ks->state >= S_ACTIVE && ks->authenticated != KS_AUTH_TRUE)
+{
+msg(D_MULTI_DROPPED,
+"Key %s [%d] not authorized%s, dropping packet.",
+source, key_id,
+(ks->authenticated == KS_AUTH_DEFERRED) ? " (deferred)" : "");
+gc_free(&gc);
+return;
+}
+}
 
+msg(D_TLS_ERRORS,
+"TLS Error: local/remote TLS keys are out of sync: %s "
+"(received key id: %d, known key ids: %s)",
+source, key_id,
+print_key_id(multi, &gc));
+gc_free(&gc);
+}
+
+/**
+ * Check the keyid of the an incoming data channel packet and
+ * return the matching crypto parameters in \c opt if found.
+ * Also move the \c buf to the start of the encrypted data, skipping
+ * the opcode and peer id header and setting also set \c ad_start for
+ * AEAD ciphers to the start of the authenticated data.
+ */
 static inline void
 handle_data_channel_packet(struct tls_multi *multi,
const struct link_socket_actual *from,
@@ -3221,7 +3272,6 @@ handle_data_channel_packet(struct tls_multi *multi,
 int op = c >> P_OPCODE_SHIFT;
 int key_id = c & P_KEY_ID_MASK;
 
-/* data channel packet */
 for (int i = 0; i < KEY_SCAN_SIZE; ++i)
 {
 struct key_state *ks = get_key_scan(multi, i);
@@ -3243,14 +3293,7 @@ handle_data_channel_packet(struct tls_multi *multi,
 && ks->authenticated == KS_AUTH_TRUE
 && (floated || link_socket_actual_match(from, &ks->remote_addr)))
 {
-if (!ks->crypto_options.key_ctx_bi.initialized)
-{
-msg(D_MULTI_DROPPED,
-"Key %s [%d] not initialized (yet), dropping packet.",
-print_link_socket_actual(from, &gc), key_id);
-goto done;
-}
-
+ASSERT(ks->crypto_options.key_ctx_bi.initialized);
 /* return appropriate data channel decrypt key in opt */
 *opt = &ks->crypto_options;
 if (op == P_DATA_V2)
@@ -3284,17 +3327,13 @@ handle_data_channel_packet(struct tls_multi *multi,
 }
 }
 
-msg(D_TLS_ERRORS,
-"TLS Error: local/remote TLS keys are out of sync: %s "
-"(received key id: %d, known key ids: %s)",
-print_link_socket_actual(from, &gc), key_id,
-print_key_id(multi, &gc));
+print_key_id_not_found_reason(multi, from, key_id);
 
 done:
+gc_free(&gc);
 tls_clear_error();
 buf->len = 0;
 *opt = NULL;
-gc_free(&gc);
 }
 
 /*
-- 
2.32.1 (Apple Git-133)



___
Openvpn-devel mailing list
Openvpn-deve

[Openvpn-devel] Summary of the community meeting (24th August 2022)

[Samuli Seppänen]

Hi,

Here's the summary of the IRC meeting.

---

COMMUNITY MEETING

Place: #openvpn-meeting on libera.chat
Date: Wed 24th August 2022
Time: 10:30 CEST (9:30 UTC)

Planned meeting topics for this meeting were here:



Your local meeting time is easy to check from services such as



SUMMARY

d12fk, dazo, djpig, kp, lev, mattock, MaxF, ordex, Pippin and plaisthos 
participated in this meeting.


---

Agreed that ommunity services migration tomorrow (Thursday) during EET 
working time (9-17) is ok. Mattock will take care of it.


---

Talked about the hackathon. It seems like November 26th is the best 
option right now.


---

Talked about OpenVPN 2.5. The next release, which is not urgent, should 
include an updated tap-windows6 driver.


---

Talked about OpenVPN 2.6. Lev had dco-win pretty comprehensively tested 
by OpenVPN Inc. internal QA. A report of the results has been sent to 
the mailing list. On the Linux dco side some changes were made which 
required simultaneous userspace (openvpn) and kernel-space (ovpn-dco) 
changes to maintain compatibility.


Set the tentative release schedule for first 2.6 RC to mid-September, 
which would make 2.6.0 November 1st a possibility.


--

Full chatlog attached
(11:30:44) ordex: ay
(11:30:52) MaxF: hi!
(11:30:57) mattock: hi!
(11:33:18) mattock: who do we have here today?
(11:33:41) dazo: o/
(11:33:46) mattock: https://community.openvpn.net/openvpn/wiki/Topics-2022-08-24
(11:33:50) lev__: guten tag
(11:34:00) mattock: I have one topic I want to discuss before I head to my 
customary lunch
(11:34:23) mattock: "Community services migration tomorrow (Thursday) during 
EET working time (9-17) is ok?"
(11:34:32) mattock: more details on the topic page
(11:35:02) mattock: trac will be affected briefly tomorrow, otherwise you 
(=developers) should not notice anything
(11:35:06) mattock: will/would
(11:35:22) ordex: fine with me, as long as we have some time to double check 
that things are working before you disappear :D
(11:35:36) mattock: I've allocated a full day (plus Friday) for fixing any 
issues
(11:36:03) mattock: I can typically focus on Thursdays from morning to late 
evening
(11:36:10) dazo: LGTM; and I agree with ordex
(11:36:14) ordex: :-D
(11:36:16) mattock: +1
(11:36:52) mattock: hackathon?
(11:36:54) d12fk: ooh late, hi
(11:37:05) mattock: hi!
(11:37:28) MaxF: hackathon! Has everyone responded to the poll?
(11:37:36) mattock: I have not!
(11:37:38) mattock: forgot
(11:37:40) mattock: link?
(11:37:43) mattock: I'll do it now
(11:38:06) Pippin_: https://doodle.com/meeting/participate/id/dRgEwERe
(11:38:07) vpnHelper: Title: Doodle (at doodle.com)
(11:38:15) mattock: thanks!
(11:40:12) mattock: done
(11:40:41) MaxF: looks like nov 26 is the least worst
(11:40:58) mattock: that late we could all stay for Christmas :)
(11:41:38) dazo: yeah ... that date might mean I need to leave late on Saturday 
 I'll see what I can manage
(11:42:40) mattock: Sync up on OpenVPN 2.5 and 2.6? 
(11:42:42) MaxF: is this everyone? what about corp people?
(11:45:17) plaisthos: Hmpf I still don't see the overview
(11:45:59) dazo: MaxF: Charlie, James, Johan and Mark are the corp folks 
missing ... I'll follow up internally, but I'd say the community folks are more 
important in this context
(11:47:14) plaisthos: argh and now I removed my choices
(11:47:15) MaxF: the main point is, I need to have a list with everyone's name 
on it, and if you're not on it, I can't let you in
(11:47:43) ordex: by when is this required?
(11:48:17) MaxF: not sure, but if we settle on a date in November, I don't need 
it right now
(11:49:20) plaisthos: yeah I think we can get that very soon after we finalise 
the date
(11:49:26) d12fk: can we fix the date today?
(11:51:21) plaisthos: yeah I would say we should
(11:53:29) dazo: +1
(11:54:24) MaxF: nov 26 looks the best with one "cannot attend" and one "if 
need be" (dazo might leave on Saturday evening)
(11:54:40) MaxF: nov 12 has one "cannot attend" and two "if need be"
(11:55:07) dazo: yeah ... Just checked flights and trains, if I need to leave 
on Sat, I need to fetch a train 17:24 from Delft
(11:55:21) ordex: we don't have much choice then, unless we ask the "if need 
be" on the 12 if they can still make it to the whole meting
(11:55:28) dazo: and arrive 12:00-12:30-ish
(11:55:29) ordex: then 12 would be better to we really have everybody for the 
whole meeting
(11:55:29) MaxF: that sounds pretty close to cannot attend
(11:55:52) dazo: (arrive on Friday)
(11:56:22) ordex: still stretched I'd say. what about asking cron2__ and kp 
their plan on Nov 12th? maybe they can still do it reasonably
(11:56:23) MaxF: I haven't heard from the "if need be"s on nov 12. If there's 
*two* people who have to leave at 17.00, that's worse
(11:56:38) dazo: 12th has 2 "need be", 26th has 1
(11:56:44) ordex: MaxF: it may be they don't favour that date, but still c

[Openvpn-devel] [PATCH] Add OpenSSL 3.0 to mingw build

[Arne Schwabe]

This also updates the host system to ubuntu 22.04 and remove the ovpn-dco-win
checkout as we now include the required headers in our own repository.

Signed-off-by: Arne Schwabe 
---
 .github/workflows/build.yaml | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index b0f67a785..f2472fdcf 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -39,31 +39,25 @@ jobs:
 strategy:
   fail-fast: false
   matrix:
+osslver: [1.1.1q, 3.0.5]
+target: [mingw64, mingw]
 include:
   - target: mingw64
 chost: x86_64-w64-mingw32
   - target: mingw
 chost: i686-w64-mingw32
 
-name: "gcc-mingw - ${{matrix.target}}"
-
-runs-on: ubuntu-20.04
+name: "gcc-mingw - ${{matrix.target}} - OSSL ${{ matrix.osslver }}"
+runs-on: ubuntu-22.04
 env:
   MAKEFLAGS: -j3
   LZO_VERSION: "2.10"
   PKCS11_HELPER_VERSION: "1.29.0"
-  OPENSSL_VERSION: "1.1.1n"
+  OPENSSL_VERSION: "${{ matrix.osslver }}"
   TAP_WINDOWS_VERSION: "9.23.3"
-  CHOST: ${{ matrix.chost }}
-  TARGET: ${{ matrix.target }}
 steps:
   - name: Install dependencies
 run: sudo apt update && sudo apt install -y mingw-w64 libtool automake 
autoconf man2html unzip
-  - name: Checkout ovpn-dco-win
-uses: actions/checkout@v3
-with:
-  repository: OpenVPN/ovpn-dco-win
-  path: ovpn-dco-win
   - name: Checkout OpenVPN
 uses: actions/checkout@v3
 with:
@@ -78,7 +72,7 @@ jobs:
 uses: actions/cache@v3
 with:
   path: '~/mingw/'
-  key: ${{ matrix.target }}-mingw-${{ env.OPENSSL_VERSION }}-${{ 
env.LZO_VERSION }}-${{ env.PKCS11_HELPER_VERSION }}-${{ env.TAP_WINDOWS_VERSION 
}}
+  key: ${{ matrix.target }}-mingw-${{ matrix.osslver }}-${{ 
env.LZO_VERSION }}-${{ env.PKCS11_HELPER_VERSION }}-${{ env.TAP_WINDOWS_VERSION 
}}
 
   # Repeating  if: steps.cache.outputs.cache-hit != 'true'
   # on every step for building dependencies is ugly but
@@ -90,15 +84,15 @@ jobs:
   wget -c -P download-cache/ 
"https://build.openvpn.net/downloads/releases/tap-windows-${TAP_WINDOWS_VERSION}.zip";
   wget -c -P download-cache/ 
"https://www.oberhumer.com/opensource/lzo/download/lzo-${LZO_VERSION}.tar.gz";
   wget -c -P download-cache/ 
"https://github.com/OpenSC/pkcs11-helper/releases/download/pkcs11-helper-${PKCS11_HELPER_VERSION}/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2";
-  wget -c -P download-cache/ 
"https://www.openssl.org/source/openssl-${OPENSSL_VERSION}.tar.gz";
   tar jxf 
"download-cache/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2"
+  wget -c -P download-cache/ 
"https://www.openssl.org/source/old/1.1.1/openssl-${OPENSSL_VERSION}.tar.gz"; || 
wget -c -P download-cache/ 
"https://www.openssl.org/source/openssl-${OPENSSL_VERSION}.tar.gz"; 
   tar zxf "download-cache/openssl-${OPENSSL_VERSION}.tar.gz"
   tar zxf "download-cache/lzo-${LZO_VERSION}.tar.gz"
   unzip download-cache/tap-windows-${TAP_WINDOWS_VERSION}.zip
 
   - name: Configure OpenSSL
 if: steps.cache.outputs.cache-hit != 'true'
-run: ./Configure --cross-compile-prefix=${CHOST}- shared ${{ 
matrix.target }} no-capieng --prefix="${HOME}/mingw/opt" 
--openssldir="${HOME}/mingw/opt" -static-libgcc
+run: ./Configure --cross-compile-prefix=${{ matrix.chost }}- shared 
${{ matrix.target }} no-capieng --prefix="${HOME}/mingw/opt" 
--openssldir="${HOME}/mingw/opt" -static-libgcc
 working-directory: "./openssl-${{ env.OPENSSL_VERSION }}"
 
   - name: Build OpenSSL
@@ -106,6 +100,9 @@ jobs:
 run: make
 working-directory: "./openssl-${{ env.OPENSSL_VERSION }}"
 
+  # OpenSSL 3.0.5 installs itself into mingw/opt/lib64 instead of
+  # mingw/opt/lib, so we include both dirs in the following steps
+  # (pkcs11-helper and OpenVPN) so the libraries will be found
   - name: Install OpenSSL
 if: steps.cache.outputs.cache-hit != 'true'
 run: make install
@@ -118,7 +115,7 @@ jobs:
 
   - name: configure pkcs11-helper
 if: steps.cache.outputs.cache-hit != 'true'
-run: OPENSSL_LIBS="-L${HOME}/mingw/opt/lib -lssl -lcrypto" 
OPENSSL_CFLAGS=-I$HOME/mingw/opt/include 
PKG_CONFIG_PATH=${HOME}/mingw/opt/lib/pkgconfig ./configure --host=${CHOST} 
--program-prefix='' --libdir=${HOME}/mingw/opt/lib --prefix=${HOME}/mingw/opt 
--build=x86_64-pc-linux-gnu --disable-crypto-engine-gnutls 
--disable-crypto-engine-nss --disable-crypto-engine-polarssl 
--disable-crypto-engine-mbedtls
+run: OPENSSL_LIBS="-L${HOME}/mingw/opt/lib -L${HOME}/mingw/opt/lib64 
-lssl -lcrypto" OPENSSL_CFLAGS=-I$HOME/mingw/opt/include 
PKG_CONFIG_PATH=${HOME}/mingw/opt/lib/pkgconfig ./configure --host=${{ 
matrix.chost }} --prog

[Openvpn-devel] [PATCH v5] Implement --client-crresponse script options and plugin interface

[Arne Schwabe]

This is allows scripts and pluginsto parse/react to a CR_RESPONSE message

Patch V2: doc fixes, do not put script under ENABLE_PLUGIN
Patch V3: rebase
Patch V4: fix else branch of the verify_crresponse_script function
Patch V5: unify message when unable to create/write crresponse file

Signed-off-by: Arne Schwabe 
---
 doc/man-sections/script-options.rst | 28 +++-
 include/openvpn-plugin.h.in |  7 ++-
 src/openvpn/init.c  |  1 +
 src/openvpn/options.c   | 15 +++
 src/openvpn/options.h   |  1 +
 src/openvpn/plugin.c|  5 ++-
 src/openvpn/push.c  |  4 ++
 src/openvpn/ssl_common.h|  1 +
 src/openvpn/ssl_verify.c| 67 +
 src/openvpn/ssl_verify.h| 23 ++
 10 files changed, 148 insertions(+), 4 deletions(-)

diff --git a/doc/man-sections/script-options.rst 
b/doc/man-sections/script-options.rst
index 6be0686d7..74c6a1fc6 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -52,6 +52,11 @@ Script Order of Execution
Executed in ``--mode server`` mode on new client connections, when the
client is still untrusted.
 
+#. ``--client-crresponse``
+
+Execute in ``--mode server`` whenever a client sends a
+:code:`CR_RESPONSE` message
+
 SCRIPT HOOKS
 
 
@@ -72,7 +77,7 @@ SCRIPT HOOKS
   double-quoted and/or escaped using a backslash, and should be separated
   by one or more spaces.
 
-  If ``method`` is set to :code:`via-env`, OpenVPN will call ``script``
+  If ``method`` is set to :code:`via-env`, OpenVPN will call ``cmd``
   with the environmental variables :code:`username` and :code:`password`
   set to the username/password strings provided by the client. *Beware*
   that this method is insecure on some platforms which make the environment
@@ -80,7 +85,7 @@ SCRIPT HOOKS
 
   If ``method`` is set to :code:`via-file`, OpenVPN will write the username
   and password to the first two lines of a temporary file. The filename
-  will be passed as an argument to ``script``, and the file will be
+  will be passed as an argument to ``cmd``, and the file will be
   automatically deleted by OpenVPN after the script returns. The location
   of the temporary file is controlled by the ``--tmp-dir`` option, and
   will default to the current directory if unspecified. For security,
@@ -123,6 +128,25 @@ SCRIPT HOOKS
   For a sample script that performs PAM authentication, see
   :code:`sample-scripts/auth-pam.pl` in the OpenVPN source distribution.
 
+--client-crresponse
+Executed when the client sends a text based challenge response.
+
+Valid syntax:
+::
+
+client-crresponse cmd
+
+  OpenVPN will write the response of the client into a temporary file.
+  The filename will be passed as an argument to ``cmd``, and the file will be
+  automatically deleted by OpenVPN after the script returns.
+
+  The response is passed as is from the client. The script needs to check
+  itself if the input is valid, e.g. if the input is valid base64 encoding.
+
+  The script can either directly write the result of the verification to
+  :code:`auth_control_file or further defer it. See ``--auth-user-pass-verify``
+  for details.
+
 --client-connect cmd
   Run command ``cmd`` on client connection.
 
diff --git a/include/openvpn-plugin.h.in b/include/openvpn-plugin.h.in
index dc7c5306f..e498f94db 100644
--- a/include/openvpn-plugin.h.in
+++ b/include/openvpn-plugin.h.in
@@ -83,6 +83,10 @@ extern "C" {
  * FUNC: openvpn_plugin_func_v1 OPENVPN_PLUGIN_CLIENT_CONNECT_V2
  * FUNC: openvpn_plugin_func_v1 OPENVPN_PLUGIN_LEARN_ADDRESS
  *
+ * The OPENVPN_PLUGIN_CLIENT_CRRESPONSE function is called when the client 
sends
+ * the CR_RESPONSE message, this is *typically* after OPENVPN_PLUGIN_TLS_FINAL
+ * but may also occur much later.
+ *
  * [Client session ensues]
  *
  * For each "TLS soft reset", according to reneg-sec option (or similar):
@@ -128,7 +132,8 @@ extern "C" {
 #define OPENVPN_PLUGIN_ROUTE_PREDOWN12
 #define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER 13
 #define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2  14
-#define OPENVPN_PLUGIN_N15
+#define OPENVPN_PLUGIN_CLIENT_CRRESPONSE15
+#define OPENVPN_PLUGIN_N16
 
 /*
  * Build a mask out of a set of plug-in types.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 1da21710c..f90867e0a 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3033,6 +3033,7 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 
 to.auth_user_pass_verify_script = options->auth_user_pass_verify_script;
 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;
 if (options->ccd_exclusive)
 {
diff --git a/src/openvpn/options.c b/src/openvp

Re: [Openvpn-devel] [PATCH] Add OpenSSL 3.0 to mingw build

[Илья Шипицин]

if this is not too late, can we add --libdir=mingw/opt/lib to keep current
behaviour ?


+  # OpenSSL 3.0.5 installs itself into mingw/opt/lib64 instead of
+  # mingw/opt/lib, so we include both dirs in the following steps
+  # (pkcs11-helper and OpenVPN) so the libraries will be found

ср, 24 авг. 2022 г. в 15:55, Arne Schwabe :

> This also updates the host system to ubuntu 22.04 and remove the
> ovpn-dco-win
> checkout as we now include the required headers in our own repository.
>
> Signed-off-by: Arne Schwabe 
> ---
>  .github/workflows/build.yaml | 31 ++-
>  1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
> index b0f67a785..f2472fdcf 100644
> --- a/.github/workflows/build.yaml
> +++ b/.github/workflows/build.yaml
> @@ -39,31 +39,25 @@ jobs:
>  strategy:
>fail-fast: false
>matrix:
> +osslver: [1.1.1q, 3.0.5]
> +target: [mingw64, mingw]
>  include:
>- target: mingw64
>  chost: x86_64-w64-mingw32
>- target: mingw
>  chost: i686-w64-mingw32
>
> -name: "gcc-mingw - ${{matrix.target}}"
> -
> -runs-on: ubuntu-20.04
> +name: "gcc-mingw - ${{matrix.target}} - OSSL ${{ matrix.osslver }}"
> +runs-on: ubuntu-22.04
>  env:
>MAKEFLAGS: -j3
>LZO_VERSION: "2.10"
>PKCS11_HELPER_VERSION: "1.29.0"
> -  OPENSSL_VERSION: "1.1.1n"
> +  OPENSSL_VERSION: "${{ matrix.osslver }}"
>TAP_WINDOWS_VERSION: "9.23.3"
> -  CHOST: ${{ matrix.chost }}
> -  TARGET: ${{ matrix.target }}
>  steps:
>- name: Install dependencies
>  run: sudo apt update && sudo apt install -y mingw-w64 libtool
> automake autoconf man2html unzip
> -  - name: Checkout ovpn-dco-win
> -uses: actions/checkout@v3
> -with:
> -  repository: OpenVPN/ovpn-dco-win
> -  path: ovpn-dco-win
>- name: Checkout OpenVPN
>  uses: actions/checkout@v3
>  with:
> @@ -78,7 +72,7 @@ jobs:
>  uses: actions/cache@v3
>  with:
>path: '~/mingw/'
> -  key: ${{ matrix.target }}-mingw-${{ env.OPENSSL_VERSION }}-${{
> env.LZO_VERSION }}-${{ env.PKCS11_HELPER_VERSION }}-${{
> env.TAP_WINDOWS_VERSION }}
> +  key: ${{ matrix.target }}-mingw-${{ matrix.osslver }}-${{
> env.LZO_VERSION }}-${{ env.PKCS11_HELPER_VERSION }}-${{
> env.TAP_WINDOWS_VERSION }}
>
># Repeating  if: steps.cache.outputs.cache-hit != 'true'
># on every step for building dependencies is ugly but
> @@ -90,15 +84,15 @@ jobs:
>wget -c -P download-cache/ "
> https://build.openvpn.net/downloads/releases/tap-windows-${TAP_WINDOWS_VERSION}.zip
> "
>wget -c -P download-cache/ "
> https://www.oberhumer.com/opensource/lzo/download/lzo-${LZO_VERSION}.tar.gz
> "
>wget -c -P download-cache/ "
> https://github.com/OpenSC/pkcs11-helper/releases/download/pkcs11-helper-${PKCS11_HELPER_VERSION}/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2
> "
> -  wget -c -P download-cache/ "
> https://www.openssl.org/source/openssl-${OPENSSL_VERSION}.tar.gz";
>tar jxf
> "download-cache/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2"
> +  wget -c -P download-cache/ "
> https://www.openssl.org/source/old/1.1.1/openssl-${OPENSSL_VERSION}.tar.gz";
> || wget -c -P download-cache/ "
> https://www.openssl.org/source/openssl-${OPENSSL_VERSION}.tar.gz";
>tar zxf "download-cache/openssl-${OPENSSL_VERSION}.tar.gz"
>tar zxf "download-cache/lzo-${LZO_VERSION}.tar.gz"
>unzip download-cache/tap-windows-${TAP_WINDOWS_VERSION}.zip
>
>- name: Configure OpenSSL
>  if: steps.cache.outputs.cache-hit != 'true'
> -run: ./Configure --cross-compile-prefix=${CHOST}- shared ${{
> matrix.target }} no-capieng --prefix="${HOME}/mingw/opt"
> --openssldir="${HOME}/mingw/opt" -static-libgcc
> +run: ./Configure --cross-compile-prefix=${{ matrix.chost }}-
> shared ${{ matrix.target }} no-capieng --prefix="${HOME}/mingw/opt"
> --openssldir="${HOME}/mingw/opt" -static-libgcc
>  working-directory: "./openssl-${{ env.OPENSSL_VERSION }}"
>
>- name: Build OpenSSL
> @@ -106,6 +100,9 @@ jobs:
>  run: make
>  working-directory: "./openssl-${{ env.OPENSSL_VERSION }}"
>
> +  # OpenSSL 3.0.5 installs itself into mingw/opt/lib64 instead of
> +  # mingw/opt/lib, so we include both dirs in the following steps
> +  # (pkcs11-helper and OpenVPN) so the libraries will be found
>- name: Install OpenSSL
>  if: steps.cache.outputs.cache-hit != 'true'
>  run: make install
> @@ -118,7 +115,7 @@ jobs:
>
>- name: configure pkcs11-helper
>  if: steps.cache.outputs.cache-hit != 'true'
> -run: OPENSSL_LIBS="-L${HOME}/mingw/opt/lib -lssl -lcrypto"
> OPENSSL_CFLAGS=-I$HOME/mingw/opt/includ

Re: [Openvpn-devel] [PATCH v2] Improve data key id not found error message

[Frank Lichtenheld]

On Wed, Aug 24, 2022 at 12:46:07PM +0200, Arne Schwabe wrote:
> With delayed data key generation now with deferred auth, NCP and similar
> mechanism the "TLS Error: local/remote TLS keys are out of sync" is shown
> much too frequent and confuses a lot of people.
> 
> This also removes the dead code of printing multi not ready keys and
> replace it with an assert.
> 
> Factor out printing of error messages into an extra function to make
> the code easier to understand and also to only call into that function
> in the case that a key is not found and avoid the overhead.
> 
> Patch v2: fix comparing key_id to state value, improve message

Okay, less confusing. But I still don't understand why we loop over all keys
without checking the ks->key_id?

Regards,
-- 
  Frank Lichtenheld


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/2] uncrustify: remove newlines after return type of function prototype

[Frank Lichtenheld]

On Fri, Aug 19, 2022 at 12:18:06PM +0200, Gert Doering wrote:
> Hi,
[...] 
> It would be cool if uncrustify had a magic flag for that, like "if 
> return type + function name is < 40 characters, put on a single line,
> and if over, split" :-)

Pretty sure there is nothing like that. So should we mark this patch
just as rejected in patchwork and move on?

> Am I making sense?

I understand your reasoning. I wouldn't agree that it is worth clinging
to a style that can't be enforced just because it is slightly prettier.
But definitely not going to waste further time arguing about this
specific bikeshed ;)

Regards,
-- 
  Frank Lichtenheld


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Add OpenSSL 3.0 to mingw build

[Arne Schwabe]

Am 24.08.22 um 13:26 schrieb Илья Шипицин:
if this is not too late, can we add --libdir=mingw/opt/lib to keep 
current behaviour ?



+      # OpenSSL 3.0.5 installs itself into mingw/opt/lib64 instead of
+      # mingw/opt/lib, so we include both dirs in the following steps
+      # (pkcs11-helper and OpenVPN) so the libraries will be found



We could but I am not sure why we should force it to be in the other 
directory and deviate from the default of how the libraries are build.


Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Add OpenSSL 3.0 to mingw build

[Илья Шипицин]

It reverts 3.0 behaviour to 1.1.1
However --libdir is available for 1.1.1 as well

On Wed, Aug 24, 2022, 5:27 PM Arne Schwabe  wrote:

> Am 24.08.22 um 13:26 schrieb Илья Шипицин:
> > if this is not too late, can we add --libdir=mingw/opt/lib to keep
> > current behaviour ?
> >
> >
> > +  # OpenSSL 3.0.5 installs itself into mingw/opt/lib64 instead of
> > +  # mingw/opt/lib, so we include both dirs in the following steps
> > +  # (pkcs11-helper and OpenVPN) so the libraries will be found
> >
>
> We could but I am not sure why we should force it to be in the other
> directory and deviate from the default of how the libraries are build.
>
> Arne
>
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2 3/4] Implement AUTH_FAIL, TEMP message support

[Arne Schwabe]

diff --git a/doc/man-sections/script-options.rst


This is more related to 4/4 and should go there for code archaeology reasons.


+/* the server can suggest a backoff time to the client, it
+ * will still be capped by the max timeout between connections
+ * (300s by default) */
+int server_backoff_time;


This should be (parsed as) an unsigned value. Negative backup requires too
advanced physics. ;-)


*All* other connect related settings (connect_retry_seconds, 
connect_retry_seconds_max, connect_timeout, ...) are int. Making this

single one unsigned int means mixing integer types and having to cast
one parameter of max_int(a, b), so I would rather be consistent in 
having all the same type.






+++ b/src/openvpn/options_util.h
\ No newline at end of file


Again


uncrustify does not complain about that. Will add manually




diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 1c4e637e4..3b3dce642 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -51,7 +52,6 @@ void
  receive_auth_failed(struct context *c, const struct buffer *buffer)
  {
  msg(M_VERB0, "AUTH: Received control message: %s", BSTR(buffer));
-c->options.no_advance = true;

  if (!c->options.pull)
  {


I think we want to keep this as the default. If I read the code correctly, at
the moment the default behavior is "advance addr" if none is given, which feel
wrong to me, given what I have read about the feature.



That looks like a mistake

Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] Implement AUTH_FAIL, TEMP message support

[Arne Schwabe]

This allows a server to indicate a temporary problem on the server and
allows the server to indicate how to proceed (i.e. move to the next server,
retry the same server, wait a certain time,...)

This adds options_utils.c/h to be able to unit test the new function.

Patch v2: Improve documentation, format man page better, comment that
  protocol-flags is not a user usable option.

Patch v3: cleanup parse_auth_failed_temp to use a simple const string
  instead of a buffer

Signed-off-by: Arne Schwabe 
---
 doc/man-sections/script-options.rst  |  36 ++
 src/openvpn/Makefile.am  |   1 +
 src/openvpn/init.c   |   9 ++-
 src/openvpn/openvpn.vcxproj  |   2 +
 src/openvpn/openvpn.vcxproj.filters  |   3 +
 src/openvpn/options.h|   9 ++-
 src/openvpn/options_util.c   | 102 +++
 src/openvpn/options_util.h   |  33 +
 src/openvpn/push.c   |  33 +
 src/openvpn/ssl.c|  13 ++--
 src/openvpn/ssl.h|   3 +
 tests/unit_tests/openvpn/Makefile.am |   1 +
 tests/unit_tests/openvpn/test_misc.c |  40 +++
 13 files changed, 267 insertions(+), 18 deletions(-)
 create mode 100644 src/openvpn/options_util.c
 create mode 100644 src/openvpn/options_util.h

diff --git a/doc/man-sections/script-options.rst 
b/doc/man-sections/script-options.rst
index 74c6a1fc6..be718ef26 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -102,6 +102,42 @@ SCRIPT HOOKS
   the authentication, a :code:`1` or :code:`0` must be written to the
   file specified by the :code:`auth_control_file`.
 
+  If the file specified by :code:`auth_failed_reason` exists and has non-empty
+  content, the content of this file will be used as AUTH_FAILED message. To
+  avoid race conditions, this file should be written before
+  :code:`auth_control_file`.
+
+  This auth fail reason can be something simple like "User has been permanently
+  disabled" but there are also some special auth failed messages.
+
+  The ``TEMP`` message indicates that the authentication
+  temporarily failed and that the client should continue to retry to connect.
+  The server can optionally give a user readable message and hint the client a
+  behavior how to proceed. The keywords of the ``AUTH_FAILED,TEMP`` message
+  are comma separated keys/values and provide a hint to the client how to
+  proceed. Currently defined keywords are:
+
+  ``backoff`` :code:`s`
+instructs the client to wait at least :code:`s` seconds before the next
+connection attempt. If the client already uses a higher delay for
+reconnection attempt, the delay will not be shortened.
+
+  ``advance addr``
+Instructs the client to reconnect to the next (IP) address of the
+current server.
+
+  ``advance remote``
+Instructs the client to skip the remaining IP addresses of the current
+server and instead connect to the next server specified in the
+configuration file.
+
+  ``advance no``
+Instructs the client to retry connecting to the same server again.
+
+  For example, the message ``TEMP[backoff 42,advance no]: No free IP 
addresses``
+  indicates that the VPN connection can currently not succeed and instructs
+  the client to retry in 42 seconds again.
+
   When deferred authentication is in use, the script can also request
   pending authentication by writing to the file specified by the
   :code:`auth_pending_file`. The first line must be the timeout in
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index 5dbff4c6c..957494b10 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -102,6 +102,7 @@ openvpn_SOURCES = \
pkcs11_mbedtls.c \
openvpn.c openvpn.h \
options.c options.h \
+options_util.c options_util.h \
otime.c otime.h \
packet_id.c packet_id.h \
perf.c perf.h \
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 977f0f96c..f400495f0 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -484,13 +484,15 @@ next_connection_entry(struct context *c)
 /* Check if there is another resolved address to try for
  * the current connection */
 if (c->c1.link_socket_addr.current_remote
-&& c->c1.link_socket_addr.current_remote->ai_next)
+&& c->c1.link_socket_addr.current_remote->ai_next
+&& !c->options.advance_next_remote)
 {
 c->c1.link_socket_addr.current_remote =
 c->c1.link_socket_addr.current_remote->ai_next;
 }
 else
 {
+c->options.advance_next_remote = false;
 /* FIXME (schwabe) fix the persist-remote-ip option for real,
  * this is broken probably ever since connection lists and 
multiple
  * remote

Re: [Openvpn-devel] [PATCH] Add OpenSSL 3.0 to mingw build

[Arne Schwabe]

Am 24.08.22 um 14:36 schrieb Илья Шипицин:

It reverts 3.0 behaviour to 1.1.1
However --libdir is available for 1.1.1 as well


I understand. What I am missing is *why* reverting to 1.1.1 is a good 
idea. I think we should rather use the new default. I can see arguments 
that we add --libdir=lib64 to openSSL 1.1.1 to adjust it to OpenSSL 3.0 
but not the other way round.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Add OpenSSL 3.0 to mingw build

[Илья Шипицин]

I am fine with adding lib64 to 1.1.1

On Wed, Aug 24, 2022, 6:01 PM Arne Schwabe  wrote:

> Am 24.08.22 um 14:36 schrieb Илья Шипицин:
> > It reverts 3.0 behaviour to 1.1.1
> > However --libdir is available for 1.1.1 as well
>
> I understand. What I am missing is *why* reverting to 1.1.1 is a good
> idea. I think we should rather use the new default. I can see arguments
> that we add --libdir=lib64 to openSSL 1.1.1 to adjust it to OpenSSL 3.0
> but not the other way round.
>
> Arne
>
>
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3 4/4] Allow scripts and plugins to set a custom AUTH_FAILED message

[Arne Schwabe]

This is currently only possible when using the management interface
and the client-deny functionality.

Patch v3: add missing gc_free

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_common.h |  1 +
 src/openvpn/ssl_verify.c | 74 ++--
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index bf3ac67a5..f1cade2ef 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -155,6 +155,7 @@ struct auth_deferred_status
 {
 char *auth_control_file;
 char *auth_pending_file;
+char *auth_failed_reason_file;
 unsigned int auth_control_status;
 };
 
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index b4608517f..76cb9f19b 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -989,6 +989,12 @@ key_state_rm_auth_control_files(struct 
auth_deferred_status *ads)
 free(ads->auth_control_file);
 ads->auth_control_file = NULL;
 }
+if (ads->auth_failed_reason_file)
+{
+platform_unlink(ads->auth_failed_reason_file);
+free(ads->auth_failed_reason_file);
+ads->auth_failed_reason_file = NULL;
+}
 key_state_rm_auth_pending_file(ads);
 }
 
@@ -1007,19 +1013,47 @@ key_state_gen_auth_control_files(struct 
auth_deferred_status *ads,
 key_state_rm_auth_control_files(ads);
 const char *acf = platform_create_temp_file(opt->tmp_dir, "acf", &gc);
 const char *apf = platform_create_temp_file(opt->tmp_dir, "apf", &gc);
+const char *afr = platform_create_temp_file(opt->tmp_dir, "afr", &gc);
 
 if (acf && apf)
 {
 ads->auth_control_file = string_alloc(acf, NULL);
 ads->auth_pending_file = string_alloc(apf, NULL);
+ads->auth_failed_reason_file = string_alloc(afr, NULL);
+
 setenv_str(opt->es, "auth_control_file", ads->auth_control_file);
 setenv_str(opt->es, "auth_pending_file", ads->auth_pending_file);
+setenv_str(opt->es, "auth_failed_reason_file", 
ads->auth_failed_reason_file);
 }
 
 gc_free(&gc);
 return (acf && apf);
 }
 
+/**
+ * Checks if the auth failed reason file has any content and if yes it will
+ * be returned as string allocated in gc to the caller.
+ */
+static char *
+key_state_check_auth_failed_message_file(const struct auth_deferred_status 
*ads,
+ struct tls_multi *multi,
+ struct gc_arena *gc)
+{
+char *ret = NULL;
+if (ads->auth_failed_reason_file)
+{
+struct buffer reason = 
buffer_read_from_file(ads->auth_failed_reason_file, gc);
+
+if (BLEN(&reason))
+{
+ret = BSTR(&reason);
+}
+
+}
+return ret;
+}
+
+
 /**
  * Checks the auth control status from a file. The function will try
  * to read and update the cached status if the status is still pending
@@ -1184,12 +1218,27 @@ tls_authentication_status(struct tls_multi *multi)
 #endif
 if (failed_auth)
 {
+struct gc_arena gc = gc_new();
+const struct key_state *ks = get_primary_key(multi);
+const char *plugin_message = 
key_state_check_auth_failed_message_file(&ks->plugin_auth, multi, &gc);
+const char *script_message = 
key_state_check_auth_failed_message_file(&ks->script_auth, multi, &gc);
+
+if (plugin_message)
+{
+auth_set_client_reason(multi, plugin_message);
+}
+if (script_message)
+{
+auth_set_client_reason(multi, script_message);
+}
+
 /* We have at least one session that failed authentication. There
  * might be still another session with valid keys.
  * Although our protocol allows keeping the VPN session alive
  * with the other session (and we actually did that in earlier
  * version, this behaviour is really strange from a user (admin)
  * experience */
+gc_free(&gc);
 return TLS_AUTHENTICATION_FAILED;
 }
 else if (success)
@@ -1248,6 +1297,21 @@ tls_authenticate_key(struct tls_multi *multi, const 
unsigned int mda_key_id, con
  * this is the place to start.
  *** */
 
+/**
+ * Check if the script/plugin left a message in the auth failed message
+ * file and relay it to the user */
+static void
+check_for_client_reason(struct tls_multi *multi,
+struct auth_deferred_status *status)
+{
+struct gc_arena gc = gc_new();
+const char *msg = key_state_check_auth_failed_message_file(status, multi, 
&gc);
+if (msg)
+{
+auth_set_client_reason(multi, msg);
+}
+gc_free(&gc);
+}
 /*
  * Verify the user name and password using a script
  */
@@ -1316,6 +1380,7 @@ verify_user_pass_script(struct tls_session *session, 
struct tls_multi *multi,
 break;
 
 default:
+check_for_client_reason(multi, &

[Openvpn-devel] [PATCH v3 25/28] Ensure that control channel packet are respecting tls-mtu

[Arne Schwabe]

This ensures that control packets are actually smaller than tls-mtu.
Since OpenVPN will consider a control message packet complete
when the TLS record is complete, we have to ensure that the SSL library
will still write one record, so the receiving side will only be able
to get/read the control message content when a TLS record is
complete. To achieve this goal, this commit does:

- Split one read from TLS library into multiple control
  channel packets, splitting one TLS record into multiple
  control packets.
- increase allowed number of outstanding packets to 6 from 4 on the
  sender side. This is still okay with older implementations as
  receivers will have room for 8.
- calculate the overhead for control channel message to allow
  staying below that threshold.
- remove maxlen from key_state_read_ciphertext and related functions
  as we now always allow control channel messages to be up to
  TLS_CHANNEL_BUF_SIZE in size and no longer limit this by the mtu of
  control packets as the implemented splitting will take care of
  larger payloads from the SSL library

Patch v2: avoid assertion about to large buffer by sticking to 1250 max control 
size
  in this commit and leaving larger sizes for the --tls-mtu commit. 
Also fix
  various other small problems and grammar fixes.

Patch v3: grammar fixes

Signed-off-by: Arne Schwabe 
---
 src/openvpn/reliable.c|  55 ++
 src/openvpn/reliable.h|  32 +++-
 src/openvpn/ssl.c | 154 --
 src/openvpn/ssl_backend.h |   8 +-
 src/openvpn/ssl_mbedtls.c |  14 +---
 src/openvpn/ssl_openssl.c |  16 ++--
 src/openvpn/ssl_pkt.h |   4 +-
 7 files changed, 214 insertions(+), 69 deletions(-)

diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index 734736256..3ccb73976 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -41,6 +41,14 @@
 
 #include "memdbg.h"
 
+/* calculates test - base while allowing for base or test wraparound. test is
+ * assumed to be higher than base */
+static inline packet_id_type
+subtract_pid(const packet_id_type test, const packet_id_type base)
+{
+return test - base;
+}
+
 /*
  * verify that test - base < extent while allowing for base or test wraparound
  */
@@ -49,22 +57,7 @@ reliable_pid_in_range1(const packet_id_type test,
const packet_id_type base,
const unsigned int extent)
 {
-if (test >= base)
-{
-if (test - base < extent)
-{
-return true;
-}
-}
-else
-{
-if ((test+0x8000u) - (base+0x8000u) < extent)
-{
-return true;
-}
-}
-
-return false;
+return subtract_pid(test, base) < extent;
 }
 
 /*
@@ -498,6 +491,36 @@ reliable_get_buf(struct reliable *rel)
 return NULL;
 }
 
+int
+reliable_get_num_output_sequenced_available(struct reliable *rel)
+{
+struct gc_arena gc = gc_new();
+packet_id_type min_id = 0;
+bool min_id_defined = false;
+
+/* find minimum active packet_id */
+for (int i = 0; i < rel->size; ++i)
+{
+const struct reliable_entry *e = &rel->array[i];
+if (e->active)
+{
+if (!min_id_defined || reliable_pid_min(e->packet_id, min_id))
+{
+min_id_defined = true;
+min_id = e->packet_id;
+}
+}
+}
+
+int ret = rel->size;
+if (min_id_defined)
+{
+ret -= subtract_pid(rel->packet_id, min_id);
+}
+gc_free(&gc);
+return ret;
+}
+
 /* grab a free buffer, fail if buffer clogged by unacknowledged low packet IDs 
*/
 struct buffer *
 reliable_get_buf_output_sequenced(struct reliable *rel)
diff --git a/src/openvpn/reliable.h b/src/openvpn/reliable.h
index b9863efe3..7fffe397d 100644
--- a/src/openvpn/reliable.h
+++ b/src/openvpn/reliable.h
@@ -46,7 +46,7 @@
  *   be stored in one \c reliable_ack
  *   structure. */
 
-#define RELIABLE_CAPACITY 8 /**< The maximum number of packets that
+#define RELIABLE_CAPACITY 12/**< The maximum number of packets that
  *   the reliability layer for one VPN
  *   tunnel in one direction can store. */
 
@@ -93,7 +93,7 @@ struct reliable
 int size;
 interval_t initial_timeout;
 packet_id_type packet_id;
-int offset;
+int offset; /**< Offset of the bufs in the reliable_entry array */
 bool hold; /* don't xmit until reliable_schedule_now is called */
 struct reliable_entry array[RELIABLE_CAPACITY];
 };
@@ -178,6 +178,20 @@ reliable_ack_empty(struct reliable_ack *ack)
 return !ack->len;
 }
 
+/**
+ * Returns the number of packets that need to be acked.
+ *
+ * @param ack The acknowledgment structure to check.
+ *
+ * @returns the number of outstanding acks
+ */
+static inline int
+reliable_ack_outstanding(struct reliable_ack *

Re: [Openvpn-devel] [PATCH applied] Re: Update openssl_compat.h for newer LibreSSL

[Maximilian Fillinger]

> but they think the revamped OpenSSL 3.0 way of calculating the TLS1 PRF
> might actually not be in 2.5 yet, so they do not need a patch for that.

In 2.5, openssl_compat.h also doesn't try to define X509_OBJECT_free(), so 
there's nothing to backport there.


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Fix delcarion of pubkeys in test_provider.c in MSVC builds

[Arne Schwabe]

  Error: test_provider.c(74): error C2099: initializer is not a constant

Fix this issue by making the const char* to const char[]. This is probably
of one the weird array decay corner cases

I could not find another/better way around this issue. Godbolt link to try:

https://godbolt.org/z/s3aPb9q8q

This error only occurs when building unit tests with windows which our
normal build system does not do but my out of tree cmake build script
tries and fails

Signed-off-by: Arne Schwabe 
---
 tests/unit_tests/openvpn/test_provider.c | 36 
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/tests/unit_tests/openvpn/test_provider.c 
b/tests/unit_tests/openvpn/test_provider.c
index 3f9a26e57..9451a6e7e 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -52,24 +52,24 @@ static int mgmt_callback_called;
 static OSSL_PROVIDER *prov[2];
 
 /* public keys for testing -- RSA and EC */
-static const char *const pubkey1 = "-BEGIN PUBLIC KEY-\n"
-   
"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA7GWP6RLCGlvmVioIqYI6\n"
-   
"LUR4owA7sJ/nJxBAk+/xzD6gqgSigBsTqeb+gdZwkKjY1N4w2DUA0r5i8Eja/BWN\n"
-   
"xMZtC5nxK4MACtMqIwvlzfk130NhFXKtlZj2cyFBXqDdRyeg1ZrUQagcHVcgcReP\n"
-   
"9yiePgfO7NUOQk8edEeOR53SFCgnLBQQ9dGWtZN0hO/5BN6NSm/fd6vq0VjTRP5a\n"
-   
"BAH/BnqX9/3jV0jh8N9AE59mI1rjVVQ9VDnuAPkS8dLfdC661/CNxt0YWByTIgt1\n"
-   
"+qjW4LUvLbnU/rlPhuJ1SBZg+z/JtDBCKfs7syu5WYFqRvNFg7/91Rr/NwxvW/1h\n"
-   "8QIDAQAB\n"
-   "-END PUBLIC KEY-\n";
-
-static const char *const pubkey2 = "-BEGIN PUBLIC KEY-\n"
-   
"MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEO85iXW+HgnUkwlj1DohNVw0GsnGIh1gZ\n"
-   
"u95ff1JiUaJIkYNIkZA+hwIPFVH5aJcSCv3SPIeDS2VUAESNKHZJBQ==\n"
-   "-END PUBLIC KEY-\n";
-
-static const char *const pubkey3 = "-BEGIN PUBLIC KEY-\n"
-   
"MCowBQYDK2VwAyEA+q5xjF5hGyyqYZidJdz/0saEQabL3N4wIZJBxNGbgJE=\n"
-   "-END PUBLIC KEY-";
+static const char pubkey1[] = "-BEGIN PUBLIC KEY-\n"
+  
"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA7GWP6RLCGlvmVioIqYI6\n"
+  
"LUR4owA7sJ/nJxBAk+/xzD6gqgSigBsTqeb+gdZwkKjY1N4w2DUA0r5i8Eja/BWN\n"
+  
"xMZtC5nxK4MACtMqIwvlzfk130NhFXKtlZj2cyFBXqDdRyeg1ZrUQagcHVcgcReP\n"
+  
"9yiePgfO7NUOQk8edEeOR53SFCgnLBQQ9dGWtZN0hO/5BN6NSm/fd6vq0VjTRP5a\n"
+  
"BAH/BnqX9/3jV0jh8N9AE59mI1rjVVQ9VDnuAPkS8dLfdC661/CNxt0YWByTIgt1\n"
+  
"+qjW4LUvLbnU/rlPhuJ1SBZg+z/JtDBCKfs7syu5WYFqRvNFg7/91Rr/NwxvW/1h\n"
+  "8QIDAQAB\n"
+  "-END PUBLIC KEY-\n";
+
+static const char pubkey2[] = "-BEGIN PUBLIC KEY-\n"
+  
"MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEO85iXW+HgnUkwlj1DohNVw0GsnGIh1gZ\n"
+  
"u95ff1JiUaJIkYNIkZA+hwIPFVH5aJcSCv3SPIeDS2VUAESNKHZJBQ==\n"
+  "-END PUBLIC KEY-\n";
+
+static const char pubkey3[] = "-BEGIN PUBLIC KEY-\n"
+  
"MCowBQYDK2VwAyEA+q5xjF5hGyyqYZidJdz/0saEQabL3N4wIZJBxNGbgJE=\n"
+  "-END PUBLIC KEY-";
 
 static const char *pubkeys[] = {pubkey1, pubkey2, pubkey3};
 
-- 
2.32.1 (Apple Git-133)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel