Bug 1199363 - ssh login with credential forwarding gives me a subsidiary ccache; can't kinit to another user
Summary: ssh login with credential forwarding gives me a subsidiary ccache; can't kini...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: openssh
Version: 27
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1278017
TreeView+ depends on / blocked
 
Reported: 2015-03-06 03:34 UTC by Jason Tibbitts
Modified: 2019-01-08 21:16 UTC (History)
20 users (show)

Fixed In Version: openssh-7.6p1-2.fc27
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1278017 1278018 (view as bug list)
Environment:
Last Closed: 2017-12-10 05:04:14 UTC
Type: Bug


Attachments (Terms of Use)
proposed patch (6.59 KB, text/plain)
2017-08-01 13:29 UTC, Jakub Jelen
no flags Details
updated patch (6.48 KB, patch)
2017-08-01 18:12 UTC, Alex Scheel
no flags Details | Diff
patch v3 (7.79 KB, patch)
2017-08-04 08:50 UTC, Jakub Jelen
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
OpenSSH Project 2775 0 None None None 2019-07-09 13:38:36 UTC

Description Jason Tibbitts 2015-03-06 03:34:24 UTC
Basically, on the box that I originally logged into (via KDM):

> klist
Ticket cache: KEYRING:persistent:7225:krb_ccache_8LMqrkh
Default principal: tibbs@MATH.UH.EDU

Valid starting       Expires              Service principal
03/05/2015 21:19:44  03/06/2015 21:19:44  krbtgt/MATH.UH.EDU@MATH.UH.EDU
        renew until 03/12/2015 20:44:18

I can kinit to another principal:

> kinit tibbs/admin
Password for tibbs/admin@MATH.UH.EDU:

Now, if I first ssh to another host:

> ssh ld01
Last login: Thu Mar  5 21:25:03 2015 from epithumia.e.math.uh.edu

ld01:~> klist
Ticket cache: KEYRING:persistent:7225:7225
Default principal: tibbs@MATH.UH.EDU

Valid starting       Expires              Service principal
03/05/2015 21:25:16  03/06/2015 21:19:44  krbtgt/MATH.UH.EDU@MATH.UH.EDU
        renew until 03/12/2015 20:44:18

Then I can't kinit to another principal:

ld01:~> kinit tibbs/admin
kinit: Can't create new subsidiary cache because default cache is already a subsdiary while generating new ccache

Simo seems to know what's up and asked me to file a bug.  I'm running updated F21; here are some hopefully relevant versions:

krb5-libs-1.12.2-9.fc21.x86_64
openssh-6.6.1p1-11.1.fc21.x86_64
pam_krb5-2.4.9-3.fc21.x86_64
pam-1.1.8-16.fc21.x86_64
kernel-3.18.7-200.fc21.x86_64

Comment 1 Jakub Jelen 2015-03-17 12:22:47 UTC
OK. I can reproduce it on my machine. It also appears when you try to kinit to different user (not only principal).

When you try to list tickets with flags (klist -f), you will see that the last ticket has a "f" flag, which means that it is forwarded (and used for authentication for this host?) so you probably can't overwrite it with different ticket?
seems to be related to this one: bz974947


Simo? What do you think? What is expected behaviour?

Just tested with DIR ccache and I'm unable to get the forwarded ticket on second server.

