(sending again with the list in CC:)
Mon, Aug 8, 2016 at 3:28 PM, David Sommerseth <dav...@openvpn.net> wrote:
> This provides exactly the same systemd functionality which existed
> before the query user infrastructure got implemented.
>
> [v4 - change disapproved &= syntax ]
>
> [v3 - Remove QUERY_USER_EXEC_ALTERNATIVE macro, simplify
> alternatives definition directly in console.h. For
> now only depend on ENABLE_SYSTEMD]
>
> [v2 - Removed the QUERY_USER_FOREACH macro]
>
> Signed-off-by: David Sommerseth <dav...@openvpn.net>
> ---
>
>
-#ifdef QUERY_USER_EXEC_ALTERNATIVE
>
This was introduced in patch 1, and now removed in patch 2. Any particular
reason to split these patches into two? Much easier (and safer -- see below
for why) to review if squashed together.
>
> +++ b/src/openvpn/console_systemd.c
>
> +static bool
> +get_console_input_systemd (const char *prompt, const bool echo, char
> *input, const int capacity)
> +{
> + int std_out;
> + bool ret = false;
> + struct argv argv;
> +
> + argv_init (&argv);
> + argv_printf (&argv, SYSTEMD_ASK_PASSWORD_PATH);
> + argv_printf_cat (&argv, "%s", prompt);
> +
> + if ((std_out = openvpn_popen (&argv, NULL)) < 0) {
> + return false;
> + }
> + CLEAR (*input);
>
Oh, here is a bug that was fixed sometime ago, and its trying to sneak back
in, eh :) Looks like a copy-paste from an old version. This function was
removed in patch 1 and now added in patch 2 which made it hard to spot
this. Anyway it should be:
memset (input, 0, capacity);
> + if (read (std_out, input, capacity) != 0)
>
That leaves no room for the terminating NUL. Should be
read (std_out, input, capacity-1)
Not a fault of this patch, the bug was probably there in the original.
Selva
