Closed Bug 818686 Opened 11 years ago Closed 5 years ago

Support XDG basedir specification

Categories

(NSS :: Libraries, defect, P5)

Product:
NSS
Component:
Libraries
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: Heintzmann.Eric, Assigned: edenisfa)

References

(Depends on 1 open bug)

Details

Attachments

(3 files)

XDG base directory spec support patch
1.41 KB, patch
franziskus
: review-
Details | Diff | Splinter Review
XDG Base Directory Support for NSS with an automated migration path.
3.48 KB, patch
Details | Diff | Splinter Review
Add support for the XDG Base Directory Specification with legacy support.
47 bytes, text/x-phabricator-request
Details | Review 
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121129151900
Where in the codebase would this be applied to?
Depends on: 259356
I think these files should be moved:

$HOME/.pki/nssdb/pkcs11.txt --> $XDG_CONFIG_HOME/pki/nssdb/pkcs11.txt
$HOME/.pki/nssdb/cert9.db   --> $XDG_DATA_HOME/pki/nssdb/cert9.db
$HOME/.pki/nssdb/key4.db    --> $XDG_DATA_HOME/pki/nssdb/key4.db

http://ploum.net/post/207-modify-your-application-to-use-xdg-folders
https://live.gnome.org/GnomeGoals/XDGConfigFolders
http://standards.freedesktop.org/basedir-spec/latest/
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Andre Klapper from comment #1)
> Where in the codebase would this be applied to?

From a quick non-comprehensive look, security/nss/lib/sysinit/nsssysinit.c getUserDB()
Priority: -- → P5
Would you accept a patch for it? I've just noticed this as well, after switching to newest Kmail (that indirectly relies on libnss3).
Declutters $HOME by placing user DB in $XDG_DATA_HOME/pki/nssdb or in $HOME/.local/share/pki/nssdb if $XDG_DATA_HOME isn't set.
Comment on attachment 8929869 [details] [diff] [review]
XDG base directory spec support patch

Review of attachment 8929869 [details] [diff] [review]:
-----------------------------------------------------------------

Adding XDG_DATA_HOME support is fine in general. But it has to either keep the old default or add a migration path.

