[patch] Auth method for console users

Havoc Pennington hp at redhat.com
Sat Jul 24 21:45:08 PDT 2004


Hi,

Comments -

- unsigned long gid_uid_or_atconsole; should be at_console 
  with an underscore (yes I can nitpick with the best of them)

- 

+               _dbus_warn ("Unknown value \"%s\" for at_console in
message bus configuration file\n",
+                        at_console);

This should be a fatal error setting DBusError rather than dbus_warn,
it's not the same as the dbus_warn earlier in the function (those are
just warnings since it may be OK for the user/group to be absent)


- does it make sense to have a BusPolicyRule for at_console?

Note that there are two different things in the config file:
<policy user="foo"> and <allow user="foo">
<policy> means BusPolicy and <allow>/<deny> means BusPolicyRule

<policy user="foo"> means a policy that applies to user foo and 
<allow user="foo"> means that user foo is allowed to connect to the
message bus.

So <policy at_console="true"> means a policy that applies only if you're
at the console, and <allow at_console="true"> means you can 
only connect if you're at the console.

I don't think "only connect if at the console" makes any sense for
either the session or system bus. So we can maybe drop the whole
BusPolicyRule/BUS_POLICY_RULE_CONSOLE part of the patch.

Apologies for not noticing this before, hacking on the selinux patch
forced me to clarify in my mind.

- if we didn't drop BUS_POLICY_RULE_CONSOLE, it needs to be 
  considered in the BUS_POLICY_RULE_IS_PER_CLIENT macro

- 
+      if (!add_list_to_client (&list, client)) {
+	_dbus_list_clear (&list);
+        goto nomem;
+      }

wonky braces and indentation ;-)

- _dbus_console_policy_lookup_uid_match() is a confusing name, 
  but I think it does the wrong thing anyway; what you want is 
  in BusPolicy you have two new lists of BusPolicyRule, one 
  comes from all allow/deny inside <policy at_console=true> and one 
  from all rules inside <policy at_console=false>. 
  In create_client_policy, you want to copy one of those lists 
  but not the other to the client policy. 

- in _dbus_file_exists() the "exists" temp variable is 
  sorta pointless.

- +  _dbus_string_free(&f); space before parens

- +  DBusError *console_error;
...
  +  uid_at_console = _dbus_is_console_user (uid, console_error);
  should be:

  DBusError console_error;
  _dbus_is_console_user (uid, &console_error);

- when you call _dbus_is_console_user(), you have to handle the 
  OOM error differently from a "no such user" or other type of 
  error. Basically, the OOM error is recoverable

Havoc





More information about the dbus mailing list