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>
... | ... |
@@ -1382,6 +1382,18 @@ initialization_sequence_completed(struct context *c, const unsigned int flags) |
1382 | 1382 |
/* If we delayed UID/GID downgrade or chroot, do it now */ |
1383 | 1383 |
do_uid_gid_chroot(c, true); |
1384 | 1384 |
|
1385 |
+ /* |
|
1386 |
+ * In some cases (i.e. when receiving auth-token via |
|
1387 |
+ * push-reply) the auth-nocache option configured on the |
|
1388 |
+ * client is overridden; for this reason we have to wait |
|
1389 |
+ * for the push-reply message before attempting to wipe |
|
1390 |
+ * the user/pass entered by the user |
|
1391 |
+ */ |
|
1392 |
+ if (c->options.mode == MODE_POINT_TO_POINT) |
|
1393 |
+ { |
|
1394 |
+ delayed_auth_pass_purge(); |
|
1395 |
+ } |
|
1396 |
+ |
|
1385 | 1397 |
/* Test if errors */ |
1386 | 1398 |
if (flags & ISC_ERRORS) |
1387 | 1399 |
{ |
... | ... |
@@ -1430,7 +1430,11 @@ purge_user_pass(struct user_pass *up, const bool force) |
1430 | 1430 |
secure_memzero(up, sizeof(*up)); |
1431 | 1431 |
up->nocache = nocache; |
1432 | 1432 |
} |
1433 |
- else if (!warn_shown) |
|
1433 |
+ /* |
|
1434 |
+ * don't show warning if the pass has been replaced by a token: this is an |
|
1435 |
+ * artificial "auth-nocache" |
|
1436 |
+ */ |
|
1437 |
+ else if (!warn_shown && (!up->tokenized)) |
|
1434 | 1438 |
{ |
1435 | 1439 |
msg(M_WARN, "WARNING: this configuration may cache passwords in memory -- use the auth-nocache option to prevent this"); |
1436 | 1440 |
warn_shown = true; |
... | ... |
@@ -1444,6 +1448,7 @@ set_auth_token(struct user_pass *up, const char *token) |
1444 | 1444 |
{ |
1445 | 1445 |
CLEAR(up->password); |
1446 | 1446 |
strncpynt(up->password, token, USER_PASS_LEN); |
1447 |
+ up->tokenized = true; |
|
1447 | 1448 |
} |
1448 | 1449 |
} |
1449 | 1450 |
|
... | ... |
@@ -201,6 +201,8 @@ struct user_pass |
201 | 201 |
{ |
202 | 202 |
bool defined; |
203 | 203 |
bool nocache; |
204 |
+ bool tokenized; /* true if password has been substituted by a token */ |
|
205 |
+ bool wait_for_push; /* true if this object is waiting for a push-reply */ |
|
204 | 206 |
|
205 | 207 |
/* max length of username/password */ |
206 | 208 |
#ifdef ENABLE_PKCS11 |
... | ... |
@@ -451,6 +451,8 @@ ssl_set_auth_nocache(void) |
451 | 451 |
{ |
452 | 452 |
passbuf.nocache = true; |
453 | 453 |
auth_user_pass.nocache = true; |
454 |
+ /* wait for push-reply, because auth-token may invert nocache */ |
|
455 |
+ auth_user_pass.wait_for_push = true; |
|
454 | 456 |
} |
455 | 457 |
|
456 | 458 |
/* |
... | ... |
@@ -459,6 +461,14 @@ ssl_set_auth_nocache(void) |
459 | 459 |
void |
460 | 460 |
ssl_set_auth_token(const char *token) |
461 | 461 |
{ |
462 |
+ if (auth_user_pass.nocache) |
|
463 |
+ { |
|
464 |
+ msg(M_INFO, |
|
465 |
+ "auth-token received, disabling auth-nocache for the " |
|
466 |
+ "authentication token"); |
|
467 |
+ auth_user_pass.nocache = false; |
|
468 |
+ } |
|
469 |
+ |
|
462 | 470 |
set_auth_token(&auth_user_pass, token); |
463 | 471 |
} |
464 | 472 |
|
... | ... |
@@ -2383,7 +2393,21 @@ key_method_2_write(struct buffer *buf, struct tls_session *session) |
2383 | 2383 |
{ |
2384 | 2384 |
goto error; |
2385 | 2385 |
} |
2386 |
- purge_user_pass(&auth_user_pass, false); |
|
2386 |
+ /* if auth-nocache was specified, the auth_user_pass object reaches |
|
2387 |
+ * a "complete" state only after having received the push-reply |
|
2388 |
+ * message. |
|
2389 |
+ * This is the case because auth-token statement in a push-reply would |
|
2390 |
+ * invert its nocache. |
|
2391 |
+ * |
|
2392 |
+ * For this reason, skip the purge operation here if no push-reply |
|
2393 |
+ * message has been received yet. |
|
2394 |
+ * |
|
2395 |
+ * This normally happens upon first negotiation only. |
|
2396 |
+ */ |
|
2397 |
+ if (!auth_user_pass.wait_for_push) |
|
2398 |
+ { |
|
2399 |
+ purge_user_pass(&auth_user_pass, false); |
|
2400 |
+ } |
|
2387 | 2401 |
} |
2388 | 2402 |
else |
2389 | 2403 |
{ |
... | ... |
@@ -4226,6 +4250,13 @@ done: |
4226 | 4226 |
return BSTR(&out); |
4227 | 4227 |
} |
4228 | 4228 |
|
4229 |
+void |
|
4230 |
+delayed_auth_pass_purge(void) |
|
4231 |
+{ |
|
4232 |
+ auth_user_pass.wait_for_push = false; |
|
4233 |
+ purge_user_pass(&auth_user_pass, false); |
|
4234 |
+} |
|
4235 |
+ |
|
4229 | 4236 |
#else /* if defined(ENABLE_CRYPTO) */ |
4230 | 4237 |
static void |
4231 | 4238 |
dummy(void) |