Hi,
On Mon, Aug 8, 2016 at 3:28 PM, David Sommerseth <dav...@openvpn.net> wrote:
> This is will provide an interface for other mechanisms to be used to
> query the user for information, such as usernames, passwords, etc.
>
> It has also been a goal to make it possible to query for all the
> information in one call and not do it sequencially as before.
>
> [v4 - add a simple wrapper combining query_user_{init,add,exec}()
> - change disapproved &= syntax ]
>
> [v3 - Avoid the dynamic list, use a static list of QUERY_USER_NUMSLOTS
> - The list of query_user data is now a global variable
> - Replaced query_user_init() with query_user_clear()
> - Make query_user_add() a void function
> - Rebased against master/600dd9a16fc61 ]
>
> [v2 - Removed the QUERY_USER_FOREACH macro
>
> diff --git a/src/openvpn/console.h b/src/openvpn/console.h
> index 268f3fe..44d49ef 100644
> --- a/src/openvpn/console.h
> +++ b/src/openvpn/console.h
> @@ -6,6 +6,8 @@
> * packet compression.
> *
> * Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <sa...@openvpn.net
> >
> + * Copyright (C) 2014-2015 David Sommerseth <dav...@redhat.com>
> + * Copyright (C) 2016 David Sommerseth <dav...@openvpn.net>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2
> @@ -27,7 +29,90 @@
>
> #include "basic.h"
>
> -bool
> -get_console_input (const char *prompt, const bool echo, char *input,
> const int capacity);
> +/**
> + * Configuration setup for declaring what kind of information to ask a
> user for
> + */
> +struct _query_user {
+ char *prompt; /**< Prompt to present to the user */
> + size_t prompt_len; /**< Lenght of the prompt string */
> + char *response; /**< The user's response */
> + size_t response_len; /**< Lenght the of the user reposone */
> + bool echo; /**< True: The user should see what is
> being typed, otherwise mask it */
> +};
> +
> +#define QUERY_USER_NUMSLOTS 10
> +extern struct _query_user query_user[]; /**< Global variable, declared
> in console.c */
>
Hmm... this is wading into style territory, so I'll move on by just
mumbling that leading underscore in struct names serves no useful purpose.
In misc.c:
> }
> @@ -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...
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.
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.
Its more like a solution looking for a problem. Which is not a bad thing
per se..
Regards,
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
