Bug 848223

Summary: cifs.upcall will need to search locations other than /tmp for Kerberos credential caches
Product: [Fedora] Fedora Reporter: Nalin Dahyabhai <nalin>
Component: cifs-utilsAssignee: Jeff Layton <jlayton>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: jlayton, sgallagh, sprabhu, ssorce, steved
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-09-17 18:41:54 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
if an entry is a directory, set the ccname type to DIR and proceed (untested)
none
search the user's directory for ccaches first, then try /tmp (untested) none

Description Nalin Dahyabhai 2012-08-14 18:57:13 EDT
Description of problem:
As part of the Fedora 18 features https://fedoraproject.org/wiki/Features/KRB5CacheMove and https://fedoraproject.org/wiki/Features/KRB5DirCache, cifs.upcall will need to be taught to look in additional locations for user credential caches.

In a nutshell, beginning with Fedora 18, we're going to be changing credential caches from being "FILE" type caches under /tmp to "FILE" or "DIR" type caches under /run/user/$UID.

The upcall helper currently only knows to search /tmp when it's looking for a user's Kerberos credentials, so we need to teach it about also searching the new locations to keep it working after the various components which create those credential caches start putting credential caches in the new locations.

Version-Release number of selected component (if applicable):
cifs-utils-5.6-1.fc18

How reproducible:
N/A
Comment 1 Nalin Dahyabhai 2012-08-16 17:45:49 EDT
Created attachment 605030 [details]
if an entry is a directory, set the ccname type to DIR and proceed (untested)
Comment 2 Nalin Dahyabhai 2012-08-16 18:02:46 EDT
Created attachment 605032 [details]
search the user's directory for ccaches first, then try /tmp (untested)

Here's a stab at what supporting users with ccaches under /run/user/$UID would look like.  It's completely untested, and if you don't like the bit that tries to make it parameterized, I won't be at all upset.  Hopefully it'll be of some use.
Comment 3 Jeff Layton 2012-08-17 07:24:03 EDT
Wow, thanks Nalin! I'll plan to test these out today.
Comment 4 Jeff Layton 2012-08-17 08:39:26 EDT
Ok, a couple of small(-ish) problems with these patches:

find_krb5_cc does a scandir on the given directory. This patch will make it pass in the full directory '/run/user/0' and it will scan and then try to use each file under it as a credcache. That's clearly not what we want.

Furthermore, there's a filter function on the scandir() call, so we need to avoid using when we look for DIR: caches as well.

If we're fairly certain that each user will have one and only one DIR: cache under /run/user, we can probably just skip the scandir altogether and simply test /run/user/$UID for validity.

If it's possible that they'll have more than one DIR: cache under there, then we probably just want to pass in '/run/user' to the function and not filter the results.
Comment 5 Jeff Layton 2012-08-20 09:50:26 EDT
Nalin, could you clarify? Is it expected that a single user would ever have more than one credcache directory under /run/user?

