Closed Bug 1498909 Opened Last year Closed Last year

oskeystore: dynamically load libsecret if available (and fall back to nss otherwise)

Categories

(Core :: Security: PSM, defect, P1)

Unspecified
FreeBSD
defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: jbeich, Assigned: keeler)

References

(Blocks 1 open bug)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file)

libsecret is a Gnome thing but Gnome is available one more than just Linux e.g., BSD and Solaris derivatives. Webkit and Blink already use libsecret on FreeBSD and OpenBSD.
Firefox 64 built fine `when=toolkit_gtk` (before bug 1498450) on FreeBSD. Can you switch Linux conditional to include all GTK platforms in bug 1492305?
Flags: needinfo?(franziskuskiefer)
Since linking at build time doesn't seem to acceptable we'll have to vendor libsecret.
Blocks: 1492305
Flags: needinfo?(franziskuskiefer)
Summary: Enable libsecret on Tier3 platforms → Vendor libsecret
Summary: Vendor libsecret → Vendor libsecret in Firefox code base
Why should it be vendored ? Can't we just have --with-system-libsecret like other dependencies ? I don't understand this issue here..
Supposedly libsecret isn't installed on build machines and thus won't be picked up. Linking at runtime makes this explode as we'd have to define all the glib types necessary here. Vendoring it seems the best way to me or do you have a better option?
Flags: needinfo?(landry)
why cant you require libsecret to be installed on build machines as a blessed dependency like it's done for any other dependency like gtk/etc ? 'linking at runtime' seems crazy...
Flags: needinfo?(landry)
I really don't have strong opinions on this (other than runtime linking isn't the way to go). But I'm no build system peer and don't know how these decisions are made. Let's ask them.
Flags: needinfo?(mh+mozilla)
(In reply to Franziskus Kiefer [:franziskus] from comment #6)
> (other than runtime linking isn't the way to go)

Why?
Flags: needinfo?(mh+mozilla)
Linking at runtime requires duplicating most of the libsecret header definitions, which is really not what we want.
So what's the way forward here?
Flags: needinfo?(mh+mozilla)
I guess it'd be fair to vendor libsecret, then, although:
- Following the few code paths we use from the libsecret library, I see it's using g_main_loop_run, and I'm not sure how this is going to interact with Gecko. I also don't know where the OSKeyStore stuff from the security manager is supposed to run ; main thread or somewhere else.
- Considering the API surface being used is very narrow, I wonder if it wouldn't make sense to just use the DBUS API directly https://specifications.freedesktop.org/secret-service/.
Flags: needinfo?(mh+mozilla)
It turns out the runtime loading approach is tenable, so we're going to explore that route.
Assignee: nobody → dkeeler
Priority: -- → P1
Summary: Vendor libsecret in Firefox code base → oskeystore: dynamically load libsecret if available (and fall back to nss otherwise)
Whiteboard: [npotb] → [psm-assigned]
Enough linux-based systems don't have libsecret that we can't make it a
requirement on linux. For those that do, however, we can dynamically load the
library at runtime. For those that don't, we can fall back to NSS.
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/15af035c0cf4
dynamically load libsecret at runtime if available r=franziskus,jcj
https://hg.mozilla.org/mozilla-central/rev/15af035c0cf4
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Is this something which can ride the trains or should we consider it for uplift?
Flags: needinfo?(dkeeler)
If I'm reading the tea leaves correctly, form autofill/credit card storage isn't using this except on Nightly yet, so we should probably let this ride the trains (if I'm wrong about that, then we probably should uplift, since linux users would essentially see their credit card info disappear when they eventually upgrade to 66).
Flags: needinfo?(dkeeler)
You are right that the consumers are Nightly-only so this can ride the trains.
Thanks for the quick reply.
You need to log in before you can comment on or make changes to this bug.