[Openvpn-devel,v2,1/2] Send auth fail to client on reneg failure

Message ID cc9094b8-caa6-34b6-ed7a-3ccb34560113@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 relies on Arne's "Add send_control_channel_string_dowork
variant" patch.

This patch modifies auth so that on a renegotiation the client is
informed of a SESSION re-auth failure during a renegotiation if either
their auth-token has expired, or they enter a wrong password in the case
of auth-nocache for example.

This also addresses my previous patch for supporting a client reason
being rejected.

Regards,
Eric

Comments

Arne Schwabe June 14, 2019, 12:01 a.m. UTC | #1
Am 11.04.19 um 04:07 schrieb Eric Thorpe:
> Hi All,
> 
> This patch relies on Arne's "Add send_control_channel_string_dowork
> variant" patch.
> 
> This patch modifies auth so that on a renegotiation the client is
> informed of a SESSION re-auth failure during a renegotiation if either
> their auth-token has expired, or they enter a wrong password in the case
> of auth-nocache for example.

Unfortenately this patch modifies the same code path that the auth-token
patch set modifies and creates conflicts.

So this patch depends on the earlier patches of the patch set (which
happen to be already applied) but breaks the later ones of the patch set

So while the patch looks good enough, I would like to postpone it after
the auth-token set has been applied.

Arne
Gert Doering Aug. 10, 2020, 11:32 p.m. UTC | #2
Hi,

On Thu, Apr 11, 2019 at 12:07:27PM +1000, Eric Thorpe wrote:
> This patch relies on Arne's "Add send_control_channel_string_dowork
> variant" patch.

So.  After we left you out in the cold for over a year (apologies 
for that), we should revisit your patch set.

I checked with Arne, he says "no, that functionality is not in yet,
but still interesting" - and all the *other* auth/plugin/async/SSO
related code we had brewing is in.

So - if you're not too frustrated with us yet: can you re-send?

gert
Eric Thorpe Aug. 12, 2020, 9:32 p.m. UTC | #3
Have re-based to master and resent.

Cheers,
Eric

---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com
https://twitter.com/sparklabs
support@sparklabs.com

On 11/08/2020 7:32 pm, Gert Doering wrote:
> Hi,
>
> On Thu, Apr 11, 2019 at 12:07:27PM +1000, Eric Thorpe wrote:
>> This patch relies on Arne's "Add send_control_channel_string_dowork
>> variant" patch.
> So.  After we left you out in the cold for over a year (apologies
> for that), we should revisit your patch set.
>
> I checked with Arne, he says "no, that functionality is not in yet,
> but still interesting" - and all the *other* auth/plugin/async/SSO
> related code we had brewing is in.
>
> So - if you're not too frustrated with us yet: can you re-send?
>
> gert

Patch

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 28c3b8867..a883faa79 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2085,6 +2085,7 @@  multi_connection_established(struct multi_context *m, struct multi_instance *mi)
 
         /* set flag so we don't get called again */
         mi->connection_established_flag = true;
+        mi->context.c2.tls_multi->connection_established = true;
 
         /* increment number of current authenticated clients */
         ++m->n_clients;
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index dd5bd4163..327ae0891 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -233,6 +233,36 @@  send_auth_failed(struct context *c, const char *client_reason)
     gc_free(&gc);
 }
 
+/*
+ * Send auth failed message from server to client without scheduling.
+ * Main use for queuing a message during renegotiation
+ */
+void
+send_push_reply_auth_failed(struct tls_multi *multi, const char *client_reason)
+{
+    struct gc_arena gc = gc_new();
+    static const char auth_failed[] = "AUTH_FAILED";
+    size_t len;
+
+    len = (client_reason ? strlen(client_reason)+1 : 0) + sizeof(auth_failed);
+    if (len > PUSH_BUNDLE_SIZE)
+    {
+        len = PUSH_BUNDLE_SIZE;
+    }
+
+    {
+        struct buffer buf = alloc_buf_gc(len, &gc);
+        buf_printf(&buf, auth_failed);
+        if (client_reason)
+        {
+            buf_printf(&buf, ",%s", client_reason);
+        }
+        send_control_channel_string_dowork(multi, BSTR(&buf), D_PUSH);
+    }
+
+    gc_free(&gc);
+}
+
 /*
  * Send restart message from server to client.
  */
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index ac25ffa78..b82a014a0 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -547,6 +547,7 @@  struct tls_multi
     time_t auth_token_tstamp; /**< timestamp of the generated token */
     bool auth_token_sent; /**< If server uses --auth-gen-token and
                            *   token has been sent to client */
+    bool connection_established ; /** Notifies future auth calls this is a reneg */
     /*
      * Our session objects.
      */
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index c7e595e46..fd83cde85 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1336,6 +1336,7 @@  verify_user_pass(struct user_pass *up, struct tls_multi *multi,
             && (multi->auth_token_tstamp + session->opt->auth_token_lifetime) < now)
         {
             msg(D_HANDSHAKE, "Auth-token for client expired\n");
+            send_push_reply_auth_failed(multi, "SESSION:Auth-token expired");
             wipe_auth_token(multi);
             ks->authenticated = false;
             goto done;
@@ -1458,6 +1459,12 @@  verify_user_pass(struct user_pass *up, struct tls_multi *multi,
     }
     else
     {
+        if (multi->connection_established)
+        {
+            /* Notify the client */
+            send_push_reply_auth_failed(multi, "SESSION:Auth failed");
+
+        }
         msg(D_TLS_ERRORS, "TLS Auth Error: Auth Username/Password verification failed for peer");
     }