diff mbox series

[Openvpn-devel,2/2] Allows a plugin to provide a client_reason for authentication failure

Message ID 20200813072923.24385-2-eric@sparklabs.com
State Changes Requested
Headers
Series [Openvpn-devel,1/2] Send auth-fail messages to clients on renegotiation failures via auth-token or user-pass expiry |

Commit Message

Eric Thorpe Aug. 12, 2020, 9:29 p.m. UTC 
  Signed-off-by: Eric Thorpe <eric@sparklabs.com>
---
 src/openvpn/ssl_verify.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

Comments

Arne Schwabe March 24, 2021, 1 p.m. UTC | #1 
I am just going through some older patches and this one of them that I
didn't review last time.

This patch does not apply cleanly to master anymore but there some other
things that should be fixed regardless. So it would be good to have a
version 2 of this patch.

> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
> index 8d8531c7..356b30fd 100644
> --- a/src/openvpn/ssl_verify.c
> +++ b/src/openvpn/ssl_verify.c
> @@ -1157,6 +1157,9 @@ verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi,
>      /* Is username defined? */
>      if ((session->opt->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL) || strlen(up->username))
>      {
> +        struct plugin_return pr, prfetch;
Move to when it is used.

> +        plugin_return_init(&pr);


>          /* set username/password in private env space */
>          setenv_str(session->opt->es, "username", up->username);
>          setenv_str(session->opt->es, "password", up->password);
> @@ -1180,7 +1183,23 @@ verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi,
>  #endif
>  
>          /* call command */
> -        retval = plugin_call(session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, NULL, session->opt->es);
> +        retval = plugin_call(session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, &pr, session->opt->es);
> +
> +        /* Fetch client reason */
> +        plugin_return_get_column(&pr, &prfetch, "client_reason");
> +        if (plugin_return_defined(&prfetch))
> +        {
> +            int i;> +            for (i = 0; i < prfetch.n; ++i)
Mov int i into for loop.


> +            {
> +                if (prfetch.list[i] && prfetch.list[i]->value)
> +                {
> +                    auth_set_client_reason(multi, prfetch.list[i]->value);
> +                }
> +            }
> +        }
> +
> +        plugin_return_free(&pr);
>  
>  #ifdef PLUGIN_DEF_AUTH
>          /* purge auth control filename (and file itself) for non-deferred returns */
> @@ -1436,8 +1455,16 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
>          if (multi->connection_established)
>          {
>              /* Notify the client */
> -            send_push_reply_auth_failed(multi, "SESSION:Auth failed");
> -
> +            const char *client_reason;
> +            if (multi->client_reason != NULL)
> +            {
> +                client_reason = multi->client_reason;
> +            }
> +            else
> +            {
> +                client_reason = "SESSION:Auth failed";
> +            }
> +            send_push_reply_auth_failed(multi, client_reason);

The patch is missing documentation in the openvpn-plugin.h. Currently
the patch adds an undocumented feature, which we want to avoid.

Arne
diff mbox series

Patch

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 8d8531c7..356b30fd 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1157,6 +1157,9 @@  verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi,
     /* Is username defined? */
     if ((session->opt->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL) || strlen(up->username))
     {
+        struct plugin_return pr, prfetch;
+        plugin_return_init(&pr);
+
         /* set username/password in private env space */
         setenv_str(session->opt->es, "username", up->username);
         setenv_str(session->opt->es, "password", up->password);
@@ -1180,7 +1183,23 @@  verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi,
 #endif
 
         /* call command */
-        retval = plugin_call(session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, NULL, session->opt->es);
+        retval = plugin_call(session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, &pr, session->opt->es);
+
+        /* Fetch client reason */
+        plugin_return_get_column(&pr, &prfetch, "client_reason");
+        if (plugin_return_defined(&prfetch))
+        {
+            int i;
+            for (i = 0; i < prfetch.n; ++i)
+            {
+                if (prfetch.list[i] && prfetch.list[i]->value)
+                {
+                    auth_set_client_reason(multi, prfetch.list[i]->value);
+                }
+            }
+        }
+
+        plugin_return_free(&pr);
 
 #ifdef PLUGIN_DEF_AUTH
         /* purge auth control filename (and file itself) for non-deferred returns */
@@ -1436,8 +1455,16 @@  verify_user_pass(struct user_pass *up, struct tls_multi *multi,
         if (multi->connection_established)
         {
             /* Notify the client */
-            send_push_reply_auth_failed(multi, "SESSION:Auth failed");
-
+            const char *client_reason;
+            if (multi->client_reason != NULL)
+            {
+                client_reason = multi->client_reason;
+            }
+            else
+            {
+                client_reason = "SESSION:Auth failed";
+            }
+            send_push_reply_auth_failed(multi, client_reason);
         }
         ks->authenticated = KS_AUTH_FALSE;
         msg(D_TLS_ERRORS, "TLS Auth Error: Auth Username/Password verification failed for peer");