[SSSD] [PATCH] add utility call check_and_open_readonly

Stephen Gallagher sgallagh at redhat.com
Mon Oct 5 11:10:19 UTC 2009


-----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 at 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 at 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 at 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 at 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-----



More information about the sssd-devel mailing list