1562683 - Enable higher NSS MP KDF default iteration count for LEGACY key3 storage by default

Status: NEW

(NSS :: Libraries, enhancement, P2)

Description

[Kai Engert [:KaiE:]]

This bug is similar to bug 1562671.
However, this bug is about the legacy DBM storage (key3.db).

Bob offered code in bug 524403 to update the legacy DBM file format and record the iteration count in the salt.

As a result of that change, DBM databases created by newer NSS versions would no longer be readable to older NSS versions.

We need to decide if this change is worth it, given that legacy DBM database should go unsupported in the near future.

If it's decided to fix DBM, then it's necessary to work out all potential consequences.

Comment 1

[Kai Engert [:KaiE:]]

Bob, do you have a need to support the iteration count in the DBM database?

Comment 2

[Mark Straver]

given that legacy DBM database should go unsupported in the near future

Why should it? It's a mature and extremely efficient (and fast) db format. The downside of exclusive write access is completely unimportant for its typical use in NSS. AFAIK, shared databases are also not a thing even in Firefox.
Because of its maturity, maintenance cost is extremely low or even negligible.
As stated, in the parent bug, NSS is widely used outside of the Mozilla scope and that should always be considered.

Consider this an official request to continue supporting DBM for the foreseeable future. The MP KDF code improvement for strengthening is already provided in the (now obsoleted) patch in bug 524403, meaning if included, no custom builds of NSS will be required by those needing DBM with acceptable MP encryption.

Comment 3

[Kai Engert [:KaiE:]]

