Improve Secret Service interoperability

The current implementation of Secret Service keyring client assumes that
the last component of an item path is integer, which is not true for some
Secret Service server implementations (e.g. KeePassXC). Besides,
the Secret Service API documents advises against recording object path
(not to mentioning parsing it in any way), recommending using lookup attributes
instead[1].

This commit fixes the code to behave in more interoperable way.

- The item path (called "keyid" in code) is no longer parsed and stored anywhere.
- The secret item is looked up in the Secret Service using hvuri and machine
  uuid attributes.
- /console-password with (username, keyid) is removed from GSettings
  storage. Instead, only username is stored in /console-username.

[1] https://specifications.freedesktop.org/secret-service/latest/ch03.html

Resolves: #237
This commit is contained in:
WGH 2021-03-21 20:44:04 +03:00 committed by Cole Robinson
parent d9b5090e06
commit fae3fecc1e
3 changed files with 45 additions and 46 deletions

View File

@ -14,10 +14,10 @@
<description>When to scale the VM graphical console. -1 = global default, 0 = never, 1 = only when in full screen mode, 2 = Always</description>
</key>
<key name="console-password" type="(si)">
<default>("", -1)</default>
<summary>Username and secrets ID for graphical password</summary>
<description>Username and secrets ID for graphical password</description>
<key name="console-username" type="s">
<default>""</default>
<summary>Username for graphical password</summary>
<description>Username for graphical password</description>
</key>
<key name="resize-guest" type="i">

View File

@ -61,8 +61,19 @@ class vmmKeyring(vmmGObject):
def _cleanup(self):
pass # pragma: no cover
def _find_secret_item_path(self, uuid, hvuri):
attributes = {
"uuid": uuid,
"hvuri": hvuri,
}
unlocked, locked = self._service.SearchItems("(a{ss})", attributes)
if not unlocked:
if locked:
log.warning("Item found, but it's locked")
return None
return unlocked[0]
def _add_secret(self, secret):
ret = None
try:
props = {
"org.freedesktop.Secret.Item.Label": GLib.Variant("s", secret.get_name()),
@ -73,17 +84,17 @@ class vmmKeyring(vmmGObject):
"text/plain; charset=utf8")
replace = True
_id = self._collection.CreateItem("(a{sv}(oayays)b)",
props, params, replace)[0]
ret = int(_id.rsplit("/")[-1])
self._collection.CreateItem("(a{sv}(oayays)b)",
props, params, replace)
except Exception: # pragma: no cover
log.exception("Failed to add keyring secret")
return ret
def _del_secret(self, _id):
def _del_secret(self, uuid, hvuri):
try:
path = self._collection.get_object_path() + "/" + str(_id)
path = self._find_secret_item_path(uuid, hvuri)
if path is None:
return None
iface = Gio.DBusProxy.new_sync(self._dbus, 0, None,
"org.freedesktop.secrets", path,
"org.freedesktop.Secret.Item", None)
@ -96,10 +107,13 @@ class vmmKeyring(vmmGObject):
except Exception:
log.exception("Failed to delete keyring secret")
def _get_secret(self, _id):
def _get_secret(self, uuid, hvuri):
ret = None
try:
path = self._collection.get_object_path() + "/" + str(_id)
path = self._find_secret_item_path(uuid, hvuri)
if path is None:
return None
iface = Gio.DBusProxy.new_sync(self._dbus, 0, None,
"org.freedesktop.secrets", path,
"org.freedesktop.Secret.Item", None)
@ -118,7 +132,7 @@ class vmmKeyring(vmmGObject):
ret = _vmmSecret(label, secret, attrs)
except Exception: # pragma: no cover
log.exception("Failed to get keyring secret id=%s", _id)
log.exception("Failed to get keyring secret uuid=%r hvuri=%r", uuid, hvuri)
return ret
@ -137,41 +151,26 @@ class vmmKeyring(vmmGObject):
if not self.is_available():
return ("", "") # pragma: no cover
username, keyid = vm.get_console_password()
if keyid == -1:
return ("", "")
secret = self._get_secret(keyid)
if secret is None or secret.get_name() != self._get_secret_name(vm):
secret = self._get_secret(vm.get_uuid(), vm.conn.get_uri())
if secret is None:
return ("", "") # pragma: no cover
if (secret.attributes.get("hvuri", None) != vm.conn.get_uri() or
secret.attributes.get("uuid", None) != vm.get_uuid()):
return ("", "") # pragma: no cover
return (secret.get_secret(), username or "")
return (secret.get_secret(), vm.get_console_username() or "")
def set_console_password(self, vm, password, username=""):
if not self.is_available():
return # pragma: no cover
secret = _vmmSecret(self._get_secret_name(vm), password,
{"uuid": vm.get_uuid(),
"hvuri": vm.conn.get_uri()})
keyid = self._add_secret(secret)
if keyid is None:
return # pragma: no cover
vm.set_console_password(username, keyid)
vm.set_console_username(username)
self._add_secret(secret)
def del_console_password(self, vm):
if not self.is_available():
return # pragma: no cover
ignore, keyid = vm.get_console_password()
if keyid == -1:
return
self._del_secret(keyid)
vm.del_console_password()
self._del_secret(vm.get_uuid(), vm.conn.get_uri())
vm.del_console_username()

View File

@ -1597,14 +1597,14 @@ class vmmDomain(vmmLibvirtObject):
ret = self.config.get_pervm(self.get_uuid(), "/vm-window-size")
return ret
def get_console_password(self):
return self.config.get_pervm(self.get_uuid(), "/console-password")
def set_console_password(self, username, keyid):
return self.config.set_pervm(self.get_uuid(), "/console-password",
(username, keyid))
def del_console_password(self):
return self.config.set_pervm(self.get_uuid(), "/console-password",
("", -1))
def get_console_username(self):
return self.config.get_pervm(self.get_uuid(), "/console-username")
def set_console_username(self, username):
return self.config.set_pervm(self.get_uuid(), "/console-username",
username)
def del_console_username(self):
return self.config.set_pervm(self.get_uuid(), "/console-username",
"")
def get_cache_dir(self):
ret = os.path.join(self.conn.get_cache_dir(), self.get_uuid())