[SSSD] [PATCH] Do not try to set password when authtok_length is zero

Pavel Březina pbrezina at redhat.com
Thu Jun 27 12:13:53 UTC 2013


On 06/27/2013 01:39 PM, Sumit Bose wrote:
> On Thu, Jun 27, 2013 at 01:32:59PM +0200, Ondrej Kos wrote:
>> On 06/27/2013 01:17 PM, Pavel Březina wrote:
>>> On 06/25/2013 02:55 PM, Lukas Slebodnik wrote:
>>>> On (24/06/13 17:04), Ondrej Kos wrote:
>>>>> The problem here wasn't in returned error code, but in faultly read
>>>>> DBUS message, due to condition in sss_authtok_set_string.
>>>>>
>>>>> When password is empty, it passes 0 as length, which is
>>>>> misinterpreted, and the function tries to determine the length of
>>>>> string by itself, reaching over boundaries of authtok string.
>>>>>
>>>>> trac issue: https://fedorahosted.org/sssd/ticket/1814
>>>>>
>>>>> Patch is attached
>>>>>
>>>>> Ondra
>>>>> --
>>>>> Ondrej Kos
>>>>> Associate Software Engineer
>>>>> Identity Management - SSSD
>>>>> Red Hat Czech
>>>>
>>>> >From 7f7e34e3d64b2b7f3c02225ed506791106c8d8a9 Mon Sep 17 00:00:00 2001
>>>>> From: Ondrej Kos <okos at redhat.com>
>>>>> Date: Mon, 24 Jun 2013 16:58:23 +0200
>>>>> Subject: [PATCH] Do not try to set password when authtok_length is zero
>>>>>
>>>>> https://fedorahosted.org/sssd/ticket/1814
>>>>>
>>>>> When the authtok_length is zero, it shouldn't call
>>>>> sss_authtok_set_password, because it tries to determine lenght of passed
>>>>> string by itself and would read parts of DBus message behind boundaries
>>>>> of authtok.
>>>>> ---
>>>>> src/responder/pam/pamsrv_cmd.c | 8 ++++++--
>>>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/src/responder/pam/pamsrv_cmd.c
>>>>> b/src/responder/pam/pamsrv_cmd.c
>>>>> index
>>>>> ff86a13a5ac13856a65d1618056caf4657cb473a..bf9a686230e0deb39f7387ed3f51c08f97575007
>>>>> 100644
>>>>> --- a/src/responder/pam/pamsrv_cmd.c
>>>>> +++ b/src/responder/pam/pamsrv_cmd.c
>>>>> @@ -65,8 +65,12 @@ static int extract_authtok_v2(TALLOC_CTX *mem_ctx,
>>>>> struct sss_auth_token *tok,
>>>>>          sss_authtok_set_empty(tok);
>>>>>          break;
>>>>>      case SSS_AUTHTOK_TYPE_PASSWORD:
>>>>> -        ret = sss_authtok_set_password(tok, (const char
>>>>> *)auth_token_data,
>>>>> -                                       auth_token_length);
>>>>> +        if (auth_token_length == 0) {
>>>>> +            sss_authtok_set_empty(tok);
>>>>> +        } else {
>>>>> +            ret = sss_authtok_set_password(tok, (const char
>>>>> *)auth_token_data,
>>>>> +                                           auth_token_length);
>>>>> +        }
>>>>>          break;
>>>>>      default:
>>>>>          return EINVAL;
>>>>> --
>>>>> 1.8.1.4
>>>>>
>>>> I tried this patch and you did not reach krb5 authentication in
>>>> krb5_child,
>>>> but reason is:
>>>> #>[krb5_auth_send] (0x0020): Wrong authtok type for user [usersssd01].
>>>> #>                           Expected [1], got [0]
>>>> So it failed in sssd_be
>>>>
>>>> I tried next patch:
>>>> +        if (auth_token_length == 0) {
>>>> +            ret = sss_authtok_set_password(tok, "", 0);
>>>> +        } else {
>>>> +            ret = sss_authtok_set_password(tok, (const char
>>>> *)auth_token_data,
>>>> +                                           auth_token_length);
>>>> +        }
>>>> and then it will fail in sssd_pam
>>>> #> [pam_parse_in_data_v3] (0x0020): pam_parse_in_data_v2 failed,
>>>> because sss_authtok_set_password could not be run with empty password.
>>>>
>>>> So there is question:
>>>> where should we fail? (in sssd_be or sssd_pam)
>>>
>>> The question is: do we want to allow empty password for non krb
>>> authentication? If yes, than we should fail in sssd_be. If no, we can
>>> fail in sssd_pam.
>>>
>>> The current authtoken code suggest that we do not allow empty passwords
>>> at all. See: sss_authtok_set_string().
>>>
>>> Does LDAP support bind with empty password?
>>>
>>
>> LDAP with empty password results into anonymous bind, we shouldn't
>> support empty passwords in any case.
>
> yes, but in general I think the decision should be made in the backend.
> The PAM modules should just send what it got (or not got).
>
> bye,
> Sumit

Then the approach is correct.




More information about the sssd-devel mailing list