On Wed, Mar 10, 2010 at 09:19:14AM -0500, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 03/09/2010 12:30 PM, Sumit Bose wrote:
>> I have disabled the '%P' sequence for the directory expansion and
change
>> setting of the permissions as described above.
>
>> bye,
>> Sumit
>
Nack
Using "expand_ccache_dir" as the boolean for disallowing %P is
confusing. Please rename the variable to make it more obvious why you
can't use %P. (Maybe "bool restrict_substitions")
done, renamed to file_mode
Using lstat() in find_ccdir_parent_data() is a bad idea. We should just
be using stat() to determine if we're ultimately dealing with a
directory, unless your intent here is to mandate that krb5_ccachedir
must never contain a symlink, in which case this needs to be documented
in the manpage, along with why.
done
Please save errno before calling DEBUG(). It won't always be guaranteed
that errno is unchanged within the DEBUG macro. This occurs in both
find_ccdir_parent_data() and create_ccache_dir().
done
li->s = talloc_strdup(mem_ctx, dirname);
Instead of mem_ctx, you should be using li for the memory parent.
done
Please talloc_zfree(parent) before returning from
find_ccdir_parent_data(), otherwise you'll be carrying that unnecessary
memory around.
You have similar memory issues in create_ccache_dir(). You create
dirname with the mem_ctx as the parent, but you never free it. And since
you're moving the pointer on it, you probably want to save the start of
the string so you can call talloc_free() on that.
done, additionally I introduced a temporary talloc context to get rid of
other allocated memory, too.
if (!private_path && (parent_stat.st_uid != 0 || parent_stat.st_gid != 0))
This check doesn't make sense to me. You're only going to allow the
creation of a public directory if the parent dir is owned by root and
has group root? I think the check should be for
(parent_stat.st_mode & (S_ISVTX | S_IWOTH | S_IXOTH)) (aka
world-writeable and world-executable and sticky-bit)
Similarly, the private path needs to check for
(parent_stat.st_uid == uid && parent_stat.st_mode & (S_IWUSR | S_IXUSR))
||
(parent_stat.st_mode & (S_ISVTX | S_IWOTH | S_IXOTH))
(aka owned by the current user and writeable/executable or
(world-writeable, world-executable and sticky-bit))
the sticky-bit won't help here if the directory does not belong to root.
However I added the x-bit check and removed the gid check.
New version attached.
bye,
Sumit
- --
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/
iEYEARECAAYFAkuXqmEACgkQeiVVYja6o6N7GACgkBeQm+NcZ4l0BCOKhlNgiaka
op4AoKXik40TL9yPpQUOwKxV0JJqDi5b
=GnHu
-----END PGP SIGNATURE-----
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel