Re: [Openvpn-devel] [PATCH] Ignore auth-nocache for auth-user-pass if auth-token is pushed

2022-10-09 Thread Gert Doering 
Hi,

On Sun, Oct 09, 2022 at 10:13:32AM +0200, Gert Doering wrote:
> This patch was still sitting "unanswered" in the list archives (though
> it never landed in patchwork, as far as I can see).

*sigh*

More coffee on a sunday morning, it seems - this was actually merged
just fine, just never recorded in the referenced trac ticket.

So, just ignore me :-)

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Ignore auth-nocache for auth-user-pass if auth-token is pushed

2022-10-09 Thread Gert Doering 
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.
> 
> Trac: #840
> Cc: David Sommerseth 
> Signed-off-by: Antonio Quartulli 

This patch was still sitting "unanswered" in the list archives (though
it never landed in patchwork, as far as I can see).

NAK :-) - on the basis of "multiple rounds of patches from Arne related
to auth-nocache and/or auth-token have landed in 2.4 and 2.5, so I think
the underlying problem has been fixed for good, AND this patch won't
apply anymore anyway".

(Going through my "open issues" mail heap for all that has auth-user-pass
in the subject, since we finally have inline auth-user-passed done... :-) )

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Ignore auth-nocache for auth-user-pass if auth-token is pushed

2017-05-23 Thread Arne Schwabe 
Am 25.02.17 um 14:10 schrieb David Sommerseth:
> 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.
> 

Code looks good. So ACK. We probably need another revision on this
auth-token client support (for reconnects) but this is going in the
right direction.

Arne




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

Re: [Openvpn-devel] [PATCH] Ignore auth-nocache for auth-user-pass if auth-token is pushed

2017-02-25 Thread David Sommerseth 
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 (_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 (_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(_user_pass, false);
+}
 }
   else
 {
@@ -3620,6 +3643,13 @@ done:
   return BSTR ();
 }
 
+void
+delayed_auth_pass_purge(void)
+{
+auth_user_pass.wait_for_push = false;
+

Re: [Openvpn-devel] [PATCH] Ignore auth-nocache for auth-user-pass if auth-token is pushed

2017-02-25 Thread Gert Doering 
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...

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP 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

[Openvpn-devel] [PATCH] Ignore auth-nocache for auth-user-pass if auth-token is pushed

2017-02-24 Thread Antonio Quartulli 
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 
Signed-off-by: Antonio Quartulli 
---
 src/openvpn/init.c | 12 
 src/openvpn/misc.c |  7 ++-
 src/openvpn/misc.h |  2 ++
 src/openvpn/ssl.c  | 33 -
 src/openvpn/ssl.h  |  2 ++
 5 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index ff1551ea..3c0bb32b 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1383,6 +1383,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 a2f45b61..e6678ec8 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -1480,7 +1480,11 @@ purge_user_pass(struct user_pass *up, const bool force)
 secure_memzero(up, sizeof(*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;
@@ -1494,6 +1498,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 16be6219..201ebf62 100644
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -206,6 +206,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 86450fe0..ce301560 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -452,6 +452,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;
 }
 
 /*
@@ -460,6 +462,14 @@ 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(_user_pass, token);
 }
 
@@ -2383,7 +2393,21 @@ key_method_2_write(struct buffer *buf, struct 
tls_session *session)
 {
 goto error;
 }
-purge_user_pass(_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(_user_pass, false);
+}
 }
 else
 {
@@ -4214,6 +4238,13 @@ done:
 return BSTR();
 }
 
+void
+delayed_auth_pass_purge(void)
+{
+auth_user_pass.wait_for_push = false;
+purge_user_pass(_user_pass, false);
+}
+
 #else  /* if defined(ENABLE_CRYPTO) */
 static void
 dummy(void)
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index ed1344e7..33a02544 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -591,6 +591,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 */
 
 #endif /* ifndef OPENVPN_SSL_H */
-- 
2.11.1