Hi,
On Mon, Aug 15, 2016 at 6:49 AM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:
>
> > if (username_from_stdin && !(flags & GET_USER_PASS_PASSWORD_ONLY))
> > { - if (!get_console_input (BSTR (&user_prompt),
> > true, up->username, USER_PASS_LEN)) - msg
> > (M_FATAL, "ERROR: could not read %s username from stdin", prefix);
> > + query_user_add (BSTR(&user_prompt),
> > BLEN(&user_prompt), +
> > up->username, USER_PASS_LEN,
> true);
> > + } + + query_user_add
> > (BSTR(&pass_prompt),
> BLEN(&pass_prompt),
> > + up->password, USER_PASS_LEN,
> > false);
> >
> >
> > The above call should be conditional on "if
> > (password_from_stdin)". Else it will get called even after reading
> > password from a file. Unfortunately get_user_pass_cr() has become
> > so fragile after repeated hacks, its so easy to break it...
>
> Good catch! I've noticed that in some very few configurations git
> master already behaves somewhat odd with systemd enabled. I think
> what you've spotted here is one of the issues I've seen but not had
> time to investigate yet. This week I'm fairly tied up in some other
> projects, but I'll dig into this and test these things. You've
> pointed me into a very sane code path, though.
>
I too saw some systemd weirdness when openvpn is started from init scripts:
the password prompt came up before username followed by password again. But
that was before fixing the CLEAR(*input) bug in 2/4. Now the patchset
appears to behave fine (except for the above bug) on my test setups (Debian
jessie and stretch/sid).
> > The following may sound harsh, but that's not the intention. I've
> > spent hours on these patches now, so just trying to get the best
> > out of it: In the end, I'm not sure what is achieved by this new
> > framework. I see multiple queries can be combined into one exec
> > call, but the way its used in get_user_pass_cr() does not make full
> > use of it. Sure, username and password are prompted in one call,
> > but static challenge is still handled by an additional call. FWIW,
> > to be true to the spirit of the implementation, a request for
> > username/password/static-challenge-response should result in only
> > one call to query_user_exec, not two. Only then one can even
> > remotely hope to have a way prompting the user for all that info in
> > one dialog by some future ask-password agent. Unless you set a
> > clear style and guidance here, over time this will degenerate into
> > something logically identical to the original code except for
> > different function names and twice the function calls.
>
> Fair point! The thing is that *currently* systemd-ask-password isn't
> clever at all. I've been discussing this with a few upstream systemd
> developers already, and basically they are going to re-write much of
> the systemd-ask-password stuff to be far more intelligent. But much
of that work has halted as a new approach is planned to use DBus.
> That again creates other challenges at boot time as DBus might not be
> running when systemd-ask-password is called. This will be resolved
> once the kdbus work gets into an upstream kernel, then things can
> start to move forward too.
>
> > Even with one exec call, finally systemd-ask-password has to be
> > called multiple times and it seems currently there are no agents
> > that could take multiple queries and prompt the user in one dialog,
> > for example.
>
> And these patches are just the first step. First we need a more sane
> API to work with, then we can start juggling things more around. My
> plan is to move as much of these user query calls as early as possible
> during the init phase. Then depending on the backend (standard
> console, systemd, dbus, whatever fancy someone else writes), the user
> is queried for this information as efficiently as possible.
>
Yes, that's all what I meant by this being ahead of its time. Which is
fine..
Irrespective of future plans, its pretty easy to combine the prompt for
username/password/static-challenge into one query_exec call.
> I've also started to ponder how we can make use of this via the
> management interface too, for the current GUIs we have around. Which
> means that an end user can be asked for private key password or
> PKCS#11 PIN, username and password in one go - as OpenVPN can now tell
> the UI what it needs to establish a connection. This could also give
> some benefits to the OpenVPN plug-in in NetworkManager too.
>
Actually the management interface (MI) works pretty well as is. I think
most GUI's use the MI and can, at least in principle, prompt for user-auth
and challenge response together --- see the Windows GUI as an example.
Dynamic challenge has to be prompted for in a follow-up dialog as the
challenge is not known in advance. Add to that GUI's ability to remember
passwords, all that has to be repeatedly prompted for, in many cases, is
the challenge response (static or dynamic). Of course there is room for
improvement along the lines of query user by having a uniform way of
passing all user-input request to the management.
If someone sees a better approach for resolving this, then I'm willing
> to look into that ... but considering this process have been going on
> for over a year without too much fuzz around this approach, the
> barrier to redesign everything from scratch again is quite high.
I have no idea for a better approach and the design of query_user looks
good to me. I only wanted to point out some minor ways in which the patch
set could be improved. And to indicate that only when user-input agents
that can make full use of it becomes available we'll know whether the
design needs further tweaks or not.
Best,
Selva
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity
planning reports. http://sdm.link/zohodev2dev
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
