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

  From: Selva Nair <selva.nair@gmail.com>

If only username is found in the file, redirect the auth-user-pass
query to the management on Windows if (i) management-query-passwords
is enabled and (ii) stdout is redirected to a log file. These
restrictions avoid regressive behaviour: those running from the
command line will continue to get the prompt on the console
and if both username and password are in the file those will
continue to get used.

Note that the management will prompt for both username and password
ignoring the username read from the file. As the GUI saves 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 on windows if log file is
redirected (prompt goes to the log file), or the console
is not available as happens when the GUI is in use.

Trac # 757

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/error.c |  9 +++++++++
 src/openvpn/error.h |  3 +++
 src/openvpn/misc.c  | 17 +++++++++++++++++
 3 files changed, 29 insertions(+)
  

Hi,

On Sun, Mar 29, 2020 at 4:34 PM <selva.nair@gmail.com> wrote:
>
> From: Selva Nair <selva.nair@gmail.com>
>
> If only username is found in the file, redirect the auth-user-pass
> query to the management on Windows if (i) management-query-passwords
> is enabled and (ii) stdout is redirected to a log file. These
> restrictions avoid regressive behaviour: those running from the
> command line will continue to get the prompt on the console
> and if both username and password are in the file those will
> continue to get used.
>
> Note that the management will prompt for both username and password
> ignoring the username read from the file. As the GUI saves 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 on windows if log file is
> redirected (prompt goes to the log file), or the console
> is not available as happens when the GUI is in use.

Why only Windows? I'd like this for macOS, too!

On a Mac using Tunnelblick (which uses the management interface with
management-query-passwords enabled), if the auth-user-pass file
contains only the password (and a LF), then the following occurs:

     neither stdin nor stderr are a tty device and you have neither a
controlling tty nor systemd - can't ask for 'Enter Auth Password:'.
If you used --daemon, you need to use --askpass to make
passphrase-protected keys work, and you can not use --auth-nocache.
     Exiting due to fatal error

Note: Tunnelblick uses the "--log" option to redirect output to a
file. I am assuming that's what is meant by "stdout is redirected to a
log file".

It would be nice to have the same behavior on both Windows and Mac.
For Tunnelblick, too, the username from the file would be lost but, as
with the Windows GUI, the user can opt to save it so it isn't asked
for again.

Best regards,

Jon Bullard
  

Hi,

On Sun, Mar 29, 2020 at 7:13 PM Jonathan K. Bullard <jkbullard@gmail.com> wrote:
>
> Hi,
>
> On Sun, Mar 29, 2020 at 4:34 PM <selva.nair@gmail.com> wrote:
> >
> > From: Selva Nair <selva.nair@gmail.com>
> >
> > If only username is found in the file, redirect the auth-user-pass
> > query to the management on Windows if (i) management-query-passwords
> > is enabled and (ii) stdout is redirected to a log file. These
> > restrictions avoid regressive behaviour: those running from the
> > command line will continue to get the prompt on the console
> > and if both username and password are in the file those will
> > continue to get used.
> >
> > Note that the management will prompt for both username and password
> > ignoring the username read from the file. As the GUI saves 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 on windows if log file is
> > redirected (prompt goes to the log file), or the console
> > is not available as happens when the GUI is in use.
>
> Why only Windows? I'd like this for macOS, too!

I did not know what other platforms were affected and, in particular,
how to handle them.

We can't make this the default as some have systemd. Also on
unix-like OSes, we can and we do prompt on the controlling tty
even if stdout and stderr are redirected to files or /dev/null.

>
> On a Mac using Tunnelblick (which uses the management interface with
> management-query-passwords enabled), if the auth-user-pass file
> contains only the password (and a LF), then the following occurs:
>
>      neither stdin nor stderr are a tty device and you have neither a
> controlling tty nor systemd - can't ask for 'Enter Auth Password:'.
> If you used --daemon, you need to use --askpass to make
> passphrase-protected keys work, and you can not use --auth-nocache.
>      Exiting due to fatal error

