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

Simo Sorce simo at redhat.com
Fri Mar 14 18:18:08 UTC 2014


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.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list