-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 13/08/16 05:48, Selva Nair wrote:
> Hi,
> 
> On Mon, Aug 8, 2016 at 3:28 PM, David Sommerseth
> <dav...@openvpn.net <mailto:dav...@openvpn.net>> wrote:

[...snip...]

> } @@ -1184,32 +1192,46 @@ get_user_pass_cr (struct user_pass *up, 
> struct buffer user_prompt = alloc_buf_gc (128, &gc); struct buffer
> pass_prompt = alloc_buf_gc (128, &gc);
> 
> +             query_user_clear (); buf_printf (&user_prompt, "Enter
> %s Username:",
prefix);
> buf_printf (&pass_prompt, "Enter %s Password:",
prefix);
> 
> 
> 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.

> 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.

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.

There are absolutely some challenges which needs to be resolved with
the challenge/response protocols.  But this needs to be looked into
far more closely when a reasonable framework is in place.  I prefer to
start at the low hanging fruits to gain experience and then start
solving the harder ones, especially when they're related.  For the
challenge/response protocol there are actually two protocol, which
works quite differently.  Anyway, this is on the TODO list as well.

I am also considering if we should have some plug-in API hooks for
this as well, so that a third-party can add a separate query mechanism
without needing to recompile OpenVPN.  But this comes much later once
we know where and how things move forward.  Currently the goal is to
have the query-user interface somewhat similar to the SSL library,
where you flip it at compile time for the default.

> Its more like a solution looking for a problem. Which is not a bad 
> thing per se..

As this patch-set has been discussed for a year now or so, I can
understand this point of view if you've not followed the discussions
over time.

My main goal is to improve the end user interaction with OpenVPN, as I
find the current model less user friendly and very limited when
integrating OpenVPN with today's more modern user interfaces.  So
these patches, are just laying a foundation for what can come next.
Without a new approach to query users, it is fairly limited what you
can do with the situation as it is today.

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.


- --
kind regards,

David Sommerseth
OpenVPN Technologies, Inc
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iEYEARECAAYFAlexnkUACgkQDC186MBRfroidQCeMMKyVVJYxdTlgQz9VvhgjQ4T
0JcAoJEqUtx9N6jbgkNzvCwY91Uhk6b5
=3wuk
-----END PGP SIGNATURE-----

------------------------------------------------------------------------------
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

Reply via email to