On 25/02/17 10:19, Gert Doering wrote: > Hi, > > On Sat, Feb 25, 2017 at 08:40:14AM +0800, Antonio Quartulli wrote: >> 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. > > Without looking at the patch itself - is this suitable material for > inclusion in 2.3? We do have quite a few "slow adopters" - and this > is a very useful feature to mitigate SWEET32 in 2FA environments...
The code paths involved shouldn't be very differ too much between v2.3 and v2.4. So I would say this should go into v2.3 as well. Attached is a very preliminary (and only compile and 'make check' tested) patch of a backport to v2.3. This needs to get a thorough test as well before we'll send an official patch to this ML. Btw. since I have worked closely with Antonio on this patch, testing and debugging and discussing it for some time, I think it would be good if someone else than me does the final code review and ACK/NAK it. I'm not able to be objective on this patch. -- kind regards, David Sommerseth OpenVPN Technologies, Inc
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index dc63475..3603c36 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1253,6 +1253,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 39aa936..546d87b 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -1343,7 +1343,11 @@ purge_user_pass (struct user_pass *up, const bool force)
CLEAR (*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;
@@ -1357,6 +1361,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 2fc281d..43d6b6c 100644
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -196,6 +196,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 32d0b6b..831b003 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -430,6 +430,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;
}
/*
@@ -438,6 +440,13 @@ 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);
}
@@ -1944,7 +1953,21 @@ key_method_2_write (struct buffer *buf, struct tls_session *session)
goto error;
if (!write_string (buf, auth_user_pass.password, -1))
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
{
@@ -3620,6 +3643,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
static void dummy(void) {}
#endif /* ENABLE_CRYPTO && ENABLE_SSL*/
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index c3d32c7..136ad75 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -503,6 +503,8 @@ void show_tls_performance_stats(void);
/*#define EXTRACT_X509_FIELD_TEST*/
void extract_x509_field_test (void);
+void delayed_auth_pass_purge(void);
+
#endif /* ENABLE_CRYPTO && ENABLE_SSL */
#endif
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ 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
