-----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(a)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(a)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(a)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(a)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?
-----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-----