Support XDG basedir specification

RESOLVED FIXED in 3.42

Status

defect
P5
normal
RESOLVED FIXED
7 years ago
5 months ago

People

(Reporter: Heintzmann.Eric, Assigned: edenisfa)

Tracking

(Depends on 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Reporter

Description

7 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121129151900

Comment 1

7 years ago
Where in the codebase would this be applied to?
Depends on: 259356
Reporter

Comment 2

7 years ago
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/

Updated

5 years ago
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()

Comment 4

2 years ago
Would you accept a patch for it? I've just noticed this as well, after switching to newest Kmail (that indirectly relies on libnss3).

Comment 5

2 years ago
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-

Comment 7

2 years ago
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?

Comment 8

2 years ago
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.

Comment 10

2 years ago
Would it be OK to add that migration right here in getUserDB function?
Assignee

Comment 11

10 months ago
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.

Comment 12

7 months ago
(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?
Assignee

Comment 13

7 months ago
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

Comment 14

7 months ago
Which version will this be fixed in?
Assignee

Comment 15

7 months ago
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

Comment 17

6 months ago

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)

Comment 18

6 months ago

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)
Assignee: nobody → edenisfa
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 3.42

Comment 22

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