Re: patch DIGEST-MD5 hashed password

[Gazsó Attila]

Hello Simon!

I cannot apply your patch (guess because of indenting lost in
conversion somewhere), but I made the same changes and sending back a
patch to the latest branch which I also tested.
I don't think there is too much difference between the two ordering,
but for those who use hashed passwords (which is the Right Thing to
do) it's one condition less in the code.

Attila

On Tue, Oct 7, 2008 at 5:15 PM, Simon Josefsson <address@hidden> wrote:
> "Gazsó Attila" <address@hidden> writes:
>
>> Hello Simon!
>>
>> I'm sending the patch to the latest git branch. I also changed the
>> field name in Gsasl_session from hpassword to
>> hashed_digest_md5_password. Otherwise the code is unchanged since the
>> latest patch.
>> I also tested it.
>
> Hi Attila!  I have applied the patch, with some modifications:
>
> http://git.savannah.gnu.org/gitweb/?p=gsasl.git;a=commitdiff;h=c63a071b1686d2281238ec6de32743703626de9f
>
> One comment though.  The code will now try to get a password first, and
> if that fails it tries to get a hashed password.  It may be wiser to
> attempt to get a hashed password first, and only if that fails try to
> get a password.  What do you think?  See patch below for my thoughts,
> although untested.
>
> /Simon
>
> diff --git a/lib/digest-md5/server.c b/lib/digest-md5/server.c
> index 3d4782c..f8a1351 100644
> --- a/lib/digest-md5/server.c
> +++ b/lib/digest-md5/server.c
> @@ -216,8 +216,19 @@ _gsasl_digest_md5_server_step (Gsasl_session * sctx,
>        const char *passwd;
>        const char *hashed_passwd;
>
> -       passwd = gsasl_property_get (sctx, GSASL_PASSWORD);
> -       if (passwd)
> +       hashed_passwd =
> +         gsasl_property_get (sctx, GSASL_DIGEST_MD5_HASHED_PASSWORD);
> +       if (hashed_passwd)
> +         {
> +           if (strlen (hashed_passwd) != (DIGEST_MD5_LENGTH * 2))
> +             return GSASL_AUTHENTICATION_ERROR;
> +
> +           rc = _gsasl_digest_md5_set_hashed_secret (state->secret,
> +                                                     hashed_passwd);
> +           if (rc != GSASL_OK)
> +             return rc;
> +         }
> +       else if ((passwd = gsasl_property_get (sctx, GSASL_PASSWORD)) != NULL)
>          {
>            char *tmp, *tmp2;
>            int rc;
> @@ -239,18 +250,6 @@ _gsasl_digest_md5_server_step (Gsasl_session * sctx,
>            memcpy (state->secret, tmp2, DIGEST_MD5_LENGTH);
>            free (tmp2);
>          }
> -       /* Retrieve hashed secret */
> -       else if ((hashed_passwd = gsasl_property_get
> -                 (sctx, GSASL_DIGEST_MD5_HASHED_PASSWORD)) != NULL)
> -         {
> -           if (strlen (hashed_passwd) != (DIGEST_MD5_LENGTH * 2))
> -             return GSASL_AUTHENTICATION_ERROR;
> -
> -           rc = _gsasl_digest_md5_set_hashed_secret (state->secret,
> -                                                     hashed_passwd);
> -           if (rc != GSASL_OK)
> -             return rc;
> -         }
>        else
>          {
>            return GSASL_NO_PASSWORD;
>

DIGEST_MD5_HASHED_PASSWORD_ORDER.diff
Description: Binary data