Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --talk-name=org.freedesktop.secrets so it can talk to local secre… #729

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

Justinzobel
Copy link
Member

…ts storage providers

@flathubbot
Copy link
Contributor

Started test build 149340

@flathubbot
Copy link
Contributor

Build 149340 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/132432/org.signal.Signal.flatpakref

@cyrneko
Copy link

cyrneko commented Sep 25, 2024

Not sure why this wasn't done earlier. Anyways I'm 90% sure it should be org.freedesktop.Secrets (capitalised) and not org.freedesktop.secrets, at least my testing leads me to believe that.

@Justinzobel
Copy link
Member Author

Not sure why this wasn't done earlier. Anyways I'm 90% sure it should be org.freedesktop.Secrets (capitalised) and not org.freedesktop.secrets, at least my testing leads me to believe that.

I don't think it matters. I copied this from another Flatpak that uses freedesktop secrets already.

@cyrneko
Copy link

cyrneko commented Sep 25, 2024

Not sure why this wasn't done earlier. Anyways I'm 90% sure it should be org.freedesktop.Secrets (capitalised) and not org.freedesktop.secrets, at least my testing leads me to believe that.

I don't think it matters. I copied this from another Flatpak that uses freedesktop secrets already.

On my Fedora Kinoite system it didn't access the keyring until I manually set it to uppercase 🤔

@Justinzobel
Copy link
Member Author

Not sure why this wasn't done earlier. Anyways I'm 90% sure it should be org.freedesktop.Secrets (capitalised) and not org.freedesktop.secrets, at least my testing leads me to believe that.

I don't think it matters. I copied this from another Flatpak that uses freedesktop secrets already.

On my Fedora Kinoite system it didn't access the keyring until I manually set it to uppercase 🤔

Interesting, thanks. I've updated it now.

@Justinzobel
Copy link
Member Author

bot, build

@flathubbot
Copy link
Contributor

Queued test build for org.signal.Signal.

@flathubbot
Copy link
Contributor

Started test build 149362

@flathubbot
Copy link
Contributor

Started test build 149363

@flathubbot
Copy link
Contributor

Build 149362 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/132454/org.signal.Signal.flatpakref

@flathubbot
Copy link
Contributor

Build 149363 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/132455/org.signal.Signal.flatpakref

@barthalion barthalion merged commit f88079a into flathub:master Sep 25, 2024
1 check passed
@Justinzobel Justinzobel deleted the freedesktop.secrets branch September 25, 2024 06:49
@barthalion
Copy link
Member

Fairly sure it should be lowercase… Please test whether data is correctly encrypted once it gets published.

@Justinzobel
Copy link
Member Author

Fairly sure it should be lowercase… Please test whether data is correctly encrypted once it gets published.

+1 point for pre-build linting. Confirm finishing arguments are valid and appropriately cased.

@barthalion
Copy link
Member

Yeah, gonna add it to the linter once you or @cyrneko confirm.

@Altonss
Copy link

Altonss commented Sep 25, 2024

Just out of curiosity, why does Signal need to talk to org.freedesktop.Secrets, or what does it need to store there?

@pm4rcin
Copy link
Contributor

pm4rcin commented Sep 25, 2024

Just out of curiosity, why does Signal need to talk to org.freedesktop.Secrets, or what does it need to store there?

Because otherwise the key is stored in plaintext. Recently there was a drama and they have fixed the hole that allowed to steal your desktop session with malicious program without you ever noticing.

@pm4rcin
Copy link
Contributor

pm4rcin commented Sep 25, 2024

@barthalion as for the capitalization it's the lowercase. An example app that requests access to keyring is Element.
PS. If anyone uses keepassxc as secret storage provider like me please confirm if it works with Signal for you as I have no luck with that.

@Altonss
Copy link

Altonss commented Sep 25, 2024

Just out of curiosity, why does Signal need to talk to org.freedesktop.Secrets, or what does it need to store there?

Because otherwise the key is stored in plaintext. Recently there was a drama and they have fixed the hole that allowed to steal your desktop session with malicious program without you ever noticing.

Thanks for the info, I found an official comment about that here signalapp/Signal-Desktop#6944 (comment) :)

@Justinzobel
Copy link
Member Author

Yeah, gonna add it to the linter once you or @cyrneko confirm.

Oh I don't use Signal, just came here to fix this as I saw some whining on the Fediverse and figured it should be an easy fix.

@Altonss
Copy link

Altonss commented Sep 25, 2024

I still get getSQLKey: using legacy key when running latest version, on Fedora Workstation (40).

@Justinzobel
Copy link
Member Author

I still get getSQLKey: using legacy key when running latest version, on Fedora Workstation (40).

If you remove the flatpak data first and setup again fresh does it still show the message? It may have chosen the key storage location based on the first time you ran the flatpak.

@Altonss
Copy link

