On Mon, Mar 10, 2014 at 10:25:56AM +0100, Sumit Bose wrote:
> On Sun, Mar 09, 2014 at 01:04:38PM +0100, Jakub Hrozek wrote:
> > On Fri, Mar 07, 2014 at 12:23:18PM -0500, Nathaniel McCallum wrote:
> > > Before this patch, a different set of options was used when calling
> > > krb5_get_init_creds_password() for the changepw principal. Because
> > > this set of options did not contain the same FAST settings as the
> > > options for normal requests, all authentication would fail when the
> > > password of a FAST-only account would expire.
> > > 
> > > The two sets approach was cargo-cult from kinit where multiple
> > > requests could be issued using the same options set. However, in the
> > > case of krb5_child, only one request (or occasionally a well-defined
> > > second request) will be issued. Two option sets are therefore not
> > > required.
> > > 
> > > To fix this problem we removed the second option set used for changepw
> > > requests. All requests now use a single option set which is modified,
> > > if needed, for well-defined subsequent requests.
> > 
> > Password changes with both expired and current password work fine with
> > the patch. Tested with IPA and MS-AD.
> > 
> > The code looks good to me, there's no reason for set_changepw_options()
> > to return anything, so making it 'void' makes sense.
> > 
> > I can't think of a reason to keep the two option structures separate.
> > The way the kr->options instance is used clobbers the lifetime set with
> > krb5_get_init_creds_opt_set_tkt_life but only when changepw principal is
> > used.
> > 
> > ACK from me.
> 
> ACK from me as well. I checked the original tickets leading to this
> patch and found no reason which would force us to use a second set of
> options here.
> 
> bye,
> Sumit

Thanks, now that the patch has +2, pushed to master and sssd-1-11
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to