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

Lukas Slebodnik lslebodn at redhat.com
Wed Nov 12 16:08:09 UTC 2014


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



More information about the sssd-devel mailing list