[Openvpn-devel,v2,2/2] When auth-user-pass file has no password, query the management

Message ID 1585591527-23734-2-git-send-email-selva.nair@gmail.com
State Accepted
Headers show
Series
  • [Openvpn-devel,v2,1/2] Move querying username/password from management to a function
Related show

Commit Message

Selva Nair March 30, 2020, 6:05 p.m.
From: Selva Nair <selva.nair@gmail.com>

When only username is found in the file, redirect the auth-user-pass
query to the management if management-query-passwords is enabled.
Otherwise the user is prompted on console, if available, as before.

This changes the behaviour for those who run from the command line,
with --management-query-passwords, but still expect the prompt
on the console.

Note that the management will prompt for both username and password
ignoring the username read from the file. As most GUIs can save the
the username, this is a one-time inconvenience.

Currently, the password is queried on the console (or systemd)
in such cases. This is not sensible when console is not available
(windows GUI, tunnelblick etc.) or when the log is redirected
to a file on Windows (for some reason prompt goes to the log file).

Trac # 757

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---

v2: Following discussions with Jonathan and Gert, removed the dependence
on stdout redirection and applied to all platforms.

 src/openvpn/misc.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Jonathan K. Bullard April 2, 2020, 4:55 p.m. | #1
Hi,

On Mon, Mar 30, 2020 at 2:06 PM <selva.nair@gmail.com> wrote:
>
> From: Selva Nair <selva.nair@gmail.com>
>
> When only username is found in the file, redirect the auth-user-pass
> query to the management if management-query-passwords is enabled.
> Otherwise the user is prompted on console, if available, as before.
>
> This changes the behaviour for those who run from the command line,
> with --management-query-passwords, but still expect the prompt
> on the console.
>
> Note that the management will prompt for both username and password
> ignoring the username read from the file. As most GUIs can save the
> the username, this is a one-time inconvenience.
>
> Currently, the password is queried on the console (or systemd)
> in such cases. This is not sensible when console is not available
> (windows GUI, tunnelblick etc.) or when the log is redirected
> to a file on Windows (for some reason prompt goes to the log file).
>
> Trac # 757
>
> Signed-off-by: Selva Nair <selva.nair@gmail.com>
> ---
>
> v2: Following discussions with Jonathan and Gert, removed the dependence
> on stdout redirection and applied to all platforms.
>
>  src/openvpn/misc.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
> index 0d5ac30..546cd71 100644
> --- a/src/openvpn/misc.c
> +++ b/src/openvpn/misc.c
> @@ -261,6 +261,22 @@ get_user_pass_cr(struct user_pass *up,
>              {
>                  strncpy(up->password, password_buf, USER_PASS_LEN);
>              }
> +            /* The auth-file does not have the password: get both username
> +             * and password from the management 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", prefix, auth_file);
> +                if (!auth_user_pass_mgmt(up, prefix, flags, auth_challenge))
> +                {
> +                    return false;
> +                }
> +            }
> +#endif
>              else
>              {
>                  password_from_stdin = 1;

Works for Tunnelblick, thanks!

One minor point: in all four places, plus in the email subject, "the
management" should be changed to "the management interface".

"Management interface" is the term that is used on the man page.

Best regards,

Jon Bullard
Selva Nair April 2, 2020, 5:09 p.m. | #2
Hi,

On Thu, Apr 2, 2020 at 12:56 PM Jonathan K. Bullard <jkbullard@gmail.com>
wrote:

> Hi,
>
> On Mon, Mar 30, 2020 at 2:06 PM <selva.nair@gmail.com> wrote:
> >
> > From: Selva Nair <selva.nair@gmail.com>
> >
> > When only username is found in the file, redirect the auth-user-pass
> > query to the management if management-query-passwords is enabled.
> > Otherwise the user is prompted on console, if available, as before.
> >
> > This changes the behaviour for those who run from the command line,
> > with --management-query-passwords, but still expect the prompt
> > on the console.
> >
> > Note that the management will prompt for both username and password
> > ignoring the username read from the file. As most GUIs can save the
> > the username, this is a one-time inconvenience.
> >
> > Currently, the password is queried on the console (or systemd)
> > in such cases. This is not sensible when console is not available
> > (windows GUI, tunnelblick etc.) or when the log is redirected
> > to a file on Windows (for some reason prompt goes to the log file).
> >
> > Trac # 757
>
> snip..


>
> Works for Tunnelblick, thanks!
>

Good to know.


>
> One minor point: in all four places, plus in the email subject, "the
> management" should be changed to "the management interface".
>
>
agreed.

Selva
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 2, 2020 at 12:56 PM Jonathan K. Bullard &lt;<a href="mailto:jkbullard@gmail.com">jkbullard@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Mon, Mar 30, 2020 at 2:06 PM &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt; wrote:<br>
&gt;<br>
&gt; From: Selva Nair &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt;<br>
&gt;<br>
&gt; When only username is found in the file, redirect the auth-user-pass<br>
&gt; query to the management if management-query-passwords is enabled.<br>
&gt; Otherwise the user is prompted on console, if available, as before.<br>
&gt;<br>
&gt; This changes the behaviour for those who run from the command line,<br>
&gt; with --management-query-passwords, but still expect the prompt<br>
&gt; on the console.<br>
&gt;<br>
&gt; Note that the management will prompt for both username and password<br>
&gt; ignoring the username read from the file. As most GUIs can save the<br>
&gt; the username, this is a one-time inconvenience.<br>
&gt;<br>
&gt; Currently, the password is queried on the console (or systemd)<br>
&gt; in such cases. This is not sensible when console is not available<br>
&gt; (windows GUI, tunnelblick etc.) or when the log is redirected<br>
&gt; to a file on Windows (for some reason prompt goes to the log file).<br>
&gt;<br>
&gt; Trac # 757<br><br></blockquote><div>snip..</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Works for Tunnelblick, thanks!<br></blockquote><div><br></div><div>Good to know.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
One minor point: in all four places, plus in the email subject, &quot;the<br>
management&quot; should be changed to &quot;the management interface&quot;.<br>
<br></blockquote><div><br></div><div>agreed.</div><div><br></div><div>Selva</div></div></div>
Gert Doering April 3, 2020, 8:45 a.m. | #3
Acked-by: Gert Doering <gert@greenie.muc.de>

Stared-at-code, basic compile / t_client test (which would not excercise
this particular code path) on mingw + freebsd.  Happily taking Jonathan's
"I've tested this on MacOS X with Tunnelblick" as "good enough for me" :-)

I have adjusted all occurrences of "the management" to "the management
interface", though I find "I have to go and ask the management" to be 
a fairly telling statement in itself :-) - I have also changed the
commit message subject slightly to point out that there this does not
happen "always".  The main text is clear enough.

Your patch has been applied to the master branch.

Can I have a joint patch for 2.4 that also updates Changes.rst, please?

(Changes.rst in the master branch needs a "redo from scratch" for 
the 2.5 release, but this should be mentioned for the 2.4.9 release
in release/2.4/Changes.rst)

thanks :)

commit 57578310992d1fbe8eff97049087c5308089acb5
Author: Selva Nair
Date:   Mon Mar 30 14:05:27 2020 -0400

     When auth-user-pass file has no password, query the management interface

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <1585591527-23734-2-git-send-email-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19655.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 0d5ac30..546cd71 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -261,6 +261,22 @@  get_user_pass_cr(struct user_pass *up,
             {
                 strncpy(up->password, password_buf, USER_PASS_LEN);
             }
+            /* The auth-file does not have the password: get both username
+             * and password from the management 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", prefix, auth_file);
+                if (!auth_user_pass_mgmt(up, prefix, flags, auth_challenge))
+                {
+                    return false;
+                }
+            }
+#endif
             else
             {
                 password_from_stdin = 1;