In those cases it looks obviously wrong to use auth-file with username
only, and I would consider that a user error. The purpose of
my patch was to handle only some naive usages where the user
expects the console prompt to get automatically directed to the GUI.
Indeed, that does happen (from user's POV) for all cases except user-pass
with only username in a file.

But I agree, we should do something like this for other GUIs such as
tunnelblick too.

>
> Note: Tunnelblick uses the "--log" option to redirect output to a
> file. I am assuming that's what is meant by "stdout is redirected to a
> log file".

Yes, that's right. However, that logic wont be proper on OS-X, would it?
Command line users who use --log can still see password
prompt on /dev/tty. We'll be breaking that behaviour.

I considered checking for env vars like IV_UI_VER set by the UI
client, but that's not readily accessible from auth_user_pass_cr()
call. Alternatives like checking whether /dev/tty can be opened and/or
systemd is available didn't appeal to me. If at all, that would have
to be a separate patch.

Any suggestions?

Selva
  

Hi,

On Sun, Mar 29, 2020 at 7:58 PM Selva Nair <selva.nair@gmail.com> wrote:
>
> Hi,
>
> On Sun, Mar 29, 2020 at 7:13 PM Jonathan K. Bullard <jkbullard@gmail.com> wrote:
<snip>
> > On a Mac using Tunnelblick (which uses the management interface with
> > management-query-passwords enabled), if the auth-user-pass file
> > contains only the password (and a LF), then the following occurs:
> >
> >      neither stdin nor stderr are a tty device and you have neither a
> > controlling tty nor systemd - can't ask for 'Enter Auth Password:'.
> > If you used --daemon, you need to use --askpass to make
> > passphrase-protected keys work, and you can not use --auth-nocache.
> >      Exiting due to fatal error
>
> In those cases it looks obviously wrong to use auth-file with username
> only, and I would consider that a user error. The purpose of
> my patch was to handle only some naive usages where the user
> expects the console prompt to get automatically directed to the GUI.
> Indeed, that does happen (from user's POV) for all cases except user-pass
> with only username in a file.
>
> But I agree, we should do something like this for other GUIs such as
> tunnelblick too.
>
> >
> > Note: Tunnelblick uses the "--log" option to redirect output to a
> > file. I am assuming that's what is meant by "stdout is redirected to a
> > log file".
>
> Yes, that's right. However, that logic wont be proper on OS-X, would it?
> Command line users who use --log can still see password
> prompt on /dev/tty. We'll be breaking that behaviour.

If the OS X command line user was using --management-query-passwords
(as Tunnelblick does), they wouldn't see the password prompt on
/dev/tty, would they? Your patch checks for that, so wouldn't you only
need to change
     && defined(_WIN32)
to something like
     && (defined(_WIN32) || TARGET_OSX)
 (and add OS X to the comment at the start of the patch).
  

Hi,

On Sun, Mar 29, 2020 at 07:58:15PM -0400, Selva Nair wrote:
> Yes, that's right. However, that logic wont be proper on OS-X, would it?
> Command line users who use --log can still see password
> prompt on /dev/tty. We'll be breaking that behaviour.
> 
> I considered checking for env vars like IV_UI_VER set by the UI
> client, but that's not readily accessible from auth_user_pass_cr()
> call. Alternatives like checking whether /dev/tty can be opened and/or
> systemd is available didn't appeal to me. If at all, that would have
> to be a separate patch.

Not sure if the case "there is an active management client, and 
--management-query-passwords is set, but we *could* ask on /dev/tty" 
is really worth considering.

(There might be cases where the management interface is not used
for password prompting, in which case /dev/tty is the way to go).

Not sure I'd worry too much about systemd here - as far as I understand,
this is somewhat orthogonal to "management interface".  So if run from
systemd, and querying via systemd, you have no management client 
connected.

Am I making sense?  It's monday morning, halfway through my first cup
of tea :-)

gert
  

Hi,

On Mon, Mar 30, 2020 at 2:07 AM Gert Doering <gert@greenie.muc.de> wrote:
>
> Hi,
>
> On Sun, Mar 29, 2020 at 07:58:15PM -0400, Selva Nair wrote:
> > Yes, that's right. However, that logic wont be proper on OS-X, would it?
> > Command line users who use --log can still see password
> > prompt on /dev/tty. We'll be breaking that behaviour.
> >
> > I considered checking for env vars like IV_UI_VER set by the UI
> > client, but that's not readily accessible from auth_user_pass_cr()
> > call. Alternatives like checking whether /dev/tty can be opened and/or
> > systemd is available didn't appeal to me. If at all, that would have
> > to be a separate patch.
>
> Not sure if the case "there is an active management client, and
> --management-query-passwords is set, but we *could* ask on /dev/tty"
> is really worth considering.

I agree, but the problem is that we currently do that when auth-file has
username but no password.

Current behaviour:
   if auth-file is set always read from it
   else query management if management-query-passwords is set etc.
   else ask on /dev/tty or windows console
with a quirk that if auth-file has username but no password, ask on /dev/tty
or console, not the management ever..

(ignoring systemd which just replaces the query on console).

I'm all for changing the latter behaviour to query the management if
possible,
/dev/tty otherwise. But that's a regression and some may complain.

Jonathan K. Bullard <jkbullard@gmail.com> wrote:
>
> If the OS X command line user was using --management-query-passwords
> (as Tunnelblick does), they wouldn't see the password prompt on
> /dev/tty, would they?

In case of auth-file missing password, they would see it on /dev/tty
on linux, and I would guess on OSX as well, but I've not checked.


Selva
<div dir="ltr">Hi,<br><br>On Mon, Mar 30, 2020 at 2:07 AM Gert Doering &lt;<a href="mailto:gert@greenie.muc.de">gert@greenie.muc.de</a>&gt; wrote:<br>&gt;<br>&gt; Hi,<br>&gt;<br>&gt; On Sun, Mar 29, 2020 at 07:58:15PM -0400, Selva Nair wrote:<br>&gt; &gt; Yes, that&#39;s right. However, that logic wont be proper on OS-X, would it?<br>&gt; &gt; Command line users who use --log can still see password<br>&gt; &gt; prompt on /dev/tty. We&#39;ll be breaking that behaviour.<br>&gt; &gt;<br>&gt; &gt; I considered checking for env vars like IV_UI_VER set by the UI<br>&gt; &gt; client, but that&#39;s not readily accessible from auth_user_pass_cr()<br>&gt; &gt; call. Alternatives like checking whether /dev/tty can be opened and/or<br>&gt; &gt; systemd is available didn&#39;t appeal to me. If at all, that would have<br>&gt; &gt; to be a separate patch.<br>&gt;<br>&gt; Not sure if the case &quot;there is an active management client, and<br>&gt; --management-query-passwords is set, but we *could* ask on /dev/tty&quot;<br>&gt; is really worth considering.<br><br>I agree, but the problem is that we currently do that when auth-file has<div>username but no password.<br><br>Current behaviour:<br>   if auth-file is set always read from it<br>   else query management if management-query-passwords is set etc.<br>   else ask on /dev/tty or windows console<br>with a quirk that if auth-file has username but no password, ask on /dev/tty<br>or console, not the management ever..</div><div><br>(ignoring systemd which just replaces the query on console).<br><br>I&#39;m all for changing the latter behaviour to query the management if possible,<br>/dev/tty otherwise. But that&#39;s a regression and some may complain.<br><br>Jonathan K. Bullard &lt;<a href="mailto:jkbullard@gmail.com">jkbullard@gmail.com</a>&gt; wrote:<br>&gt;<br>&gt; If the OS X command line user was using --management-query-passwords<br>&gt; (as Tunnelblick does), they wouldn&#39;t see the password prompt on<br>&gt; /dev/tty, would they?<br><div><div><br></div><div>In case of auth-file missing password, they would see it on /dev/tty</div><div>on linux, and I would guess on OSX as well, but I&#39;ve not checked.</div><div> <br></div><blockquote style="margin:0 0 0 40px;border:none;padding:0px"></blockquote><blockquote style="margin:0 0 0 40px;border:none;padding:0px"></blockquote><blockquote style="margin:0 0 0 40px;border:none;padding:0px"></blockquote><blockquote style="margin:0 0 0 40px;border:none;padding:0px"></blockquote><blockquote style="margin:0 0 0 40px;border:none;padding:0px"></blockquote><blockquote style="margin:0 0 0 40px;border:none;padding:0px"></blockquote>Selva<br></div></div></div>
  

Hi,

On Mon, Mar 30, 2020 at 11:12 AM Selva Nair <selva.nair@gmail.com> wrote:
> Jonathan K. Bullard <jkbullard@gmail.com> wrote:
> >
> > If the OS X command line user was using --management-query-passwords
> > (as Tunnelblick does), they wouldn't see the password prompt on
> > /dev/tty, would they?
>
> In case of auth-file missing password, they would see it on /dev/tty
> on linux, and I would guess on OSX as well, but I've not checked.

The password prompt appears on /dev/tty on OS X only if --daemon is not used.

If --daemon and --management-query-passwords are used but --askpass is
not (whether or not --auth-nocache is also used), which is typical for
a Tunnelblick configuration on OS X, the following appears in the log:

     neither stdin nor stderr are a tty device and you have neither a
          controlling tty nor systemd - can't ask for 'Enter Auth Password:'.
          If you used --daemon, you need to use --askpass to make
          passphrase-protected keys work, and you can not use
          --auth-nocache.
     Exiting due to fatal error

if --daemon, --management-query-passwords, and --askpass are all used
(whether or not --auth-nocache is used), you get:

     Need password(s) from management interface, waiting...

If Windows GUI uses --daemon, that could be an additional requirement
that would work for Tunnelblick and OS X, which would mean one less
incompatibility between Windows and OS X.

Or it could test for Windows || (OS X && --daemon).

Best regards,

Jon Bullard
  

Hi,

On Mon, Mar 30, 2020 at 12:11 PM Jonathan K. Bullard <jkbullard@gmail.com>
wrote:

> Hi,
>
> On Mon, Mar 30, 2020 at 11:12 AM Selva Nair <selva.nair@gmail.com> wrote:
> > Jonathan K. Bullard <jkbullard@gmail.com> wrote:
> > >
> > > If the OS X command line user was using --management-query-passwords
> > > (as Tunnelblick does), they wouldn't see the password prompt on
> > > /dev/tty, would they?
> >
> > In case of auth-file missing password, they would see it on /dev/tty
> > on linux, and I would guess on OSX as well, but I've not checked.
>
> The password prompt appears on /dev/tty on OS X only if --daemon is not
> used.
>
> If --daemon and --management-query-passwords are used but --askpass is
> not (whether or not --auth-nocache is also used), which is typical for
> a Tunnelblick configuration on OS X, the following appears in the log:
>
>      neither stdin nor stderr are a tty device and you have neither a
>           controlling tty nor systemd - can't ask for 'Enter Auth
> Password:'.
>           If you used --daemon, you need to use --askpass to make
>           passphrase-protected keys work, and you can not use
>           --auth-nocache.
>      Exiting due to fatal error
>
> if --daemon, --management-query-passwords, and --askpass are all used
> (whether or not --auth-nocache is used), you get:
>
>      Need password(s) from management interface, waiting...
>
> If Windows GUI uses --daemon, that could be an additional requirement
> that would work for Tunnelblick and OS X, which would mean one less
> incompatibility between Windows and OS X.
>

--daemon is a unix/linux option (not supported on Windows) and after
deamonizing there is no controlling tty leading to the behaviour you mention
above. I think that's documented.


> Or it could test for Windows || (OS X && --daemon).
>

Personally I would prefer to enable this code for all platforms although
its a minor regression.

That is, if management-query-passwords is enabled and auth file is
missing password, query the management, not on console irrespective
of other options and OS. If that's acceptable, I'll submit a v2.

Selva



> Best regards,
>
> Jon Bullard
>
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 30, 2020 at 12:11 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 11:12 AM Selva Nair &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt; wrote:<br>
&gt; Jonathan K. Bullard &lt;<a href="mailto:jkbullard@gmail.com" target="_blank">jkbullard@gmail.com</a>&gt; wrote:<br>
&gt; &gt;<br>
&gt; &gt; If the OS X command line user was using --management-query-passwords<br>
&gt; &gt; (as Tunnelblick does), they wouldn&#39;t see the password prompt on<br>
&gt; &gt; /dev/tty, would they?<br>
&gt;<br>
&gt; In case of auth-file missing password, they would see it on /dev/tty<br>
&gt; on linux, and I would guess on OSX as well, but I&#39;ve not checked.<br>
<br>
The password prompt appears on /dev/tty on OS X only if --daemon is not used.<br>
<br>
If --daemon and --management-query-passwords are used but --askpass is<br>
not (whether or not --auth-nocache is also used), which is typical for<br>
a Tunnelblick configuration on OS X, the following appears in the log:<br>
<br>
     neither stdin nor stderr are a tty device and you have neither a<br>
          controlling tty nor systemd - can&#39;t ask for &#39;Enter Auth Password:&#39;.<br>
          If you used --daemon, you need to use --askpass to make<br>
          passphrase-protected keys work, and you can not use<br>
          --auth-nocache.<br>
     Exiting due to fatal error<br>
<br>
if --daemon, --management-query-passwords, and --askpass are all used<br>
(whether or not --auth-nocache is used), you get:<br>
<br>
     Need password(s) from management interface, waiting...<br>
<br>
If Windows GUI uses --daemon, that could be an additional requirement<br>
that would work for Tunnelblick and OS X, which would mean one less<br>
incompatibility between Windows and OS X.<br></blockquote><div><br></div><div>--daemon is a unix/linux option (not supported on Windows) and after</div><div>deamonizing there is no controlling tty leading to the behaviour you mention</div><div>above. I think that&#39;s documented.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Or it could test for Windows || (OS X &amp;&amp; --daemon).<br></blockquote><div><br></div><div>Personally I would prefer to enable this code for all platforms although</div><div>its a minor regression.</div><div><br></div><div>That is, if management-query-passwords is enabled and auth file is</div><div>missing password, query the management, not on console irrespective</div><div>of other options and OS. If that&#39;s acceptable, I&#39;ll submit a v2.</div><div><br></div><div>Selva</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Best regards,<br>
<br>
Jon Bullard<br>
</blockquote></div></div>
  

Hi,

On Mon, Mar 30, 2020 at 12:29:44PM -0400, Selva Nair wrote:
> Personally I would prefer to enable this code for all platforms although
> its a minor regression.
> 
> That is, if management-query-passwords is enabled and auth file is
> missing password, query the management, not on console irrespective
> of other options and OS. If that's acceptable, I'll submit a v2.

I find this reasonable, and can't really think of a situation where 
the current behaviour might be desirable.

So, yes, changing user-visible behaviour, but in a way that avoids
one particular hard-to-explain (or even document) corner case.

For master = 2.5, I'd definitely do this.

For 2.4, I'm not totally sure, but this is something we can discuss in
the community meeting if people have a strong opposing opinion (*I*
consider the current behaviour to be a bug, and the change a bugfix).

gert
  

On Mon, Mar 30, 2020 at 12:30 PM Selva Nair <selva.nair@gmail.com> wrote:

> That is, if management-query-passwords is enabled and auth file is
> missing password, query the management, not on console irrespective
> of other options and OS. If that's acceptable, I'll submit a v2.

That's fine with me (and Tunnelblick), thanks.

Best regards,

Jon Bullard