Fallback to password authentication when auth-token fails

Under some circumstances, the auth-token stored by a client may not
be valid anymore.

For example, if the server is restarted, the client will try to
reconnect and resend the old token during authentication. Unfortunately
this attempt will fail, because the server does not keep track of tokens
used during previous runs.

With the current behaviour, depending on how auth-retry was configured,
the client will either just quit, prompt the user for username and password
as the original values are overwritten by the token, or endelessly fail
authentication by sending the old token over and over.

This patch changes the behaviour of the client so that, upon failed
authentication using a token, it will drop the token, perform a soft
restart (USR1) and attempt re-authenticating with the original password
provided by the user if auth-nocache was not specified.

Patch V2: properly formatted commit message, fix openvpn3 detection

Patch V3: remove all server changes, include only minimal non
intrusive client changes that only improve error recovery but don't
change overall behaviour.

Patch V4: forget add push.c to git index, now also included

Patch V5: is fixing overlong lines and one minor style problem.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20181010143051.27163-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17718.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 07cf3113daec9ca25e95e1f1f104a6fc689f9b44..7abcaf1eb46d8751af7f7272c3cef06b9ab8f641 100644 (file)
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -5412,6 +5412,12 @@ into the file/buffer for dynamic configuration data.  This
 will then make the OpenVPN server to push this value to the
 client, which replaces the local password with the
 UNIQUE_TOKEN_VALUE.
+
+Newer clients (2.4.7+) will fall back to the original password method
+after a failed auth. Older clients will keep using the token value
+and react acording to
+.B \-\-auth-retry
+.
 .\"*********************************************************
 .TP
 .B \-\-tls\-verify cmd

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index f5a27dc16cfe5c1ca2dd9890d0bbe3faec4e5bd8..1931149bb4b2a65d1e4dd80658625f864a8f3fd1 100644 (file)
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -469,7 +469,7 @@ purge_user_pass(struct user_pass *up, const bool force)
      * don't show warning if the pass has been replaced by a token: this is an
      * artificial "auth-nocache"
      */
-    else if (!warn_shown && (!up->tokenized))
+    else if (!warn_shown)
     {
         msg(M_WARN, "WARNING: this configuration may cache passwords in memory -- use the auth-nocache option to prevent this");
         warn_shown = true;
@@ -477,14 +477,18 @@ purge_user_pass(struct user_pass *up, const bool force)
 }
 
 void
-set_auth_token(struct user_pass *up, const char *token)
+set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token)
 {
-    if (token && strlen(token) && up && up->defined && !up->nocache)
+
+    if (token && strlen(token) && up && up->defined)
     {
-        CLEAR(up->password);
-        strncpynt(up->password, token, USER_PASS_LEN);
-        up->tokenized = true;
+        strncpynt(tk->password, token, USER_PASS_LEN);
+        strncpynt(tk->username, up->username, USER_PASS_LEN);
+        tk->defined = true;
     }
+
+    /* Cleans user/pass for nocache */
+    purge_user_pass(up, false);
 }
 
 /*

diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h
index 009425f0dd319ee3482b34240fdc2fb21d041e91..991b7df21b8ea7db7b3fba65b805885493d99db7 100644 (file)
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -63,7 +63,6 @@ struct user_pass
 {
     bool defined;
     bool nocache;
-    bool tokenized; /* true if password has been substituted by a token */
     bool wait_for_push; /* true if this object is waiting for a push-reply */
 
 /* max length of username/password */
