Closed Bug 1377940 Opened 7 years ago Closed 6 years ago

Change NSS default storage file format (currently DBM), when no prefix is given, to SQL

Categories

(NSS :: Libraries, defect, P3)

3.31
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

Attachments

(2 files)

As of today, if an application initializes NSS with a database storage location, but doesn't specify a prefix for the type of storage, NSS will use the classic DBM file storage.

The suggestion is to change this in the next release of the NSS library, and use the SQL storage format, if no prefix was provided by the application.
See Also: → 783994
Summary: Change NSS default storage file format, when no prefix is given, to SQL (not DBM) → Change NSS default storage file format (currently DBM), when no prefix is given, to SQL
What happens if you have a dbm store at the location given at NSS initialization? That is, what happens to, say, evolution (which iirc uses nss), when the system nss library changes its default under it?
(In reply to Mike Hommey [:glandium] from comment #2)
> What happens if you have a dbm store at the location given at NSS
> initialization? That is, what happens to, say, evolution (which iirc uses
> nss), when the system nss library changes its default under it?

I just reconfirmed the behavior by experimenting with certutil.

If the directory contains DBM (cert8/key3), but not SQL files (cert9/key4), the following will happen:

NSS will try to open the SQL files. If no such files exist, it will fall back to DBM files, and if they exist, will open them.

If the application is restricted to read only operations, no new files will be created.

If there's the need for any write action, then NSS will autoamtically create the new SQL files, containing the converted contents, plus the new modifications.

Future executions with SQL being the default will find the SQL files, open the SQL files, and ignore the DBM files.
Comment on attachment 8883265 [details] [diff] [review]
patch v1

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

r+ this change is a few decades overdue. Thanks for pushing this through kai.

bob
Attachment #8883265 - Flags: review?(rrelyea) → review+
Some additional notes on update:

What kai described is the behavior if the key database does not have a password. If the key database does have a password, it is updated when the database is openned r/w and the token has been logged in. 

Applications using the system database usually open the system database with the sql: prefix. Nothing changes for them. Kai's patch only changes the default (what happens if you don't specify a prefix). Applications that explicitly select the database will continue to use the database they have selected.

bob
I should not that this also does not change the behavior for users that set the NSS_DEBAULT_DB_TYPE environment variable.
Argh the first 'not' should be 'note'
Bob, hypothetical question:

If we change the NSS default, but Firefox makes a decision that they want to stick with the old DBM default, how could we achieve the following:
- ensure that Firefox uses DBM if no environment variable is set
- ensure that the override with NSS_DEFAULT_DB_TYPE=SQL still works

If we simply changed Firefox to hardcode the dbm: prefix for the path, it would break the NSS_DEFAULT_DB_TYPE variable.

Is there a way we could allow Firefox to request "dbm unless NSS_DEFAULT_DB_TYPE is defined" ?
If not, could we implement a way to do that?

A potential idea could be to invent yet another prefix, like "prefer-dbm:", which would be ignored by NSS, if NSS_DEFAULT_DB_TYPE has been defined to any value.
Flags: needinfo?(rrelyea)
ISTR from a long time ago that the switch hadn't been done in Firefox because it required manual migration on the part of the application. Comment 3 suggests that's handled by NSS now?
You might remember an additional, more ambitious goal from the past.

In addition to the file format change, we once had the intention to also migrate the location of the NSS database to a different directory. That directory would be outside the profile directory. It would allow multiple applications, such as Firefox and Thunderbird to share a single NSS database. This migration would require application assistance, as it could involve merging with an already existing destination. If both directories had master passwords set, but different ones, the migration would require to prompt the user for the second password. This applications assistance hasn't been implemented, and therefore we aren't ready to perform the migration to a command database directory.

The intention of this bug is limited to migrating to the newer file format, because we'd like to drop support for the old DBM format eventually.
> ISTR from a long time ago that the switch hadn't been done in Firefox because it required manual migration on the part of
> the application. Comment 3 suggests that's handled by NSS now?

The switch has always been automatic. There were two separate data base switches.

1) switching to a common database:

While I spent a lot of time on the NSS side to make this as automatic as possible, it still required a lot of work at the application layer to make things smooth. This work was difficult and potentially error prone. It also could lead to user confusion. The issue is that there may be a different password on your common database than was on your original. As a result, the upgrade would ask the user for passwords that they may not understand and didn't have really good failure modes when those passwords weren't supplied.

