Add secret access, as the safeStorage api is now used - #530
Conversation
|
🚧 Test build enqueued. |
|
🚧 Started test build. |
|
❌ Test build failed. |
|
🚧 Test build enqueued. |
|
🚧 Started test build. |
|
✅ Test build succeeded. To test this build, install it from the testing repository: |
|
Manually tested this change and it seems to work fine here :) |
|
This shouldn't be needed, electron should use the portal (it even does use the portal, but it seems either electron or the secrets portal falls short on the encryption API). Therefore, for now, I would avoid adding this API and instead let people click through the warning dialogue. It's not amazing, but it also doesn't change the status quo. Adding the bypass for the secrets API, can confuse access token storage once more, and I would like to avoid that. References: |
|
Test build working for me (Debian + xfce + gnome-keyring). |
The current popup makes it sound, like NOT adding the permission confuses token storage once more. Is it misleading?
|
|
It should've been using libsecret with The secrets portal is quite literally broken for chromium and electron applications. Now migrating back and forth to fix this will obviously corrupt everyone's secrets storage. |
|
Does not work for me in a clean install. kwallet6, doesn't help granting org.kde.kwallet6 either Edit: granting it, still same issue |
Unfortunately, the pop-up is full of technical jargon and the button says "use no encryption", in an app for encrypted communication. I just stopped using the desktop app instead of clicking the button and assumed it would be fixed in the next update. I would hope that many non-technical users do the same without access to clear information on this problem. |
|
🚧 Test build enqueued. |
|
🚧 Started test build. |
|
✅ Test build succeeded. To test this build, install it from the testing repository: |
|
So with these changes this happens on start, which I guess is expected and not preventable.
|
c38997b to
a883e09
Compare
|
🚧 Test build enqueued. |
|
🚧 Started test build. |
|
✅ Test build succeeded. To test this build, install it from the testing repository: |
|
The really annoying thing here is we have to choose between: Short term gain: No popup and we get encrypted messages, but we might break sessions and we have to keep the permissions and update the permissions to read secrets from unsandboxed session store, that might include all your stored website passwords 👍🏻 Long term gain: We wait for Electron and xdg-secret-portal to get along again and work as expected, including support for safeStorage. Sandbox stays intact, no direct access for Electron to your keyring, but we have an ugly popup. 🤔 ⏳ Mid term gain: We add the permissions, we get encrypted messages, but as soon as Electron and portal get along again, we remove the permissions. We just break everyone's sessions, drop the encryption secrets and make everyone re-log. That will certainly not cause any data loss, because everyone uses key backups, right? 😡 💥 Right now, I'm still pushing for the long term gains, because I believe that keeping the sandbox intact, will benefit us in the long term more than avoiding the popup. But feel free to share your thoughts, especially because I see more people in the issues starting to add the permissions manually instead of clicking through the popup. |
|
@SISheogorath Can you give an example of what "clicking through the popup" means for us who didn't do it, since you mention it in many places? Maybe in the issue #528? I'm reluctant to clicking a button "Use no encryption" and I'm also hesitant to install a patch that forces me to reauthenticate, to revalidate and to reestablish encryption. A clean migration path would be preferable. On Flathub this repository is marked as officially provided by New Vector Inc. Eventually they might want to chime in here? |
That's a wrong statement. Flathub does not claim that anywhere. |
|
The thing that got me is that, prior to the problematic update, Electron wasn't using the keyring either. There is something in Electron that requests the secret from the Desktop portal, but it doesn't seem to be using it. I can change or outright delete the secret and Element still starts up fine. But because it does request the Secret, it is looked up (by the desktop portal, not the application) in the keyring, which requests an unlock, etc, which lead me (and maybe others?) to assume that Element was using the system keyring (at least indirectly). This also means that the session reset on "Use no encryption" is entirely unnecessary, it wasn't "using encryption" to begin with, so there should be the migration path of "doing nothing"... @SISheogorath: I'd argue for the long term gain, because allowing Element access to the whole keyring weakens security much more than it strenghtens it, for most threat models: With all current desktops, the keyring only provides "encryption at rest", and no "encryption at screen lock" or anything similar. Linux users wishing for encryption at rest will mostly already use some form of disk encryption which provides this. Not allowing keyring access (which has already been the status quo) is a practical weakness only in the case of not using any kind of disk or filesystem encryption which includes $HOME already; and it is only vulnerable to physical attacks. |
|
I did notice that clicking 'Use no encryption' had apparent no effect too. |
| - /share/man | ||
| - /man | ||
| modules: | ||
| - name: libsecret |
There was a problem hiding this comment.
I would suggest adding this as a submodule https://github.com/flathub/shared-modules?tab=readme-ov-file
Edit: Not sure if it is the right thing to do overall, but if needed, then adding it as a submodule is probably better.
There was a problem hiding this comment.
Shouldn't be an option, as it's using different options, which would invalidate our use case.
|
Any news on this? Iiuc the long-term approach is to wait for upstream release that fixes this? Is there an upstream tracker for it? |
@LecrisUT upstream issue is [Bug]: Safestorage detection fails on linux outside of predefined desktop environments #39789 |
|
How is this an upstream issue from 2023 when Element worked perfectly fine no less than 6 months ago. Then the developers decided to change things and it broke the app. |
|
xdg-run/keyring can probably be removed right why is that even needed, I know no other flatpaks that do this |
The sandbox is already not intact; As far as I can tell also, (maybe this is only in KDE) the secrets portal isn't even that much better in its implementation, it doesn't seem to provide any protection against unsandboxed apps, as long as one has access to the keyring, they can decrypt the Element database. Ideally I feel like when the secrets portal is available it would migrate the secrets, not just always depend on it. |
| # Required for access to the safeStorage https://www.electronjs.org/de/docs/latest/api/safe-storage | ||
| - --talk-name=org.freedesktop.secrets |
There was a problem hiding this comment.
| # Required for access to the safeStorage https://www.electronjs.org/de/docs/latest/api/safe-storage | |
| - --talk-name=org.freedesktop.secrets | |
| # Required for access to the safeStorage https://www.electronjs.org/de/docs/latest/api/safe-storage | |
| - --talk-name=org.freedesktop.secrets | |
| - --talk-name=org.kde.kwalletd6 | |
| - --talk-name=org.kde.kwalletd5 |
|
2026 and I searched for and found this still open? |
electron/electron#49054 was just merged so maybe the building blocks are now there? |
|
Doesn't seem to have reached a release |
|
Short-term progress on this issue is unlikely since the maintainer of this Flathub package is stepping down and looking for a replacement. More at #568. |
17 participants
Closes #528