On Fri, Oct 02, 2009 at 03:20:33PM -0400, Stephen Gallagher wrote: > On 09/28/2009 03:05 PM, Sumit Bose wrote: > > On Mon, Sep 28, 2009 at 02:51:11PM -0400, Stephen Gallagher wrote: > >> On 09/28/2009 01:52 PM, Stephen Gallagher wrote: > >>> On 09/28/2009 12:24 PM, Stephen Gallagher wrote: > >>>> On 09/28/2009 11:49 AM, Sumit Bose wrote: > >>>>> Hi, > >>> > >>>>> with the patch the config file is only read if it is > >>>>> - a regular file > >>>>> - owner and group are 0 (root) > >>>>> - file permissions are 600 > >>> > >>>>> This patch depends on the config_from_fd patch currently under review. > >>> > >>>>> bye, > >>>>> Sumit > >>> > >>> > >>>>> ------------------------------------------------------------------------ > >>> > >>>>> _______________________________________________ > >>>>> sssd-devel mailing list > >>>>> sssd-devel@lists.fedorahosted.org > >>>>> https://fedorahosted.org/mailman/listinfo/sssd-devel > >>> > >>>> Nack. > >>> > >>>> As discussed on IRC, the lstat is redundant. All of the necessary > >>>> file-type checks can be performed with the fstat, with no risk of race > >>>> condition. > >>> > >>> > >>> Per conversation on IRC, this patch is approved. I didn't realize at > >>> first that we want to exclude symlinks as well from the SSSD. > >>> _______________________________________________ > >>> sssd-devel mailing list > >>> sssd-devel@lists.fedorahosted.org > >>> https://fedorahosted.org/mailman/listinfo/sssd-devel > >>> > >> > >> Updated Sumit's patch to use the new interface. > > > > ACK (is this a self-ACK?) > > > > bye, > > Sumit > > _______________________________________________ > > sssd-devel mailing list > > sssd-devel@lists.fedorahosted.org > > https://fedorahosted.org/mailman/listinfo/sssd-devel > > After a bit of thought, I realized that the code in confdb_init_db > needed to be reordered a bit. If the permissions on the file changed, > but not its contents, we would never detect it, because we were checking > whether the modification time has changed first. Changing file > permissions only does not update the modification time, so until the > file actually had new data written into it, it would have happily kept > loading a potentially world-readable config file. > > I've now moved the check_and_open_readonly() call to the beginning of > the confdb_init_db routine and converted the modification time stat() to > an fstat() on the returned file descriptor. > > Please re-review. >
Although I cannot follow the argument I think it is a good idea to move check_and_open_readonly() to the top, ACK. I have updated the sssd.conf man page a the check_and_open_readonly() tests in "[PATCH] more documentation and test for sssd.conf", please review. bye, Sumit _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
