Note: There are a few cases of duplicates in user autocompletion which are being worked on.

nsNSSCertificateDB::DeleteCertificate has race conditions

RESOLVED DUPLICATE of bug 1267861

Status

()

Core
Security: PSM
RESOLVED DUPLICATE of bug 1267861
9 years ago
a year ago

People

(Reporter: Mike Perry, Unassigned)

Tracking

({privacy})

unspecified
privacy
Points:
---

Firefox Tracking Flags

(blocking2.0 -, blocking1.9.2 -, status1.9.2 wanted, blocking1.9.1 -, status1.9.1 wanted)

Details

(Whiteboard: [psm-fatal])

(Reporter)

Description

9 years ago
I'm implementing SSL certificate isolation for Torbutton, where client, server, and CA SSL certificates for different Tor states are written out to disk and kept separate to minimize the ability of websites and exit nodes from probing to see if a client has a certificate for a certain corporation, website, or client certificate installed.

The code works like this:
1. Go through the nsICertTree tree views and record all the user-added ssl certs for each cert type.
2. Write these out to disk using the BER binary format to a file for this tor state.
3. call nsIX509CertDB::deleteCertificate() on each one
4. Import in certificates saved in the opposite tor state from that BER file on disk

I've managed to get it working in the general case, but it appears that there is a race condition on certificate deletion. It appears as if nsIX509Cert::deleteCertificate doesn't actually delete the certificates from the DB, but only lazily marks them for later deletion. This is actually fine, but the problem is that when the same certs are re-inserted before the deletion actually takes place (like for example, if the user toggles tor back and forth real fast, or if they have the same cert in both jars), then the whole process fails due to the duplicate cert. Worse still, this duplicate cert is then later deleted because it is still marked for deletion.

A fix for this would be a check during each of the nsNSSCertificateDB::Import* functions to see if the cert already exists but has the nsNSSCertificate::mPermDelete flag set. If so, these functions should merely clear the flag instead of complaining that the cert already exists.

Furthermore, iterations over all the current certs in a nsICertTree treeView should skip certs with this mPermDelete flag set (looks like this check should be done in nsCertTree::GetCertsByTypeFromCertList).

Updated

7 years ago
Flags: blocking1.9.0.19?

Updated

7 years ago
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Keywords: privacy
OS: Windows XP → All
Hardware: x86 → All

Updated

7 years ago
Flags: blocking1.9.0.19?
Version: 2.0 Branch → unspecified
Assignee: nobody → kaie
Component: File Handling → Security: PSM
Product: Firefox → Core
QA Contact: file.handling → psm
This isn't going to block one of our security/stability branch releases, but it is worth considering once it's fixed on the trunk
blocking1.9.1: ? → -
blocking1.9.2: ? → -
status1.9.1: --- → wanted
status1.9.2: --- → wanted

Updated

7 years ago
Assignee: kaie → nobody
Whiteboard: [psm-fatal]
blocking2.0: ? → -

Comment 2

6 years ago
The Tor Project / Electronic Frontier Foundation is paying to have this bug fixed.

"If you know C++ and/or Firefox internals, we should be able to pay you for your time to address these issues and shepherd the relevant patches through Mozilla's review process."

Source: https://blog.torproject.org/blog/web-developers-and-firefox-hackers-help-us-firefox-4

Updated

a year ago
Duplicate of this bug: 1175124

Updated

a year ago
Duplicate of this bug: 873814

Updated

a year ago
Duplicate of this bug: 876971

Updated

a year ago
Duplicate of this bug: 931346
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1267861
You need to log in before you can comment on or make changes to this bug.