[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
Expired != Disabled this change is intentional. Simo. - Original Message - > From: "Lukas Slebodnik" > To: "Development of the System Security Services Daemon" > > Cc: "Simo Sorce" > Sent: Friday, January 29, 2016 9:22:23 AM > Subject: Re: [SSSD] Re: [PATCH] fix account lockout reporting with the krb5 > provider > > On (14/01/16 18:38), Jakub Hrozek wrote: > >On Thu, Jan 14, 2016 at 12:09:12PM -0500, Simo Sorce wrote: > >> > OK to push now? > >> > >> Yes please :-) > >> > >> Simo > > > >* master: 19e44537c28f6d5f011cd7ac885c74c1e892605f > I have a question about this patch. > > I can see some inconsistencies for expired/disabled user. > > Here is a LDIF for expiration of user > dn: cn=$username,$ou,$basedn > changetype: modify > replace: accountExpires > accountExpires: 1294650180 > > and for disabling user > dn: cn=$username,$ou,$basedn > changetype: modify > replace: userAccountControl > userAccountControl: 514 > > > There are test with ssh + password (pam auth) > and ssh + key (pam pam account) > > and here is current state with master. > -- > disabled AD user > pam_sss(sshd:auth): received for user testuser01-17923: 6 (Permission > denied) > > pam_sss(sshd:account): system info: [The user account is disabled on the AD > server] > pam_sss(sshd:account): Access denied for user testuser01-17923: 6 > (Permission denied) > > expired AD user > pam_sss(sshd:auth): received for user testuser01-17923: 6 (Permission > denied) > > pam_sss(sshd:account): system info: [The user account is expired on the AD > server] > pam_sss(sshd:account): Access denied for user testuser01-17923: 13 (User > account has expired) > > > Previously, we could see info "User account has expired" > even in auth phase. And it's unusual that auth and account returned different > error codes. > > I think that this patch fixed "auth" PAM error code for disabled user > but it broke for expired user or did I miss something? > > LS > ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
On (29/01/16 18:48), Jakub Hrozek wrote: >On Fri, Jan 29, 2016 at 03:22:23PM +0100, Lukas Slebodnik wrote: >> On (14/01/16 18:38), Jakub Hrozek wrote: >> >On Thu, Jan 14, 2016 at 12:09:12PM -0500, Simo Sorce wrote: >> >> > OK to push now? >> >> >> >> Yes please :-) >> >> >> >> Simo >> > >> >* master: 19e44537c28f6d5f011cd7ac885c74c1e892605f >> I have a question about this patch. >> >> I can see some inconsistencies for expired/disabled user. >> >> Here is a LDIF for expiration of user >> dn: cn=$username,$ou,$basedn >> changetype: modify >> replace: accountExpires >> accountExpires: 1294650180 >> >> and for disabling user >> dn: cn=$username,$ou,$basedn >> changetype: modify >> replace: userAccountControl >> userAccountControl: 514 >> >> >> There are test with ssh + password (pam auth) >> and ssh + key (pam pam account) > >I will try to take a look when I work on >https://fedorahosted.org/sssd/ticket/2927 (unless you just started on >that ticket and this is how you found out..in that case self-assign the >ticket, please.. At any rate, thanks for the heads up.) > NO, It was pure AD testing of sssd master. >> >> and here is current state with master. >> -- >> disabled AD user >> pam_sss(sshd:auth): received for user testuser01-17923: 6 (Permission >> denied) >> >> pam_sss(sshd:account): system info: [The user account is disabled on the >> AD server] >> pam_sss(sshd:account): Access denied for user testuser01-17923: 6 >> (Permission denied) >> >> expired AD user >> pam_sss(sshd:auth): received for user testuser01-17923: 6 (Permission >> denied) >> >> pam_sss(sshd:account): system info: [The user account is expired on the AD >> server] >> pam_sss(sshd:account): Access denied for user testuser01-17923: 13 (User >> account has expired) >> >> >> Previously, we could see info "User account has expired" >> even in auth phase. And it's unusual that auth and account returned different >> error codes. > >I think the difference is because the auth phase converts the error PAM code >from Kerberos error code, while the account phase looks at the >adUserAccountControl sysdb attribute. Chances are we need to take a look >if our handling of the attribute values is correct. > >> >> I think that this patch fixed "auth" PAM error code for disabled user >> but it broke for expired user or did I miss something? > >I think those should be completely independent, the AD provider should >read the info in sdap_account_expired_ad(). But this is based just on >reading the code, I haven't actually done any tests. previously we return pam error code 13 for disabled AD user (sshd:auth) expired AD user (sshd:auth) expired AD user (sshd:account) and with current master we return 13 only for expired AD user (sshd:account) So my assumption is that we should return *6* for disabled AD user (sshd:auth) disabled AD user (sshd:account) and return *13* for expired AD user (sshd:auth) expired AD user (sshd:account) If I'm wrong please correct me. LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
On Fri, Jan 29, 2016 at 03:22:23PM +0100, Lukas Slebodnik wrote: > On (14/01/16 18:38), Jakub Hrozek wrote: > >On Thu, Jan 14, 2016 at 12:09:12PM -0500, Simo Sorce wrote: > >> > OK to push now? > >> > >> Yes please :-) > >> > >> Simo > > > >* master: 19e44537c28f6d5f011cd7ac885c74c1e892605f > I have a question about this patch. > > I can see some inconsistencies for expired/disabled user. > > Here is a LDIF for expiration of user > dn: cn=$username,$ou,$basedn > changetype: modify > replace: accountExpires > accountExpires: 1294650180 > > and for disabling user > dn: cn=$username,$ou,$basedn > changetype: modify > replace: userAccountControl > userAccountControl: 514 > > > There are test with ssh + password (pam auth) > and ssh + key (pam pam account) I will try to take a look when I work on https://fedorahosted.org/sssd/ticket/2927 (unless you just started on that ticket and this is how you found out..in that case self-assign the ticket, please.. At any rate, thanks for the heads up.) > > and here is current state with master. > -- > disabled AD user > pam_sss(sshd:auth): received for user testuser01-17923: 6 (Permission > denied) > > pam_sss(sshd:account): system info: [The user account is disabled on the AD > server] > pam_sss(sshd:account): Access denied for user testuser01-17923: 6 > (Permission denied) > > expired AD user > pam_sss(sshd:auth): received for user testuser01-17923: 6 (Permission > denied) > > pam_sss(sshd:account): system info: [The user account is expired on the AD > server] > pam_sss(sshd:account): Access denied for user testuser01-17923: 13 (User > account has expired) > > > Previously, we could see info "User account has expired" > even in auth phase. And it's unusual that auth and account returned different > error codes. I think the difference is because the auth phase converts the error PAM code from Kerberos error code, while the account phase looks at the adUserAccountControl sysdb attribute. Chances are we need to take a look if our handling of the attribute values is correct. > > I think that this patch fixed "auth" PAM error code for disabled user > but it broke for expired user or did I miss something? I think those should be completely independent, the AD provider should read the info in sdap_account_expired_ad(). But this is based just on reading the code, I haven't actually done any tests. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
On (14/01/16 18:38), Jakub Hrozek wrote: >On Thu, Jan 14, 2016 at 12:09:12PM -0500, Simo Sorce wrote: >> > OK to push now? >> >> Yes please :-) >> >> Simo > >* master: 19e44537c28f6d5f011cd7ac885c74c1e892605f I have a question about this patch. I can see some inconsistencies for expired/disabled user. Here is a LDIF for expiration of user dn: cn=$username,$ou,$basedn changetype: modify replace: accountExpires accountExpires: 1294650180 and for disabling user dn: cn=$username,$ou,$basedn changetype: modify replace: userAccountControl userAccountControl: 514 There are test with ssh + password (pam auth) and ssh + key (pam pam account) and here is current state with master. -- disabled AD user pam_sss(sshd:auth): received for user testuser01-17923: 6 (Permission denied) pam_sss(sshd:account): system info: [The user account is disabled on the AD server] pam_sss(sshd:account): Access denied for user testuser01-17923: 6 (Permission denied) expired AD user pam_sss(sshd:auth): received for user testuser01-17923: 6 (Permission denied) pam_sss(sshd:account): system info: [The user account is expired on the AD server] pam_sss(sshd:account): Access denied for user testuser01-17923: 13 (User account has expired) Previously, we could see info "User account has expired" even in auth phase. And it's unusual that auth and account returned different error codes. I think that this patch fixed "auth" PAM error code for disabled user but it broke for expired user or did I miss something? LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
On Thu, Jan 14, 2016 at 12:09:12PM -0500, Simo Sorce wrote: > > OK to push now? > > Yes please :-) > > Simo * master: 19e44537c28f6d5f011cd7ac885c74c1e892605f ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
On Thu, 2016-01-14 at 17:30 +0100, Jakub Hrozek wrote: > On Thu, Jan 14, 2016 at 11:03:51AM -0500, Simo Sorce wrote: > > On Thu, 2016-01-14 at 12:41 +0100, Jakub Hrozek wrote: > > > On Wed, Jan 13, 2016 at 02:56:25PM -0500, Simo Sorce wrote: > > > > subj says it all, > > > > bug: https://fedorahosted.org/sssd/ticket/2924 > > > > > > > > I have compiled and run make check|intgcheck but "not" actively tested > > > > this patch. > > > > > > I did test the patch by crating an account in AD and then ticking the > > > "Account is disabled" box. PAM returned error code 6, which is what I'd > > > expect. > > > > > > ACK > > > > Did the old code return the wrong error too ? > > Yes, before the patch I see: > Jan 14 16:23:51 adclient.win.trust.test su[3745]: pam_sss(su-l:auth): > received for user lockuser: 13 (User account has expired) > > With the patched code I see: > Jan 14 16:26:13 adclient.win.trust.test su[27524]: pam_sss(su-l:auth): > received for user lockuser: 6 (Permission denied) > > > I haven't looked at the AD case only IPA when I coded it but AD and IPA > > share the krb5 provider so I guess testing either would be fine ... > > Yes, the PAM error codes are exactly the same after "ipa user-disable". > > OK to push now? Yes please :-) Simo -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
On Thu, Jan 14, 2016 at 11:03:51AM -0500, Simo Sorce wrote: > On Thu, 2016-01-14 at 12:41 +0100, Jakub Hrozek wrote: > > On Wed, Jan 13, 2016 at 02:56:25PM -0500, Simo Sorce wrote: > > > subj says it all, > > > bug: https://fedorahosted.org/sssd/ticket/2924 > > > > > > I have compiled and run make check|intgcheck but "not" actively tested > > > this patch. > > > > I did test the patch by crating an account in AD and then ticking the > > "Account is disabled" box. PAM returned error code 6, which is what I'd > > expect. > > > > ACK > > Did the old code return the wrong error too ? Yes, before the patch I see: Jan 14 16:23:51 adclient.win.trust.test su[3745]: pam_sss(su-l:auth): received for user lockuser: 13 (User account has expired) With the patched code I see: Jan 14 16:26:13 adclient.win.trust.test su[27524]: pam_sss(su-l:auth): received for user lockuser: 6 (Permission denied) > I haven't looked at the AD case only IPA when I coded it but AD and IPA > share the krb5 provider so I guess testing either would be fine ... Yes, the PAM error codes are exactly the same after "ipa user-disable". OK to push now? ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
On Thu, 2016-01-14 at 12:41 +0100, Jakub Hrozek wrote: > On Wed, Jan 13, 2016 at 02:56:25PM -0500, Simo Sorce wrote: > > subj says it all, > > bug: https://fedorahosted.org/sssd/ticket/2924 > > > > I have compiled and run make check|intgcheck but "not" actively tested > > this patch. > > I did test the patch by crating an account in AD and then ticking the > "Account is disabled" box. PAM returned error code 6, which is what I'd > expect. > > ACK Did the old code return the wrong error too ? I haven't looked at the AD case only IPA when I coded it but AD and IPA share the krb5 provider so I guess testing either would be fine ... Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] fix account lockout reporting with the krb5 provider
On Wed, Jan 13, 2016 at 02:56:25PM -0500, Simo Sorce wrote: > subj says it all, > bug: https://fedorahosted.org/sssd/ticket/2924 > > I have compiled and run make check|intgcheck but "not" actively tested > this patch. I did test the patch by crating an account in AD and then ticking the "Account is disabled" box. PAM returned error code 6, which is what I'd expect. ACK ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
