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