-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 04/05/2010 08:40 AM, Dmitri Pal wrote:
Stephen Gallagher wrote:
> On 04/05/2010 08:13 AM, Dmitri Pal wrote:
>> Stephen Gallagher wrote:
>>> On 04/02/2010 06:00 AM, Jakub Hrozek wrote:
>>>> On 04/01/2010 10:20 PM, Stephen Gallagher wrote:
>>>>> There is a small window between running lstat() on a filename and
>>>>> opening it where it's possible for the file to have been
modified.
>>>>> We were protecting against this by saving the stat data from the
>>>>> original file and verifying that it was the same file (by device
>>>>> and inode) when we opened it again, but this is an imperfect
>>>>> solution, as it is still possible for an attacker to modify the
>>>>> permissions during this window.
>>>>> It is much better to simply open the file and test on the active
>>>>> file descriptor.
>>>>> Resolves
https://fedorahosted.org/sssd/ticket/425 incidentally, as
>>>>> without the initial lstat, we are implicitly accepting symlinks
>>>>> and only verifying the target file.
>>>
>>>> Well, this changes the semantics of the calls a little as it is not
>>>> possible to check for symlinks anymore..granted we don't need it
> in the
>>>> code right now
>>> Realistically, it's going to be exceptionally rare that we would
>>> actually care whether something was a symlink. If we desperately
> need to
>>> know this, check_file() can be called individually.
>>>
>>> The problem with symlinks is that there's no way to test for them that
>>> is not a race-condition. When you call stat() or fstat(), it's
>>> impossible to determine whether the resulting file being stat()-ed
>>> exists at the original specified location or was resolved through
>>> symlinks. So the only way to determine if a file is a symlink is by
>>> polling it with lstat() before or after doing the stat()/fstat(). In
>>> either case, it's a race-condition, because someone with an inotify()
>>> watch on the file could change it between one stat and the next.
>>>
>>> So it's always a security risk to care about whether a file is a
>>> symlink. It's always better to work only with the resulting file.
>>>
>> I might be missing something but I do not get it.
>> If you open the file first and then do fstat() on the file descriptor
>> you do not care if it is a symlink or not.
>> You care that the file you opened has the right mode and ownership and
>> this is what you are checking.
>> And nobody can do anything using inotify() to the file you already
> opened.
>> I do not see a race condition here and I do not see a security issue
> here.
>> Am I missing something?
>
> No, you've got it exactly correct. I'm saying that if you do fstat(),
> it's perfectly safe. lstat() is an easy way to guarantee a
> race-condition. And lstat() is the only way to be certain that you're
> using a symlink. So in short, it's unsafe to care whether a symlink is
> in use.
>
> In the older code, we were first checking whether a file was a symlink,
> and then opening it if it was a real file. This is an unsafe premise, as
> there's no way to guarantee that the file is STILL not a symlink when
> the open() call happens. And it's not really sensible to care if a
> symlink is in use anyway.
>
Ah Ok. Yes. What I did not get is why we ever cared about symlinks vs.
files but I guess it does not matter since we are on the same page.
So then why do we need check_file function that has a race condition?
Why it is not just open file using passed in path and call check_fd on
the file descriptor?
But I guess you already have another function to do exactly that.
So why not just remove check_file function then?
I left it there in case there is a genuine reason that we might care
about a symlink, since this is the only way to read it. I also added a
noisy warning in the header file that it's unsafe to use in most
situations :)
Also, there's one other case in the code where we're stuck with using
it. When we open a D-BUS connection, we don't have a way of getting the
file descriptor of the socket file back from the D-BUS libraries, so the
best we can do is call check_file AFTER D-BUS has opened the socket.
This is a paranoia check, since the D-BUS libraries should already be
ensuring that the communication is safe. I'm still planning to open an
enhancement request against D-BUS to request the actual file descriptor.
- --
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAku526wACgkQeiVVYja6o6MXNwCgkcbMGRKhc4ppgbN55ZImiDj5
qOcAoKLEdcnGDjJJK+Zsw9dxv46qpqP1
=TpRX
-----END PGP SIGNATURE-----