@@ -145,7 +144,8 @@ void fail_user_pass(const char *prefix,
 
 void purge_user_pass(struct user_pass *up, const bool force);
 
-void set_auth_token(struct user_pass *up, const char *token);
+void set_auth_token(struct user_pass *up, struct user_pass *tk,
+                    const char *token);
 
 /*
  * Process string received by untrusted peer before

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 72f09962143a17fcc1845792b3ed0fb9b000451a..8befc6f595f853ad7ea88b8c033ad5967eb29ba3 100644 (file)
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -55,8 +55,20 @@ receive_auth_failed(struct context *c, const struct buffer *buffer)
 
     if (c->options.pull)
     {
-        switch (auth_retry_get())
+        /* Before checking how to react on AUTH_FAILED, first check if the
+         * failed auth might be the result of an expired auth-token.
+         * Note that a server restart will trigger a generic AUTH_FAILED
+         * instead an AUTH_FAILED,SESSION so handle all AUTH_FAILED message
+         * identical for this scenario */
+        if (ssl_clean_auth_token())
         {
+            c->sig->signal_received = SIGUSR1; /* SOFT-SIGUSR1 -- Auth failure error */
+            c->sig->signal_text = "auth-failure (auth-token)";
+        }
+        else
+        {
+            switch (auth_retry_get())
+            {
             case AR_NONE:
                 c->sig->signal_received = SIGTERM; /* SOFT-SIGTERM -- Auth failure error */
                 break;
@@ -70,8 +82,9 @@ receive_auth_failed(struct context *c, const struct buffer *buffer)
 
             default:
                 ASSERT(0);
+            }
+            c->sig->signal_text = "auth-failure";
         }
-        c->sig->signal_text = "auth-failure";
 #ifdef ENABLE_MANAGEMENT
         if (management)
         {

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 74b88ce6fa7b0c2b001ced63e17f9b29ab36887f..c0bc7a47e630ad1f813db0a90ce0721bc4316a06 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -398,6 +398,7 @@ pem_password_callback(char *buf, int size, int rwflag, void *u)
 
 static bool auth_user_pass_enabled;     /* GLOBAL */
 static struct user_pass auth_user_pass; /* GLOBAL */
+static struct user_pass auth_token;     /* GLOBAL */
 
 #ifdef ENABLE_MANAGEMENT
 static char *auth_challenge; /* GLOBAL */
@@ -407,7 +408,7 @@ void
 auth_user_pass_setup(const char *auth_file, const struct static_challenge_info *sci)
 {
     auth_user_pass_enabled = true;
-    if (!auth_user_pass.defined)
+    if (!auth_user_pass.defined && !auth_token.defined)
     {
 #ifdef ENABLE_MANAGEMENT
         if (auth_challenge) /* dynamic challenge/response */
@@ -445,7 +446,7 @@ ssl_set_auth_nocache(void)
 {
     passbuf.nocache = true;
     auth_user_pass.nocache = true;
-    /* wait for push-reply, because auth-token may invert nocache */
+    /* wait for push-reply, because auth-token may still need the username */
     auth_user_pass.wait_for_push = true;
 }
 
@@ -455,15 +456,18 @@ ssl_set_auth_nocache(void)
 void
 ssl_set_auth_token(const char *token)
 {
-    if (auth_user_pass.nocache)
-    {
-        msg(M_INFO,
-            "auth-token received, disabling auth-nocache for the "
-            "authentication token");
-        auth_user_pass.nocache = false;
-    }
+    set_auth_token(&auth_user_pass, &auth_token, token);
+}
 
-    set_auth_token(&auth_user_pass, token);
+/*
+ * Cleans an auth token and checks if it was active
+ */
+bool
+ssl_clean_auth_token (void)
+{
+    bool wasdefined = auth_token.defined;
+    purge_user_pass(&auth_token, true);
+    return wasdefined;
 }
 
 /*
@@ -2408,19 +2412,26 @@ key_method_2_write(struct buffer *buf, struct tls_session *session)
 #else
         auth_user_pass_setup(session->opt->auth_user_pass_file, NULL);
 #endif
-        if (!write_string(buf, auth_user_pass.username, -1))
+        struct user_pass *up = &auth_user_pass;
+
+        /*
+         * If we have a valid auth-token, send that instead of real
+         * username/password
+         */
+        if (auth_token.defined)
+            up = &auth_token;
+
+        if (!write_string(buf, up->username, -1))
         {
             goto error;
         }
-        if (!write_string(buf, auth_user_pass.password, -1))
+        else if (!write_string(buf, up->password, -1))
         {
             goto error;
         }
         /* if auth-nocache was specified, the auth_user_pass object reaches
          * a "complete" state only after having received the push-reply
          * message.
-         * This is the case because auth-token statement in a push-reply would
-         * invert its nocache.
          *
          * For this reason, skip the purge operation here if no push-reply
          * message has been received yet.

diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index ac6fef75ebdf215209c65547b1184080fbb655ae..eafb235ef0d474b8dc055b036ce2a2569632ad83 100644 (file)
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -439,6 +439,8 @@ void ssl_set_auth_token(const char *token);
  */
 void ssl_purge_auth_challenge(void);
 
+bool ssl_clean_auth_token(void);
+
 void ssl_put_auth_challenge(const char *cr_str);
 
 #endif