Our current recommendation here is if you want to use a common database, let users either create it new, or merge by hand using the certutil. This change does not try to switch to a common database.

2) just switching to the sql lite db, using the existing profile:

This has always been automatic. The issues with 'switch 1' may have confused people thinking this switch would be an issue here.

There is an issue that if you never enter your password (if you have one), you can never switch. This shouldn't be a problem because the 'no switch' just happens silently. Users will continue to 'use' the old key database. I put 'use' in quotes because they really can't use that database until they supply the password anyway.

We really need a plan for how to get firefox off of dbm. That code is older than some of our mozilla developers, and there is no upstream maintaining it.
> Is there a way we could allow Firefox to request "dbm unless NSS_DEFAULT_DB_TYPE is defined" ?
> If not, could we implement a way to do that?

> A potential idea could be to invent yet another prefix, like "prefer-dbm:", which would be ignored by NSS, if NSS_DEFAULT_DB_TYPE has 
> been defined to any value.

There isn't. If we need to supply something I'd rather supply a pref api that sets the default (which the environment variable could override). If firefox really needs to use dbm I'd rather switch it back for one release or so. The issue here is sql has been available for over a decade and it's time for dbm to go away. At some point going back to dbm will not be possible because we would remove the library that supports it, so the point of this change is to get us to the point where we can make the old library go away.

bob
Flags: needinfo?(rrelyea)
Depends on: 1379154
Depends on: 1379160
Depends on: 1379273
Blocks: 783994
See Also: 783994
Comment on attachment 8883265 [details] [diff] [review]
patch v1

I'm setting feedback- to indicate this isn't yet ready for checkin. We should fix the blocker bugs first.
Attachment #8883265 - Flags: feedback-
See Also: → 1383072
Depends on: 1382278
Priority: -- → P3
Depends on: 1403691
Blocks: 1409708
This incremental patch is needed to fix a test failure.

While "export -n" effectively removes the variables from the environment seen by tools, the bash scripts that drive the test still see the variable, which was previously set to value "dbm".

We have one test in chains.sh that tests for different results based on the database type used (bug 435314, broken in dbm, fixed in sql), and unless we apply this fix, the scripts are surprised that the test succeseds with the variable set to dbm.
Attachment #8926362 - Flags: review?(franziskuskiefer)
Comment on attachment 8926362 [details] [diff] [review]
1377940-incremental.patch

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

Looks like it's doing what it's supposed to.
Attachment #8926362 - Flags: review?(franziskuskiefer) → review+
Attachment #8883265 - Flags: feedback-
It seems we are ready to proceed with this task.

- We have fixed all known blocker bugs in NSS.

- Fedora Linux has already made this change in the development branch (Rawhide), targetting Fedora 28.

- We improved the Mozilla Firefox application code to behave correctly with the NSS sql database.

- Firefox 58 has been configured to use it, and it's been in the Firefox beta channel for 11 days.

I haven't seen any negative feedback yet.
I suggest that we proceed with this change for the current NSS 3.35 cycle.
Target Milestone: --- → 3.35
try looks good
  https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=3545a4af1887ea14854bf82444d5d669c257fb4b

both patches checked in:
  https://hg.mozilla.org/projects/nss/rev/b0658ed36763
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
See Also: → 1415912
See Also: → 1433464
After upgrading nss package to 3.35 downstream did you notice firefox-esr stabilitiy degraded when built --with-system-nss? If so you may want to backport at least bug 1380706. mozilla-esr52 wasn't prepared to ignore default database type change.
I haven't been reported such crashes, but i'll try tmrw with esr @work and a fresh profile. Thanks for the notice! This was an unfortunate move... as such, nss 3.35 isnt backwards-compatible at runtime with esr *and* mainline..
Using an 'old' esr profile, starting 52.6.0 with a systemwide nss 3.35 'upgraded' said profile (ie i now have key3.db, key4.db, cert8.db, cert9.db..) without issues. I've been able to visit a page and exit fine.

creating a new empty profile with the same esr didnt yield crashes at startup, and i only have cert9.db & key4.db in the profile. It didnt crash upon exit either. That was using systemwide sqlite 3.21.0.
Depends on: 1432484
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: