[SSSD] KRB: check ccache directory for user's GID

Pavel Reichl preichl at redhat.com
Tue Apr 15 16:23:30 UTC 2014


On Fri, 2014-03-14 at 14:18 -0400, Simo Sorce wrote:
> On Fri, 2014-03-14 at 17:09 +0100, Jakub Hrozek wrote:
> > On Fri, Mar 14, 2014 at 04:44:00PM +0100, Sumit Bose wrote:
> > > On Fri, Mar 14, 2014 at 04:14:15PM +0100, Pavel Reichl wrote:
> > > > Hello,
> > > > 
> > > > I noticed that debug message does not reflect condition. 
> > > > I suppose it was intended to to check user's gid too.
> > > > 
> > > > Please have mercy if I'm wrong :-).
> > > > 
> > > > Regards,
> > > > 
> > > > Pavel Reichl
> > > 
> > > > From ebc90225c5f8141b997e40bebfc08e4f0a0a222b Mon Sep 17 00:00:00 2001
> > > > From: Pavel Reichl <preichl at redhat.com>
> > > > Date: Fri, 14 Mar 2014 14:46:45 +0000
> > > > Subject: [PATCH 1/2] KRB: check ccache directory for user's GID
> > > > 
> > > > ---
> > > >  src/providers/krb5/krb5_utils.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c
> > > > index 27572c4ea749c6953f73cea10ef184ec539bf25b..858029a5aa46c3582adeae0f912632acd87377d3 100644
> > > > --- a/src/providers/krb5/krb5_utils.c
> > > > +++ b/src/providers/krb5/krb5_utils.c
> > > > @@ -419,8 +419,8 @@ done:
> > > >  static errno_t check_parent_stat(struct stat *parent_stat,
> > > >                                   uid_t uid, gid_t gid)
> > > >  {
> > > > -    if (!((parent_stat->st_uid == 0 && parent_stat->st_gid == 0) ||
> > > > -           parent_stat->st_uid == uid)) {
> > > > +    if ((parent_stat->st_uid != 0 || parent_stat->st_gid != 0) &&
> > > > +        (parent_stat->st_uid != uid || parent_stat->st_gid != gid)) {
> > > 
> > > I think the original check is ok because it is sufficient that the
> > > directory belongs to the user, it does not need to belong to the users
> > > primary group as well. As a result gid can be removed form
> > > check_parent_stat()'s parameter list.
> > > 
> > > But it would nice to hear other opinoins as well.
> > > 
> > > bye,
> > > Sumit
> > 
> > That's what I thought, too, but I wasn't sure either when we discussed
> > the code with Pavel in the office earlier today.
> > 
> > Simo, do you remember?
> 
> No, however the debug message is misleading and this code change would
> make it true.
> That said I am not sure we really want to care about the group owner
> given we create the subdirs with 700 permission anyway so no permission
> is associated with group ownership.
> 
> Perhaps we should just check:
> if (parent_stat->st_uid != 0 && parent_stat->st_uid != uid) { ...
> 
> Simo.
> 

Hello,

patch addressing Simo's comments is attached.

Thanks for review.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-KRB-do-not-check-ccache-directory-for-GID.patch
Type: text/x-patch
Size: 1659 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140415/9047bc12/attachment.bin>


More information about the sssd-devel mailing list