openssh bugs - Sep 2017 - [Bug 2775] New: Improve kerberos credential forwarding support

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

            Bug ID: 2775
           Summary: Improve kerberos credential forwarding support
           Product: Portable OpenSSH
           Version: 7.5p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: Kerberos support
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jjelen at redhat.com

Created attachment 3051
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3051&action=edit
proposed patch from Fedora (rebased on master)

The following bug, which is finally closing to be resolved made me
filling this bug report to catch up also in upstream:

https://bugzilla.redhat.com/show_bug.cgi?id=1199363

So what is the problem? OpenSSH is using by default FILE: credential
cache under TMP, which has a great advantage of being portable, but
might not be ideal from security point of view, it might have some
limitations (not actually collection -- can handle only single
principal so it is always overwritten by next kinit in session) and is
nowhere to configure on server. The "pointer" to this ccache is
preserved in the environment variable KRBCCNAME, which makes the
forwarded credentials pretty much isolated in session and not available
after the session is closed and some jobs are executed on his behalf.
It can be considered as an advantage or not.

Years ago, we implemented a patch, that was picking up default ccache
location from krb5 configuration files (there is a krb5 function to do
that). It was having some issues, but it looks like we pinpointed them
and in attachment is the proposed patch. What it does in short?

* Gets default_ccache_name from kerberos configuration files.
* Replaces expansions templates (we cant use krb5 functions, since we
are still running under the root context to my understanding and we
need to replace UID).
* The code tries to use this default value, but if it fails to retrieve
it, falls back to FILE: ccache as it always did.
* We do not set the environment KRBCCNAME if the default ccache is
used. Tools using kerberos know where the cache is, if it is in default
location. When we were setting it, we were hitting the error explained
in the above bugzilla.
* Adds some portability bits for windows (CCAPI ccache if supported
instead of files under /tmp/)

Feel free to comment if there is anything unclear in the patch or in
the comments. The variation of this patch is used in both Fedora and
RHEL.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

Jakub Jelen <jjelen at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3051|0                           |1
        is obsolete|                            |

--- Comment #1 from Jakub Jelen <jjelen at redhat.com> ---
Created attachment 3161
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3161&action=edit
Allow using default ccache or unique one as before

I attached a new patch adding an option KerberosUniqueTicket, allowing
to generate unique ccache names as before.

This new approach also utilizes the cc_new_unique() for credential
cache creation, avoids overwriting existing ccaches, sanitizes cleanup
procedure.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

Charles Hedrick <hedrick at rutgers.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hedrick at rutgers.edu

--- Comment #2 from Charles Hedrick <hedrick at rutgers.edu> ---
What happened to this? It's still a problem for us. In the most recent
version, the credential is actually going into /tmp. (I had been using
the version from Centos, which has patches from Redhat that caused the
problem reported here.) This is clearly not the right behavior. Thus it
doesn't appear that the patch referred to here was actually done. Using
/tmp is clearly wrong, and will interfere with Redhat's move to KCM:.

I've submitted a bug report to Redhat, since it's their code in Centos,
but I'd rather see it fixed here.

