user auth patches

[Havoc Pennington]

Hi,

Ralf Habacker wrote:
> DBusUserInfo and DBUSGroupInfo are used in several functions in 
> dbus-sysdeps-win.c and dbus-sysdeps-win-util.c.

Right, but you should be able to delete all the code that uses them once 
we sort out the below items.

> but there are still many functions unresolved for example
>    _DBUS_DEFINE_GLOBAL_LOCK(system_users)

This is simplest to just move to -sysdeps so it's present on both 
platforms, I think we did something similar before with another one of 
the locks.

> _dbus_homedir_from_username (username, &homedir)
> which are defined in dbus-userdb.c and referenced by dbus\dbus-keyring.c

Basically the detail that the keyring lives in a user homedir needs to 
be factored out of dbus-keyring.c, so we need a dbus-sysdeps.h thing 
like _dbus_get_keyring_location(DBusCredentials *desired_identity)

I will sort this out.

Alternatively you could implement these on Windows but would not need to 
mess with dbus_uid_t in order to do so (just use a windows sid for 
"username") - however I think it will be slightly cleaner to rearrange 
the code a bit more so I will do that.

> F:\daten\windbus\cvs-commit-3\dbus\dbus-auth-script.c(525):                
> if (!_dbus_username_from_current_process (&u) ||
>  F:\daten\windbus\cvs-commit-3\dbus\dbus-auth.c(788):  if 
> (!_dbus_username_from_current_process (&username))
>  F:\daten\windbus\cvs-commit-3\dbus\dbus-keyring.c(741):      if 
> (!_dbus_username_from_current_process (&username) ||
> 

The new _dbus_append_desired_identity() should replace these three I 
believe.

> dbus-transport-socket.c _dbus_read_credentials_socket" in Funktion 
> "_exchange_credentials".
> dbus-transport-socket.c _dbus_send_credentials_socket" in Funktion 
> "_exchange_credentials".

These will just need to be implemented on Windows; if you always want to 
use COOKIE_SHA1 on windows and not EXTERNAL, then you can make them 
no-ops and it will work fine.

>> Basically at this point there should be no abstraction of users or 
>> groups. The public API has a string "sid" for Windows users, and a uid 
>> for UNIX users, and the two are never treated interchangeably.
>>
> But if those function are limited to a specific platform, why should 
> they defined on a platform on where they would never be used.

The functions are not limited to a platform. Conceptually the API has 
the same specification on all platforms. That is, on either platform you 
can call dbus_connection_get_unix_user() or 
dbus_connection_get_windows_user(). It's even conceivable that by making 
the Windows port NIS-aware or Kerberos-enabled or something, it could 
authenticate a UNIX user logging on to the Windows bus.

The functions of course give different answers on different platforms. 
If the client did not auth as a unix user, as will be the norm on 
Windows, then get_unix_user() will return false. But it will also return 
false if the client is on UNIX but logged in anonymously, for example.

As I've pointed out a couple of times, adding a DBusUser object does not 
change anything. The DBusUser object will just have methods 
user.get_unix_uid() and user.get_windows_sid(). So you would do
connection.get_user().get_unix_uid(), while today you do
connection.get_unix_user(). It is exactly the same.

The only way to avoid this is to be a "framework" like Qt, where if you 
have a QUser or something, you can expect the *app* will only want the 
QUser, and won't want to convert to another representation. We do not 
have that expectation. Instead, we would expect apps to do something like:
  QUser user;
  if (connection.get_unix_user(&uid))
     user = new QUnixUser(uid);
  else if (connection.get_windows_user(&windows_sid))
     user = new QWindowsUser(windows_sid);

D-Bus bindings such as the Qt bindings are welcome to do this.

Writing the following is not in any way better:
  QUser user;
  if (connection.get_user().get_unix_user(&uid))
     user = new QUnixUser(uid);
  else if (connection.get_user().get_windows_user(&windows_sid))
     user = new QWindowsUser(windows_sid);

So that is why having DBusUser is pointless. Also, if we don't have 
DBusUser we get two advantages:
  - we can delete all the stuff in dbus-sysdeps-win.c that needlessly
    emulates UNIX, and move the abstractions up to a higher level.
  - we can be clear about which functionality is UNIX-specific,
    and avoid accidentally implementing things that don't make
    sense on Windows in the Windows port. UNIX emulation leads
    to code that builds but makes no sense to have.

Say we have had an API like:
   uid = current_process_uid()
   homedir = homedir_from_uid(uid)
   keyring_location = homedir + /.dbus-keyring

But the cross-platform API can just be:
   keyring_location = get_keyring_location()

And then you can choose a location that makes sense on Windows, without 
trying to emulate a bunch of stuff about uids and home directories.

Note the huge difference from something like Qt: we can just add a 
function like get_keyring_location(), we don't need a fully generic 
platform abstraction, just a purpose-built one.

> Would it not be lesser work and more readable to simply comment out the 
> related code on a not supported platform, which is done by many multiple 
> platform projects like Qt and KDE  also in the core code ?

I don't think it would be either easier or more readable.

>> The one case where we pass around "windows user OR unix user" is the 
>> DBusCredentials object, which explicitly can contain either.
> Which still requires a dbus_uid_t on windows. I will take a deeper look 
> into that stuff.

It requires a dbus_uid_t (which is just an integer), but it doesn't 
require Windows to have any way to create a dbus_uid_t (i.e. it will 
always be DBUS_UID_UNSET on Windows)

Havoc