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

Use GnuTLS as the crypto backend for libsecret #294

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZVNexus
Copy link
Collaborator

@ZVNexus ZVNexus commented Feb 16, 2024

No description provided.

GnuTLS is present in the base Freedesktop platform & SDK, this is
also what Fedora has been shipping with as of a couple months ago.

https://src.fedoraproject.org/rpms/libsecret/c/a858664cc30cb9103a9818cdb97a20879ab8cfb5
@ZVNexus ZVNexus requested a review from Lctrs as a code owner February 16, 2024 11:41
@ZVNexus
Copy link
Collaborator Author

ZVNexus commented Feb 16, 2024

This needs gnutls >= 3.8.2, the runtime ships 3.8.0 it silently falls back to libgcrypt.

Even if it is updatable in runtime, it's not going to reach Flathub before next month. Also 22.08 can never update to 3.8 branch.

https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/merge_requests/17826

@TingPing
Copy link
Member

the runtime ships 3.8.0 it silently falls back to libgcrypt.

This means that -Dcrypto=gnutls is silently ignored by libsecret? That sounds wrong.

@bbhtt
Copy link
Contributor

bbhtt commented Feb 16, 2024

It's at least not exiting when it can't find the required version of gnutls 3.8.2

@Erick555
Copy link
Collaborator

Erick555 commented Feb 16, 2024

This means that -Dcrypto=gnutls is silently ignored by libsecret? That sounds wrong.

Actually this may be advantage here otherwise it will stop working with fdsdk 22.08 which as noted won't have relevant gnutls.

Fedora commit says it's to have one less lib loaded in memory - I wonder if this is really worth it. The fact it makes libsecret depend on minor runtime release isn't great.

@bbhtt
Copy link
Contributor

bbhtt commented Feb 16, 2024

As @Erick555 noted in #freedesktop-sdk, updating gnutls to 3.8.2 in 23.08 branch of freedesktop-sdk will break forward ABI guarantees.

So this needs to wait until 24.08 comes out this October or November. The update to master was pushed yesterday https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/merge_requests/17825

Or even if the update lands in freedesktop-sdk, libsecret cannot start depending on it in the 22.08 and 23.08 cycle.

@TingPing
Copy link
Member

It's at least not exiting when it can't find the required version of gnutls 3.8.2

It just disables all crypto:

https://gitlab.gnome.org/GNOME/libsecret/-/blob/master/meson.build?ref_type=heads#L51-62

@bbhtt
Copy link
Contributor

bbhtt commented Feb 16, 2024

That's worse. Then it can't land in any case, until every supported runtime has the dependency.

@bbhtt bbhtt marked this pull request as draft February 16, 2024 18:58
@TingPing
Copy link
Member

https://gitlab.gnome.org/GNOME/libsecret/-/merge_requests/137

@A6GibKm
Copy link
Collaborator

A6GibKm commented Feb 17, 2024

Just to be clear, if you disable crypto it only means apps will use the dbus backend rather than the secrets portal, it does not disable encryption at the library level.

Having said that, there is no migration path between the two backends, from the app pov secrets existing secrets would just not be there anymore.

@xanscale
Copy link

i think the solution is to update runtime not force a behavior

@bbhtt bbhtt marked this pull request as ready for review September 29, 2024 13:31
@bbhtt
Copy link
Contributor

bbhtt commented Sep 29, 2024

If there is no migration path, then this needs to be a separate manifest libsecret-gnutls.json so that existing apps don't suddenly break.

Those wanting to use that one can explicitly add it after assessing it for their app.

@bbhtt
Copy link
Contributor

bbhtt commented Oct 1, 2024

Libsecret was added to freedesktop-sdk for 25.08 (default build using gcrypt backend as it will replace this shared module). The current shared modules has no maintainer and will most likely be removed once 25.08 is out.

Chromium and Electron does not work with the default build, they need the DBus backends. I switched chromium to that already flathub/org.chromium.Chromium.BaseApp@3937abe

Electron baseapp will be switched to that in the 25.08 branch as it is a breaking change.

These should cover all the needs of majority of the apps.

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.

6 participants