Leaving KRB5CCNAME unset would normally do the right thing, but I don't
recommend it. There will still have to be code added that understands
collections. (configure.ac will have to be modified to see whether
krb5_cc_cache_match exists. It was added in 2012. openssh probably
wants to support OSs older than that.) You want behavior to be the same
as in kinit and sssd. To avoid overwriting a cache having a different
principal, you need to do krb5_cc_cache_match to find a credential in
the cache that matches the one you're logging in with. If there isn't
then you have to create a new credential in the cache explicitly (if
you don't you could overwrite one with a different principal), and
arguably make it primary. In the end, you can set KRB5CCNAME to the
collection or leave it unset. In principle it doesn't matter. However
for consistency with sssd I'd set it. You really don't want behavior to
be different depending upon whether you used a password or not.

I'm willing to write the code if you'll accept it.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #3 from Charles Hedrick <hedrick at rutgers.edu> ---
I'm going to try the patch. But it's been a while. Is that patch going
to happen?

One comment: You say you have to do the template replacement yourself
because you're running as root. Another approach, which is probably
safer because you don't have to duplicate code that could change, is to
temporarily change your UID. I've done that in my own code.
Incidentally, make sure you check this code with KCM:. The default is
normally KCM:, with no uid. If you're going to use the default, you
have to be the user when you generate the ccache, or you'll get root's
KCM.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #4 from Charles Hedrick <hedrick at rutgers.edu> ---
What version of Kerberos do you intend to support?
krb5_cc_support_switch was added in 1.10, in 2012. We've built openssh
on older systems than that. If you intend to continue supporting old
Kerberos, I suggest modifying the autoconf stuff to check whether
krb5_cc_support_switch exists. All the collection stuff was added in a
single release, so it should be enough to check one function added in
that release.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #5 from Charles Hedrick <hedrick at rutgers.edu> ---
OK, I tested master with the second patch. Some issues:

default is KCM:. on the target system I have an expired ticket in
KCM:1003. I do kerberized ssh. The login happens, but the default cc is
still KCM:1003, and the ticket is still expired. I suspect the problem
is that you didn't do seteuid to the user. The KCM: implementation is
weird. There's no way for root to refer to the default collection of a
user. With KEYRING, you can use KEYRING:persistent:%{uid}, but KCM:1003
isn't a collection; it's a specific ccache. The only way to refer to
the collection is KCM: alone, and that only works if you're the right
user. I actually think this is a problem. I think KCM:1003 should be a
collection, and the first ticket should be something like
KCM:1003:1003, but the implementor doesn't see this as a problem.

using KEYRING:persistent:%{uid}

I have two things in the collection, hedrick and hedrick.admin, with
hedrick.admin selected. It adds a third cache for hedrick and selects
it. It should really use krb5_cc_cache_match to find the original
hedrick, update it with the new credential, and switch to it.

I have one credential, for hedrick. It adds a second one. I think this
is a mistake. sssd will reuse the existing credential cache.

sssd will also set KRB5CCNAME, which I think is preferred, though
leaving it unset isn't really a bug.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #6 from Charles Hedrick <hedrick at rutgers.edu> ---
You can make reasonable arguments for how sssd behaves and how this
patch behaves. Generating a new cache for each connection lets you
clean up the connection's cache separately without interfering with
other collections. That approach works as long as no one starts any
processes that continue running after their logout. Since that's pretty
common for us, this doesn't make sense.

Having a single cache which is used by all collections makes sense as
long as you have a separate way of cleaning it up when no one is using
it. We do, and sssd is going to implement it.

But whichever approach you think is better, it doesn't make sense for
your cache management to differ depending upon whether you login with a
password (which involves sssd) or with a Kerberos credential. If you
are ever going to use a password, you can't let sshd cleanup your
credentials, or you're going to pull a cache out from under a different
session. If sshd isn't going to cleanup, then it should reuse a current
credential *with the same principal* if there is one, to avoid a
continuing buildup of caches.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #7 from Charles Hedrick <hedrick at rutgers.edu> ---
sorry, collection --> connection in the previous comment.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #8 from Jakub Jelen <jjelen at redhat.com> ---
Thank you very much for review, trying the patch and valuable comments.

I did not know about krb5_cc_cache_match() function and it would
certainly make sense to reuse the same principal instead of creating a
new entry.

About the compatibility with older kerberoses, this is probably a
question on upstream. Here in Red Hat, we will probably not need to
support anything older than 1.15.

About switching to user context, we indeed to it, but at the time of
the credential creating, we can not switch both euid and real uid
permanently yet and kerberos code is using the real uids for the
template expansions.

As you describe for the KCM cache, it is not possible to use since it
does not have any collection that would be accessible from root and the
user under the same name, which I also consider as a bad design, but
the kerberos guys do not see it as an issue.

We used to set KRB5CCNAME, but it has its own drawbacks. After creating
the credention in collection, I did not find any unified way how to get
from kerberos the name of the containing collection, which I could use
for the above environment variable. There used to be several
workarounds for various collections (FILE, DIR, ...) and setting it
wrongly caused issues such as [1].

I will try to have a look into your proposed changes and incorporate
them into the patch.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1199363

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #9 from Charles Hedrick <hedrick at rutgers.edu> ---
I wasn't suggesting changing permanently to the user's context, just
changing temporarily while you generate the ccname, and while you open
the ccache (for KCM). I'm not sure which uid you need to set for each
of these operations, but as long as the saved uid remains 0 you can get
back. Look at setresuid. That assumes there aren't portability problems
with setresuid. I don't know whether portable openssh supports any OS's
without saved uid, but if so they'll probably be old enough to use the
old code, the same as if they have an old Kerberos.

RHEL has announced that KCM: will be the default for the next release.
I don't think you want sshd to not support it. This is portable ssh,
not the RHEL-specific patch, so KCM support could be added in
RHEL-specific code, but since KCM is going to be on all versions
through sssd, I'd prefer to see it done portably.

As to Kerberos versions, this is the bugzilla for portable openssh.
Isn't this the version that would most likely be used for old Linuxs,
Solaris 2.8, etc.? It's not hard to accommodate old Kerberos. Just omit
all the new code. I believe the latest version of the patch leaves in
the code to act the old way but doesn't use it be default. It should
become the only code if collections don't exist. This has to be done at
compile time, not run time, since you won't be able to compile code
with the collections API on an old Kerberos.

As to KRB5CCNAME. I understnad the problems with setting it. The old
code sets it wrong. But sssd sets it, and I don't think you want
behavior to depend upon whether you typed a password or used Kerberos.
I conjecture that you can simply use the expanded value of the default
from krb5.conf. I think that will always do the right thing. Can you
think of counterexamples?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #10 from Charles Hedrick <hedrick at rutgers.edu> ---
I just checked Solaris 2.8. I has setuid and seteuid, but not setreid
or setresuid. Setuid sets real, effective, and saved. Seteuid sets
effective only. So you can get back from seteuid, but  not setuid. That
says that for Solaris 2.8 it isn't possible to do a temporary change of
real uid. That means if you want to support stuff that old you'll need
an #ifdef. If I were doing it, I woudl do that. (Indeed I'm willing to
write the code if you want, and test it on both Centos 7.5 and Solaris
2.8.)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #11 from Charles Hedrick <hedrick at rutgers.edu> ---
The code I recommend is

