After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 585301 - Use NSS SQLite database, if available
Use NSS SQLite database, if available
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: general
2.28.x (obsolete)
Other All
: Normal normal
: ---
Assigned To: Evolution Shell Maintainers Team
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2009-06-10 03:29 UTC by Craig Ringer
Modified: 2011-02-24 09:02 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26


Attachments
Use SQL access in e-d-s if NSS new enough (2.23 KB, patch)
2009-06-10 08:56 UTC, Craig Ringer
needs-work Details | Review
Use SQL access in Evolution s/mime if NSS new enough (2.08 KB, patch)
2009-06-10 09:02 UTC, Craig Ringer
reviewed Details | Review
Use system database where appropriate (2.70 KB, patch)
2010-06-10 12:55 UTC, David Woodhouse
committed Details | Review
Use camel_init() to initialize NSS in evolution (2.13 KB, patch)
2010-06-10 12:57 UTC, David Woodhouse
committed Details | Review
use correct db for non-system case too (1.56 KB, patch)
2010-06-12 11:27 UTC, David Woodhouse
committed Details | Review

Description Craig Ringer 2009-06-10 03:29:24 UTC
Please describe the problem:
Both E-D-S and Evolution open the NSS certificate database, and do so in read/write mode. They seem to get away with it, but it's unsafe, and may cause issues now or in the future.

The only safe way to share the NSS database is when multiple clients all open it read-only. It is not safe to open it read-write unless you are the only client. This includes opening it read-write in one client and read-only in the others.

Evolution needs to open the certificate store read-only, as does e-d-s. When Evolution's S/MIME management interface is opened, e-d-s should close the certificate store and shut down NSS and Evolution should re-open NSS in read/write mode. When cert management is done, e-d-s can re-open the DB in read-only mode, once evo has also reverted to read-only mode.

This isn't pretty.

Alternately, Evolution could do its crypto via IPC to/from e-d-s. However, e-d-s isn't always built with NSS; it might be built with GnuTLS, which doesn't provide a certificate store. So that's not viable.

A much better soultion in the long run is for Evolution to open the database read only, eliminate the S/MIME management UI entirely, and do all certificate management via gnome-keyring's PKCS#11 provider and the Seahorse key manager.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Craig Ringer 2009-06-10 03:32:13 UTC
See bug 585300. bug 534219, bug 270893.
Comment 2 Craig Ringer 2009-06-10 06:57:34 UTC
Work is proceeding on addressing this in NSS:

https://wiki.mozilla.org/NSS:Roadmap#SQLite-Based_Multiaccess_Certificate_and_Key_Databases
Comment 3 Craig Ringer 2009-06-10 08:37:48 UTC
It's possible to launch e-d-s and evo with the NSS_DEFAULT_DB_TYPE="sql" environment variable set to get them to use the new NSS SQLite database support, which *IS* safe to share among multiple readers/writers.

The soon-to-be-attached patches to e-d-s and evolution will explicitly request SQL format databases if NSS supports them, but otherwise use the old format.

Possible issues:

Comment 4 Craig Ringer 2009-06-10 08:45:05 UTC
Note that old databases are automatically migrated to the new format by NSS. The old database files are left in place, so users won't get nasty surprises if they test a new version then go back to the old one.
Comment 5 Craig Ringer 2009-06-10 08:56:31 UTC
Created attachment 136260 [details] [review]
Use SQL access in e-d-s if NSS new enough
Comment 6 Craig Ringer 2009-06-10 09:02:17 UTC
Created attachment 136261 [details] [review]
Use SQL access in Evolution s/mime if NSS new enough
Comment 7 Matthew Barnes 2009-08-03 19:00:40 UTC
I'm not an NSS expert, but your reasoning seems sound and the patches look good.

However the E-D-S patch conflicts slightly with another recent NSS-related bug.  See bug #580620.  Can you resolve your changes with this?

Comment 8 Milan Crha 2010-03-26 20:04:53 UTC
Is this still needed in 2.30.0+? If so, could you update those patches for actual master (thus for 2.31+), please?
Comment 9 Craig Ringer 2010-03-27 03:50:51 UTC
Unknown. I've stopped using evo because my patches for client cert support were ignored for so long that I got sick of maintaining my own patched custom builds and went back to thunderbird. Sorry.

At this point work's keeping me too busy to re-investigate this.
Comment 10 Tobias Mueller 2010-05-23 12:38:53 UTC
Hm. Matthew: Given that bug 580620 is fixed, is patch 136261 good enough then?
Comment 11 Matthew Barnes 2010-05-29 19:48:17 UTC
I modified the Camel patch to accomodate bug #580620.  Some peer review from Craig or anyone else knowledgable about NSS would be appreciated.

Committing only to master until these get sufficient testing:

http://git.gnome.org/browse/evolution/commit/?id=0f474eaef3ac90f9dd5eb946bca8b36040f4c85e

http://git.gnome.org/browse/evolution-data-server/commit/?id=9116943ec239bed51e031c239895bb76edc9e2d4

I agree that ultimately we should integrate with gnome-keyring's new certificate and encryption key storage [1], but that's on file elsewhere so retitling this bug to reflect what the patches do and closing as fixed.


