Browse code

auth-token: Ensure tokens are always wiped on de-auth

If tls_deauthenticate() was called, it could in some scenarios leave the
authentication token for a session in memory. This change just ensures
auth-tokens are always wiped as soon as a TLS session is considered
broken.

Signed-off-by: David Sommerseth <davids@openvpn.net>

Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20170328205346.18844-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14344.html
Signed-off-by: David Sommerseth <davids@openvpn.net>

David Sommerseth authored on 2017/03/29 05:53:46
Showing 1 changed files
... ...
@@ -80,6 +80,28 @@ setenv_untrusted(struct tls_session *session)
80 80
     setenv_link_socket_actual(session->opt->es, "untrusted", &session->untrusted_addr, SA_IP_PORT);
81 81
 }
82 82
 
83
+
84
+/**
85
+ *  Wipes the authentication token out of the memory, frees and cleans up related buffers and flags
86
+ *
87
+ *  @param multi  Pointer to a multi object holding the auth_token variables
88
+ */
89
+static void
90
+wipe_auth_token(struct tls_multi *multi)
91
+{
92
+    if(multi)
93
+    {
94
+        if (multi->auth_token)
95
+        {
96
+            secure_memzero(multi->auth_token, AUTH_TOKEN_SIZE);
97
+            free(multi->auth_token);
98
+        }
99
+        multi->auth_token = NULL;
100
+        multi->auth_token_sent = false;
101
+    }
102
+}
103
+
104
+
83 105
 /*
84 106
  * Remove authenticated state from all sessions in the given tunnel
85 107
  */
... ...
@@ -88,10 +110,10 @@ tls_deauthenticate(struct tls_multi *multi)
88 88
 {
89 89
     if (multi)
90 90
     {
91
-        int i, j;
92
-        for (i = 0; i < TM_SIZE; ++i)
91
+        wipe_auth_token(multi);
92
+        for (int i = 0; i < TM_SIZE; ++i)
93 93
         {
94
-            for (j = 0; j < KS_SIZE; ++j)
94
+            for (int j = 0; j < KS_SIZE; ++j)
95 95
             {
96 96
                 multi->session[i].key[j].authenticated = false;
97 97
             }
... ...
@@ -1219,21 +1241,6 @@ verify_user_pass_management(struct tls_session *session, const struct user_pass
1219 1219
 }
1220 1220
 #endif /* ifdef MANAGEMENT_DEF_AUTH */
1221 1221
 
1222
-/**
1223
- *  Wipes the authentication token out of the memory, frees and cleans up related buffers and flags
1224
- *
1225
- *  @param multi  Pointer to a multi object holding the auth_token variables
1226
- */
1227
-static void
1228
-wipe_auth_token(struct tls_multi *multi)
1229
-{
1230
-    secure_memzero(multi->auth_token, AUTH_TOKEN_SIZE);
1231
-    free(multi->auth_token);
1232
-    multi->auth_token = NULL;
1233
-    multi->auth_token_sent = false;
1234
-}
1235
-
1236
-
1237 1222
 /*
1238 1223
  * Main username/password verification entry point
1239 1224
  */
... ...
@@ -1285,7 +1292,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
1285 1285
         /* Ensure that the username has not changed */
1286 1286
         if (!tls_lock_username(multi, up->username))
1287 1287
         {
1288
-            wipe_auth_token(multi);
1288
+            /* auth-token cleared in tls_lock_username() on failure */
1289 1289
             ks->authenticated = false;
1290 1290
             goto done;
1291 1291
         }
... ...
@@ -1306,7 +1313,6 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
1306 1306
         if (memcmp_constant_time(multi->auth_token, up->password,
1307 1307
                                  strlen(multi->auth_token)) != 0)
1308 1308
         {
1309
-            wipe_auth_token(multi);
1310 1309
             ks->authenticated = false;
1311 1310
             tls_deauthenticate(multi);
1312 1311
 
... ...
@@ -1478,6 +1484,7 @@ verify_final_auth_checks(struct tls_multi *multi, struct tls_session *session)
1478 1478
         if (!cn || !strcmp(cn, CCD_DEFAULT) || !test_file(path))
1479 1479
         {
1480 1480
             ks->authenticated = false;
1481
+            wipe_auth_token(multi);
1481 1482
             msg(D_TLS_ERRORS, "TLS Auth Error: --client-config-dir authentication failed for common name '%s' file='%s'",
1482 1483
                 session->common_name,
1483 1484
                 path ? path : "UNDEF");