getting cache name:

#ifdef hassetresuid
   setresuid(user,user,-1)
   name = krb5 default name
   setresuid(0,0,-1)
#else
   name = your current code to emulate the library
#endif

for opening the ccache

#ifdef hassetresuid
   setresuid(user,user,-1
#endif
   current code doing cc_resolv, etc
#ifdef hassetresuid
   setresuid(0,0,-1)
#endif

In the second case you don't need an else. The change of uid is really
only needed for KCM: (assuming you chown the cache properly). KCM: is
unlikely to be present on Solaris 2.8 and other systems without
setresuid.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #12 from Jakub Jelen <jjelen at redhat.com> ---
Hmm ... this sounds for me like an ugly hack to work around the way how
KCM is (intentionally) simple (and limited?). Generally, processes
should not need to change the real UID in order to do something in some
user's behalf.

I will try to get in touch with SSSD developers what is their view on
this, because I would really like to have this resolved somehow.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #13 from Charles Hedrick <hedrick at rutgers.edu> ---
I think you're going to find that the sssd team is stuck. They're
implementing an existing API from Heimdal. The MIT implementation of
KCM is based on Heimdal's spec. The MIT libraries are designed so you
can use an MIT-based application on a system where Heimdal is the
primary version of Kerberos. An example would be a Mac, where we want
to be able to use Macports applications based on MIT Kerberos, but the
system (including the service that supports KCM) is based on Heimdal.

I agree with your opinion of the design, but I think the sssd team is
stuck. I don't think they can make the change you propose.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #14 from Jakub Jelen <jjelen at redhat.com> ---
It showed up that the KCM implementation in SSSD is probably wrong and
the operations that I used were not correctly handled so this is
probably going to be fixed in sssd-kcm:

https://pagure.io/SSSD/sssd/issue/3873

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

Andreas Hasenack <andreas at canonical.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andreas at canonical.com

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

--- Comment #15 from Andreas Hasenack <andreas at canonical.com> ---
Hi, quick ping to see if this discussion/review can be revived :)

Is there an interest in openssh upstream to adopt this work?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

Pat Riehecky <riehecky at fnal.gov> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |riehecky at fnal.gov

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2775

Oliver Freyermuth <o.freyermuth at googlemail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |o.freyermuth at googlemail.com

-- 
You are receiving this mail because:
You are watching the assignee of the bug.