Comment 2 Simo Sorce 2015-03-17 13:13:23 UTC
(In reply to Jakub Jelen from comment #1)
> OK. I can reproduce it on my machine. It also appears when you try to kinit
> to different user (not only principal).
> 
> When you try to list tickets with flags (klist -f), you will see that the
> last ticket has a "f" flag, which means that it is forwarded (and used for
> authentication for this host?) so you probably can't overwrite it with
> different ticket?
> seems to be related to this one: bz974947
> 
> 
> Simo? What do you think? What is expected behaviour?
> 
> Just tested with DIR ccache and I'm unable to get the forwarded ticket on
> second server.

It's a tricky situation, on the one hand it may make sense to have a subsidiary cache, on the other it causes a number of restrictions we shouldn't really suffer.

We probably need to discuss what are the expectations as an ssh user and see if we need some tunables or not.

My initial thinking is that, if you have a Cache Collection, what you can do is to stick the delegated credentials in a subsidiary, but still put the whole collection in KRB5CCNAME and just kswitch to the newly stored credentials as the default creds.
However, if you are already logged into the console or from another ssh session, doing a kswitch will affect all terminals, so I am hesitant.

Perhaps, it should be a tunable where you can tell sshd what is the preference: stick to subsidiary vs expose the whole collection.

Comment 3 Jakub Jelen 2015-03-26 16:14:23 UTC
OK. I had the second look at this issue and from my point of view, we should do it all the same way how the FILE and DIR cache does it. My testing with different default_ccache_name showed that you can do the above mentioned kinit with all the other configurations:
http://paste.fedoraproject.org/203252/27382909/

So only KEYRING is set up as subsidiary cache.

There is no reason to block kinit, because there is workaround to use "kdestroy; kinit user" to change to different principal.

sshd is handling ccache by hand and KEYRING is supported after this bz991186. We are using functions krb5_cc_initialize, krb5_cc_store_cred from krb5.h so if I'm right we can

Unfortunately, there is not much documentation about KEYRING ccache. I don't think that we specify somewhere it it should be subsidiary or do handle this cache different than the others. If it is a bug, probably in krb5 libraries IMHO.

Comment 4 Jason Tibbitts 2015-03-26 16:46:47 UTC
Unfortunately the workaround of "kdestroy; kinit whatever" has implications for kerberized NFS so it's not entirely a perfect solution, but I can deal either way.  I just thought it was odd that whether you can kinit depends on both your ccache type and how you happened to log in.

Comment 5 Simo Sorce 2015-03-26 17:33:14 UTC
Ok I think we can move this to krb5, if the KEYRING ccache behaves subnstantially differently than a FILE/DIR ccache we may consider it a KEYRING ccache bug indeed.

Comment 6 Roland Mainz 2015-03-26 23:52:17 UTC
(In reply to Simo Sorce from comment #5)
> Ok I think we can move this to krb5, if the KEYRING ccache behaves
> subnstantially differently than a FILE/DIR ccache we may consider it a
> KEYRING ccache bug indeed.

Who wrote the initial KEYRING support in krb5 ?

Comment 7 Simo Sorce 2015-03-27 13:13:31 UTC
(In reply to Roland Mainz from comment #6)
> Who wrote the initial KEYRING support in krb5 ?

I think Nalin did the old KEYRING code, and I refactored it heavily to make it a Collection Type.

Comment 8 Nalin Dahyabhai 2015-03-27 13:47:20 UTC
You flatter me.  The git log attributes it to Kevin Coffman.

Comment 9 Charles Slivkoff 2015-06-01 15:20:32 UTC
This is also an issue for RHEL 7. 

Is another BZ needed?

Comment 10 Fedora End Of Life 2015-11-04 10:36:30 UTC
This message is a reminder that Fedora 21 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 21. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '21'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 21 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 11 Jason Tibbitts 2015-11-04 15:19:49 UTC
This does not appear to be an issue in F22 and I don't have much in the way of F21 around any longer so I'm happy to let this autoclose.

Comment 12 Jakub Jelen 2015-11-06 12:46:26 UTC
(In reply to Jason Tibbitts from comment #11)
> This does not appear to be an issue in F22 and I don't have much in the way
> of F21 around any longer so I'm happy to let this autoclose.

Thank you for testing again. I am not sure if and what has changed and it would be useful for krb5 guys to have a track about it if they will be fixing it also in rhel7. Closing bugs as RESOLVED is always more pleasant then EOL, isn't it?

As pointed out, it is still reproducible on RHEL7, but I can reproduce it also on all Fedoras from 21 to 23. Moving to F23 since it looks like nothing changed here (or you switched ccache to FILE/DIR from KEYRING?

My basic test case looks like this (for future self or QA):
 setup gssapi authenticatin for server and client
 make sure "GSSAPIDelegateCredentials yes" is in ssh_config
$ kinit tester
$ ssh tester@kerberos.example.com
[tester@]$ kinit root # fail
kinit: Can't create new subsidiary cache because default cache is already a subsidiary while generating new ccache

Comment 13 Robbie Harwood 2016-01-06 14:37:13 UTC
I'm having a hard time understanding the last two comments.  It looks to me like this is not an issue anymore; is that correct?

Comment 14 Charles Slivkoff 2016-01-06 16:45:38 UTC
F23 continues to behave the same.

I ssh to a F23 server and authenticate via GSSAPI with GSSAPIDelegateCredentials=yes in ~/.ssh/config

Attempts to kinit to another instance without first kdestroy return this failure:

kinit: Can't create new subsidiary cache because default cache is already a subsidiary while generating new ccache

Comment 15 Jason Tibbitts 2016-01-06 21:00:22 UTC
I can confirm that this is a problem (when ssh'ing from an F23 machine to an F22 or an F23 machine; both fully updated).  I swear that when I tested it a while back the problem was gone, but it's certainly here now.

Oddly, I'm _not_ seeing the issue when sshing from an F23 machine to a Centos7 machine (fully updated).

Comment 16 Jakub Jelen 2016-01-08 08:01:13 UTC
My testing log on fpaste is gone (poor me who post it there and believe it will survive), but as far as I recall from the comment #1, the problem is only with KEYRING ccache (based on default_ccache_name in /etc/krb5.conf).

Jason, my guess is that your CentOS 7 has configured to use some different ccache (thought KEYRING is default in RHEL as far as I know)? Can you check also this one?

Comment 17 Fedora End Of Life 2016-11-24 11:32:12 UTC
This message is a reminder that Fedora 23 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 23. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '23'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 23 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 18 Fedora End Of Life 2016-12-20 13:20:12 UTC
Fedora 23 changed to end-of-life (EOL) status on 2016-12-20. Fedora 23 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.

Comment 19 Robbie Harwood 2017-07-21 18:15:48 UTC
(Reproduced on rawhide.)

Comment 20 Alex Scheel 2017-07-21 19:28:38 UTC
I believe this to be a bug in OpenSSH: https://github.com/openssh/openssh-portable/blob/master/gss-serv-krb5.c#L184

(Note that Fedora carries patches modifying this behavior that have the same issue: http://pkgs.fedoraproject.org/cgit/rpms/openssh.git/tree/openssh-7.2p1-gsskex.patch#n815, http://pkgs.fedoraproject.org/cgit/rpms/openssh.git/tree/openssh-6.3p1-krb5-use-default_ccache_name.patch#n246)

In the Fedora situation, default_ccache_name is read from /etc/krb5.conf and is used to create a new ccache with the forwarded creds. However, calling krb5_cc_get_name() and setting KRB5CCNAME to that value forces krb5 to only work with that single credential. In the upstream case, default_ccache_name in /etc/krb5.conf is ignored and a random file ccache is used, so KRB5CCNAME must be set. 

It should be noted that the reason DIR works correctly in Fedora is due to these branch: http://pkgs.fedoraproject.org/cgit/rpms/openssh.git/tree/openssh-7.2p1-gsskex.patch#n829

krb5_cc_get_name() returns the name for a specific cache-collection item. For FILE with default_ccache_name specified in krb5.conf, this is the same as default_ccache_name (without leading FILE:). For DIR, this would be the path to the ticket inside the directory. For KEYRING, this would be the name of the subsidiary cache. However, that Fedora patch checks if the type is DIR and effectively removes the file portion of the name. As there is no similar branch for KEYRING, KRB5CCNAME is set to the specific subsidiary and kinit will not add to it.

From krb5's point of view, the behavior of krb5_cc_get_name() is consistent among FILE/DIR/KEYRING. Unsetting KRB5CCNAME fixes the problem.

(From Fedora+OpenSSH's point of view, they'd either need to introduce another branch handling the forms of KEYRING correctly, or they'd need to not set KRB5CCNAME when /etc/krb5.conf is has a default_ccache_name, as in the case default_ccache_name is missing, they fall back to the upstream random FILE behavior).

Comment 21 Jakub Jelen 2017-07-31 16:51:35 UTC
Alex, thank you for your investigation!

Let me clarify what is summary and proposal for OpenSSH:

* the FILE already works out of the box

* the DIR already strips the cache-collection part of the KRB5CCNAME

* in case of KEYRING ccache, we will get from krb5_cc_get_name() something like "KEYRING:persistent:1000:1000", which is cache-collection item, but we should set the KRB5CCNAME to only first portion of it: "KEYRING:persistent:1000", so the kinit can create a new item there?

How is the new KCM cache by default [1] going to affect this behavior? Should we treat it in some different way? Or is there already some krb5 function that would do this magic for us?

I will try to put together testing environment and verify the fix in next days.

Thanks,
Jakub

[1] https://fedoraproject.org/wiki/Changes/KerberosKCMCache

Comment 22 Alex Scheel 2017-07-31 17:57:28 UTC
Jakub,

There's no reason to set KRB5CCNAME except in the specific case where OpenSSH creates a FILE ccache with random name per fallback here [1]. So I'd just not set it unless in that fallback, which will solve the KCM case as well. Also, there is no supported way of querying the cache-collection from the ccache [2], so supporting KEYRING and KCM would have to be carried hacks which (might need to be updated later; see [3] for additional forms of KEYRING that'd need to be tested as well).


(krb5/kinit/etc. will default to using the same ccaches that OpenSSH'd be reading and manually setting in KRB5CCNAME, so setting it to the default is redundant at best)


Thanks,

Alex


[1]: http://pkgs.fedoraproject.org/cgit/rpms/openssh.git/tree/openssh-6.3p1-krb5-use-default_ccache_name.patch#n220
[2]: http://colabti.org/irclogger/irclogger_log/krbdev?date=2017-07-21#l49
[3]: https://web.mit.edu/kerberos/krb5-1.12/doc/basic/ccache_def.html#ccache-types

Comment 23 Jakub Jelen 2017-08-01 13:29:17 UTC
Created attachment 1307585 [details]
proposed patch

Thanks for clarification.

There was a lot going on around this part of code downstream, but the changes mostly come from the bug #848228, which moved default cache from /tmp to /run and adding a workaround for the DIR cache and the bug #991186 which introduced reading of the krb5 configuration file.

Unfortunately, the file path is created and resolved on completely different place than where we are setting the environment variable and these functions do not share any common context variables. Therefore the patch is quite intrusive.

I put together a patch that builds and that should prevent setting up environment variable for default ccache location, but I don't have time to test it at the moment. So far attaching a patch here and I hope I will get to that in coming weeks.

Comment 24 Alex Scheel 2017-08-01 18:12:51 UTC
Created attachment 1307742 [details]
updated patch

I have tested the patch and have attached an update to it.

The patch didn't work for two reasons. Line numbers refer to code before this patch was applied.

1) In gss-serv-krb5.c:ssh_gssapi_krb5_storecreds:474, the guard for do_pam_putenv needs to include a check for set_env (technically redundant with my changes).
2) The end of the same function sets client->store.env{val,var} so the later call to ssh_gssapi_do_child() will expose them in the new session. Thus KRB5CCNAME is still set.

To fix that, I've added an early return path on lines 453-457.

Also: I noticed you changed the return type of ssh_gssapi_krb5_storecreds to int, but didn't update ssh_gssapi_storecreds() (its caller) to do anything with that. This fix would allow you to change that back to void like the other mechs.

Comment 25 Jakub Jelen 2017-08-03 10:08:51 UTC
Thanks Alex for testing and for the addition.

You are right, also the PAM modules should be able to know the location of the ccache if it is default and they should not need a environment variable.

But I am not sure if we can skip setting the store variables. They are used in both rekeying and during the credential cleanup (conditional), if I understand the code correctly (the krb5_free_context() needs the context, which is not passed to it if we use your early-return).

Comment 26 Alex Scheel 2017-08-03 14:34:22 UTC
Jakub,

I'm not sure I follow your parenthetical note: "(the krb5_free_context() needs the context, which is not passed to it if we use your early-return)"

I definitely agree that krb5_free_context(...) needs a context, and my early-return ensures that the context gets set and the credentials cache is closed:

453     if (!set_env) {
454          krb5_cc_close(krb_context, ccache);
455          client->store.data = krb_context;
456          return 0;
457     }


However, krb5_free_context(...) doesn't need ccache locations, so not setting env{val,var} shouldn't affect that code. 

With respect to rekey-ing, the only reason rekeying would need the ccache name is if the default changed or it was non-default to begin with. The latter is currently handled already, which leaves the former. If the default changed then certainly _other_ gss/krb5 programs will be using the new default (unless the user sets KRB5CCNAME), so there's no reason for SSH not to. Thus I'd prefer to update ssh_gssapi_krb5_updatecreds(...) and friends to again query the default ccache if one is not specified in envval.

With respect to credential cleanup, I'm not entirely sure how we want to handle this and defer to Robbie on this. With a persistent keystore like KEYRING, there are interesting edge cases:

- What if the ccache contains other principals generated while inside an SSH session by kinit?
- What if the ccache was non-empty upon ssh connection? 
- What if the ccache already contained a ticket for the delegated principal?

I think the behavior we want is at "ssh only removes what it explicitly created" but beyond that, I'm not sure.



However, none of this answers the question of "why does setting env{val,var} set environment variables". What I find most interesting is that even the client will know about it:


debug1: Setting KRB5CCNAME to KEYRING:persistent:1001:krb_ccache_8L9ii1E


(from ssh -vvv...)


Thanks! Any thoughts would be appreciated.

-- Alex

Comment 27 Jakub Jelen 2017-08-04 08:50:34 UTC
Created attachment 1308961 [details]
patch v3

Oh ... my bad. I missed you stored the krb5_context. But the cleanup is still using store.envval as an argument to the krb5_cc_resolve(). If the NULL is passed, it will resolve the default location? I didn't see it documented anywhere. If not, we still need it to find what we wrote/created to be able to clean up after ourselves.

My initial idea was to set up all the envval, envvar variables as we did before not to break other things, but gate only the passing of environment variables to the user environment and PAM, but you are right, there needs to be more checks (added in the current patch, including the debug1 line you posted in previous comment).

About the rekeying, if I understand the code correctly, the ssh_gssapi_rekey_creds() is helpful only for the updating the existing non-default files so we should not need it? With your patch, this function is completely skipped (as it was so far for non-FILE and non-DIR ccache).

Comment 28 Alex Scheel 2017-08-04 13:45:10 UTC
Jakub,

I was not able to get that patch to build as authctxt is not defined in session.c:1087. If you change it to s->authctxt, then it compiles and works. I think you can thus safely remove the changes to gss-serv-krb5.c:453-457 (which I see you've commented out). 

Regarding krb5_cc_resolve with NULL name, per here [0], that will fail (though the docs on the matter are vague [1]). I'd make it a call to krb5_cc_default(...) in the case when it is NULL though [2], as this will both query the default name and resolve the ccache.

It appears that cleanup doesn't actually remove any creds (and didn't before this patch either), but I'd call that a separate issue, so I'm not sure how much the code there matters.

Further, I'd have to defer to Robbie again on this one, but the behavior in the following scenario seems weird:

- Two principals, alice and bob (or bob and bob/admin, etc.)
- Client->Server
- An existing session for `bob` on the server, say, via console.
- `bob` on the server kinits to alice with an empty cache. 

This will then look like this:

[bob@bz-c2 ~]$ kinit alice
Password for alice@CIPHERBOY.COM: 
[bob@bz-c2 ~]$ klist -A
Ticket cache: KEYRING:persistent:1001:krb_ccache_h3s0SlR
Default principal: alice@CIPHERBOY.COM

Valid starting       Expires              Service principal
08/04/2017 09:38:46  08/04/2017 21:38:46  krbtgt/CIPHERBOY.COM@CIPHERBOY.COM


If `bob` from the client then kinits as bob and ssh-s with delegated creds into the server, the result is as follows:

[bob@bz-c1 ~]$ ssh bob@bz-c2 
Last login: Fri Aug  4 09:35:05 2017 from 192.168.122.135
[bob@bz-c2 ~]$ klist -A
Ticket cache: KEYRING:persistent:1001:krb_ccache_h3s0SlR
Default principal: bob@CIPHERBOY.COM

Valid starting       Expires              Service principal
08/04/2017 09:39:28  08/04/2017 21:11:23  krbtgt/CIPHERBOY.COM@CIPHERBOY.COM

That is, Alice's credential is overwritten. This is not the same behavior as when kinit is called in two separate sessions, so I'd say there's something funny with ssh's adding of the delegated cred, but I'd have to look into it more.

(in that case it looks like:

[bob@bz-c2 ~]$ kinit alice
Password for alice@CIPHERBOY.COM: 
[bob@bz-c2 ~]$ klist -A
Ticket cache: KEYRING:persistent:1001:krb_ccache_7gFFCnM
Default principal: alice@CIPHERBOY.COM

Valid starting       Expires              Service principal
08/04/2017 09:41:18  08/04/2017 21:41:18  krbtgt/CIPHERBOY.COM@CIPHERBOY.COM

Ticket cache: KEYRING:persistent:1001:krb_ccache_h3s0SlR
Default principal: bob@CIPHERBOY.COM

Valid starting       Expires              Service principal
08/04/2017 09:41:13  08/04/2017 21:41:13  krbtgt/CIPHERBOY.COM@CIPHERBOY.COM

from a different/existing session)



Besides the above, I'd give a LGTM, but I've not tested rekey as I'm not quite sure how to. But `kinit` works!


Thanks for your help!


[0]: https://github.com/krb5/krb5/blob/85d64c43dbf7a7faa56a1999494cdfa49e8bd2c9/src/lib/krb5/ccache/ccbase.c#L204-L205
[1]: http://web.mit.edu/kerberos/krb5-current/doc/appdev/refs/api/krb5_cc_resolve.html
[2]: http://web.mit.edu/kerberos/krb5-current/doc/appldev/refs/api/krb5_cc_default.html

Comment 29 Robbie Harwood 2017-08-04 17:41:09 UTC
> With respect to credential cleanup, I'm not entirely sure how we want to handle this and defer to Robbie on this. With a persistent keystore like KEYRING, there are interesting edge cases:
>
> - What if the ccache contains other principals generated while inside an SSH session by kinit?
> - What if the ccache was non-empty upon ssh connection? 
> - What if the ccache already contained a ticket for the delegated principal?

Clearly we don't want to delete principals we didn't create.

Our ccaches don't really have a notion of "deleting" principals.  This is why, for instance, there is no such option in kdestroy.  We've had conversations about adding verbs for this in the past, but the current consensus is that there is very little gain to doing so (because ccaches can be copied around, and the only way to expire credentials is to let them time out).

So we can't cleanup just the ones we've created through ssh.  But even where we could (e.g., empty ccache on remote), we don't want to.  The user may ssh in and run jobs; yanking the creds out from under these jobs would be problematic.  Consider running a mosh-server, or something in screen/tmux, for instance.

So let's not do any cleanup here.

> Further, I'd have to defer to Robbie again on this one, but the behavior in the following scenario seems weird:

For similar reasons to above, this behavior is problematic ("rude").

It looks like this is caused by the ssh_gssapi_krb5_storecreds function (gss-serv-krb5.c) which, when building against MIT krb5, calls several functions that empty the ccache.  Probably this should be built around krb5_cc_resolve() instead (side note: krb5_cc_gen_new is deprecated).

Comment 30 Jan Kurik 2017-08-15 09:01:34 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.

Comment 31 Jakub Jelen 2017-08-17 13:19:51 UTC
> It appears that cleanup doesn't actually remove any creds (and didn't before this patch either), but I'd call that a separate issue, so I'm not sure how much the code there matters.

AFAIK this is because the configuration option GSSAPICleanupCredentials is set to no in the shipped configuration file. This use case might not be default nor most important, but we should certainly not break it (at least not in RHEL).

> Clearly we don't want to delete principals we didn't create.

That sounds reasonable. The safe default is not-to-delete, but I believe there should be still the option to delete them (there is ... as above) and it should be tested and supported.

You can simply copy FILE/DIR ccaches, but can you do the same with KEYRING/KCM?

> It looks like this is caused by the ssh_gssapi_krb5_storecreds function (gss-serv-krb5.c) which, when building against MIT krb5, calls several functions that empty the ccache.

Which one do you mean?

> Probably this should be built around krb5_cc_resolve() instead (side note: krb5_cc_gen_new is deprecated).

I don't see we would use krb5_cc_gen_new() with MIT in current Fedora. All the usage is in HEIMDAL ifdefs.

Also the ssh_krb5_cc_gen() in the end calls krb5_cc_resolve() as you propose.

Comment 32 Robbie Harwood 2017-08-17 20:02:56 UTC
(In reply to Jakub Jelen from comment #31)
>> Clearly we don't want to delete principals we didn't create.
> 
> That sounds reasonable. The safe default is not-to-delete, but I believe
> there should be still the option to delete them (there is ... as above) and
> it should be tested and supported.
> 
> You can simply copy FILE/DIR ccaches, but can you do the same with
> KEYRING/KCM?

Yes, there shouldn't be anything to stop you doing that.  I'm happy to support operations using the provided GSSAPI/krb5 APIs, but you remove elements from ccaches by hand at your own peril :)
 
>> It looks like this is caused by the ssh_gssapi_krb5_storecreds function (gss-serv-krb5.c) which, when building against MIT krb5, calls several functions that empty the ccache.
> 
> Which one do you mean?

Well, there's a straight up call to krb5_cc_initialize(), for one.

> > Probably this should be built around krb5_cc_resolve() instead (side note: krb5_cc_gen_new is deprecated).
> 
> I don't see we would use krb5_cc_gen_new() with MIT in current Fedora. All
> the usage is in HEIMDAL ifdefs.

You're right, I misread the nesting, apologies.

> Also the ssh_krb5_cc_gen() in the end calls krb5_cc_resolve() as you propose.

It may also benefit from krb5_cc_default_name().  (Strongly preferred to reading krb5.conf yourself.)

Comment 33 Jakub Jelen 2017-08-18 10:43:41 UTC
(In reply to Robbie Harwood from comment #32)
> (In reply to Jakub Jelen from comment #31)
> >> Clearly we don't want to delete principals we didn't create.
> > 
> > That sounds reasonable. The safe default is not-to-delete, but I believe
> > there should be still the option to delete them (there is ... as above) and
> > it should be tested and supported.
> > 
> > You can simply copy FILE/DIR ccaches, but can you do the same with
> > KEYRING/KCM?
> 
> Yes, there shouldn't be anything to stop you doing that.  I'm happy to
> support operations using the provided GSSAPI/krb5 APIs, but you remove
> elements from ccaches by hand at your own peril :)

In gss-serv.c, function ssh_gssapi_cleanup_creds() I see a call to krb5_cc_destroy(), which sounds like a proper API.

The other thing is that the code is trying to remove forwarded DIR ccache in auth-krb5.c, function krb5_cleanup_proc() in case of failure during the authentication. Do you believe that krb5_cc_destroy() call should be enough? To my understanding, that code is removing the ccache directory, only if there is primary file/directory inside. But it should not matter for other ccache types.

> >> It looks like this is caused by the ssh_gssapi_krb5_storecreds function (gss-serv-krb5.c) which, when building against MIT krb5, calls several functions that empty the ccache.
> > 
> > Which one do you mean?
> 
> Well, there's a straight up call to krb5_cc_initialize(), for one.

Yes, that makes sense. Is there a way to check the ccache is initialized so we would not have to destroy it? Or do the initialization without the destroying the context?

> > > Probably this should be built around krb5_cc_resolve() instead (side note: krb5_cc_gen_new is deprecated).
> > 
> > I don't see we would use krb5_cc_gen_new() with MIT in current Fedora. All
> > the usage is in HEIMDAL ifdefs.
> 
> You're right, I misread the nesting, apologies.
> 
> > Also the ssh_krb5_cc_gen() in the end calls krb5_cc_resolve() as you propose.
> 
> It may also benefit from krb5_cc_default_name().  (Strongly preferred to
> reading krb5.conf yourself.)

That sounds like a good idea. Is it something new that was not generally available some years ago, when this patch was written? Does it also expand templates (uid, euid, USERID, TEMP) as written in configuration file, or this is still up to the caller?

Comment 34 Robbie Harwood 2017-08-24 20:15:09 UTC
(In reply to Jakub Jelen from comment #33)
> (In reply to Robbie Harwood from comment #32)
> > (In reply to Jakub Jelen from comment #31)
> > > > Clearly we don't want to delete principals we didn't create.
> > > 
> > > That sounds reasonable. The safe default is not-to-delete, but I believe
> > > there should be still the option to delete them (there is ... as above) and
> > > it should be tested and supported.
> > > 
> > > You can simply copy FILE/DIR ccaches, but can you do the same with
> > > KEYRING/KCM?
> > 
> > Yes, there shouldn't be anything to stop you doing that.  I'm happy to
> > support operations using the provided GSSAPI/krb5 APIs, but you remove
> > elements from ccaches by hand at your own peril :)
> 
> In gss-serv.c, function ssh_gssapi_cleanup_creds() I see a call to
> krb5_cc_destroy(), which sounds like a proper API.

Sure, that destroys the entire ccache though.  (I was referring to pulling out individual elements.)

> The other thing is that the code is trying to remove forwarded DIR ccache in
> auth-krb5.c, function krb5_cleanup_proc() in case of failure during the
> authentication. Do you believe that krb5_cc_destroy() call should be enough?
> To my understanding, that code is removing the ccache directory, only if
> there is primary file/directory inside. But it should not matter for other
> ccache types.

Well, DIR isn't the only collection type, right: KEYRING and KCM are also collection types.

krb5_cc_destroy will empty the ccache, so yes, that should be enough, if I understand the question.  If you want to destroy the collection, you have to iterate over the ccaches in it and destroy each one in turn.  (The kdestroy code has an example of this, in the -A path: https://github.com/krb5/krb5/blob/master/src/clients/kdestroy/kdestroy.c#L148-L171 if you're curious, but I don't think you actually want to empty the collection in its entirety.)

> > > > It looks like this is caused by the ssh_gssapi_krb5_storecreds function (gss-serv-krb5.c) which, when building against MIT krb5, calls several functions that empty the ccache.
> > > 
> > > Which one do you mean?
> > 
> > Well, there's a straight up call to krb5_cc_initialize(), for one.
> 
> Yes, that makes sense. Is there a way to check the ccache is initialized so
> we would not have to destroy it? Or do the initialization without the
> destroying the context?

It shouldn't need an explicit step.  Initialize for us is meant in the calloc() sense - that is, resetting everything to 0/defaults.

> > > > Probably this should be built around krb5_cc_resolve() instead (side note: krb5_cc_gen_new is deprecated).
> > > 
> > > I don't see we would use krb5_cc_gen_new() with MIT in current Fedora. All
> > > the usage is in HEIMDAL ifdefs.
> > 
> > You're right, I misread the nesting, apologies.
> > 
> > > Also the ssh_krb5_cc_gen() in the end calls krb5_cc_resolve() as you propose.
> > 
> > It may also benefit from krb5_cc_default_name().  (Strongly preferred to
> > reading krb5.conf yourself.)
> 
> That sounds like a good idea. Is it something new that was not generally
> available some years ago, when this patch was written? Does it also expand
> templates (uid, euid, USERID, TEMP) as written in configuration file, or
> this is still up to the caller?

Good question!  The function dates well back into the early krb5 days and appears to have been committed in early 1990 (which makes it older than I am), but it's hard to tell because a cvs2svn migration was in progress at that point.

Empirically, yes, it expands template variables.  For instance, it will spit out strings like: "KEYRING:persistent:21259" or "FILE:/tmp/krb5cc_21259".

Comment 35 Jakub Jelen 2017-09-05 09:49:33 UTC
Thank you for valuable answers and patience :)

The first step to resolve the original problems is the patch attached in the comment #27, which does most of the expected.

The clearing ccache on new (forwarded) connection was not problem before using the default ccache location, because every session had its own.

If I would remove the cc_initialize(), what all can go wrong? How is the ccache getting the information about the default principal, which is passed to this function? I can not seem to find any function that could do that.

It could also give us the case, when we would insert tickets into the cache, but it would have some different default principal and the connected user/application would not see them or would not be able to use them, isn't it?

These are my concerns about the modification/skipping the initialization. I believe we will probably have to live with cleaning the cache on forwarding.

Looking into the krb5_cc_default_name(), it is using the currently running user context (getuid()), which might be a problem, if the process still runs privileged as a root, but we need to prepare caches for connecting users (we are using geteuid() for both UID and EUID expansions). Therefore I don't think we can simplify it here.

I will probably push the patch as it is into Fedora soon. It is improvement in generic case, but if you have some comments/ideas hot to improve the initialization without side effects for the default, feel free to comment.

Comment 36 Jakub Jelen 2017-09-05 12:08:37 UTC
I am planning to send something from this upstream, but it is little bit more complicated, because they do not have key exchange, so I will come with some more questions:

 * Comment #20 talks about workaround for DIR which made it working before. In case we will not be setting the KRB5CCNAME, this workaround is not needed (branch stripping the file part from the path):

https://src.fedoraproject.org/rpms/openssh/blob/master/f/openssh-6.3p1-krb5-use-default_ccache_name.patch#_45

 * Can you try to explain what can the cleanup_proc() do? I believe it is related to the above point (stripping the filename from the path), but is it needed if I remove the above modification? the cc_destroy should take care of removing the files, but I don't see it would be removing the directory. Is it expected?

https://src.fedoraproject.org/rpms/openssh/blob/master/f/openssh-6.3p1-krb5-use-default_ccache_name.patch#_72

Comment 37 Robbie Harwood 2017-09-06 19:37:20 UTC
(In reply to Jakub Jelen from comment #36)
> I am planning to send something from this upstream, but it is little bit
> more complicated, because they do not have key exchange, so I will come with
> some more questions:

Sounds good!  We'll try to answer.

>  * Comment #20 talks about workaround for DIR which made it working before.
> In case we will not be setting the KRB5CCNAME, this workaround is not needed
> (branch stripping the file part from the path):
> 
> https://src.fedoraproject.org/rpms/openssh/blob/master/f/openssh-6.3p1-krb5-use-default_ccache_name.patch#_45

Not sure there's a question here, but the workaround indeed shouldn't be needed.

>  * Can you try to explain what can the cleanup_proc() do? I believe it is
> related to the above point (stripping the filename from the path), but is it
> needed if I remove the above modification? the cc_destroy should take care
> of removing the files, but I don't see it would be removing the directory.
> Is it expected?
> 
> https://src.fedoraproject.org/rpms/openssh/blob/master/f/openssh-6.3p1-krb5-use-default_ccache_name.patch#_72

This is a good question.  The code segment you highlight looks to be removing the /primary entry of a DIR ccache?  Not supported, whatever it is :)

Comment 38 Jakub Jelen 2017-09-11 14:19:10 UTC
There were some more questions/clarification in the Comment #35.
The upstream bug addressing what is answered is already filled.

I submitted a scratch build [1] with these changes and if it will work as expected, I will push it into Fedora 27. If you have some environment, where you can test it, it would be very nice.

[1] https://koji.fedoraproject.org/koji/taskinfo?taskID=21796356

Comment 39 Robbie Harwood 2017-09-21 20:04:01 UTC
I spun a f26 build and it seems to work as expected there.  Thanks Jakub!

Comment 40 Fedora Update System 2017-11-22 09:05:38 UTC
openssh-7.6p1-2.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2017-96d1995b70

Comment 41 Fedora Update System 2017-11-22 21:42:29 UTC
openssh-7.6p1-2.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-96d1995b70

Comment 42 Fedora Update System 2017-12-10 05:04:14 UTC
openssh-7.6p1-2.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.