Closed Bug 1449169 Opened 2 years ago Closed 2 years ago

NSS should assume SQlite >= 3.5.0 and support read-only database opening using the sqlite3_open_v2 API

Categories

(NSS :: Libraries, enhancement, P3)

3.36
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: KaiE, Assigned: KaiE)

Details

Attachments

(2 files)

An application initializes NSS with the following call.

  NSS_InitContext(nsscertdir, prefix, prefix, SECMOD_DB,
                  &initparams, NSS_INIT_READONLY);

The NSS SQL database is used, which uses SQLite.

Despite the request to open read-only, we see attempts to open the files in write mode.

We haven't yet debugged which code causes the attempt to open in write mode.
Before we debug this inside NSS, we'd like to ask:

Could it be a possible cause, that SQLite always attempt to open its files in write mode, potentially for locking purposes?
Which SQLite files are we talking about?  The main database file?

SQLite should be attempting to open main database file read-only if you use the SQLITE_OPEN_READONLY flag on sqlite3_open_v2().  Are you seeing something difference? How exactly are you trying to open the database?  Can you provide additional context?
Thanks for the initial feedback, which makes it clear that we likely cannot solve this without debugging what's really going on at the NSS level.
NSS never calls sqlite3_open_v2.

Inside the NSS internal function sdb_openDB, NSS doesn't make any use of the read-only flag that was provided as a parameter (flags == 1 == SDB_RDONLY).

That function has the following comment:
    /*
     * in sqlite3 3.5.0, there is a new open call that allows us
     * to specify read only. Most new OS's are still on 3.3.x (including
     * NSS's internal version and the version shipped with Firefox).
     */

Bob, it appears that because of the situation described in the above comment, which was written at the time of the original implementation, NSS never uses read-only access when opening sqlite database files.

We'd have to change NSS to use the new sqlite3_open_v2.

I see that even on RHEL 6, we already have SQLite 3.6.20 available, so I think it should be acceptable to change NSS.
Summary: Investigate why SQlite attempts to open files in read/write mode, if NSS requests read/only access → NSS should assume SQlite >= 3.5 and support read-only database opening using the sqlite3_open_v2 API
SQLite 3.6.20 is quite old.  The latest releases of SQLite are more than 3x faster and contain security fixes.  Can you statically link against SQLite (as FF does) and hence always have the latest code?
No, on Linux distributions we commonly use shared linking. This allows us to reduce the OS code size, and in times of security incidents, fewer places must be updated.
Old version of SQLite (or for that matter, any package) on RHEL don't mean that they lack security fixes, it just specifies the oldest code that was shipped as part of upstream release – more or less the API of the package. The code needs to link to system libraries.

Please see https://access.redhat.com/security/updates/backporting and https://fedoraproject.org/wiki/Bundled_Libraries for background info.
Using a test program, I have confirmed that version 3.6.20 of SQLite does honor the SQLITE_OPEN_READONLY flag to sqlite3_open_v2(), causing it to open the database file using open(..., O_RDONLY).  (I only tested unix - presumably it works on Windows too.)  So I think this takes us back to the question of:  What SQLite API is NSS using to open the database?
Hello Richard, I'm sorry if I haven't been clear in the previous statements.

The fault is inside NSS. NSS never uses the v2 API. NSS never asks SQLite to open read-only.
Based on that, I suggest to change NSS to start using the v2 API.

The question is, does this change align with the backwards compatibility requirements of NSS.
In the past, compatibility with SQLite 3.3 was required.

I believe we can increase NSS' minimal SQLite version expectation to 3.6.20, and change NSS to use the v2 API.
We intend to upgrade the source code snapshot distributed with NSS to verion 3.6.20 (the currently oldest supported version), and change NSS to use the v2 open API, and support read only opening.
Bob has suggested, as part of the work on this bug, we should check if NSS has any #ifdef based on dynamically detected sqlite version, and possible cleaning up those we can.
It think it's fine to use the 3.6.20 API now. When the original code went in, there were still users on the older API. The embedded sqlite that NSS uses is quite old, not not really updated (unlike the frozen version in RHEL 6), so statically linking is definitely out. Most NSS users supply their own sqlite (either through the OS or in their application), and those versions are likely to be more up to date than the one embedded in the NSS source tree (which is really there just to allow NSS to build and test stand alone).
Priority: -- → P3
Assignee: nobody → kaie
Attached patch 1449169-v1.patchSplinter Review
Attachment #8967710 - Flags: review?(rrelyea)
try build running here:
https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=bc71ec59c3b41f01adfbd417d19f8f2e51908ec6

I'd like to do the update of the bundled sqlite snapshot in a separate step.
Summary: NSS should assume SQlite >= 3.5 and support read-only database opening using the sqlite3_open_v2 API → NSS should assume SQlite >= 3.6.20 and support read-only database opening using the sqlite3_open_v2 API
(In reply to Kai Engert (:kaie:) from comment #14)
> I'd like to do the update of the bundled sqlite snapshot in a separate step.

Unnecessary, we already bundle the newer version 3.10.2 (bug 1234698).
Comment on attachment 8967710 [details] [diff] [review]
1449169-v1.patch

shouldn't the minimal expected version be changed also by this patch?
or it's not a check run by code/doesn't live in nss repo?
I don't think there's NSS code that checks for the version. But I'll doublecheck.
(we didn't update any version checks in the past, when we updated the bundled copy to newer versions, which suggests that there is no such check)
Target Milestone: --- → 3.37
Attachment #8967710 - Flags: review?(dueno)
Comment on attachment 8967710 [details] [diff] [review]
1449169-v1.patch

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

::: lib/softoken/sdb.c
@@ +648,4 @@
>      /*
>       * in sqlite3 3.5.0, there is a new open call that allows us
>       * to specify read only. Most new OS's are still on 3.3.x (including
>       * NSS's internal version and the version shipped with Firefox).

I would suggest to update this comment reflecting the current situation (3.6.20 is required now and 3.10.2 is included).
Attachment #8967710 - Flags: review?(dueno) → review+
Thanks. Version 3.6.20 was mentioned because it's probably the oldest version we currently have to support. But the newest APIs we're using were from version 3.5.0. Increasing the minimal version requirement to 3.5.0 should be sufficient. We should mention that in the NSS release notes for this bugfix.

I'll remove the above comment, and add a comment mentioning the 3.5.0 requirement.
Summary: NSS should assume SQlite >= 3.6.20 and support read-only database opening using the sqlite3_open_v2 API → NSS should assume SQlite >= 3.5.0 and support read-only database opening using the sqlite3_open_v2 API
Attached patch 1449169-v2.patchSplinter Review
Attachment #8967710 - Flags: review?(rrelyea)
https://hg.mozilla.org/projects/nss/rev/e7ccf89f73a2
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.