[Openvpn-devel,v3,5/5] Implement forwarding client CR_RESPONSE messages to management

Message ID 20200519220004.25136-6-arne@rfc2549.org
State Accepted
Headers show
Series Implement additional two step authentication methods | expand

Commit Message

Arne Schwabe May 19, 2020, noon UTC
When signalling the client that it should do Challenge response
without reconnecting (IV_SSO=crtext/INFOPRE=CR_TEXT), the server
needs forward the response via the management console.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 doc/management-notes.txt | 30 +++++++++++++++++++++++++++++-
 src/openvpn/forward.c    |  4 ++++
 src/openvpn/manage.c     | 28 +++++++++++++++++++++++++++-
 src/openvpn/manage.h     |  5 +++++
 src/openvpn/push.c       | 22 ++++++++++++++++++++++
 src/openvpn/push.h       |  2 ++
 6 files changed, 89 insertions(+), 2 deletions(-)

Comments

David Sommerseth May 27, 2020, 11:15 a.m. UTC | #1
On 20/05/2020 00:00, Arne Schwabe wrote:
> When signalling the client that it should do Challenge response
> without reconnecting (IV_SSO=crtext/INFOPRE=CR_TEXT), the server
> needs forward the response via the management console.
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  doc/management-notes.txt | 30 +++++++++++++++++++++++++++++-
>  src/openvpn/forward.c    |  4 ++++
>  src/openvpn/manage.c     | 28 +++++++++++++++++++++++++++-
>  src/openvpn/manage.h     |  5 +++++
>  src/openvpn/push.c       | 22 ++++++++++++++++++++++
>  src/openvpn/push.h       |  2 ++
>  6 files changed, 89 insertions(+), 2 deletions(-)
> 
[...]

Basing this also on prior testing, looking good.  Only done compile testing in
this round.

Acked-By: David Sommerseth <davids@openvpn.net>
Gert Doering June 20, 2020, 12:45 a.m. UTC | #2
Your patch has been applied to the master branch.

Fixed a typo in doc/management-notes.txt ("sequentiality" was meant to be
exactly this, not "sequentially").

Stared a bit at the code and ran basic t_client tests, but no real testing
and no real review.

commit 06498f21cdf051b0643606efda96b27b3c358e0c
Author: Arne Schwabe
Date:   Wed May 20 00:00:04 2020 +0200

     Implement forwarding client CR_RESPONSE messages to management

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: David Sommerseth <davids@openvpn.net>
     Message-Id: <20200519220004.25136-6-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19910.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index ce32b85f..5cfbb70a 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -1052,7 +1052,7 @@  The ">CLIENT:" notification is enabled by the --management-client-auth
 OpenVPN configuration directive that gives the management interface client
 the responsibility to authenticate OpenVPN clients after their client
 certificate has been verified.  CLIENT notifications may be multi-line, and
-the sequentiality of a given CLIENT notification, its associated environmental
+the sequentially of a given CLIENT notification, its associated environmental
 variables, and the terminating ">CLIENT:ENV,END" line are guaranteed to be
 atomic.
 
@@ -1094,6 +1094,34 @@  CLIENT notification types:
 
     >CLIENT:ADDRESS,{CID},{ADDR},{PRI}
 
+(5) Text based challenge/Response
+
+   >CLIENT:CR_RESPONSE,{CID},{KID},{response_base64}
+   >CLIENT:ENV,name1=val1
+   >CLIENT:ENV,name2=val2
+   >CLIENT:ENV,...
+   >CLIENT:ENV,END
+
+   Using the cr-response command on the client side will trigger this
+   message on the server side.
+
+   CR_RESPONSE notification. The >CR_RESPONSE fulfils the same purpose as the
+   CRV1 response in the traditional challenge/response. See that section
+   below for more details. Since this still uses the same cid as the original
+   response, we do not use the username and opaque session data in this
+   response but only contains the actual response.
+
+   It is important to note that OpenVPN2 merely passes the authentication
+   information and does not do any further checks. (E.g. if a CR was issued
+   before or if multiple CR responses were sent from the client or if
+   data has a valid base64 encoding)
+
+   This interface should be be sufficient for almost all challenge/response
+   system that can be implemented with a single round and base64 encoding the
+   response. Mechanisms that need multiple rounds or more complex answers
+   should implement a different response type than CR_RESPONSE.
+
+
 Variables:
 
 CID --  Client ID, numerical ID for each connecting client, sequence = 0,1,2,...
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 3b088f87..885cf126 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -403,6 +403,10 @@  check_incoming_control_channel_dowork(struct context *c)
             {
                 server_pushed_info(c, &buf, 4);
             }
