VPN-6647: Ensure the app is started with a systemd application scope #10079
Conversation
|
I'm hoping someone else knows this area better than I do. Given the risks this could come with I'd removing myself from being a reviewer. If we need me to review it, I'm happy to - please let me know and I'll spend time coming up to speed on some of the details here. |
| @@ -108,3 +112,131 @@ QString XdgPortal::parentWindow() { | |||
| // Otherwise, we don't support this windowing system. | |||
| return QString(""); | |||
| } | |||
|
|
|||
| // Decode systemd escape characters in the unit names. | |||
| static QString decodeSystemdEscape(const QString& str) { | |||
| // And now for the gross part. StartTransientUnit() returns a systemd job that | ||
| // will create our scope for us, and move us into it. But we have to wait for | ||
| // that job to finish before we can proceed. However, we can't do this async | ||
| // because we don't have a QCoreApplication setup yet. The only way I can |
There was a problem hiding this comment.
I think this is is fine, we propbably could create the qApp as first thing but then again if we have this async task to move the scope. Settingsholder would need to wait for that ... we would need to touch a lot of places to work with a Future<Settingholder*> so i am fine with polling, unless you think that is too long :)
There was a problem hiding this comment.
I don't think it's worth the effort. The job seems to finish in tens of milliseconds and it's almost always done in the first iteration of the loop. We mostly just need it to ensure we're not in a some kind of race condition when SettingsHolder starts up.
Description
In PR #9922 we added support to migrate users off the
libsecretsAPI for storing the settings encryption key and onto the XDG Secrets portal. This brought with it a quirk that applications which are launched interactively (eg: on a command line) can wind up with a different encryption key than when launched via the start menu.This is due to the fact that the XDG portal looks up our application ID from the systemd scope, but launching the VPN from the command line results in a different scope. Different scope means a different encryption key and this causes the user to experience a loss of their settings.
To resolve this, we can invoke the systemd D-Bus method
StartTransientUnit()to move our process into a new scope and ensure that it has the correct application ID as far as the XDG portals are concerned.I may have gone a bit overboard on the refactoring while implementing this though. Here's why:
StartTransientUnit()method call, so they were added todbustypeslinux.hplatforms/linux/dbustypes.hLinuxDependenciestoLinuxUtilsmostly so that it's consistent with other platforms.LinuxUtilsisn't really a class, just a collection of static methods. So I converted it into a namespace instead of a class.Once that was all done, we add a new method:
XdgPortal::setupAppScope()which checks to see if the application is currently running in a systemd scope and invoking theStartTransientUnit()method over D-Bus to create a scope matching the Linux application ID oforg.mozilla.vpnif we are not currently in a systemd scope. If a valid application scope is found, we do nothing and assume that systemd knows what's best for how the client was launched. And then to ensure that we can correctly fetch the settings encryption keys, we must call this method before we create theSettingsManagerclass.My motivation for this work: It gets really annoying really fast when you have to log in again every time you manually launch the client via the command line.
Reference
Introduced by #9922
JIRA Issue: VPN-6647
Checklist