[PATCH] Make safe_dbus module's functions less 'safe'
Martin Kolman
mkolman at redhat.com
Thu Mar 27 13:12:35 UTC 2014
On Thu, 2014-03-27 at 11:14 +0100, Vratislav Podzimek wrote:
> The module is named safe_dbus, there is no need to put words 'dbus' and 'safe'
> in all functions' names.
>
> Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> ---
> anaconda | 6 +--
> pyanaconda/keyboard.py | 25 ++++++------
> pyanaconda/safe_dbus.py | 98 +++++++++++++++++++++++------------------------
> pyanaconda/screensaver.py | 10 ++---
> 4 files changed, 69 insertions(+), 70 deletions(-)
>
> diff --git a/anaconda b/anaconda
> index 492ebef..88a3690 100755
> --- a/anaconda
> +++ b/anaconda
> @@ -894,14 +894,14 @@ if __name__ == "__main__":
> flags.noverifyssl = True
>
> if opts.liveinst:
> - from pyanaconda.safe_dbus import dbus_get_session_connection, DBusCallError
> from pyanaconda.screensaver import inhibit_screensaver
> + from pyanaconda import safe_dbus
>
> flags.livecdInstall = True
>
> try:
> - anaconda.dbus_session_connection = dbus_get_session_connection()
> - except DBusCallError as e:
> + anaconda.dbus_session_connection = safe_dbus.get_new_session_connection()
> + except safe_dbus.DBusCallError as e:
> log.info("Unable to connect to DBus session bus: %s", e)
> else:
> anaconda.dbus_inhibit_id = inhibit_screensaver(anaconda.dbus_session_connection)
> diff --git a/pyanaconda/keyboard.py b/pyanaconda/keyboard.py
> index 390f4bc..abaf3ec 100644
> --- a/pyanaconda/keyboard.py
> +++ b/pyanaconda/keyboard.py
> @@ -42,8 +42,7 @@ from collections import namedtuple
>
> from pyanaconda import iutil
> from pyanaconda import flags
> -from pyanaconda.safe_dbus import dbus_call_safe_sync, dbus_get_property_safe_sync, dbus_get_new_system_bus_connection_sync
> -from pyanaconda.safe_dbus import DBusPropertyError
> +from pyanaconda import safe_dbus
> from pyanaconda.constants import DEFAULT_VC_FONT, DEFAULT_KEYBOARD, THREAD_XKL_WRAPPER_INIT
> from pyanaconda.threads import threadMgr, AnacondaThread
>
> @@ -683,17 +682,17 @@ class LocaledWrapper(object):
> """
>
> def __init__(self):
> - self._connection = dbus_get_new_system_bus_connection_sync()
> + self._connection = safe_dbus.get_new_system_connection()
>
> @property
> def keymap(self):
> try:
> - keymap = dbus_get_property_safe_sync(LOCALED_SERVICE,
> + keymap = safe_dbus.get_property_sync(LOCALED_SERVICE,
> LOCALED_OBJECT_PATH,
> LOCALED_IFACE,
> "VConsoleKeymap",
> self._connection)
> - except DBusPropertyError:
> + except safe_dbus.DBusPropertyError:
> # no value for the property
> log.error("Failed to get the value for the systemd-localed's "
> "VConsoleKeymap property")
> @@ -705,24 +704,24 @@ class LocaledWrapper(object):
> @property
> def layouts_variants(self):
> try:
> - layouts = dbus_get_property_safe_sync(LOCALED_SERVICE,
> + layouts = safe_dbus.get_property_sync(LOCALED_SERVICE,
> LOCALED_OBJECT_PATH,
> LOCALED_IFACE,
> "X11Layout",
> self._connection)
> - except DBusPropertyError:
> + except safe_dbus.DBusPropertyError:
> # no value for the property
> log.error("Failed to get the value for the systemd-localed's "
> "X11Layout property")
> return [""]
>
> try:
> - variants = dbus_get_property_safe_sync(LOCALED_SERVICE,
> + variants = safe_dbus.get_property_sync(LOCALED_SERVICE,
> LOCALED_OBJECT_PATH,
> LOCALED_IFACE,
> "X11Variant",
> self._connection)
> - except DBusPropertyError:
> + except safe_dbus.DBusPropertyError:
> # no value for the property
> log.error("Failed to get the value for the systemd-localed's "
> "X11Variant property")
> @@ -748,12 +747,12 @@ class LocaledWrapper(object):
> @property
> def options(self):
> try:
> - options = dbus_get_property_safe_sync(LOCALED_SERVICE,
> + options = safe_dbus.get_property_sync(LOCALED_SERVICE,
> LOCALED_OBJECT_PATH,
> LOCALED_IFACE,
> "X11Options",
> self._connection)
> - except DBusPropertyError:
> + except safe_dbus.DBusPropertyError:
> # no value for the property
> log.error("Failed to get the value for the systemd-localed's "
> "X11Options property")
> @@ -780,7 +779,7 @@ class LocaledWrapper(object):
> # should ask for credentials or not
> args = GLib.Variant('(ssbb)', (keymap, "", convert, False))
>
> - dbus_call_safe_sync(LOCALED_SERVICE, LOCALED_OBJECT_PATH, LOCALED_IFACE,
> + safe_dbus.call_sync(LOCALED_SERVICE, LOCALED_OBJECT_PATH, LOCALED_IFACE,
> "SetVConsoleKeyboard", args, self._connection)
>
> def convert_keymap(self, keymap):
> @@ -857,7 +856,7 @@ class LocaledWrapper(object):
> # should ask for credentials or not
> args = GLib.Variant("(ssssbb)", (layouts_str, "", variants_str, opts_str,
> convert, False))
> - dbus_call_safe_sync(LOCALED_SERVICE, LOCALED_OBJECT_PATH, LOCALED_IFACE,
> + safe_dbus.call_sync(LOCALED_SERVICE, LOCALED_OBJECT_PATH, LOCALED_IFACE,
> "SetX11Keyboard", args, self._connection)
>
> def set_and_convert_layout(self, layout_variant):
> diff --git a/pyanaconda/safe_dbus.py b/pyanaconda/safe_dbus.py
> index 58dd9c6..10c0866 100644
> --- a/pyanaconda/safe_dbus.py
> +++ b/pyanaconda/safe_dbus.py
> @@ -41,7 +41,7 @@ class DBusPropertyError(DBusCallError):
>
> pass
>
> -def dbus_get_new_system_bus_connection_sync():
> +def get_new_system_connection():
> """Return a new connection to the system bus."""
>
> return Gio.DBusConnection.new_for_address_sync(
> @@ -50,8 +50,49 @@ def dbus_get_new_system_bus_connection_sync():
> Gio.DBusConnectionFlags.MESSAGE_BUS_CONNECTION,
> None, None)
>
> -def dbus_call_safe_sync(service, obj_path, iface, method, args,
> - connection=None):
> +def get_new_session_connection():
> + """
> + Get a connection handle for the per-user-login-session message bus.
> +
> + !!! RUN THIS EARLY !!! like, before any other threads start. Connections to
> + the session bus must be made with the effective UID of the login user,
> + which in live installs is not the UID of anaconda. This means we need to
> + call seteuid in this method, and doing that after threads have started will
> + probably do something weird.
> +
> + Live installs use consolehelper to run as root, which sets the original
> + UID in $USERHELPER_UID.
> +
> + :return: the session connection handle
> + :rtype: Gio.DBusConnection
> + :raise DBusCallError: if some DBus related error appears
> + :raise OSError: if unable to set the effective UID
> + """
> +
> + old_euid = None
> + if "USERHELPER_UID" in os.environ:
> + old_euid = os.geteuid()
> + os.seteuid(int(os.environ["USERHELPER_UID"]))
> +
> + try:
> + connection = Gio.DBusConnection.new_for_address_sync(
> + Gio.dbus_address_get_for_bus_sync(Gio.BusType.SESSION, None),
> + Gio.DBusConnectionFlags.AUTHENTICATION_CLIENT|
> + Gio.DBusConnectionFlags.MESSAGE_BUS_CONNECTION,
> + None, None)
> + except GLib.GError as gerr:
> + raise DBusCallError("Unable to connect to session bus: %s", gerr)
> + finally:
> + if old_euid is not None:
> + os.seteuid(old_euid)
> +
> + if connection.is_closed():
> + raise DBusCallError("Connection is closed")
> +
> + return connection
> +
> +def call_sync(service, obj_path, iface, method, args,
> + connection=None):
> """
> Safely call a given method on a given object of a given service over DBus
> passing given arguments. If a connection is given, it is used, otherwise a
> @@ -79,7 +120,7 @@ def dbus_call_safe_sync(service, obj_path, iface, method, args,
>
> if not connection:
> try:
> - connection = dbus_get_new_system_bus_connection_sync()
> + connection = get_new_system_connection()
> except GLib.GError as gerr:
> raise DBusCallError("Unable to connect to system bus: %s", gerr)
>
> @@ -102,8 +143,8 @@ def dbus_call_safe_sync(service, obj_path, iface, method, args,
>
> return ret.unpack()
>
> -def dbus_get_property_safe_sync(service, obj_path, iface, prop_name,
> - connection=None):
> +def get_property_sync(service, obj_path, iface, prop_name,
> + connection=None):
> """
> Get value of a given property of a given object provided by a given service.
>
> @@ -128,51 +169,10 @@ def dbus_get_property_safe_sync(service, obj_path, iface, prop_name,
> """
>
> args = GLib.Variant('(ss)', (iface, prop_name))
> - ret = dbus_call_safe_sync(service, obj_path, DBUS_PROPS_IFACE, "Get", args,
> - connection)
> + ret = call_sync(service, obj_path, DBUS_PROPS_IFACE, "Get", args,
> + connection)
> if ret is None:
> msg = "No value for the %s object's property %s" % (obj_path, prop_name)
> raise DBusPropertyError(msg)
>
> return ret
> -
> -def dbus_get_session_connection():
> - """
> - Get a connection handle for the per-user-login-session message bus.
> -
> - !!! RUN THIS EARLY !!! like, before any other threads start. Connections to
> - the session bus must be made with the effective UID of the login user,
> - which in live installs is not the UID of anaconda. This means we need to
> - call seteuid in this method, and doing that after threads have started will
> - probably do something weird.
> -
> - Live installs use consolehelper to run as root, which sets the original
> - UID in $USERHELPER_UID.
> -
> - :return: the session connection handle
> - :rtype: Gio.DBusConnection
> - :raise DBusCallError: if some DBus related error appears
> - :raise OSError: if unable to set the effective UID
> - """
> -
> - old_euid = None
> - if "USERHELPER_UID" in os.environ:
> - old_euid = os.geteuid()
> - os.seteuid(int(os.environ["USERHELPER_UID"]))
> -
> - try:
> - connection = Gio.DBusConnection.new_for_address_sync(
> - Gio.dbus_address_get_for_bus_sync(Gio.BusType.SESSION, None),
> - Gio.DBusConnectionFlags.AUTHENTICATION_CLIENT|
> - Gio.DBusConnectionFlags.MESSAGE_BUS_CONNECTION,
> - None, None)
> - except GLib.GError as gerr:
> - raise DBusCallError("Unable to connect to session bus: %s", gerr)
> - finally:
> - if old_euid is not None:
> - os.seteuid(old_euid)
> -
> - if connection.is_closed():
> - raise DBusCallError("Connection is closed")
> -
> - return connection
> diff --git a/pyanaconda/screensaver.py b/pyanaconda/screensaver.py
> index ce62e5a..f95a98f 100644
> --- a/pyanaconda/screensaver.py
> +++ b/pyanaconda/screensaver.py
> @@ -19,7 +19,7 @@
> # Author(s): David Shea <dshea at redhat.com>
> #
>
> -from pyanaconda.safe_dbus import dbus_call_safe_sync, DBusCallError
> +from pyanaconda import safe_dbus
> from gi.repository import GLib
>
> import logging
> @@ -46,12 +46,12 @@ def inhibit_screensaver(connection):
> """
>
> try:
> - inhibit_id = dbus_call_safe_sync(SCREENSAVER_SERVICE, SCREENSAVER_PATH, SCREENSAVER_IFACE,
> + inhibit_id = safe_dbus.call_sync(SCREENSAVER_SERVICE, SCREENSAVER_PATH, SCREENSAVER_IFACE,
> SCREENSAVER_INHIBIT_METHOD, GLib.Variant('(ss)',
> (SCREENSAVER_APPLICATION, SCREENSAVER_REASON)),
> connection)
> return inhibit_id[0]
> - except DBusCallError as e:
> + except safe_dbus.DBusCallError as e:
> log.info("Unable to inhibit the screensaver: %s", e)
>
> return None
> @@ -67,7 +67,7 @@ def uninhibit_screensaver(connection, inhibit_id):
> """
>
> try:
> - dbus_call_safe_sync(SCREENSAVER_SERVICE, SCREENSAVER_PATH, SCREENSAVER_IFACE,
> + safe_dbus.call_sync(SCREENSAVER_SERVICE, SCREENSAVER_PATH, SCREENSAVER_IFACE,
> SCREENSAVER_UNINHIBIT_METHOD, GLib.Variant('(u)', (inhibit_id,)), connection)
> - except DBusCallError as e:
> + except safe_dbus.DBusCallError as e:
> log.info("Unable to uninhibit the screensaver: %s", e)
Looks good & I don't feel any less safe, ACK! :)
More information about the anaconda-patches
mailing list