Bug 99622 - On bus startup check given auth in config file against supported mechanisms.
Summary: On bus startup check given auth in config file against supported mechanisms.
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard: review+
Keywords: patch
Depends on:
Blocks: 99512
  Show dependency treegraph
 
Reported: 2017-01-31 21:26 UTC by Ralf Habacker
Modified: 2017-02-02 11:44 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
On bus startup check given auth in config file against supported mechanisms. (4.30 KB, patch)
2017-01-31 21:26 UTC, Ralf Habacker
Details | Splinter Review
On bus startup check given auth in config file against supported mechanisms (4.29 KB, patch)
2017-02-01 21:08 UTC, Ralf Habacker
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Habacker 2017-01-31 21:26:45 UTC
Moved from bug 96577 and fixes from partial review at https://bugs.freedesktop.org/show_bug.cgi?id=96577#c62 has been applied.
Comment 1 Ralf Habacker 2017-01-31 21:26:47 UTC
Created attachment 129263 [details] [review]
On bus startup check given auth in config file against supported mechanisms.

With recent code starting dbus-daemon with an unsupported auth mechanism
let dbus-daemon silently ignore this issue. Clients connecting to this
server fails to connect without any descriptive explanation of the
root cause, only the message 'Rejected client connection due to lack
of memory' error is reported in dbus-daemon verbose log, which is disabled
in production environments.

With this patch dbus-daemon checks the supported auth mechanisms on startup
and shuts down with a descriptive error message, which gives admins an
immediate feedback on service startup/restart.

Signed-off-by: Ralf Habacker <ralf.habacker@freenet.de>

Bug:
Comment 2 Philip Withnall 2017-01-31 23:41:31 UTC
Comment on attachment 129263 [details] [review]
On bus startup check given auth in config file against supported mechanisms.

Review of attachment 129263 [details] [review]:
-----------------------------------------------------------------

::: dbus/dbus-auth.c
@@ +2863,5 @@
> +  int i;
> +  _dbus_assert (buffer != NULL);
> +
> +  i = 0;
> +  while (all_mechanisms[i].mechanism != NULL)

You could turn this `while` loop into a `for` and compress the book-keeping code for `i` a bit.

::: dbus/dbus-auth.h
@@ +92,5 @@
>  
>  void          _dbus_auth_set_unix_fd_possible(DBusAuth               *auth, dbus_bool_t b);
>  dbus_bool_t   _dbus_auth_get_unix_fd_negotiated(DBusAuth             *auth);
> +DBUS_PRIVATE_EXPORT
> +dbus_bool_t   _dbus_auth_is_supported_mechanism(DBusString         *name);

Column alignment is a bit odd for the parameter here.
Comment 3 Simon McVittie 2017-02-01 19:03:11 UTC
Comment on attachment 129263 [details] [review]
On bus startup check given auth in config file against supported mechanisms.

Review of attachment 129263 [details] [review]:
-----------------------------------------------------------------

What Philip said.
Comment 4 Ralf Habacker 2017-02-01 21:08:36 UTC
Created attachment 129279 [details] [review]
On bus startup check given auth in config file against supported mechanisms

- use for loop
- indention fix

Signed-off-by: Ralf Habacker <ralf.habacker@freenet.de>
Comment 5 Philip Withnall 2017-02-01 23:01:43 UTC
Comment on attachment 129279 [details] [review]
On bus startup check given auth in config file against supported mechanisms

Review of attachment 129279 [details] [review]:
-----------------------------------------------------------------

LGTM with this nitpick.

::: dbus/dbus-auth.c
@@ +2859,5 @@
> + */
> +dbus_bool_t
> +_dbus_auth_dump_supported_mechanisms (DBusString *buffer)
> +{
> +  int i;

unsigned int.
Comment 6 Ralf Habacker 2017-02-02 10:32:50 UTC
commit pushed to git master
Comment 7 Ralf Habacker 2017-02-02 10:35:38 UTC
(In reply to Ralf Habacker from comment #6)
> commit pushed to git master
... with mentioned fix applied.
Comment 8 Simon McVittie 2017-02-02 11:44:20 UTC
(Will be in 1.11.10)


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.