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

Jakub Hrozek jhrozek at redhat.com
Thu Jul 18 14:14:33 UTC 2013


On Thu, Jul 18, 2013 at 04:04:08PM +0200, Jakub Hrozek wrote:
> On Wed, Jul 17, 2013 at 07:17:13PM +0200, Ondrej Kos wrote:
> > On 07/17/2013 06:47 PM, Jakub Hrozek wrote:
> > >On Wed, Jul 17, 2013 at 02:04:16PM +0200, Ondrej Kos wrote:
> > >>On 07/16/2013 08:10 PM, Jakub Hrozek wrote:
> > >>>On Tue, Jul 09, 2013 at 11:15:09PM +0200, Jakub Hrozek wrote:
> > >>>>On Fri, Jun 28, 2013 at 02:23:01PM +0200, 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
> > >>>>>
> > >>>>>We found out that it is right approach (after long discussion :-)
> > >>>>>
> > >>>>>ACK
> > >>>>
> > >>>>The approach might be good but on entering a blank password I'm getting
> > >>>>"System Error" with this patch, while without the patch I was getting
> > >>>>"Authentication failure". PAM_AUTH_ERR is the correct return code for
> > >>>>this use case.
> > >>>
> > >>>Hi,
> > >>>
> > >>>Ondra asked me to re-test atop the current git HEAD. Here is what I see
> > >>>when I type "su - jhrozek" and then just type "Enter":
> > >>>
> > >>>[sssd[be[redhat.com]]] [be_pam_handler] (0x0100): Got request with the following data
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): command: PAM_AUTHENTICATE
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): domain: redhat.com
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): user: jhrozek
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): service: su-l
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): tty: pts/20
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): ruser: jhrozek
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): rhost:
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): authtok type: 0
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): newauthtok type: 0
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): priv: 0
> > >>>[sssd[be[redhat.com]]] [pam_print_data] (0x0100): cli_pid: 22305
> > >>>[sssd[be[redhat.com]]] [krb5_pam_handler] (0x1000): Wait queue of user [jhrozek] is empty, running request immediately.
> > >>>[sssd[be[redhat.com]]] [krb5_auth_send] (0x0020): Wrong authtok type for user [jhrozek]. Expected [1], got [0]
> > >>>[sssd[be[redhat.com]]] [check_wait_queue] (0x1000): Wait queue for user [jhrozek] is empty.
> > >>>[sssd[be[redhat.com]]] [be_pam_handler_callback] (0x0100): Backend returned: (0, 4, <NULL>) [Success]
> > >>>[sssd[be[redhat.com]]] [be_pam_handler_callback] (0x0100): Sending result [4][redhat.com]
> > >>>[sssd[be[redhat.com]]] [be_pam_handler_callback] (0x0100): Sent result [4][redhat.com]
> > >>>
> > >>>
> > >>>4 == System Error
> > >>>_______________________________________________
> > >>>sssd-devel mailing list
> > >>>sssd-devel at lists.fedorahosted.org
> > >>>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >>>
> > >>
> > >>Hi,
> > >>
> > >>Now I see what you meant, new patches rebased on top of current master are
> > >>attached.
> > >>
> > >>The third patch replaces mutiple calls of sss_authtok_get_type with variable
> > >>and single call, just a small refactoring.
> > >>
> > >>Ondra
> > >>
> > >>--
> > >>Ondrej Kos
> > >>Associate Software Engineer
> > >>Identity Management - SSSD
> > >>Red Hat Czech
> > >
> > >[PATCH 1/3] Do not try to set password when authtok_length is zero
> > >Ack
> > >
> > >> From 3968df17f835a8dac892b3756f2a4f35a86a0baa Mon Sep 17 00:00:00 2001
> > >>From: Ondrej Kos <okos at redhat.com>
> > >>Date: Wed, 17 Jul 2013 13:42:57 +0200
> > >>Subject: [PATCH 2/3] KRB: Handle empty password gracefully
> > >>
> > >>https://fedorahosted.org/sssd/ticket/1814
> > >>
> > >>Return authentication error when empty password is passed.
> > >>---
> > >>  src/providers/krb5/krb5_auth.c | 13 +++++++++++++
> > >>  1 file changed, 13 insertions(+)
> > >>
> > >>diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
> > >>index 22495f57079d979b6c3484acf916ecc0736292d1..5b4d4abb6c9f97cac394853d73eb4e5f8b045c44 100644
> > >>--- a/src/providers/krb5/krb5_auth.c
> > >>+++ b/src/providers/krb5/krb5_auth.c
> > >>@@ -495,6 +495,19 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
> > >>          case SSS_PAM_AUTHENTICATE:
> > >>          case SSS_PAM_CHAUTHTOK:
> > >>              if (sss_authtok_get_type(pd->authtok) != SSS_AUTHTOK_TYPE_PASSWORD) {
> > >>+                /* handle empty password gracefully */
> > >>+                if (sss_authtok_get_type(pd->authtok) == SSS_AUTHTOK_TYPE_EMPTY &&
> > >>+                    sss_authtok_get_size(pd->authtok) == 0) {
> > >
> > >The second part is not needed, empty authtok cannot have size != 0 by
> > >definiton.
> > >
> > >>+
> > >>+                    DEBUG(SSSDBG_CRIT_FAILURE,
> > >>+                          ("Illegal zero-length authtok for user [%s]\n",
> > >>+                           pd->user));
> > >>+                    state->pam_status = PAM_AUTH_ERR;
> > >>+                    state->dp_err = DP_ERR_OK;
> > >>+                    ret = EOK;
> > >>+                    goto done;
> > >>+                }
> > >>+
> > >>                  DEBUG(SSSDBG_CRIT_FAILURE,
> > >>                        ("Wrong authtok type for user [%s]. " \
> > >>                         "Expected [%d], got [%d]\n", pd->user,
> > >>--
> > >>1.8.1.4
> > >>
> > >
> > >> From e9f44ae4e487b0cddfb676c7f52bcef9780c4393 Mon Sep 17 00:00:00 2001
> > >>From: Ondrej Kos <okos at redhat.com>
> > >>Date: Wed, 17 Jul 2013 13:58:36 +0200
> > >>Subject: [PATCH 3/3] KRB: Replace multiple calls with variable
> > >
> > >Meh, the compiler would optimize the call anyway :-) But in general I'm
> > >not opposed.
> > >_______________________________________________
> > >sssd-devel mailing list
> > >sssd-devel at lists.fedorahosted.org
> > >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> > 
> > Resending all three patches, modified and rebased.
> > 
> > Ondra
> 
> Thank you, ack

Pushed to master and sssd-1-10



More information about the sssd-devel mailing list