::: lib/sysinit/nsssysinit.c
@@ +54,5 @@
> +    if (userdir != NULL) {
> +        nssdir = PORT_Alloc(strlen(userdir) + sizeof(NSS_USER_PATH1) +
> +                 sizeof(NSS_USER_PATH2));
> +    } else {
> +        nssdir = PORT_Alloc(strlen(homedir) + sizeof(NSS_USER_PATH0) +

Always using ~/.local/share/pki/nssdb now as default will break old installations. If you want to change the default folder, you have to add an automatic migration.
Attachment #8929869 - Flags: review-
You can't keep old default, the whole point is to get rid of it, to conform to XDG spec. How do you propose to introduce migration?
I'm not sure exactly about use cases, but for example in my particular situation Kmail is using it, and simply deleting $HOME/.pki/nss creates it again when Kmail is restarted. So it's not any worse than it creating it again in different location. Is there a situation when this directory switch can cause a problem?
> You can't keep old default, the whole point is to get rid of it, to conform to XDG spec. How do you propose to introduce migration?

There has to be something that detects the old config folder and moves it if it's there. This would break applications that hard code the path but that might be ok.
I think there's a redhat bug that was closed for this change. They probably know better.
Would it be OK to add that migration right here in getUserDB function?
I would like to also submit a patch for support but with an automated migration path included.

At this moment, I check if the folder "$HOME/.pki" exists and I rename it to "${XDG_DATA_HOME:-$HOME/.local/share}/pki".

I tried some tests, but it seems some apps hardcode this path to "$HOME/.pki/nssdb". I don't quite understand why they don't use this function, but I am still learning so maybe there is a very good explanation for it.
(In reply to Franziskus Kiefer [:franziskus] from comment #9)
> There has to be something that detects the old config folder and moves it if
> it's there. This would break applications that hard code the path but that
> might be ok.

Is the above solution good?
We check if $HOME/.pki and $HOME/.pki/nssdb exist; if they do, then we use
this path. Otherwise, use ${XDG_DATA_HOME:-$HOME/.local/share}/pki/nssdb
Which version will this be fixed in?
Hey @Shmerl(In reply to Shmerl from comment #14)
> Which version will this be fixed in?

Hey! Just a FYI about this, pushing this MR will not solve this issue once for all. The code on NSS is used as a reference of where applications should store their certificate files (see https://bugzilla.mozilla.org/show_bug.cgi?id=494481 ). The function getUserDB is not exposed. 

This change would motivate for us to go push on other software (I am trying this right now on chromium: https://chromium-review.googlesource.com/c/chromium/src/+/1364131 ). At least on chromium, they hardcoded this path "because NSS does like this and we would only change after the upstream NSS discussion".

Also, they are using phabricator at mozilla. If they are not replying to you here, check https://phabricator.services.mozilla.com for updates.

Bob,

As part of fixing this, we made a function in the nsssysinit.c non-static (so it could be tested).

I've added a .def file to the nsssysinit.so build so that we don't pollute the ABI. This does result in ABI changes (from the ABI compatibility check):

+Function symbols changes summary: 2 Removed, 0 Added function symbols not referenced by debug info
+Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info
+
+2 Removed function symbols not referenced by debug info:
+
+  _fini
+  _init
+

It seems like this is very likely OK: https://sourceware.org/ml/binutils/2018-05/msg00012.html

However, I just want to confirm that hiding the _init and _fini symbols isn't an issue in your experience? We hide those symbols for other libraries, and I don't see any special behavior here that warrants caution, but I thought that I'd double check.

Flags: needinfo?(rrelyea)
QA Contact: jjones

Loosing those symbols should be fine.

On the base issue, there is a migration issue for us. We'll need to keep the old behavior on our existing RHEL platforms (at least the behavior without the environment variable). Fedora can move forward because applications that depend on the hard coded location can be changed to accommodate the move. I'll leave daiki to comment on how he wants to handle this in RHEL

Flags: needinfo?(rrelyea) → needinfo?(dueno)

Martin/Bob: Does Firefox use nsssysinit? Is my understanding correct that this change would only be applicable to Fedora/RHEL?

Ryan, that's a good question. Firefox builds don't exclude this library. That might be a mistake, because the make builds exclude it everywhere except non-Android Linux. Unfortunately, that will require some testing.

We also noticed that the file contains #ifdef XP_WIN and #ifdef XP_UNIX, which are pointless given the build conditions on it. I'll open a bug to remove that code.

Daiki, are you OK with landing this and dealing with the RHEL changes later, or would you like to hold it for those?

Blocks: 1519228
Blocks: 1519231

(In reply to Martin Thomson [:mt:] from comment #19)

Daiki, are you OK with landing this and dealing with the RHEL changes later, or would you like to hold it for those?

Yes, we can adjust the RHEL packages later for this change (if the migration is really needed; the approach in comment 13 sounds good enough).

Flags: needinfo?(dueno)

https://hg.mozilla.org/projects/nss/rev/da45424cb9a0b4d8e45e5040e2e3b574d994e254

Assignee: nobody → edenisfa
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.42

Opened one for Kmail: https://bugs.kde.org/show_bug.cgi?id=403752

Can someone please explain why this is not working on my archlinux (nss v3.45-1) system? On my test case, everytime I launch qutebrowser it will create a ~/.pki, no matter if I remove this dir before the launching or even creating an empty one on ~/.local/share/pki/nssdb.

Still same issue as @AlexandreProvencio, but I don't even know which application creates it

Same issue on Ubuntu 20.04.1 LTS

Some app, likely firefox or vscode, appears to create a ~/.pki folder on startup

(In reply to lgarczyn from comment #25)

Same issue on Ubuntu 20.04.1 LTS

Some app, likely firefox or vscode, appears to create a ~/.pki folder on startup

Can confirm it's visual studio code, version 1.52.1

You're seeing this because the file that was patched is not normally used by NSS using applications. That's what I was asking about in Comment #18. NSS using applications are very unlikely to be using this, the file is a bit more like a 'contrib' file. It's mostly harmless, it'll do something for some platforms (like RHEL), but it's not the main entry point most main NSS-using apps use.

You need to log in before you can comment on or make changes to this bug.