Re: [Openvpn-devel] [PATCH v3] Implement --client-crresponse script options and plugin interface
On Dienstag, 18. Mai 2021 14:26:35 CEST Arne Schwabe wrote:
> This is allows scripts and pluginsto parse/react to a CR_RESPONSE message
This commit message needs a makeover, I think.
> - If ``method`` is set to :code:`via-env`, OpenVPN will call ``script``
> + If ``method`` is set to :code:`via-env`, OpenVPN will call ``cmd``
These drive-by fixes Antonio spotted make sense and are not intrusive enough
that I care.
> +verify_crresponse_script(struct tls_multi *multi, const char *cr_response)
[...]
> +const char *tmp_file = platform_create_temp_file(session->opt->tmp_dir,
> "cr", );
> +if (tmp_file)
> +{
> +struct status_output *so = status_open(tmp_file, 0, -1, NULL,
> + STATUS_OUTPUT_WRITE);
> +status_printf(so, "%s", cr_response);
> +if (!status_close(so))
> +{
> +msg(D_TLS_ERRORS, "TLS CR Response Error: could not write cr"
> + "responsed to file: %s",
> +tmp_file);
> +tls_deauthenticate(multi);
> +goto done;
> +}
> +}
> +else
> +{
> +msg(D_TLS_ERRORS, "TLS Auth Error: could not create write "
> + "username/password to temp file");
> +}
This else branch should be the same as the "if (!status_close(so))" one above
I think, as you don't want to call the script without a valid tempfile.
Besides that the error message is copy/paste wrong anyway. So, maybe introduce
a bool and do the error handling in one place, might help future copy/pastes
as well.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Implement --client-crresponse script options and plugin interface
Hi,
On 18/05/2021 14:26, 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
> Patch V3: rebase
>
> 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| 65 +
> src/openvpn/ssl_verify.h| 23 ++
> 10 files changed, 146 insertions(+), 4 deletions(-)
>
> diff --git a/doc/man-sections/script-options.rst
> b/doc/man-sections/script-options.rst
> index 22990f4f4..f48e5818d 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``
this fix seems unrelated
>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
this fix seems unrelated
>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 1f60f8861..550ce2746 100644
> --- a/include/openvpn-plugin.h.in
> +++ b/include/openvpn-plugin.h.in
> @@ -84,6 +84,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):
> @@ -131,7 +135,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 49c742928..ffb3c78d5 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -2916,6 +2916,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 =
>
Re: [Openvpn-devel] [PATCH v3] Implement --client-crresponse script options and plugin interface
Hi, On 10/08/2021 11:38, Antonio Quartulli wrote: > Hi, > > On 18/05/2021 14:26, 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 >> Patch V3: rebase >> >> Signed-off-by: Arne Schwabe > > I have a high level question about this patch. > > At the moment there is no trace of CR_RESPONSE or request in the > OpenVPN2 code. > > To the best of my knowledge that's because this protocol is entirely > handled outside of OpenVPN using INFO_PRE and custom control messages. > > With this patch we are aiming at moving a piece of this "external" logic > inside OpenVPN. > > Question 1: > > What is the benefit of moving the CR response handling inside OpenVPN, > if there is no code to generate the request? Maybe there are advantages > that are just assumed? > > Question 2: > > If we implement handling CR-RESPONSE, do we want to implement such > handling in the future with every custom control message that may be > sent by the management interface? > > Question 3: > > IMHO, this is a very good case where OpenVPN can be transparent and let > the upper layer deal with it (i.e. via mgmt interface), why changing that? I have to take this back. OpenVPN already has some logic to parse the CR_RESPONSE and react to it accordingly. Therefore my comments above are wrong. Please ignore my earlier message. Best Regards, -- Antonio Quartulli ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Implement --client-crresponse script options and plugin interface
Hi, On 18/05/2021 14:26, 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 > Patch V3: rebase > > Signed-off-by: Arne Schwabe I have a high level question about this patch. At the moment there is no trace of CR_RESPONSE or request in the OpenVPN2 code. To the best of my knowledge that's because this protocol is entirely handled outside of OpenVPN using INFO_PRE and custom control messages. With this patch we are aiming at moving a piece of this "external" logic inside OpenVPN. Question 1: What is the benefit of moving the CR response handling inside OpenVPN, if there is no code to generate the request? Maybe there are advantages that are just assumed? Question 2: If we implement handling CR-RESPONSE, do we want to implement such handling in the future with every custom control message that may be sent by the management interface? Question 3: IMHO, this is a very good case where OpenVPN can be transparent and let the upper layer deal with it (i.e. via mgmt interface), why changing that? Best Regards, -- Antonio Quartulli ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3] Implement --client-crresponse script options and plugin interface
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
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| 65 +
src/openvpn/ssl_verify.h| 23 ++
10 files changed, 146 insertions(+), 4 deletions(-)
diff --git a/doc/man-sections/script-options.rst
b/doc/man-sections/script-options.rst
index 22990f4f4..f48e5818d 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 1f60f8861..550ce2746 100644
--- a/include/openvpn-plugin.h.in
+++ b/include/openvpn-plugin.h.in
@@ -84,6 +84,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):
@@ -131,7 +135,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 49c742928..ffb3c78d5 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2916,6 +2916,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/openvpn/options.c
index db460796a..3be0e2c35 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1327,6 +1327,7 @@
