Re: [Openvpn-devel] [PATCH 2/2] More explicit versioning compatibility in sample-plugins/defer/simple.c

2021-01-29 Thread David Sommerseth
On 27/01/2021 21:21, Greg Cox wrote: While not required, adding openvpn_plugin_min_version_required_v1 helps by making an example for others to copy, and helps to explicitly call attention to the difference between the API version number and the struct version number in v3 calls. ---

Re: [Openvpn-devel] [PATCH 1/2] Update openvpn_plugin_func_v2 to _v3 in sample-plugins/defer/simple.c

2021-01-29 Thread David Sommerseth
On 27/01/2021 21:21, Greg Cox wrote: This isn't strictly required, but it modernizes the functions used. This change makes _open the same parameter form as _func (for better parallelism in function writing) and includes a check for the correct struct version, as recommended by openvpn-plugin.h

[Openvpn-devel] [PATCH] sample-plugin/defer: Add simple test case for additional web-auth

2021-01-29 Thread David Sommerseth
From: David Sommerseth Extend the defer/simple sample-plugin supporting test_defer_timeout and test_defer_openurl environment variables to trigger an additional web based authentication. Both variables are required to enable this feature. Since this plug-in will require clients to use

Re: [Openvpn-devel] [PATCH v2 09/11] Implement deferred auth for scripts

2021-01-29 Thread David Sommerseth
On 25/01/2021 13:56, Arne Schwabe wrote: This patch also refactors the if condition that checks the result of the authentication since that has become quite unreadable. It renames s1/s2 and extracts some parts of the condition into individual variables to make the condition better understandle

Re: [Openvpn-devel] [PATCH v2 10/11] Implement --client-crresponse script options and plugin interface

2021-01-29 Thread David Sommerseth
On 25/01/2021 13:56, Arne Schwabe wrote: This is allows scripts and pluginsto parse/react to a CR_RESPONSE message Patch V2: doc fixes, do not put script under ENABLE_PLUGIN Signed-off-by: Arne Schwabe --- doc/man-sections/script-options.rst | 28 - include/openvpn-plugin.h.in

Re: [Openvpn-devel] [PATCH v2 11/11] Add example script demonstrating TOTP via auth-pending

2021-01-29 Thread David Sommerseth
On 25/01/2021 13:56, Arne Schwabe wrote: Signed-off-by: Arne Schwabe --- Changes.rst | 9 +++ doc/man-sections/script-options.rst | 3 + sample/sample-scripts/totpauth.py | 107 3 files changed, 119 insertions(+) create mode

Re: [Openvpn-devel] [PATCH v2 08/11] Allow pending auth to be send from a auth plugin

2021-01-29 Thread David Sommerseth
On 25/01/2021 13:56, Arne Schwabe wrote: Patch v2: removed change that slipped into this patch and belongs into the next Signed-off-by: Arne Schwabe --- doc/man-sections/generic-options.rst | 3 +- include/openvpn-plugin.h.in | 8 ++ src/openvpn/ssl.c

Re: [Openvpn-devel] [PATCH 5/5] Prefer TLS libraries TLS PRF function, fix OpenVPN in FIPS mode

2021-01-29 Thread Antonio Quartulli
Hi, witht his review I want to open a broader discussion about the use of ASSERT in the OpenVPN code. My comments below will get to the point. On 07/09/2020 18:22, Arne Schwabe wrote: > This moves from using our own copy of the TLS1 PRF function to using > TLS library provided function where

Re: [Openvpn-devel] [PATCH v2 4/5] Check return values in md_ctx_init and hmac_ctx_init

2021-01-29 Thread Arne Schwabe
> > I saw that you missed this case earlier, but I thought that this call > cannot really fail. > > Assuming it can fail under certain conditions, wouldn't the M_FATAL > somewhat become a DoS on the server side? The condition it can fail is basically that the crypto library is unable or

Re: [Openvpn-devel] [PATCH v2 4/5] Check return values in md_ctx_init and hmac_ctx_init

