[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