From: Antonio Quartulli <a@unstable.cc>
Date: Sat, 25 Feb 2017 00:40:14 +0000 (+0800)
Subject: Ignore auth-nocache for auth-user-pass if auth-token is pushed
X-Git-Tag: v2.5_beta1~662
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=571165360db0392fa83ec8e6f8de145f623c53fe;p=thirdparty%2Fopenvpn.git

Ignore auth-nocache for auth-user-pass if auth-token is pushed

When the auth-token option is pushed from the server to the client,
the latter has to ignore the auth-nocache directive (if specified).

The password will now be substituted by the unique token, therefore
it can't be wiped out, otherwise the next renegotiation will fail.

Trac: #840
Cc: David Sommerseth <openvpn@sf.lists.topphemmelig.net>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20170225004014.28638-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14194.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
---

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 91ab28e04..0832f1d1b 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1382,6 +1382,18 @@ initialization_sequence_completed(struct context *c, const unsigned int flags)
     /* If we delayed UID/GID downgrade or chroot, do it now */
     do_uid_gid_chroot(c, true);
 
+    /*
+     * In some cases (i.e. when receiving auth-token via
+     * push-reply) the auth-nocache option configured on the
+     * client is overridden; for this reason we have to wait
+     * for the push-reply message before attempting to wipe
+     * the user/pass entered by the user
+     */
+    if (c->options.mode == MODE_POINT_TO_POINT)
+    {
+        delayed_auth_pass_purge();
+    }
+
     /* Test if errors */
     if (flags & ISC_ERRORS)
     {
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index df108b08d..fbd993854 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -1430,7 +1430,11 @@ purge_user_pass(struct user_pass *up, const bool force)
         secure_memzero(up, sizeof(*up));
         up->nocache = nocache;
     }
-    else if (!warn_shown)
+    /*
+     * 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))
     {
         msg(M_WARN, "WARNING: this configuration may cache passwords in memory -- use the auth-nocache option to prevent this");
         warn_shown = true;
@@ -1444,6 +1448,7 @@ set_auth_token(struct user_pass *up, const char *token)
     {
         CLEAR(up->password);
         strncpynt(up->password, token, USER_PASS_LEN);
+        up->tokenized = true;
     }
 }
 
diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h
index 94573b217..ce965492e 100644
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -201,6 +201,8 @@ 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 */
 #ifdef ENABLE_PKCS11
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index c658f3778..29280dcea 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -451,6 +451,8 @@ ssl_set_auth_nocache(void)
 {
     passbuf.nocache = true;
     auth_user_pass.nocache = true;
+    /* wait for push-reply, because auth-token may invert nocache */
+    auth_user_pass.wait_for_push = true;
 }
 
 /*
@@ -459,6 +461,14 @@ 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, token);
 }
 
@@ -2383,7 +2393,21 @@ key_method_2_write(struct buffer *buf, struct tls_session *session)
         {
             goto error;
         }
-        purge_user_pass(&auth_user_pass, false);
+        /* 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.
+         *
+         * This normally happens upon first negotiation only.
+         */
+        if (!auth_user_pass.wait_for_push)
+        {
+            purge_user_pass(&auth_user_pass, false);
+        }
     }
     else
     {
@@ -4226,6 +4250,13 @@ done:
     return BSTR(&out);
 }
 
+void
+delayed_auth_pass_purge(void)
+{
+    auth_user_pass.wait_for_push = false;
+    purge_user_pass(&auth_user_pass, false);
+}
+
 #else  /* if defined(ENABLE_CRYPTO) */
 static void
 dummy(void)
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index b119c166d..56ea60137 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -598,6 +598,8 @@ void extract_x509_field_test(void);
  */
 bool is_hard_reset(int op, int key_method);
 
+void delayed_auth_pass_purge(void);
+
 #endif /* ENABLE_CRYPTO */
 
 #endif /* ifndef OPENVPN_SSL_H */