2021-01-29 Thread Antonio Quartulli
Hi, On 29/01/2021 11:47, Arne Schwabe wrote: > Without this OpenVPN will later segfault on a FIPS enabled system due > to the algorithm available but not allowed. > > Patch V2: Use (!func) instead (func != 1) > > Signed-off-by: Arne Schwabe > --- > src/openvpn/crypto_openssl.c | 15

[Openvpn-devel] [PATCH v2 4/5] Check return values in md_ctx_init and hmac_ctx_init

2021-01-29 Thread Arne Schwabe
Without this OpenVPN will later segfault on a FIPS enabled system due to the algorithm available but not allowed. Patch V2: Use (!func) instead (func != 1) Signed-off-by: Arne Schwabe --- src/openvpn/crypto_openssl.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff

Re: [Openvpn-devel] [PATCH 4/5] Check return values in md_ctx_init and hmac_ctx_init

2021-01-29 Thread Antonio Quartulli
Hi, On 07/09/2020 18:22, Arne Schwabe wrote: > Without this OpenVPN will later segfault on a FIPS enabled system due > to the algorithm available but not allowed. > > Signed-off-by: Arne Schwabe > --- > src/openvpn/crypto_openssl.c | 10 -- > 1 file changed, 8 insertions(+), 2

Re: [Openvpn-devel] [PATCH v2 07/11] Refactor extract_var_peer_info into standalone function and add ssl_util.c

2021-01-29 Thread Lev Stipakov
Hi, > Patch v2: add newline add the end of sll_util.h and ssl_util.c sll -> ssl Compared to v1, only changes are added newlines. Compiled with MSVC. Acked-by: Lev Stipakov ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net

Re: [Openvpn-devel] [PATCH v2] Allow running a default configuration with TLS libraries without BF-CBC

2021-01-29 Thread Antonio Quartulli
Hi, On 25/01/2021 13:43, Arne Schwabe wrote: > Modern TLS libraries might drop Blowfish by default or distributions > might disable Blowfish in OpenSSL/mbed TLS. We still signal OCC > options with BF-CBC compatible strings. To avoid requiring BF-CBC > for this, special this one usage of BF-CBC

Re: [Openvpn-devel] [PATCH v2 06/11] Add S_EXITCODE flag for openvpn_run_script to report exit code

2021-01-29 Thread Lev Stipakov
Hi, V1 has been acked and this has minor style/comment fixes. No other changes compared to V1. Compiled with MSVC. Acked-by: Lev Stipakov ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net

Re: [Openvpn-devel] [PATCH v2 03/11] Implement server side of AUTH_PENDING with extending timeout

2021-01-29 Thread Lev Stipakov
Hi, Looks fine, except > -send_control_channel_string(c, "AUTH_PENDING", D_PUSH); > +struct key_state *ks = _multi->session[TM_ACTIVE].key[KS_PRIMARY]; >C:\Users\lev\Projects\openvpn\src\openvpn\push.c(350,1): error C2220: the >following warning is treated as an error

Re: [Openvpn-devel] [PATCH v2 02/11] Implement client side handling of AUTH_PENDING message

2021-01-29 Thread Lev Stipakov
Compared with V1 - all concerns are addressed. Compiled with MSVC. Acked-by: Lev Stipakov ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2 05/11] Change parameter of send_auth_pending_messages from context to tls_multi

2021-01-29 Thread Lev Stipakov
Hi, The patch is fine and the comment has been fixed. It also fixes compilation errors introduced in 03/11. However I think we need to fix it back in 03/11, not here. -Lev ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net

Re: [Openvpn-devel] [PATCH v2 04/11] Introduce management client state for AUTH_PENDING notifications

2021-01-29 Thread Lev Stipakov
Since I already acked v1, I checked that v2 is no different to v1. I got a compilation error because 03/11 is broken, but it is unrelated to this patch. Acked-by: Lev Stipakov ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net

Re: [Openvpn-devel] [PATCH v2 01/11] Change pull request timeout use a timeout rather than a number

2021-01-29 Thread Lev Stipakov
Compared with V1 and spelling is now fixed. Compiled with MSVC, code looks fine. Acked-by: Lev Stipakov ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel