GNOME Bugzilla – Bug 751040
smartcard plugin doesn't function if sharing plugin is active
Last modified: 2017-01-11 14:30:24 UTC
The smartcard plugin and the sharing plugin both uses nss (the latter indirectly, via NMClient). Unfortunatey, the sharing plugin runs first and initializes nss insufficiently, so gnome-settings-daemon can't find the smartcard drivers from the secmod database. This ends up preventing the smartcard plugin from working in the logged in session (debugged by Sumit Bose). NSS apparently has an alternative API to initialize itself, such that a second call won't noop and instead will perform any subsequent initialization requested. That API is called NSS_InitContext(). Documented here: https://wiki.mozilla.org/NSS_Library_Init We should change the smartcard plugin to use it instead of NSS_Initialize. Also we should fix NMClient, but that should go in another bug.
I filed bug 751041 to cover the NMClient side
Created attachment 305398 [details] [review] smartcard: use NSS_InitContext instead of NSS_Initialize NSS_Initialize is a noop if called multiple times. We currently call NSS_Initialize twice in gnome-settings-daemon. Once by NMClient and once by the smartcard plugin. NMClient does it first, and it does it without initializing the secmod database. When the smartcard plugin tries to initialize NSS with the secmod database later, it's call is turned to a noop. This commit changes the smartcard plugin to use NSS_InitContext instead, which can properly handle being initialized multiple times with different configurations. See: https://wiki.mozilla.org/NSS_Library_Init
The above patch is so far untested, but I think it will work.
Review of attachment 305398 [details] [review]: ::: plugins/smartcard/gsd-smartcard-manager.c @@ +95,3 @@ GsdSmartcardManagerPrivate *priv = self->priv; + NSSInitContext *context = NULL; + NSSInitParameters parameters = { sizeof (parameters), }; I don't understand that. @@ +142,3 @@ + if (self->priv->nss_context != NULL) { + NSS_ShutdownContext (self->priv->nss_context); g_clear_pointer() ?
Review of attachment 305398 [details] [review]: ::: plugins/smartcard/gsd-smartcard-manager.c @@ +95,3 @@ GsdSmartcardManagerPrivate *priv = self->priv; + NSSInitContext *context = NULL; + NSSInitParameters parameters = { sizeof (parameters), }; The first field in the structure is the size of the structure in bytes so that they can grow the structure in a forward compatible way. If you'd prefer I could put NSSInitParameters parameters = { .length = sizeof (parameters) }; or NSSInitParameters parameters = { 0 }; parameters.length = sizeof (parameters); @@ +142,3 @@ + if (self->priv->nss_context != NULL) { + NSS_ShutdownContext (self->priv->nss_context); I thought about that, though I'd lose the debug message
(In reply to Ray Strode [halfline] from comment #5) > Review of attachment 305398 [details] [review] [review]: > > ::: plugins/smartcard/gsd-smartcard-manager.c > @@ +95,3 @@ > GsdSmartcardManagerPrivate *priv = self->priv; > + NSSInitContext *context = NULL; > + NSSInitParameters parameters = { sizeof (parameters), }; > > The first field in the structure is the size of the structure in bytes so > that they can grow the structure in a forward compatible way. Just a comment saying that would be enough for me. > @@ +142,3 @@ > > + if (self->priv->nss_context != NULL) { > + NSS_ShutdownContext (self->priv->nss_context); > > I thought about that, though I'd lose the debug message Ha, sure.
Created attachment 305407 [details] [review] smartcard: use NSS_InitContext instead of NSS_Initialize NSS_Initialize is a noop if called multiple times. We currently call NSS_Initialize twice in gnome-settings-daemon. Once by NMClient and once by the smartcard plugin. NMClient does it first, and it does it without initializing the secmod database. When the smartcard plugin tries to initialize NSS with the secmod database later, it's call is turned to a noop. This commit changes the smartcard plugin to use NSS_InitContext instead, which can properly handle being initialized multiple times with different configurations. See: https://wiki.mozilla.org/NSS_Library_Init I went ahead and used g_clear_pointer(). It looks better to me, even with the extra if() statement.
(still need to actually, you know, test it and stuff)
Of course, *all* initialisations ought to be automatically loading the set of modules specified by the p11-kit configuration, unless NSS_NoDB_Init() is used... See https://bugzilla.mozilla.org/show_bug.cgi?id=1161219 which should probably be a duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=248722
(we've been using this in rhel for a while now) Attachment 305407 [details] pushed as 3b4c585 - smartcard: use NSS_InitContext instead of NSS_Initialize
(In reply to Ray Strode [halfline] from comment #10) > (we've been using this in rhel for a while now) > Attachment 305407 [details] pushed as 3b4c585 - smartcard: use > NSS_InitContext instead of NSS_Initialize Why commit this to master when it isn't true there? The smartcard plugin doesn't use NetworkManager, so nobody else calling NSS_Initialize(). Plugins are now independent daemons, since gnome-settings-daemon 3.23.2.
And it wasn't committed to gnome-3-22 or any earlier versions where this fix might do something.
Pushed to gnome-3-20 and gnome-3-22. I won't revert the patch on master, so that backporting is easier, if needed.
It still makes sense for master even though the bug isnt in master... Its using better apis. I cherry picked for other branches but didnt notice my git-bz push didnt send them