On Fri, 2009-11-13 at 15:29 +0100, Sumit Bose wrote: > > Nack. > > > > In the sssd-ipa manpage, I think we should change the "please note" > to > > "Please note that this default differs from the traditional kerberos > > provider backend." > > > > I think that referring to the "underlying Kerberos provider" makes > it > > unclear. > > done > > > > > In create_send_buffer(), you assign buf->size based on sizeof(int), > but > > you're using uint32_t for the actual data. This is a waste of memory > on > > 64-bit integer systems, and a serious error on a 16-bit integer > system. > > (Not that we ever expect to support such a system) If you're copying > in > > a 32-bit number, please guarantee that the space is allocated for a > > 32-bit number. > > > > done > > > Please add a comment in fork_child() stating why the value of > > KRB5_VALIDATE dictates whether to assume the user's identity. > > > > done > > > I think this is a serious error: you're only validating against the > > first entry in the keytab. It's possible for a keytab to have many > > different principals, as well as multiple enctypes for the same > > principal. We need to iterate through all keytab entries and test > first > > for the principal we need to validate against and not fail until all > > enctypes for the sought-after principal have been tried. > > > > ok, I look for the first key with a matching realm or try the last one > in the keytab file. > > > get_and_save_tgt(): Again a comment would be nice around > become_user() > > noting that it was being done here after being deferred from earlier > so > > that we can validate the TGT. > > done > > > > > General question: if we're moving where become_user() is called, > will > > this affect our SELinux policy? > > > > I think it will not affect the policy, because the krb5_child inherits > the SELinux labels from the parent, but I will check with Dan.
Looks good to me, ack. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel