Further improve the memory management when a clients --auth-token
fails the server side token authentication enabled via --auth-gen-token.

v2 - Add ASSERT() if base64 encoding of token fails
v3 - Use proper boolean logic in ASSERT()

Signed-off-by: David Sommerseth <dav...@openvpn.net>
---
 src/openvpn/ssl_verify.c | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 4328828..d661473 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1108,6 +1108,21 @@ verify_user_pass_management (struct tls_session 
*session, const struct user_pass
 }
 #endif
 
+/**
+ *  Wipes the authentication token out of the memory, frees and cleans up 
related buffers and flags
+ *
+ *  @param multi  Pointer to a multi object holding the auth_token variables
+ */
+static void
+wipe_auth_token(struct tls_multi *multi)
+{
+  secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
+  free (multi->auth_token);
+  multi->auth_token = NULL;
+  multi->auth_token_sent = false;
+}
+
+
 /*
  * Main username/password verification entry point
  */
@@ -1157,6 +1172,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
       /* Ensure that the username has not changed */
       if (!tls_lock_username(multi, up->username))
         {
+          wipe_auth_token(multi);
           ks->authenticated = false;
           goto done;
         }
@@ -1168,6 +1184,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");
+          wipe_auth_token(multi);
           ks->authenticated = false;
           goto done;
         }
@@ -1176,10 +1193,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
       if (memcmp_constant_time(multi->auth_token, up->password,
                  strlen(multi->auth_token)) != 0)
         {
-          secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
-          free (multi->auth_token);
-          multi->auth_token = NULL;
-          multi->auth_token_sent = false;
+          wipe_auth_token(multi);
           ks->authenticated = false;
           tls_deauthenticate (multi);
 
@@ -1255,24 +1269,11 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
          /* The token should be longer than the input when
            * being base64 encoded
            */
-         if( openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
-                                    &multi->auth_token) < AUTH_TOKEN_SIZE)
-           {
-             msg(D_TLS_ERRORS, "BASE64 encoding of token failed. "
-                  "No auth-token will be activated now");
-             if (multi->auth_token)
-               {
-                 secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
-                 free (multi->auth_token);
-                 multi->auth_token = NULL;
-               }
-           }
-         else
-           {
-             multi->auth_token_tstamp = now;
-             dmsg (D_SHOW_KEYS, "Generated token for client: %s",
-                    multi->auth_token);
-           }
+          ASSERT(openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
+                                       &multi->auth_token) > AUTH_TOKEN_SIZE);
+          multi->auth_token_tstamp = now;
+          dmsg (D_SHOW_KEYS, "Generated token for client: %s",
+                multi->auth_token);
        }
 
       if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME))
-- 
1.8.3.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to