Bug 7157 - OpenAFS tokens owned by root
Summary: OpenAFS tokens owned by root
Status: NEW
Alias: None
Product: Samba 3.4
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.4.5
Hardware: All Linux
: P3 major
Target Milestone: ---
Assignee: Volker Lendecke
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-19 09:16 UTC by Thomas J. Moore
Modified: 2010-02-21 11:04 UTC (History)
0 users

See Also:


Attachments
Make setreuid path set real id (333 bytes, patch)
2010-02-21 11:04 UTC, Thomas J. Moore
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas J. Moore 2010-02-19 09:16:32 UTC
The setresuid path in set_effective_uid() behaves differently from the other paths:  it sets the real user ID as well as the effective one.  The other paths do not set the real user ID.  Whether or not this is a good thing, it is actually necessary for OpenAFS to work.  Otherwise all tokens are created for the root user, overwriting any previous tokens, which makes it impossible for more than one person to log in.

This bug has been around for a long time, but it wasn't really triggered until:

http://gitweb.samba.org/?p=samba.git;a=commit;h=d9f61dbdc91fae6560361f98d981b1f7bea80422

So the trigger has been in place since 3.2.4.  I opened it against 3.4.5, because that's the version I was trying to upgrade to from 3.0.33 (RHEL5).

I suggest adding set_re_uid() after setreuid().  We tried 2 fixes:  reverting the above by defining USE_SETRESUID in lib/util_sec.c, and set_re_uid().  Either works.  Since set_re_uid() doesn't work in the USE_SETEUID case, I suppose there should also be a guard against using that with WITH_FAKE_KASERVER.
Comment 1 Jeremy Allison 2010-02-19 19:21:58 UTC
Can you attach the patch you need please so I can understand exactly what changes you're proposing ?

Thanks,

Jeremy.
Comment 2 Thomas J. Moore 2010-02-21 11:04:43 UTC
Created attachment 5409 [details]
Make setreuid path set real id

I described the change rather than providing a patch because I do not know what the expected semantics are.  It's possible that this breaks things in subtle ways, but it works well enough for us.