Bug 11876 - EXTERNAL authentication fails on setuid applications
Summary: EXTERNAL authentication fails on setuid applications
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: Havoc Pennington
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-08-06 20:44 UTC by Andrea Luzzardi
Modified: 2008-01-14 12:11 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Testcase (659 bytes, text/plain)
2007-08-06 20:45 UTC, Andrea Luzzardi
Details
Patch against CVS HEAD (1.26 KB, patch)
2007-08-06 21:15 UTC, Andrea Luzzardi
Details | Splinter Review
Patch against CVS HEAD (fixed) (1.22 KB, patch)
2007-08-06 21:52 UTC, Andrea Luzzardi
Details | Splinter Review

Description Andrea Luzzardi 2007-08-06 20:44:27 UTC
The EXTERNAL authentication method doesn't work if the effective UID of the client is different from its real UID, which happens for instance on setuid applications.

That's because the client sends its real UID while the server checks the effective UID.

Here's a pseudo-callstack of the client:
1/ handle_client_response_mech()
2/ dbus_credentials_add_from_current_process()
3/ _dbus_credentials_add_unix_uid()
4/ _dbus_getuid()
5/ getuid()

While the server gets the effective uid in _dbus_read_credentials_socket() (through SO_PEERCRED and other methods).

I guess _dbus_credentials_add_unix_uid() should call _dbus_geteuid() (which doesn't exist yet), but i'm not sure about how much code depends on the current behaviour of that function.
Comment 1 Andrea Luzzardi 2007-08-06 20:45:59 UTC
Created attachment 11019 [details]
Testcase
Comment 2 Andrea Luzzardi 2007-08-06 21:15:02 UTC
Created attachment 11020 [details] [review]
Patch against CVS HEAD
Comment 3 Andrea Luzzardi 2007-08-06 21:52:18 UTC
Created attachment 11021 [details] [review]
Patch against CVS HEAD (fixed)

The previous patch replaced _dbus_getuid() by _dbus_geteuid() in the wrong place. My bad.
Comment 4 Havoc Pennington 2007-08-07 09:07:12 UTC
Thanks, when applying this it would be worth grepping for all other uses of _dbus_getuid() and see if they should be euid as well.

Comment 5 Andrea Luzzardi 2007-08-19 15:15:25 UTC
After a quick look, I'd say that every getuid should be changed to geteuid, except the one in dbus-userdb.c.

Also, my patch lacks of a _dbus_geteuid() in dbus-sysdeps-win.c (which should return DBUS_UID_UNSET).
Comment 6 John (J5) Palmieri 2007-10-03 14:43:09 UTC
Havoc, can I apply this and do the other _dbus_geteuid fixes?

Andrea, I don't totally understand your last comment about dbus-sysdeps-win.c.  Can you clarify? Is it an indepth fix or something easy?  
Comment 7 Havoc Pennington 2007-10-03 15:27:58 UTC
patch looks fine to me. John, the windows fix is to just cut-and-paste the dbus_geteuid() implementation into the windows file, but have it always return DBUS_UID_UNSET. You could do the windows fix or just leave it for the windows team.
Comment 8 Benjamin Close 2008-01-11 02:38:42 UTC
Bugzilla Upgrade Mass Bug Change

NEEDSINFO state was removed in Bugzilla 3.x, reopening any bugs previously listed as NEEDSINFO.

  - benjsc
    fd.o Wrangler
Comment 9 John (J5) Palmieri 2008-01-14 12:11:26 UTC
committed, and other files fixed.  Thanks


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.