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(a)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.
--
Ondrej Kos
Associate Software Engineer
Identity Management - SSSD
Red Hat Czech