+            else if (buf_string_match_head_str(&buf, "CR_RESPONSE"))
+            {
+                receive_cr_response(c, &buf);
+            }
             else
             {
                 msg(D_PUSH_ERRORS, "WARNING: Received unknown control message: %s", BSTR(&buf));
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 3ebe72ec..898cb3b3 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -2908,7 +2908,7 @@  management_notify_generic(struct management *man, const char *str)
 #ifdef MANAGEMENT_DEF_AUTH
 
 static void
-man_output_peer_info_env(struct management *man, struct man_def_auth_context *mdac)
+man_output_peer_info_env(struct management *man, const struct man_def_auth_context *mdac)
 {
     char line[256];
     if (man->persist.callback.get_peer_info)
@@ -2958,6 +2958,32 @@  management_notify_client_needing_auth(struct management *management,
     }
 }
 
+void
+management_notify_client_cr_response(unsigned mda_key_id,
+                                     const struct man_def_auth_context *mdac,
+                                     const struct env_set *es,
+                                     const char *response)
+{
+    struct gc_arena gc;
+    if (management)
+    {
+        gc = gc_new();
+
+        struct buffer out = alloc_buf_gc(256, &gc);
+        msg(M_CLIENT, ">CLIENT:CR_RESPONSE,%lu,%u,%s",
+            mdac->cid, mda_key_id, response);
+        man_output_extra_env(management, "CLIENT");
+        if (management->connection.env_filter_level>0)
+        {
+            man_output_peer_info_env(management, mdac);
+        }
+        man_output_env(es, true, management->connection.env_filter_level, "CLIENT");
+        management_notify_generic(management, BSTR(&out));
+
+        gc_free(&gc);
+    }
+}
+
 void
 management_connection_established(struct management *management,
                                   struct man_def_auth_context *mdac,
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index e28b11d1..8c824ca7 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -434,6 +434,11 @@  void management_learn_addr(struct management *management,
                            const struct mroute_addr *addr,
                            const bool primary);
 
+void management_notify_client_cr_response(unsigned mda_key_id,
+                                          const struct man_def_auth_context *mdac,
+                                          const struct env_set *es,
+                                          const char *response);
+
 #endif
 
 char *management_query_pk_sig(struct management *man, const char *b64_data,
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index a5fa87d8..26460490 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -209,6 +209,28 @@  server_pushed_info(struct context *c, const struct buffer *buffer,
     msg(D_PUSH, "Info command was pushed by server ('%s')", m);
 }
 
+void
+receive_cr_response(struct context *c, const struct buffer *buffer)
+{
+    struct buffer buf = *buffer;
+    const char *m = "";
+
+    if (buf_advance(&buf, 11) && buf_read_u8(&buf) == ',' && BLEN(&buf))
+    {
+        m = BSTR(&buf);
+    }
+#ifdef MANAGEMENT_DEF_AUTH
+    struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE];
+    struct man_def_auth_context *mda = session->opt->mda_context;
+    struct env_set *es = session->opt->es;
+    int key_id = session->key[KS_PRIMARY].key_id;
+
+
+    management_notify_client_cr_response(key_id, mda, es, m);
+#endif
+    msg(D_PUSH, "CR response was sent by client ('%s')", m);
+}
+
 /**
  * Add an option to the given push list by providing a format string.
  *
diff --git a/src/openvpn/push.h b/src/openvpn/push.h
index 42ab100d..2faf19a6 100644
--- a/src/openvpn/push.h
+++ b/src/openvpn/push.h
@@ -53,6 +53,8 @@  void server_pushed_signal(struct context *c, const struct buffer *buffer, const
 void server_pushed_info(struct context *c, const struct buffer *buffer,
                         const int adv);
 
+void receive_cr_response(struct context *c, const struct buffer *buffer);
+
 void incoming_push_message(struct context *c, const struct buffer *buffer);
 
 void clone_push_list(struct options *o);