[patch] Auth method for console users

[John (J5) Palmieri]

Third times a charm.

On Sun, 2004-07-25 at 00:45 -0400, Havoc Pennington wrote:
> Hi,
> 
> Comments -
> 
> - unsigned long gid_uid_or_atconsole; should be at_console 
>   with an underscore (yes I can nitpick with the best of them)

fixed

> - 
> 
> +               _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)
> 
fixed

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

Took the code out for this.  If it is ever needed we can always add it
back.

> 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 ;-)

Switching between GNOME and GNU coding styles can be a bitch
sometimes ;-)

> - _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. 

Axed.  The two lists is more intuitive.

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

Forgot to take out my debugging code.  It is now nicely wrapped up into
one return statement.

> - +  _dbus_string_free(&f); space before parens

Doh! Fixed.

> - +  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

Fixed.  Returns the error to the calling function for proper handling.

-- 
John (J5) Palmieri
Associate Software Engineer
Desktop Group
Red Hat, Inc.
Blog: http://martianrock.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbus.console-auth-3.patch
Type: text/x-patch
Size: 17828 bytes
Desc: not available
Url : http://freedesktop.org/pipermail/dbus/attachments/20040726/f5d647ab/dbus.console-auth-3-0001.bin