diff mbox series

[Openvpn-devel,v1] Inlined credentials: read missing password from management interface

Message ID 20260414055900.17132-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] Inlined credentials: read missing password from management interface | expand

Commit Message

Gert Doering April 14, 2026, 5:58 a.m. UTC 
From: Selva Nair <selva.nair@gmail.com>

When commit 39619b7fab added support for inlining username only,
fallback for password was from console. This is not ideal when
graphical UI is in use as there is no console. Instead, query the
management interface when possible.

This patch just extends a similar fix when username is read from
a file and password is missing. As before, any username read
from file or inlined is not peserved as we currently have no way
of locking the username in the management interface prompt.

Change-Id: Ieeb2f980330d485739dbf3d722f107c1dbf704fc
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1599
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1599
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>

Comments

Gert Doering April 19, 2026, 2:08 p.m. UTC | #1 
The original commit looked so simple and straightforward, but these
user-password-things never are... so thanks for fixing that omission.
I have not tested it, but if Arne ACKs it - and the code looks more
logical now, just using "password_from_stdin" as an indicator, not
multiple nested variants.

I'm not sure if this could be considered a "bug", but it's definitely
irritating behaviour for GUI users ("no credentials in file" leads to
"both are queried from mgmt" while "username inlined" leads to "password
queried from console only") so for a low minor version number, this
definitely qualified as "we want to fix this in the current release".

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

commit 287acce1acaad25a92f2112e967b9686418ef72a (master)
commit b450414d10309575c28ce00f175e8374e0e99480 (release/2.7)
Author: Selva Nair
Date:   Tue Apr 14 07:58:54 2026 +0200

     Inlined credentials: read missing password from management interface

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1599
     Message-Id: <20260414055900.17132-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36608.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index c00a3ce..979293f 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -305,24 +305,6 @@ 
                 {
                     strncpy(up->password, password_buf, USER_PASS_LEN);
                 }
-                /* The auth-file does not have the password: get both username
-                 * and password from the management interface if possible.
-                 * Otherwise set to read password from console.
-                 */
-#if defined(ENABLE_MANAGEMENT)
-                else if (management && (flags & GET_USER_PASS_MANAGEMENT)
-                         && management_query_user_pass_enabled(management))
-                {
-                    msg(D_LOW,
-                        "No password found in %s authfile '%s'. Querying the management interface",
-                        prefix, auth_file);
-                    if (!auth_user_pass_mgmt(up, prefix, flags, auth_challenge))
-                    {
-                        fclose(fp);
-                        return false;
-                    }
-                }
-#endif
                 else
                 {
                     password_from_stdin = 1;
@@ -348,7 +330,23 @@ 
         if (username_from_stdin || password_from_stdin || response_from_stdin)
         {
 #ifdef ENABLE_MANAGEMENT
-            if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE) && response_from_stdin)
+            /* If management-query-passwords is true, we could be here because
+             * of no password present in auth-file or inline. In that case
+             * query via the management interface instead of stdin/console.
+             */
+            if (management && (flags & GET_USER_PASS_MANAGEMENT)
+                && management_query_user_pass_enabled(management)
+                && !(flags & GET_USER_PASS_USERNAME_ONLY))
+            {
+                msg(D_LOW,
+                    "No '%s' password found in authfile or inline. Querying the management interface",
+                    prefix);
+                if (!auth_user_pass_mgmt(up, prefix, flags, auth_challenge))
+                {
+                    return false;
+                }
+            }
+            else if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE) && response_from_stdin)
             {
                 struct auth_challenge_info *ac = parse_auth_challenge(auth_challenge, &gc);
                 if (ac)