[SSSD] [PATCHES] Simplify credential krb5 cache manipulation

Simo Sorce simo at redhat.com
Fri Sep 6 13:20:58 UTC 2013


On Fri, 2013-09-06 at 15:04 +0200, Sumit Bose wrote:
> On Tue, Sep 03, 2013 at 10:07:13PM -0400, Simo Sorce wrote:
> > After the recent patches to explicitly enable the KEYRING type in SSSD I
> > realized that the code that manipulates ccaches had grown too much, and,
> > most importantly, was doing unnecessary operations already performed in
> > an abstract way by krb5 functions.
> > 
> > This patch set mostly addresses ticket #2061
> > 
> > The aims has been to remove as much as possible type-specific code,
> > resorting to type specific behavior only as an explicit exception where
> > necessary due to historical or other reasons.
> > 
> > The combined diff gives a nice total stat of:
> >   815 insertions(+)
> >   1529 deletions(-)
> > 
> > 
> > The last patch is an attempt to address ticket #2071,
> > 
> > It was necessary to add it here otherwise sssd will fail to operate
> > correctly with some templates (as noted in #2071).
> > However I am not sure that's the way we want to resolve the problem.
> > The patch aimed at maintaining as much as possible a reasonable
> > behavior, although the intended behavior was not really written
> > anywhere. Personally I would rather scrap the patch and instead provide
> > a new one that would simply stop creating public directories at all, I
> > do not think it is sssd's role to fix/create directories that should be
> > set up by the admin appropriately ahead of time (either manually of via
> > tmpfiles.d or whatever).
> > 
> > I will try to follow up with a proposed patch that 'simplifies' sssd
> > behavior instead of fixing it for #2071
> > 
> > Simo.
> > 
> > -- 
> > Simo Sorce * Red Hat, Inc * New York
> 
> I've reviewed the first 14 patches and my comments and the result of a
> discussion on irc about how security can be improved when running as a
> user are below.

Thank you, comments inline and new patchset attached.

> I'll review the 15th patch together with the alternative one and send my
> comments separately.
> 
> bye,
> Sumit


[..]

>                                         ret, strerror(ret)));
> > +            /* free ssc immediately otherwise the code will try to restore
> > +             * wrong creds */
> > +            free(ssc);
> 
> talloc_free or talloc_zfree

ouch thanks a lot for catching this one.

[..]

> > +    /* change gid now, (leaves saved gid to current, so we can restore) */
> > +    ret = setresgid(gid, gid, -1);
> 
> only setting the egid should be sufficient and safer

[..]

> > +    if (uid != 0) {
> > +        /* change uid, (leaves saved uid to current, so we can restore) */
> > +        ret = setresuid(uid, uid, -1);
> 
> only setting the euid should be sufficient and safer


As discussed on IRC change both the above to retain the real uid/gid by
leaving -1 in there, this way a user process cannot successfully send
SIGKILL to sssd_be.


[..]


> > +static errno_t handle_randomized(TALLOC_CTX *mem_ctx, char *in)
> 
> mem_ctx is not used in handle_randomized()

Ah remnants of a previous version, removed the mem_ctx here and also
removed the tmp_ctx from the calling function, as handle_randomized()
was actually the only user.


Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-krb5-Add-calls-to-change-and-restore-credentials.patch
Type: text/x-patch
Size: 5768 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-krb5-Add-helper-to-destroy-ccache-as-user.patch
Type: text/x-patch
Size: 4453 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-krb5-Use-krb5_cc_destroy-to-remove-old-ccaches.patch
Type: text/x-patch
Size: 9900 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-krb5-Replace-type-specific-ccache-principal-check.patch
Type: text/x-patch
Size: 11688 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-krb5-Move-determination-of-user-being-active.patch
Type: text/x-patch
Size: 7827 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-krb5-move-template-check-to-initializzation.patch
Type: text/x-patch
Size: 6466 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-krb5-Make-check_for_valid_tgt-static.patch
Type: text/x-patch
Size: 6504 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-krb5-Use-new-function-to-validate-ccaches.patch
Type: text/x-patch
Size: 17384 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-krb5-Unify-function-to-create-ccache-files.patch
Type: text/x-patch
Size: 14254 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-krb5-Remove-unused-ccache-backend-infrastructure.patch
Type: text/x-patch
Size: 11762 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0011-krb5-Remove-unused-function.patch
Type: text/x-patch
Size: 2173 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0012-krb5-Add-file-dir-path-precheck.patch
Type: text/x-patch
Size: 3548 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0011.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0013-krb5_child-Simplify-ccache-creation.patch
Type: text/x-patch
Size: 18309 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0012.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0014-krb5-Remove-unused-helper-functions.patch
Type: text/x-patch
Size: 3647 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0013.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0015-krb5-More-correct-approach-to-public-dir-expansion.patch
Type: text/x-patch
Size: 44515 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130906/b4aaf029/attachment-0014.bin>


More information about the sssd-devel mailing list