(In reply to Mark Straver from comment #2)

Why should it?

Because the primary developers of the related code have repeatedly said in the past that they prefer to stop maintaining it.
I believe there are known bugs without plans to fix them. Bob would know more.

AFAIK, shared databases are also not a thing even in Firefox.

Firefox was migrated to sqlite/key4 storage 1.5 years ago, see bug 783994.
Long time support versions of Firefox ESR 60.x and Thunderbird 60.x have been using the new storage, too, since mid 2018.

Consider this an official request to continue supporting DBM for the foreseeable future.

That's Bob's call as well.

EDITED: fixed year to say 2018.

Comment 4

[Mark Straver]

Firefox was migrated

NSS != Firefox.

How often do I have to repeat that NSS is used outside of the Mozilla scope? I've already said it three times now. You should consider all use of the library when making these kinds of decisions.

That's Bob's call as well.

Would it be Bob's call to not destroy usability of a common library? I don't know Bob, or what his priorities are in regards to NSS or his use of it, but I am a consumer of NSS and request that you not remove/deprecate/cripple DBM use in it.

Comment 5

[Kai Engert [:KaiE:]]

(In reply to Mark Straver from comment #4)

Firefox was migrated

NSS != Firefox.

Mark, I understand. I specifically responded to your claim that shared databases aren't a thing with Firefox, which is false, because Firefox does use the shared database file format.

Comment 6

[Kai Engert [:KaiE:]]

Bob Relyea is a senior developer of NSS. He has been with the project for more than 20 years, and has probably written a large part of it. It was also him who wrote the NSS shared DB code, and all the migration code from DBM to sqlite.

It was also Bob who has repeatedly told me in the past that the old DBM code should go away.
If he decides not to maintain it, there might not be anyone else who is willing maintain it.

So let's wait for his comments.

Comment 7

[Mark Straver]

your claim that shared databases aren't a thing with Firefox, which is false, because Firefox does use the shared database file format.

Using a shareable file format does not automatically mean that databases are actually shared, which is what I referred to. Last I checked Firefox profile components aren't shared that way and access is exclusive to a single browser instance, no matter if the format offers the possibility to share it. So shared databases really aren't a thing with Firefox, and my statement is not false.

Comment 8

[Robert Relyea]

So there are a couple of issues that come up in this bug, so I'll try to address them each separately:

1. The meta issue: Should anyone be using dbm?

DBM is based on the very old berkeley db code, which has not been maintained since the early 90's. The original developers of the code created sleepycat, which fixes a lot of issues with the old database code. About 15 years ago, I updated NSS to be able to use sqlite, which was 1) more stable, and 2) had a schema more suited to NSS's PKCS #11 use then dbm (which couldn't actually store all PKCS #11 attributes).

The new sqlite database is the new default in NSS, so you have to work to get the old database. If your application used the old database, you can update to the new NSS database by simply opening your database R/W (and saying you are opening the sqlite database) and authenticating to that database. If your application only uses sqlite, then you can use certutil to update (tell certutil to open the database R/W and list the keys).

In the not to distant future we will probably turn off dbm by default (that is you would have to make your own special builds to get it). We probably need to support it through the lifetime of RHEL-8, but you'll want to change your application over sooner. The berekeley db is very brittle, and has lots of known issues (open a database R/W and crash and you can corrupt it, any sort of sharing can crash your application, there are lots of coverity issues which we are not trying to fix.

You do not need to actually share your database to get advantage of sql. Even if you have a standalone app that has it's own database, you want to transition away from dbm.

bob

Comment 9

[Robert Relyea]

Should we add support for dbm iteration count.

I did do the initial code to support it, so my preference was to have complete support. As a general rule, I dislike having any semantic differences between the various underlying database options (though there are naturally some just do to the nature of the database).

That being said, the fact that the change creates an incompatible database for old software to read, means that in RH we would have to control the rollout of this change, getting the code that understands the iteration count out first, and then turning on the iteration count. Kaie's code that conditionally turns it on based on the db type would probably be in use in our products initially.

This incompatibility is the main reason to even think about not including the patch for dbm. This is mitigated by the fact that you could manually fix this database by removing the password with a new NSS and adding it back with an old one. I think we need a better plan though.

At a minimum, we should include this patch, but not turn on using the higher iteration count in dbm by default, and have some way to manually turn that on. The other option is add the code and set the iteration count high by default, but have an environment variable to set it back to 1 (like we use when we run our tests).

I'm agnostic about which way we should go (though for RHEL, we would probably initially release with the iteration count low in dbm by default).

bob

Comment 10

[Mark Straver]

The new sqlite database is the new default in NSS, so you have to work to get the old database.

The "work" involved isn't much. Just prefix your db opening statement with dbm: and you're good to go. Even though some devs had trouble figuring that out in time when updating NSS it's a one-time minimal patch to apply to any application and be forgotten about.

FTR we've currently moved to a manually-patched version of NSS for the time being to provide better MP security using the dbm db storage patch with iteration count stored as in the patches provided. We know dbm can be fragile in some situations, but our limited use of it doesn't expose our users to much risk of corruption or crashes since the surrounding code is mature. All things considered we'll have to think about whether to look at migrating to sql or abandoning Mozilla NSS for a fork of our own (which we'd rather not have to do, but dropping of dbm might result in that necessity).

If we are forced to move to the sql storage format, it would of course be imperative that this type of db with iteration count is also properly translated to the sql format. Would this need more code changes to have the migration path or would that work as-is?

Comment 11

[Robert Relyea]

The transition to sql format should be straightforward. The only issues people have ran into in the transition is that sql lite is more PKCS #11 compliant than dbm and it lead to some applications accidently using shortcuts that don't work in sqlite (and wouldn't work with an HSM), and some performance issues in shared file systems (which have some work arounds, and are even more dangerous to keep dbm databases in).

That being said, I think we should at least pick up the patch to handle iteration count for dbm databases. I have some older OS's that will need dbm for some time, even if we turn it off be default in most NSS builds. Any such dbm database should upgrade to an sqlite database with iteraction count without a problem (actually a dbm without an iteration count will transition to an sql with an iteration count.. The iteration count is updated anytime you do an update/change password, so it doesn't matter what the source database has).

bob

Comment 12

[Kai Engert [:KaiE:]]

Based on Bob's comments, I think we should do the following in bug 1562671:

• raise the default count for modern sql key4
• add the code that can optionally read/write the iteration count from legacy dbm key3
• keep the default for DBM at 1
• add an environment variable, than an application can set, to request that NSS enables the higher count for DBM, too

I think this should address the needs described here.
It allows a migration period for mixed environments, where NSS libraries can be deployed that already support reading the modified key3 DBM databases, while not yet writing for them.

Applications that wish to start using the higher count with an DBM database can use a setenv call before calling NSS init.

We can keep this bug open, and consider to enable the higher iteration count for DBM databases by default at a later time.