Altonss commented Sep 25, 2024

If you remove the flatpak data first and setup again fresh does it still show the message?

Does this mean also deleting all user data? 🤔 Ideally this should be migrated automatically for existing installations...

@Justinzobel
Copy link
Member Author

It would involve deleting all user data. However, you can (and should) backup the data first just in case you need to restore.

As for automatic migration, I'm not sure how that would work. It would have to be something within Signal.

@pm4rcin
Copy link
Contributor

pm4rcin commented Sep 25, 2024

You can try testing in a VM by adding it as a new device from the phone app.

@Altonss
Copy link

Altonss commented Sep 25, 2024

As for automatic migration, I'm not sure how that would work. It would have to be something within Signal.

AFAIU migration from legacy to safeStorage has been implemented, so it should migrate automatically :)

Quote from signalapp/Signal-Desktop#6849 (comment):

In addition to migrating to encrypted/keystore-backed local database encryption keys on supported platforms, our implementation also includes some additional troubleshooting steps and a temporary fallback option that will allow users to recover their message database using their legacy database encryption key if something goes wrong. This should help minimize data loss if any edge cases or other keystore-related bugs are discovered during the migration process and production rollout. The temporary fallback and legacy key will both be removed after everything has been tested and deployed on a wide variety of devices across various operating systems and OS versions.

See relevant implementation here https://github.com/signalapp/Signal-Desktop/blob/96a2d05906351b2c7caf6ee3d5d3292404606919/app/main.ts#L1641 and https://github.com/signalapp/Signal-Desktop/blob/96a2d05906351b2c7caf6ee3d5d3292404606919/app/main.ts#L1697

@Altonss
Copy link

Altonss commented Sep 25, 2024

I still get getSQLKey: using legacy key when running latest version, on Fedora Workstation (40).

Just a note of what I tried so far, and didn't work, so be carefull!

  • changed from org.freedesktop.Secrets to org.freedesktop.secrets
  • Launched Signal, this worked and showed it is now using encrypted storage
  • Closed Signal
  • Somehow seahorse wouldn't show me the secrets anymore, had to restart gnome-keyring with gnome-keyring-daemon -r to fix this seahorse issue (and so Signal could also access the key again)
  • Now Signal installation is broken even though it can access key (I see getSQLKey: decrypting key during start-up), with the same error message as signal-desktop won't start after upgrade to 7.24.X (and sudo user pw change) signalapp/Signal-Desktop#7018 !

@bbhtt
Copy link
Contributor

bbhtt commented Sep 25, 2024

This is in lowercase the second commit is incorrect. You can check the name with

dbus-send --print-reply --dest=org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus.ListNames|grep secrets
      string "org.freedesktop.secrets"

I've added a lint rule for this and reverted the second commit.

@bermeitinger-b
Copy link
Collaborator

I need to add --password-store=gnome-libsecret as an argument so that it converts and uses the host's keyring.

This seems to be an issue of Signal, not Flatpak, though.

@sukahiroaki sukahiroaki mentioned this pull request Sep 25, 2024
@Justinzobel
Copy link
Member Author

I need to add --password-store=gnome-libsecret as an argument so that it converts and uses the host's keyring.

This seems to be an issue of Signal, not Flatpak, though.

I assume it would be --password-store=kwallet for KDE Plasma then?

@bermeitinger-b
Copy link
Collaborator

Yes, but this again seems to be either a bug in Electron or your host system. It should detect the currently running instance automatically. You can overwrite this by using this cli argument.

@ben-alkov
Copy link

I need to add --password-store=gnome-libsecret as an argument so that it converts and uses the host's keyring.
This seems to be an issue of Signal, not Flatpak, though.

I assume it would be --password-store=kwallet for KDE Plasma then?

Plasma 6, flatpak, Signal version "7.24.1 production" here. I got an error about the keyring backend, suggesting to use --password-store="kwallet6" via CLI (which took a bit of doing, as I'm not very familiar with the guts of Flatpak). Figured it out, added the flag to the end of the "Exec" line in the .desktop file, and got Signal running again with no data loss.

@salim-b
Copy link

salim-b commented Sep 27, 2024

Figured it out, added the flag to the end of the "Exec" line in the .desktop file, and got Signal running again with no data loss.

Instead of modifying your .desktop files you can set a permanent env var override with the latest version via:

flatpak override org.signal.Signal --user --env=SIGNAL_PASSWORD_STORE=kwallet6

This will then become the default for your user, i.e. apply always, regardless how you start the Signal Flatpak (unless you explicitly override the SIGNAL_PASSWORD_STORE env var in the flatpak run invocation).

@Justinzobel
Copy link
Member Author

Justinzobel commented Sep 28, 2024

Was happy to help with the initial change but as I don't use Signal I'm going to unsubscribe from this. Happy encrypted messaging to all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants