Browse code

auth-gen-token: Hardening memory cleanup on auth-token failuers

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()
v4 - Rebase against The Great Reformatting

Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1481883907-26413-1-git-send-email-davids@openvpn.net>
URL: http://www.mail-archive.com/search?l=mid&q=1481883907-26413-1-git-send-email-davids@openvpn.net

David Sommerseth authored on 2016/12/16 19:25:07
Showing 1 changed files
... ...
@@ -1213,6 +1213,21 @@ verify_user_pass_management(struct tls_session *session, const struct user_pass
1213 1213
 }
1214 1214
 #endif /* ifdef MANAGEMENT_DEF_AUTH */
1215 1215
 
1216
+/**
1217
+ *  Wipes the authentication token out of the memory, frees and cleans up related buffers and flags
1218
+ *
1219
+ *  @param multi  Pointer to a multi object holding the auth_token variables
1220
+ */
1221
+static void
1222
+wipe_auth_token(struct tls_multi *multi)
1223
+{
1224
+    secure_memzero(multi->auth_token, AUTH_TOKEN_SIZE);
1225
+    free(multi->auth_token);
1226
+    multi->auth_token = NULL;
1227
+    multi->auth_token_sent = false;
1228
+}
1229
+
1230
+
1216 1231
 /*
1217 1232
  * Main username/password verification entry point
1218 1233
  */
... ...
@@ -1264,6 +1279,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
1264 1264
         /* Ensure that the username has not changed */
1265 1265
         if (!tls_lock_username(multi, up->username))
1266 1266
         {
1267
+            wipe_auth_token(multi);
1267 1268
             ks->authenticated = false;
1268 1269
             goto done;
1269 1270
         }
... ...
@@ -1275,6 +1291,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
1275 1275
             && (multi->auth_token_tstamp + session->opt->auth_token_lifetime) < now)
1276 1276
         {
1277 1277
             msg(D_HANDSHAKE, "Auth-token for client expired\n");
1278
+            wipe_auth_token(multi);
1278 1279
             ks->authenticated = false;
1279 1280
             goto done;
1280 1281
         }
... ...
@@ -1283,10 +1300,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
1283 1283
         if (memcmp_constant_time(multi->auth_token, up->password,
1284 1284
                                  strlen(multi->auth_token)) != 0)
1285 1285
         {
1286
-            secure_memzero(multi->auth_token, AUTH_TOKEN_SIZE);
1287
-            free(multi->auth_token);
1288
-            multi->auth_token = NULL;
1289
-            multi->auth_token_sent = false;
1286
+            wipe_auth_token(multi);
1290 1287
             ks->authenticated = false;
1291 1288
             tls_deauthenticate(multi);
1292 1289
 
... ...
@@ -1374,30 +1388,18 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
1374 1374
             /* The token should be longer than the input when
1375 1375
              * being base64 encoded
1376 1376
              */
1377
-            if (openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
1378
-                                      &multi->auth_token) < AUTH_TOKEN_SIZE)
1379
-            {
1380
-                msg(D_TLS_ERRORS, "BASE64 encoding of token failed. "
1381
-                    "No auth-token will be activated now");
1382
-                if (multi->auth_token)
1383
-                {
1384
-                    secure_memzero(multi->auth_token, AUTH_TOKEN_SIZE);
1385
-                    free(multi->auth_token);
1386
-                    multi->auth_token = NULL;
1387
-                }
1388
-            }
1389
-            else
1390
-            {
1391
-                multi->auth_token_tstamp = now;
1392
-                dmsg(D_SHOW_KEYS, "Generated token for client: %s",
1393
-                     multi->auth_token);
1394
-            }
1377
+            ASSERT(openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
1378
+                                         &multi->auth_token) > AUTH_TOKEN_SIZE);
1379
+            multi->auth_token_tstamp = now;
1380
+            dmsg(D_SHOW_KEYS, "Generated token for client: %s",
1381
+                 multi->auth_token);
1395 1382
         }
1396 1383
 
1397 1384
         if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME))
1398 1385
         {
1399 1386
             set_common_name(session, up->username);
1400 1387
         }
1388
+
1401 1389
 #ifdef ENABLE_DEF_AUTH
1402 1390
         msg(D_HANDSHAKE, "TLS: Username/Password authentication %s for username '%s' %s",
1403 1391
             ks->auth_deferred ? "deferred" : "succeeded",