[Openvpn-devel,v2,3/5] Implement sending response to challenge via CR_RESPONSE

Message ID 20191109151306.18597-3-arne@rfc2549.org
State Superseded
Headers show
Series [Openvpn-devel,v2,1/5] Implement parsing and sending INFO and INFO_PRE control messages | expand

Commit Message

Arne Schwabe Nov. 9, 2019, 4:13 a.m. UTC
When a client announces its support to support text based
challenge/response via IV_SOO=cr_text,the client needs to also
be able to reply to that response.

This adds the "cr-response" management function to be able to
do this. The answer should be base64 encoded.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 doc/management-notes.txt |  8 ++++++++
 src/openvpn/init.c       | 39 +++++++++++++++++++++++++++++++++++++++
 src/openvpn/manage.c     | 39 ++++++++++++++++++++++++++++++++++++++-
 src/openvpn/manage.h     |  1 +
 4 files changed, 86 insertions(+), 1 deletion(-)

Comments

David Sommerseth March 27, 2020, 10:06 a.m. UTC | #1
On 09/11/2019 16:13, Arne Schwabe wrote:
> When a client announces its support to support text based
> challenge/response via IV_SOO=cr_text,the client needs to also

Typo.  IV_SOO -> IV_SSO

> be able to reply to that response.
> 
> This adds the "cr-response" management function to be able to
> do this. The answer should be base64 encoded.

Could we have some kind of test/sample script demonstrating this feature?  It
would also help test that this feature works and also useful for regression
testing later on.
David Sommerseth May 15, 2020, 5:23 a.m. UTC | #2
Hi,

So I'm giving this one another look again.  I started now by trying to use
this feature manually, to see that each step works as expected.  But this time
I also discovered a few other details.

On 09/11/2019 16:13, Arne Schwabe wrote:
> When a client announces its support to support text based
> challenge/response via IV_SOO=cr_text,the client needs to also

Just reminding of the IV_SOO -> IV_SSO typo-fix ... and it should also be
'crtext', not 'cr_text'.  The former is what is used in the management notes.

The more I dive into this, I'm also not sure IV_SSO is the proper term.  As it
is actually defines additional authentication mechanisms, where SSO (Single
Sign-on) is just one potential user of this feature.  What about just using
IV_AUTH? (knowing this will change the prior patches)

[...snip...]

> diff --git a/doc/management-notes.txt b/doc/management-notes.txt
> index 17645c1d..e380ca2b 100644
> --- a/doc/management-notes.txt
> +++ b/doc/management-notes.txt
> @@ -806,6 +806,14 @@ To accept connecting to the host and port directly, use this command:
>  
>    proxy NONE
>  
> +COMMAND -- cr-response (OpenVPN 2.5 or higher)
> +-------------------------------------------------
> +Provides support for sending responses a challenge/response
> +query via INFOMSG,CR_TEXT. The response should be base64 encoded:
> +
> +  cr-response SGFsbG8gV2VsdCE=
> +
> +

Could we please have more documentation how to use to enable and use this
feature properly?  Similar to the examples later down for both the static and
dynamic challenge protocols.  This does not need to happen in this patch
though, as this is also tightly connected to the next patches.

> +static bool
> +management_callback_send_cc_mesage(void *arg,

I didn't spot this earlier, but there's a typo in the function name; it is
'message' with two 's'.

>  static bool
>  management_callback_remote_cmd(void *arg, const char **p)
> @@ -3990,6 +4028,7 @@ init_management_callback_p2p(struct context *c)
>          cb.show_net = management_show_net_callback;
>          cb.proxy_cmd = management_callback_proxy_cmd;
>          cb.remote_cmd = management_callback_remote_cmd;
> +        cb.send_cc_message = management_callback_send_cc_mesage;

Same typo here in the callback function name; which is why it compiles ;-)

Patch

diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 17645c1d..e380ca2b 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -806,6 +806,14 @@  To accept connecting to the host and port directly, use this command:
 
   proxy NONE
 
+COMMAND -- cr-response (OpenVPN 2.5 or higher)
+-------------------------------------------------
+Provides support for sending responses a challenge/response
+query via INFOMSG,CR_TEXT. The response should be base64 encoded:
+
+  cr-response SGFsbG8gV2VsdCE=
+
+
 COMMAND -- pk-sig (OpenVPN 2.5 or higher, management version > 1)
 COMMAND -- rsa-sig (OpenVPN 2.3 or higher, management version <= 1)
 -----------------------------------------------------------------
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 0bdb0a9c..77c1c23b 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -288,6 +288,44 @@  ce_management_query_proxy(struct context *c)
     return ret;
 }
 
