diff mbox series

[Openvpn-devel,v2,for,2.5/2.6] Purge auth-token as well while purging passwords

Message ID 20221026185543.5378-1-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel,v2,for,2.5/2.6] Purge auth-token as well while purging passwords | expand

Commit Message

Selva Nair Oct. 26, 2022, 7:55 a.m. UTC 
From: Selva Nair <selva.nair@gmail.com>

Starting from commit e61b401a auth-token is saved in a separate struct
from auth-user-pass and is not cleared when ssl_purge_auth() is called.
This makes "forget-passwords" sent to the management
interface or "--management-forget-disconnect" option not to work
as expected.

Purging caused by --auth-nocache is not affected
(auth-token is retained in that case as it should be).

Use case:
For Pre-Logon access and persistent connections on Windows, use of
"forget-passwords" before disconnect is probably the only way to
ensure that no credentials are left behind. Note that openvpn.exe
continues to run after disconnect in these cases.

Also, the original intent of "forget-passwords" appears to be to
clear all "passwords" that can be used to reconnect.

v2:
- call ssl_clean_auth_token() directly from manage.c instead
  of amending ssl_purge_auth()
- Add a comment that ssl_purge_auth() does not clear auth-token

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/manage.c | 2 ++
 src/openvpn/ssl.h    | 1 +
 2 files changed, 3 insertions(+)

Comments

Gert Doering Oct. 26, 2022, 7:27 p.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

Thanks.  Based on our mail thread on v1, this now clears the token on
"management related events" (forget-password, management disconnect)
and does not affect other code paths.  I have not submitted this to
extremely thorough testing, just basic t_client tests for 2.5 and master
(which do not exercise these code paths at all).

Your patch has been applied to the master and release/2.5 branch.

commit ecad4839caf4c2fab9c6627ceeca9b9cb32e8929 (master)
commit 3d792ae9557b959e796710cf903866d205d979da (release/2.5)
Author: Selva Nair
Date:   Wed Oct 26 14:55:43 2022 -0400

     Purge auth-token as well while purging passwords

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20221026185543.5378-1-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25460.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 5670e594..5b288eab 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -762,6 +762,7 @@  static void
 man_forget_passwords(struct management *man)
 {
     ssl_purge_auth(false);
+    (void)ssl_clean_auth_token();
     msg(M_CLIENT, "SUCCESS: Passwords were forgotten");
 }
 
@@ -1922,6 +1923,7 @@  man_reset_client_socket(struct management *man, const bool exiting)
         if (man->settings.flags & MF_FORGET_DISCONNECT)
         {
             ssl_purge_auth(false);
+            (void)ssl_clean_auth_token();
         }
 
         if (man->settings.flags & MF_SIGNAL)
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 9ae6ae8f..94137b23 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -390,6 +390,7 @@  void ssl_set_auth_nocache(void);
 /*
  * Purge any stored authentication information, both for key files and tunnel
  * authentication. If PCKS #11 is enabled, purge authentication for that too.
+ * Note that auth_token is not cleared.
  */
 void ssl_purge_auth(const bool auth_user_pass_only);