-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10/05/2009 03:15 AM, Sumit Bose wrote: > 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
Even despite the ack, I think maybe this needs more clarification. The way the confdb_init_db() routine used to work: 1) stat the config file. If the mtime > confdb.ldb's lastUpdated time, reread the config. 2) Run check_and_open_readonly() The problem with this is that chmod doesn't update mtime on a file. This means that if someone created a valid config, then started the sssd, then chmod-ed the config file, it would not be noticed by check_and_open_readonly(). The approach in this patch makes it so that check_and_open_readonly() is ALWAYS run, but we only do the actual config file processing if the file itself has changed. I noticed when I was testing your patch that just running chmod didn't trigger the failure to load, so I investigated why and fixed it. - -- Stephen Gallagher RHCE 804006346421761 Looking to carve out IT costs? www.redhat.com/carveoutcosts/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkrJ1BcACgkQeiVVYja6o6NGdQCfQ1nATL9NM1y9DjOuFyDutIQo Je8AniPD+KNWoo21rgA9R3fLhLtr1goB =VaPM -----END PGP SIGNATURE----- _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel