[Openvpn-devel,v4] Fix OpenVPN querying user/password if auth-token with user expires

Message ID 20221009130805.1556517-1-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v4] Fix OpenVPN querying user/password if auth-token with user expires | expand

Commit Message

Arne Schwabe Oct. 9, 2022, 2:08 a.m. UTC
The problematic behaviour happens when start a profile without
auth-user-pass and connect to a server that pushes auth-token
When the auth token expires OpenVPN asks for auth User and password
again.

The problem is that the auth_user_pass_setup sets
auth_user_pass_enabled = true; This function is called from two places.
In ssl.c it is only called with an auth-token present or that
variable already set. The other one is init_query_passwords.

Move setting auth_user_pass_enabled to the second place to ensure it is
only set if we really want passwords.

Patch v2: Remove unrelated code change
Patch v3: Rebase to master
Patch v4: Rebase to master

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/init.c | 1 +
 src/openvpn/ssl.c  | 7 ++++++-
 src/openvpn/ssl.h  | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

Comments

Gert Doering Oct. 9, 2022, 2:51 a.m. UTC | #1
Verified that this is just a rebase of v3, to accomodate for the
auth_user_pass_setup() changes that have happened in the meantime.

Recording David's and Heiko's ACK, they have done the stare-at-code
and actual testing (I have run t_client tests, but they do not excercise
this problem with my current test servers - need to add more variants).

I have reworded the commit message along the lines David suggested.

This is a bugfix, so it goes into release/2.5 as well - needed a bit
of manual massaging (auth_user_pass_setup() change)

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

commit 7d291e10bccd1d6b9e584307fb5fe3ebfb114ec9 (master)
commit af546d798213587285b225cd0031944a81e8e26c (release/2.5)
Author: Arne Schwabe
Date:   Sun Oct 9 15:08:05 2022 +0200

     Fix OpenVPN querying user/password if auth-token with user expires

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: David Sommerseth <davids@openvpn.net>
     Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
     Message-Id: <20221009130805.1556517-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25367.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering Oct. 9, 2022, 8:27 p.m. UTC | #2
Hi,

On Sun, Oct 09, 2022 at 03:51:36PM +0200, Gert Doering wrote:
> Recording David's and Heiko's ACK, they have done the stare-at-code
> and actual testing (I have run t_client tests, but they do not excercise
> this problem with my current test servers - need to add more variants).

Just for the records - testing this with 2.x OpenVPN as server seems
to be impossible using built-in --auth-gen-token or using the script
interface to --auth-user-pass-verify + --client-connect, because of
various reasons (--auth-gen-token does not even trigger when there is
no initial username, and the script approach interferes with TLS
username locking - patch for that on the list).

That said, I found a way to stage the server side, and can now confirm
that *without* this patch, the sequence

 - client without --auth-user-pass
 - server pushes --auth-token-user + --auth-token
 - client sends these as future username/password
 - token expire, servers sends AUTH_FAIL

will lead to

 - client stops, asking for "Enter Auth Username:" without the patch

 - client reverts to sending "no username+password" *with* the patch
    (the client declares this a "reconnect" and waits 5 seconds, but
     the connection will recover without user interaction)

(tested for both 2.5 and master)

gert

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 80b077653..5141a35c2 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -595,6 +595,7 @@  init_query_passwords(const struct context *c)
     /* Auth user/pass input */
     if (c->options.auth_user_pass_file)
     {
+        enable_auth_user_pass();
 #ifdef ENABLE_MANAGEMENT
         auth_user_pass_setup(c->options.auth_user_pass_file,
                              c->options.auth_user_pass_file_inline,
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 4f28eb568..5ed71f0c5 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -394,6 +394,12 @@  static struct user_pass auth_token;     /* GLOBAL */
 static char *auth_challenge; /* GLOBAL */
 #endif
 
+void
+enable_auth_user_pass()
+{
+    auth_user_pass_enabled = true;
+}
+
 void
 auth_user_pass_setup(const char *auth_file, bool is_inline,
                      const struct static_challenge_info *sci)
@@ -405,7 +411,6 @@  auth_user_pass_setup(const char *auth_file, bool is_inline,
         flags |= GET_USER_PASS_INLINE_CREDS;
     }
 
-    auth_user_pass_enabled = true;
     if (!auth_user_pass.defined && !auth_token.defined)
     {
 #ifdef ENABLE_MANAGEMENT
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index f8c30762e..9ae6ae8fc 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -371,6 +371,9 @@  void tls_post_encrypt(struct tls_multi *multi, struct buffer *buf);
  */
 void pem_password_setup(const char *auth_file);
 
+/* Enables the use of user/password authentication */
+void enable_auth_user_pass();
+
 /*
  * Setup authentication username and password. If auth_file is given, use the
  * credentials stored in the file, however, if is_inline is true then auth_file