[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