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(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.
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
--
Ondrej Kos
Associate Software Engineer
Identity Management - SSSD
Red Hat Czech
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel