-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
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.
>
>> As per the code only couple of nits: the DEBUG() call on line 163
>> should say "check_fd failed"
>
> Thanks, fixed.
>
>
>> Why does the check_fd call accept an external stat buf? It is not
>> defined in any header file, perhaps it should be made static..did you
>> envision exporting it in the future similar to check_file()?
>
> I didn't just envision it, I planned it and then forgot to include it in
> the header file :)
>
> Fixed. I also added several comments to the header file to identify the
> recommended approach to performing file checks and warn of the potential
> race-conditions.
>
>
-------------------------
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
- --
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/
iEYEARECAAYFAku516gACgkQeiVVYja6o6P/5QCgnGy8Jsjz+NqpAWOCwD/08Gzk
ktYAnjwUGAwmIV8ksyRiqAu5fCpDbDkt
=TYU+
-----END PGP SIGNATURE-----