Once I understand whether that could occur, I'll see if I can fix your patches to do the right thing...
Comment 6 Nalin Dahyabhai 2012-08-20 12:01:30 EDT
(In reply to comment #4)
> Ok, a couple of small(-ish) problems with these patches:
> 
> find_krb5_cc does a scandir on the given directory. This patch will make it
> pass in the full directory '/run/user/0' and it will scan and then try to
> use each file under it as a credcache. That's clearly not what we want.

Not sure I follow here.  A ccache used by the user with UID 0 might be named something like FILE:/run/user/0/krb5cc or DIR:/run/user/0/krb5cc or FILE:/run/user/0/krb5cc_ABCDEFGH or DIR:/run/user/0/krb5cc_ABCDEFGH.

> Furthermore, there's a filter function on the scandir() call, so we need to
> avoid using when we look for DIR: caches as well.

If you like, sure.  I don't really understand why you'd use a filter when checking for FILE ccaches but not DIR ccaches, though.

> If we're fairly certain that each user will have one and only one DIR: cache
> under /run/user, we can probably just skip the scandir altogether and simply
> test /run/user/$UID for validity.
> 
> If it's possible that they'll have more than one DIR: cache under there,
> then we probably just want to pass in '/run/user' to the function and not
> filter the results.

The user's subdirectory under /run/user is not itself going to be a ccache, so unless there's an insert-the-UID step that I'm missing in there, I don't think that's correct.

(In reply to comment #5)
> Nalin, could you clarify? Is it expected that a single user would ever have
> more than one credcache directory under /run/user?

There can be cases where a user has more than one ccache in /run/user/$UID, and each of those ccaches can be either a file or a directory.
Comment 7 Jeff Layton 2012-08-20 12:18:57 EDT
(In reply to comment #6)
> (In reply to comment #4)
> > Ok, a couple of small(-ish) problems with these patches:
> > 
> > find_krb5_cc does a scandir on the given directory. This patch will make it
> > pass in the full directory '/run/user/0' and it will scan and then try to
> > use each file under it as a credcache. That's clearly not what we want.
> 
> Not sure I follow here.  A ccache used by the user with UID 0 might be named
> something like FILE:/run/user/0/krb5cc or DIR:/run/user/0/krb5cc or
> FILE:/run/user/0/krb5cc_ABCDEFGH or DIR:/run/user/0/krb5cc_ABCDEFGH.
> 

Ok, thanks -- that clarifies things. I misunderstood what the layout of the /run/user/$UID credcaches would look like. In that case I think your patches should be ok, but I'll plan to test them out today with a test rig that more closely matches what you've described above.
Comment 8 Jeff Layton 2012-08-20 12:41:35 EDT
Ok, cursory testing with it looks good. I'm testing on an f17 machine since my rawhide box is fubar. Nalin, do you mind posting those patches to linux-cifs@vger.kernel.org?

One minor thing that I noticed (unrelated to cifs.upcall):

# mkdir -p /run/user/50000

...then as testuser with uid=50000:

[testuser@f17 ~]$ kinit
kinit: Credential cache directory /run/user/50000/krb5cc does not exist while getting default ccache

...shouldn't the krb5 libs create that directory for you if it doesn't exist? With a FILE: cache, it's not required that the file already exist...
Comment 9 Nalin Dahyabhai 2012-08-20 16:14:35 EDT
(In reply to comment #8)
> Ok, cursory testing with it looks good. I'm testing on an f17 machine since
> my rawhide box is fubar. Nalin, do you mind posting those patches to
> linux-cifs@vger.kernel.org?

Do you have a pointer to a doc on creating a minimal test setup?  I didn't give these more than does-it-compile testing myself, and I'd like to have a better feel for it before taking that step.

> One minor thing that I noticed (unrelated to cifs.upcall):
> 
> # mkdir -p /run/user/50000
> 
> ...then as testuser with uid=50000:
> 
> [testuser@f17 ~]$ kinit
> kinit: Credential cache directory /run/user/50000/krb5cc does not exist
> while getting default ccache
> 
> ...shouldn't the krb5 libs create that directory for you if it doesn't
> exist? With a FILE: cache, it's not required that the file already exist...

You're right, there are a few complications for processes that create the credential caches.  I touched on the API-level bits at https://bugzilla.redhat.com/show_bug.cgi?id=848228#c1, but in a nutshell:
* krb5 1.10 expects the directory for a DIR: ccache to be created elsewhere.  In krb5 1.11, it'll start creating the final component in the pathname if it doesn't already exist.
* kdestroy on a DIR: ccache is pretty conservative in what it removes, so a process that wants to leave no trace needs to do some more cleanup work after that.
Comment 13 Nalin Dahyabhai 2012-08-21 18:59:05 EDT
Rebased on git master, added a bit more error-checking, shared the "best" state between the multiple calls to find_krb5_cc(), and posted them.
Comment 14 Fedora Update System 2012-08-24 07:37:42 EDT
cifs-utils-5.6-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/cifs-utils-5.6-2.fc18
Comment 15 Fedora Update System 2012-08-24 17:33:08 EDT
Package cifs-utils-5.6-2.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing cifs-utils-5.6-2.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-12639/cifs-utils-5.6-2.fc18
then log in and leave karma (feedback).
Comment 16 Fedora Update System 2012-09-17 18:41:54 EDT
cifs-utils-5.6-2.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.