diff mbox series

[Openvpn-devel,v2,2/2] Support client reason from auth plugin

Message ID a9f4fde1-61a1-5706-060b-58b8ba687314@sparklabs.com
State Changes Requested
Headers show
Series [Openvpn-devel,v2,1/2] Send auth fail to client on reneg failure | expand

Commit Message

Eric Thorpe April 10, 2019, 4:07 p.m. UTC 
Hi All,

This patch allows for a client reason to be returned from an auth plugin
and sent to the connecting client on an auth fail. This change is
backwards compatible with existing plugins and hasn't caused issues with
existing plugins like the included pam plugin in our testing. The main
purpose of this change is to support dynamic challenge/response from
plugins, currently this is only possible from the management interface.

Example usage for this change can be found in a new plugin here modified
from the included PAM plugin -
https://github.com/thesparklabs/openvpn-two-factor-extensions/tree/master/yubikey-u2f-pam-plugin

This is version two for this patch and addresses previous inadequacies
pointed out.

Regards,
Eric

Comments

Arne Schwabe June 14, 2019, 12:07 a.m. UTC | #1 
Am 11.04.19 um 04:07 schrieb Eric Thorpe:
> Hi All,
> 
> This patch allows for a client reason to be returned from an auth plugin
> and sent to the connecting client on an auth fail. This change is
> backwards compatible with existing plugins and hasn't caused issues with
> existing plugins like the included pam plugin in our testing. The main
> purpose of this change is to support dynamic challenge/response from
> plugins, currently this is only possible from the management interface.
> 
> Example usage for this change can be found in a new plugin here modified
> from the included PAM plugin -
> https://github.com/thesparklabs/openvpn-two-factor-extensions/tree/master/yubikey-u2f-pam-plugin
> 

Same as for the 1/2 patch, I would like to postpone this after the
auth-token set has been merged.

The patch "Sent indication that a session is expired to clients" of
auth-token also does the renaming from man_def_auth_set_client_reason to
auth_set_client_reason so this creates a big conflict.

Arne
diff mbox series

Patch

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9696e9bab..884c94a23 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1354,10 +1354,8 @@  tls_multi_free(struct tls_multi *multi, bool clear)
 
     ASSERT(multi);
 
-#ifdef MANAGEMENT_DEF_AUTH
-    man_def_auth_set_client_reason(multi, NULL);
+    set_client_reason(multi, NULL);
 
-#endif
 #if P2MP_SERVER
     free(multi->peer_info);
 #endif
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index b82a014a0..975fb983a 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -515,13 +515,13 @@  struct tls_multi
     char *locked_cn;
     char *locked_username;
     struct cert_hash_set *locked_cert_hash_set;
-
-#ifdef ENABLE_DEF_AUTH
+    
     /*
      * An error message to send to client on AUTH_FAILED
      */
     char *client_reason;
 
+#ifdef ENABLE_DEF_AUTH
     /* Time of last call to tls_authentication_status */
     time_t tas_last;
 #endif
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index fd83cde85..fa0ba28c4 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -837,21 +837,6 @@  verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
 #endif
 
 #ifdef MANAGEMENT_DEF_AUTH
-void
-man_def_auth_set_client_reason(struct tls_multi *multi, const char *client_reason)
-{
-    if (multi->client_reason)
-    {
-        free(multi->client_reason);
-        multi->client_reason = NULL;
-    }
-    if (client_reason && strlen(client_reason))
-    {
-        /* FIXME: Last alloc will never be freed */
-        multi->client_reason = string_alloc(client_reason, NULL);
-    }
-}
-
 static inline unsigned int
 man_def_auth_test(const struct key_state *ks)
 {
@@ -1055,7 +1040,7 @@  tls_authenticate_key(struct tls_multi *multi, const unsigned int mda_key_id, con
     if (multi)
     {
         int i;
-        man_def_auth_set_client_reason(multi, client_reason);
+        set_client_reason(multi, client_reason);
         for (i = 0; i < KEY_SCAN_SIZE; ++i)
         {
             struct key_state *ks = multi->key_scan[i];
@@ -1081,6 +1066,21 @@  tls_authenticate_key(struct tls_multi *multi, const unsigned int mda_key_id, con
  * this is the place to start.
  *************************************************************************** */
 
+
+void
+set_client_reason(struct tls_multi *multi, const char *client_reason)
+{
+    if (multi->client_reason)
+    {
+        free(multi->client_reason);
+        multi->client_reason = NULL;
+    }
+    if (client_reason && strlen(client_reason))
+    {
+        multi->client_reason = string_alloc(client_reason, NULL);
+    }
+}
+
 /*
  * Verify the user name and password using a script
  */
@@ -1166,7 +1166,7 @@  verify_user_pass_script(struct tls_session *session, const struct user_pass *up)
  * Verify the username and password using a plugin
  */
 static int
-verify_user_pass_plugin(struct tls_session *session, const struct user_pass *up, const char *raw_username)
+verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi, const struct user_pass *up, const char *raw_username)
 {
     int retval = OPENVPN_PLUGIN_FUNC_ERROR;
 #ifdef PLUGIN_DEF_AUTH
@@ -1176,6 +1176,9 @@  verify_user_pass_plugin(struct tls_session *session, const struct user_pass *up,
     /* 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", (raw_username ? raw_username : up->username));
         setenv_str(session->opt->es, "password", up->password);
@@ -1197,7 +1200,23 @@  verify_user_pass_plugin(struct tls_session *session, const struct user_pass *up,
 #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)
+                {
+                    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 */
@@ -1378,7 +1397,7 @@  verify_user_pass(struct user_pass *up, struct tls_multi *multi,
 #endif
     if (plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY))
     {
-        s1 = verify_user_pass_plugin(session, up, raw_username);
+        s1 = verify_user_pass_plugin(session, multi, up, raw_username);
     }
     if (session->opt->auth_user_pass_verify_script)
     {
@@ -1462,7 +1481,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);
 
         }
         msg(D_TLS_ERRORS, "TLS Auth Error: Auth Username/Password verification failed for peer");
diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h
index 3e2267aed..afcfb77be 100644
--- a/src/openvpn/ssl_verify.h
+++ b/src/openvpn/ssl_verify.h
@@ -171,6 +171,8 @@  tls_common_name_hash(const struct tls_multi *multi, const char **cn, uint32_t *c
 
 #endif
 
+void set_client_reason(struct tls_multi *multi, const char *client_reason);
+
 /**
  * Verify the given username and password, using either an external script, a
  * plugin, or the management interface.
@@ -225,19 +227,12 @@  struct x509_track
  */
 #ifdef MANAGEMENT_DEF_AUTH
 bool tls_authenticate_key(struct tls_multi *multi, const unsigned int mda_key_id, const bool auth, const char *client_reason);
-
-void man_def_auth_set_client_reason(struct tls_multi *multi, const char *client_reason);
-
 #endif
 
 static inline const char *
 tls_client_reason(struct tls_multi *multi)
 {
-#ifdef ENABLE_DEF_AUTH
     return multi->client_reason;
-#else
-    return NULL;
-#endif
 }
 
 /** Remove any X509_ env variables from env_set es */