[SSSD] [PATCH] Move ccache operations to krb5_child, allow the krb5_auth code to run unprivileged

Lukas Slebodnik lslebodn at redhat.com
Fri Nov 14 17:55:22 UTC 2014


On (14/11/14 13:52), Jakub Hrozek wrote:
>On Thu, Nov 13, 2014 at 07:30:41PM +0100, Jakub Hrozek wrote:
>> On Wed, Nov 12, 2014 at 05:08:09PM +0100, Lukas Slebodnik wrote:
>> > On (12/11/14 15:44), Jakub Hrozek wrote:
>> > >On Wed, Nov 12, 2014 at 01:45:26PM +0100, Lukas Slebodnik wrote:
>> > >> On (11/11/14 22:37), Jakub Hrozek wrote:
>> > >> >On Tue, Nov 11, 2014 at 09:11:45PM +0100, Jakub Hrozek wrote:
>> > >> >> On Tue, Nov 11, 2014 at 06:23:24PM +0100, Lukas Slebodnik wrote:
>> > >> >> > On (11/11/14 13:45), Jakub Hrozek wrote:
>> > >> >> > >On Tue, Nov 11, 2014 at 11:15:30AM +0100, Jakub Hrozek wrote:
>> > >> >> > >> Can you give me access to a host that reproduces this crash? ccname
>> > >> >> > >> should never be NULL with the new patches ...
>> > >> >> > >
>> > >> >> > >..except on access_provider=krb5...
>> > >> >> > >
>> > >> >> > >Thanks for catching that, new patches are attached.
>> > >> >> > 
>> > >> >> > There is problem with support of enterprise principals.
>> > >> >> > authentication for such users failed.
>> > >> >> 
>> > >> >> Thanks a lot for catching that, I had no trouble logging in as a user
>> > >> >> from a child domain, but I could reproduce the issue when I used a
>> > >> >> completely different suffix.
>> > >> >> 
>> > >> >> I'll work on a fix.
>> > >> >
>> > >> >Thanks again for the catch, can you test this additional fix on top of
>> > >> >your patches? (Sorry for sending a separate patch, I want to get a fresh
>> > >> >look at the set tomorrow, some other krb5_ccache.c functions might get
>> > >> >the same treatment)
>> > >> 
>> > >> >From 5d95f998643d875bcea149dde5e7a16aa42063b4 Mon Sep 17 00:00:00 2001
>> > >> >From: Jakub Hrozek <jhrozek at redhat.com>
>> > >> >Date: Tue, 11 Nov 2014 22:33:28 +0100
>> > >> >Subject: [PATCH] sss_krb5_check_ccache_princ fix
>> > >> >
>> > >> >---
>> > >> krb5 + ad tests passed with this patch.
>> > >> 
>> > >> LS
>> > >
>> > >Thanks, the whole patchset is attached again.
>> > 
>> > >From 7722435d86ab57c91a58028845fe781bc28605e0 Mon Sep 17 00:00:00 2001
>> > >From: Jakub Hrozek <jhrozek at redhat.com>
>> > >Date: Sat, 18 Oct 2014 22:03:01 +0200
>> > >Subject: [PATCH 4/6] KRB5: Move checking for illegal RE to krb5_utils.c
>> > >
>> > >Otherwise we would have to link krb5_child with pcre and transfer the
>> > >regex, which wold be cumbersome. Check for illegal patterns when
>> > >expanding the template instead.
>> > >---
>> > > src/providers/krb5/krb5_ccache.c | 38 ++------------------
>> > > src/providers/krb5/krb5_ccache.h |  7 +---
>> > > src/providers/krb5/krb5_utils.c  | 36 +++++++++++++++++--
>> > > src/providers/krb5/krb5_utils.h  |  4 +--
>> > > src/tests/krb5_utils-tests.c     | 78 ++++++++++++++++------------------------
>> > > 5 files changed, 69 insertions(+), 94 deletions(-)
>> > >
>> > Our CI does not like this patch.
>> > 
>> > src/providers/krb5/krb5_auth.c: In function 'krb5_auth_prepare_ccache_name':
>> > src/providers/krb5/krb5_auth.c:305:74: error: passing argument 4 of 'expand_ccname_template' makes pointer from integer without a cast [-Werror]
>> >              kr->ccname = expand_ccname_template(kr, kr, ccname_template, true,
>> >                                                                           ^
>> > In file included from src/providers/krb5/krb5_auth.c:41:0:
>> > src/providers/krb5/krb5_utils.h:45:7: note: expected 'struct pcre *' but argument is of type 'int'
>> >  char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
>> >        ^
>> > src/providers/krb5/krb5_auth.c:305:26: error: too few arguments to function 'expand_ccname_template'
>> >              kr->ccname = expand_ccname_template(kr, kr, ccname_template, true,
>> >                           ^
>> > In file included from src/providers/krb5/krb5_auth.c:41:0:
>> > src/providers/krb5/krb5_utils.h:45:7: note: declared here
>> >  char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
>> >        ^
>> > ../sssd/src/providers/krb5/krb5_auth.c:313:45: error: passing argument 2 of 'sss_krb5_precreate_ccache' makes integer from pointer without a cast [-Werror]
>> >                                              kr->krb5_ctx->illegal_path_re,
>> >                                              ^
>> > In file included from src/providers/krb5/krb5_auth.h:35:0,
>> >                  from src/providers/krb5/krb5_auth.c:40:
>> > src/providers/krb5/krb5_ccache.h:38:9: note: expected 'uid_t' but argument is of type 'struct pcre *'
>> >  errno_t sss_krb5_precreate_ccache(const char *ccname, uid_t uid, gid_t gid);
>> >          ^
>> > src/providers/krb5/krb5_auth.c:312:19: error: too many arguments to function 'sss_krb5_precreate_ccache'
>> >              ret = sss_krb5_precreate_ccache(kr->ccname,
>> >                    ^
>> > In file included from src/providers/krb5/krb5_auth.h:35:0,
>> >                  from src/providers/krb5/krb5_auth.c:40:
>> > src/providers/krb5/krb5_ccache.h:38:9: note: declared here
>> >  errno_t sss_krb5_precreate_ccache(const char *ccname, uid_t uid, gid_t gid);
>> >          ^
>> > cc1: all warnings being treated as errors
>> > Makefile:10727: recipe for target 'src/providers/krb5/libsss_krb5_common_la-krb5_auth.lo' failed
>> > 
>> > >From 6a854dab8a1affbc9af27b46591a0d8562829afb Mon Sep 17 00:00:00 2001
>> > >From: Jakub Hrozek <jhrozek at 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.
>> > >---
>> > > Makefile.am                             |  14 +-
>> > > src/providers/krb5/krb5_auth.c          | 222 ++++----------------------------
>> > > src/providers/krb5/krb5_ccache.c        |  62 ++++-----
>> > > src/providers/krb5/krb5_ccache.h        |   5 +-
>> > > src/providers/krb5/krb5_child.c         | 208 ++++++++++++++++++++++++++++--
>> > > src/providers/krb5/krb5_child_handler.c |  13 ++
>> > > src/tests/krb5_child-test.c             |   3 +-
>> > > src/util/util_errors.c                  |   1 +
>> > > src/util/util_errors.h                  |   1 +
>> > > 9 files changed, 282 insertions(+), 247 deletions(-)
>> > >
>> > >diff --git a/Makefile.am b/Makefile.am
>> > >index 41b1843baee41ea8a67eb47e6786286190bbdcb9..c6601468e79244aad0a1bb8d7c87190aad9ef61a 100644
>> > >--- a/Makefile.am
>> > >+++ b/Makefile.am
>> > >@@ -1604,6 +1604,7 @@ krb5_child_test_SOURCES = \
>> > >     src/providers/krb5/krb5_child_handler.c \
>> > >     src/providers/krb5/krb5_common.c \
>> > >     src/util/sss_krb5.c \
>> > >+    src/util/find_uid.c \
>> > >     src/providers/data_provider_fo.c \
>> > >     src/providers/data_provider_opts.c \
>> > >     src/providers/data_provider_callbacks.c \
>> > >@@ -2491,27 +2492,36 @@ libsss_ad_la_LDFLAGS = \
>> > This change is not necessary and, because krb5_child test is already linked
>> > with libsss_util, which contains this file.
>> > 
>> > Moreover, this change caused compilation failures on platforms with disabled
>> > link_all_deplibs.
>> > 
>> >   CCLD     krb5-child-test
>> > /usr/bin/ld: src/util/krb5_child_test-find_uid.o: undefined reference to symbol
>> > +'sd_uid_get_sessions@@LIBSYSTEMD_209'
>> > /lib64/libsystemd.so.0: error adding symbols: DSO missing from command line
>> > collect2: error: ld returned 1 exit status
>> > 
>> > LS
>> 
>> Thank you, see the attached patches.
>
>I forgot to remove the extra find_uid.c from Makefile.am

Tests passed even running sssd as unprivileged user (sssd).
Code looks good to me as well.
http://sssd-ci.duckdns.org/logs/job/3/57/summary.html

ACK

It might be good to have ACK from Sumit or Simo.

LS



More information about the sssd-devel mailing list