[1] http://live.gnome.org/GnomeKeyring/CertificatesKeys
Comment 12 David Woodhouse 2010-06-10 11:54:06 UTC
This should be using the system-wide shared database /etc/pki/nssdb when it's enabled; not making up a random new location.

And the 'user-wide shared location' if libnsssysinit.so isn't enabled would surely be ~/.pki/nssdb?
Comment 13 David Woodhouse 2010-06-10 12:55:39 UTC
Created attachment 163286 [details] [review]
Use system database where appropriate

This fixes it to use the system database in /etc/pki/nssdb when it's enabled.

We should also switch to ~/.pki/nssdb for the case when it's not (which is what Chrome uses, and what the system database setup will use for the user-specific part).
Comment 14 David Woodhouse 2010-06-10 12:57:01 UTC
Created attachment 163287 [details] [review]
Use camel_init() to initialize NSS in evolution

Without this, evolution will wrongly initialise NSS for itself and miss out on the fixes we've been making in camel_init().
Comment 15 David Woodhouse 2010-06-11 09:26:23 UTC
Eep, I just pushed the evolution part of this by accident -- does someone want to quickly approve it (it stands alone), or should I go ahead and revert it? Sorry!
Comment 16 Chenthill P 2010-06-11 11:57:10 UTC
I skimmed through https://bugzilla.redhat.com/show_bug.cgi?id=546221 and https://wiki.mozilla.org/NSS_Shared_DB_And_LINUX to get some information. It makes sense to use the system database. Will evolution be able to write changes to system database ?
Comment 17 David Woodhouse 2010-06-11 13:53:46 UTC
(In reply to comment #16)
> Will evolution be able to write changes to system database ?

Yes, it will. When you open the system database in sql:/etc/pki/nssdb, it automatically _also_ opens your user-specific database in ~/.pki/nssdb.

That's what the libnsssysinit.so plugin does, and that's why we test for that before we use the system db. If it's _not_ enabled, then we don't use the system database (and should probably use ~/.pki/nssdb instead).
Comment 18 Chenthill P 2010-06-11 14:35:24 UTC
Comment on attachment 163286 [details] [review]
Use system database where appropriate

cleared my doubts. Please commit the patches. thanks!!
Comment 19 Chenthill P 2010-06-11 14:35:41 UTC
Comment on attachment 163287 [details] [review]
Use camel_init() to initialize NSS in evolution

please commit the same.
Comment 20 Chenthill P 2010-06-11 14:39:46 UTC
Please commit it to gnome-2-30 as well if its tested well enough.
Comment 21 David Woodhouse 2010-06-11 14:50:45 UTC
Committing to 2.30 also involved backporting commits 9116943e, 3e78b5ce, 

HEAD: http://git.gnome.org/browse/evolution-data-server/commit/?id=bd704bff
2.30: http://git.gnome.org/browse/evolution-data-server/commit/?h=gnome-2-30&id=3a0d1cde

Committing to 2.30 also involved backporting commits 9116943e, 3e78b5ce, 157d2bb3.
Comment 22 David Woodhouse 2010-06-12 11:27:11 UTC
Created attachment 163464 [details] [review]
use correct db for non-system case too

(In reply to comment #13)
> We should also switch to ~/.pki/nssdb for the case when [the system db is not
> enabled] (which is what Chrome uses, and what the system database setup will 
> use for the user-specific part).

This does so, and should probably go in ASAP so people don't end up with an SQL DB in the wrong place.
Comment 23 Chenthill P 2010-06-14 10:00:44 UTC
Comment on attachment 163464 [details] [review]
use correct db for non-system case too

please commit the patch to master and gnome-2-30 branch..
Comment 24 David Woodhouse 2010-06-14 10:28:13 UTC
committed; thanks.
Comment 25 Milan Crha 2010-07-08 13:11:19 UTC
(In reply to comment #20)
> Please commit it to gnome-2-30 as well if its tested well enough.

Well, I'm not 100% sure, but it seems these changes broke things in 2.30.2. See a downstream bug report:
https://bugzilla.redhat.com/show_bug.cgi?id=612269

User cannot sign outgoing mails using S/MIME with 2.30.2, but downgrading to 2.30.1 works as expected. If in doubt, Fedora's packages don't contain any special patches in this area, it's basically the same as tarballs.

To copy&paste findings here:
a) certificate name changed, thus one needs to change his/her certificate chosen in account preferences, otherwise there is shown an error about "not able to find the certificate".

b) even when I select the right certificate, then it fails to sign with it, with an error "Failed to encode data".

Finally, I cannot import a certificate to MY store with a new version, as I guess I do not know the password for it. I saw there also my evolution certificates for the first run, its name had Evolution prefix, but I do not see them now, only if I downgrade to the previous version, to 2.30.1.
Comment 26 Milan Crha 2010-09-09 17:45:27 UTC
Pretty same issue with not saving trust information in bug #626066 and reverting these particular changes makes it work again.
Comment 27 Matthew Barnes 2011-02-12 15:07:26 UTC
As far as I can tell, this issue is resolved in 2.32.  Can this bug be closed now?

Moving it off the blocker list regardless.
Comment 28 Milan Crha 2011-02-24 09:02:19 UTC
And closing as well.