+/**
+ * This method sends a custom control channel message
+ *
+ * This will write the control message
+ *
+ *  command parm1,parm2,..
+ *  .
+ * to the control channel.
+ *
+ * @param arg           The context struct
+ * @param command       The command being sent
+ * @param parameters    the parameters to the command
+ * @return              if sending was successful
+ */
+static bool
+management_callback_send_cc_mesage(void *arg,
+                                   const char *command,
+                                   const char *parameters)
+{
+    struct context *c = (struct context *) arg;
+    size_t len = strlen(command) + 1 + sizeof(parameters) + 1;
+    if (len > PUSH_BUNDLE_SIZE)
+    {
+        return false;
+    }
+
+    struct gc_arena gc = gc_new();
+    struct buffer buf = alloc_buf_gc(len, &gc);
+    ASSERT(buf_printf(&buf, "%s", command));
+    if (parameters)
+    {
+        ASSERT(buf_printf(&buf, ",%s", parameters));
+    }
+    bool status = send_control_channel_string(c, BSTR(&buf), D_PUSH);
+
+    gc_free(&gc);
+    return status;
+}
 
 static bool
 management_callback_remote_cmd(void *arg, const char **p)
@@ -3990,6 +4028,7 @@  init_management_callback_p2p(struct context *c)
         cb.show_net = management_show_net_callback;
         cb.proxy_cmd = management_callback_proxy_cmd;
         cb.remote_cmd = management_callback_remote_cmd;
+        cb.send_cc_message = management_callback_send_cc_mesage;
 #ifdef TARGET_ANDROID
         cb.network_change = management_callback_network_change;
 #endif
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 1d97c2b6..8dada6f2 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -75,6 +75,7 @@  man_help(void)
     msg(M_CLIENT, "auth-retry t           : Auth failure retry mode (none,interact,nointeract).");
     msg(M_CLIENT, "bytecount n            : Show bytes in/out, update every n secs (0=off).");
     msg(M_CLIENT, "echo [on|off] [N|all]  : Like log, but only show messages in echo buffer.");
+    msg(M_CLIENT, "cr-response response   : Send a challenge response answer via CR_RESPONSE to server");
     msg(M_CLIENT, "exit|quit              : Close management session.");
     msg(M_CLIENT, "forget-passwords       : Forget passwords entered so far.");
     msg(M_CLIENT, "help                   : Print this message.");
@@ -779,6 +780,27 @@  man_net(struct management *man)
     }
 }
 
+static void
+man_send_cc_message(struct management *man, const char *message, const char *parameters)
+{
+    if (man->persist.callback.send_cc_message)
+        {
+            const bool status = (*man->persist.callback.send_cc_message)
+                (man->persist.callback.arg, message, parameters);
+            if (status)
+            {
+                msg(M_CLIENT, "SUCCESS: command succeeded");
+            }
+            else
+            {
+                msg(M_CLIENT, "ERROR: command failed");
+            }
+        }
+    else
+    {
+        msg(M_CLIENT, "ERROR: This command is not supported by the current daemon mode");
+    }
+}
 #ifdef ENABLE_PKCS11
 
 static void
@@ -1144,7 +1166,15 @@  man_load_stats(struct management *man)
 }
 
 #define MN_AT_LEAST (1<<0)
-
+/**
+ * Checks if the correct number of arguments to a management command are present
+ * and otherwise prints an error and returns false.
+ *
+ * @param p         pointer to the parameter array
+ * @param n         number of arguments required
+ * @param flags     if MN_AT_LEAST require at least n parameters and not exactly n
+ * @return          Return wether p has n (or at least n) parameters
+ */
 static bool
 man_need(struct management *man, const char **p, const int n, unsigned int flags)
 {
@@ -1460,6 +1490,13 @@  man_dispatch_command(struct management *man, struct status_output *so, const cha
             man_query_need_str(man, p[1], p[2]);
         }
     }
+    else if (streq(p[0], "cr-response"))
+    {
+        if (man_need (man, p, 1, 0))
+        {
+             man_send_cc_message (man, "CR_RESPONSE", p[1]);
+        }
+    }
     else if (streq(p[0], "net"))
     {
         man_net(man);
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index d24abe09..8acc18bf 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -164,6 +164,7 @@  struct management_callback
     int (*kill_by_addr) (void *arg, const in_addr_t addr, const int port);
     void (*delete_event) (void *arg, event_t event);
     int (*n_clients) (void *arg);
+    bool (*send_cc_message) (void *arg, const char *message, const char *parameter);
 #ifdef MANAGEMENT_DEF_AUTH
     bool (*kill_by_cid)(void *arg, const unsigned long cid, const char *kill_msg);
     bool (*client_auth) (void *arg,