On Mon, 10 Nov 2014 17:12:55 +0100
Jakub Hrozek <jhrozek(a)redhat.com> wrote:
On Thu, Nov 06, 2014 at 10:21:17AM -0500, Simo Sorce wrote:
> On Wed, 5 Nov 2014 18:36:06 +0100
> Jakub Hrozek <jhrozek(a)redhat.com> wrote:
>
> > From 1afae1740eb9bf232c33dba77f643f88d0eeb7a3 Mon Sep 17 00:00:00
> > 2001 From: Jakub Hrozek <jhrozek(a)redhat.com>
> > Date: Sat, 18 Oct 2014 22:03:13 +0200
> > Subject: [PATCH 5/6] KRB5: Move all ccache operations to
> > krb5_child.c
> >
> > The credential cache operations must be now performed by the
> > krb5_child completely, because the sssd_be process might be
> > running as the sssd user who doesn't have access to the ccaches.
> >
> > src/providers/krb5/krb5_ccache.c is still linked against
> > libsss_krb5 until we fix Kerberos ticket renewal as non-root.
> >
> > Also includes a new error code that indicates that the back end
> > should remove the old ccache attribute -- the child can't do that
> > if it's running as the user.
>
> I think I see an ordering issue but I am not completely sure
> (unfortunately I am looking only at the patches because git am
> fails to bring them on a copy of the master tree).
The patches depended on the SELinux changes that were since merged to
master. The attached patchset should apply on master cleanly.
>
> The main reason ccache handling was done in 2 parts is that in some
> cases we need to do some preparatory work as root.
> For example creating a user directory for DIR: ccache and similar.
> I think this patches moves things around and causes all these
> functions to be run after krb5_child has already dropped root in
> k5c_setup() though. If that's the case as it looks to me I think
> this patchset will not work as well as it should.
You're right, I admit I tested on RHEL and Fedora where we only use
KEYRING: and FILE:/tmp/ where I didn't hit this issue. I was also
confused by our removal of creating public ccache dir components --
now I realize we can still create user-owned directories under
root-writably directories and DIR:/run/user/$UID is exactly this case.
This means we need to examine the ccache as root in order to determine
whether to re-create it...with the attached patches the DIR cache
seems to work fine.
>
> I also see that you added ERR_CREDS_EXPIRED_CCACHE to delete the
> ccache once the reply is returned to the backend. But I am afraid
> deleting a ccache may also require root privs as the ccache is
> owned by the user but the sssd backend is running as the sssd user.
> Or am I missing another change that addresses this ?
>
> Also ERR_CREDS_EXPIRED_CCACHE seem not performing the same checks as
> ERR_CREDS_EXPIRED, why (and why does it need to be different) ?
No, the ERR_CREDS_EXPIRED_CCACHE semantics is different. The
krb5_child is able to remove the old ccache from disk or from the
kernel keyring, but in case the child dropped privileges to the user
who is logging in, it cannot delete the ccache attribute from
sysdb..so ERR_CREDS_EXPIRED_CCACHE is a way of telling sssd_be to
remove this attribute istead.
What happen if sssd_be is running unprivileged itself ?
Thank you for the review. New patches are attached (and I just
realized I didn't CC you in my reply to your other mail; sorry)
NP: I'll try to catch up on the replies and look at the new patches in
the afternoon.
Simo.
--
Simo Sorce * Red Hat, Inc * New York