Hi,

On Mon, Dec 14, 2015 at 4:10 PM, Selva Nair <[email protected]> wrote:
>
>> I took a quick look and it seems a simplified patch that addresses the
>> most critical-sounding issue (challenge/reponse not prompted for
>> from stdin) may be more useful.
>>
>
>
> That's exactly what that patch is.
>

Ok, now looking at the newer, version:

On Thu, Dec 10, 2015 at 11:57 AM, Wayne Davison <[email protected]> wrote:

> ---
>  src/openvpn/misc.c | 119
> +++++++++++++++++++++++++----------------------------
>  1 file changed, 57 insertions(+), 62 deletions(-)
>

A commit message that explains the "why" and "how" will be helpful.


>
> diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
> index bc411bf..83e10f7 100644
> --- a/src/openvpn/misc.c
> +++ b/src/openvpn/misc.c
> @@ -1132,74 +1132,69 @@ get_user_pass_cr (struct user_pass *up,
>            password_from_stdin = true;
>          }
>
> -      /*
> -       * Get username/password from standard input?
> -       */
> -      if (username_from_stdin || password_from_stdin)
> -       {
>  #ifdef ENABLE_CLIENT_CR
> -         if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
> -           {
> -             struct auth_challenge_info *ac = get_auth_challenge
> (auth_challenge, &gc);
> -             if (ac)
> -               {
> -                 char *response = (char *) gc_malloc (USER_PASS_LEN,
> false, &gc);
> -                 struct buffer packed_resp;
> -
> -                 buf_set_write (&packed_resp, (uint8_t*)up->password,
> USER_PASS_LEN);
> -                 msg (M_INFO|M_NOPREFIX, "CHALLENGE: %s",
> ac->challenge_text);
> -                 if (!get_console_input ("Response:",
> BOOL_CAST(ac->flags&CR_ECHO), response, USER_PASS_LEN))
> -                   msg (M_FATAL, "ERROR: could not read challenge
> response from stdin");
> -                 strncpynt (up->username, ac->user, USER_PASS_LEN);
> -                 buf_printf (&packed_resp, "CRV1::%s::%s", ac->state_id,
> response);
> -               }
> -             else
> -               {
> -                 msg (M_FATAL, "ERROR: received malformed challenge
> request from server");
> -               }
> -           }
> -         else
> +      if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
> +        {
> +          struct auth_challenge_info *ac = get_auth_challenge
> (auth_challenge, &gc);
> +          if (ac)
> +            {
> +              char *response = (char *) gc_malloc (USER_PASS_LEN, false,
> &gc);
> +              struct buffer packed_resp;
> +
> +              buf_set_write (&packed_resp, (uint8_t*)up->password,
> USER_PASS_LEN);
> +              msg (M_INFO|M_NOPREFIX, "CHALLENGE: %s",
> ac->challenge_text);
> +              if (!get_console_input ("Response:",
> BOOL_CAST(ac->flags&CR_ECHO), response, USER_PASS_LEN))
> +                msg (M_FATAL, "ERROR: could not read challenge response
> from stdin");
> +              strncpynt (up->username, ac->user, USER_PASS_LEN);
> +              buf_printf (&packed_resp, "CRV1::%s::%s", ac->state_id,
> response);
> +            }
> +          else
> +            {
> +              msg (M_FATAL, "ERROR: received malformed challenge request
> from server");
> +            }
> +        }
> +      else
>

I would either avoid excessive indentation changes or leave it for a
white-space-only patch (which will likely get ignored :). Same goes
for rest of the patch, which only moves around variable declarations,
or change whitespace.

This could be a small ~2 line patch -- easier to review and test.

Selva

Reply via email to