Open Bug 594297 Opened 15 years ago Updated 2 years ago

softoken imports certs with same nickname but different subjects, corrupts DB

Categories

(NSS :: Libraries, defect, P3)

3.12.7

Tracking

(Not tracked)

3.12.9

People

(Reporter: brauckmann, Unassigned)

Details

Attachments

(6 files, 2 obsolete files)

User-Agent: Mozilla/5.0 (X11; U; Linux i686; de; rv:1.9.1.11) Gecko/20100714 SUSE/3.5.11-0.1.1 Firefox/3.5.11 Build Identifier: nss-3.12.7 I have a certificate database with two certificates. Both have nickname "test@example.com" and contain an email address "test@example.com". The certificates have different Subject-DNs. When calling certuil with -L, both certificates are displayed. When calling certutil with certutil -L -n test@example.com, just the first certificate is displayed. Reproducible: Always Steps to Reproduce: 1. certutil -L -d testdb 2. certutil -L -d testdb -n test@example.com Actual Results: Just the first certificate is displayed Expected Results: Both should be displayed The problem seems to be certutils strategy to retrieve all certificates for a given nickname/email address: In mozilla/security/nss/cms/certutil.c:470, one certificate with matching nickname/email address is retrieved. Some lines later, all certificates with a subject-dn matching the subject-dn of the first certificates are retrieved. This is wrong, as there may be more certificates with identical email address but different subject dn. Unfortunately, the same strategy is used by thunderbird. This strategy can prevent thunderbird from finding a valid recipient certificate for email encryption.
Version: unspecified → 3.12.7
One root certificate and two email certificates for test@example.com. Nickname test@example.com. The two email certificates have a different subject-dn.
The situation described in the first paragraph of comment 0 is the very definition of a "corrupt database". In a proper database, ALL the certificates that share a nickname also shared a common subject DN. An attempt to add a certificate with a different subject DN but the same nickname as an existing cert in the DB should have any of the following outcomes: - failure - cert added with a different nickname than the one requested, one that is either: -- new and unique (if there are no other certs with the same subject), or -- common to other certificates with the same subject DN as the one added. If you can find a series of steps that take a non-corrupt database and add a certificate to it, making it be corrupt, that is, making it have two certs with the same nickname but different subject DNs, and can add those steps-to-reproduce to this bug, that would be VERY helpful. I don't doubt that they exist. I've just never found them.
Attached file Test root certificate
Creating a database with certificates with equal nicknames but different subject DNs is actually quite easy: - Use Firefox or Thunderbird - Import the certificates "Test root certificate", "Test user certificate 1" and "Test user certificate 2" with the certificate manager of Firefox/Thunderbird - Thats all. After that, the cert8.db of that application contains two certificates with nickname test@example.com but with different subject DN (CN=Joe Test,OU=Org2,O=Test,C=DE and CN=Joe Test5,OU=Org1,O=Test2,C=US) Tested with: Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.2.9) Gecko/20100825 Thunderb ird/3.1.3 Mozilla/5.0 (X11; U; Linux i686; de; rv:1.9.1.11) Gecko/20100714 SUSE/3.5.11-0.1.1 Firefox/3.5.11 It works with either an empty cert8.db or with an cert8.db that was already in use for several years. Thunderbird/Firefox always sets the email-address of a certificate as its nickname, even if there already exists a certificate with same nickname but different subject DN.
Hm, certutil uses a different way of importing a certificate than other Mozilla apps. certutil.c, function AddCerts(), approx. line 187: certutil uses PK11_ImportCert(), which expects a nickname and seems to complain if the nickname is recycled with a certificate with different SubjectDN. Other mozilla apps seem to use the code from mozilla/security/manager/ssl/src/nsNSSCertificateDB.cpp, method nsNSSCertificateDB::ImportEmailCertificate(). This method uses CERT_ImportCerts(), which does not take a nickname as parameter and which seems to "compute" the nickname from the emailaddress. I have modified certutil to use CERT_ImportCerts(), and it always adds certificates with their email addresses as nickname, regardless whether this nickname already exists or not. I can create the same type of corrupt database with the modified certutil that Firefox or Thunderbird creates when importing the certificates in comment 7.
Thanks very much for that research, Jürgen, especially for finding out how mozilla apps do it differently from certutil. It should be the case that, ultimately, both PK11_ImportCert and CERT_ImportCerts use the same function in lib/softoken to actually import the cert. It would be a function called through a pointer, named NSC_<something>, such as NSC_CreateObject or NSC_SetAttributeValue. That function, and the functions it calls, are the code ultimately responsible for ensuring that the rules about nicknames (which are encoded as CKA_LABEL attributes) and subject names are followed. Apparently, under some circumstances, that code fails to do its job, and you have now provided a very simple test case, so it should not be too difficult to "get to the bottom" of this problem now.
Jürgen, Please attach your patch to certutil, so that I can use it to reproduce your results without needing to use Thunderbird or Firefox. Thanks.
I'm elevating this to P1, in anticipation of its confirmation. It will be confirmed when an NSS team member can reproduce with a caller of CERT_ImportCerts.
Component: Tools → Libraries
Priority: -- → P1
QA Contact: tools → libraries
Summary: certutil does not list all matching certificates with -n → softoken imports certs with same nickname but different subjects
Target Milestone: --- → 3.12.9
Bob, I think you should take a look at this. Very serious.
Assignee: nobody → rrelyea
Summary: softoken imports certs with same nickname but different subjects → softoken imports certs with same nickname but different subjects, corrupts DB
Attachment #473958 - Attachment description: Modified certutil.c to use CERT_ImportCerts instead of PK11_ImportCert → Modified certutil.c to use CERT_ImportCerts instead of PK11_ImportCert, based on NSS-3.12.7 source tar ball
Just another remark: My original reason for opening this bug was a situation where thunderbird was not able to find a valid email receipient certificate in a certdb. My hope was that it was able to reproduce a very similar situation with certutil, which seems much easier do debug than Thunderbird, thus this bug report against certutil The certdb that triggered the situation contained two certificates with identical email addresses, identical nicknames, and different subject DNs. One certificate was expired, the other still valid. Thunderbird did find only the first, expired certificate and refused mail encryption (naturally). Currently, this bug tries to resolve the issue with the identical nicknams. I don't think that thats the main reason for my original problem. I think that there is a second issue, either in NSS or in Thunderbird. Thunderbird and certutil use the same strategy for searching certificates. They both do the following (certutil.c listCerts(), mozilla/security/manager/ssl/src/nsNSSCertificateDB.cpp nsNSSCertificateDB::FindCertByEmailAddress()): the_cert = CERT_FindCertByNicknameOrEmailAddr( nickname/emailadress); certs = CERT_CreateSubjectCertList(NULL, handle, &the_cert->derSubject, PR_Now(), ...); The certutil man page documents that certutil -A -n < nickname> only finds certificates with matching nickname, but in fact the code also allows for finding certs with matching emailadress (independent of the nickname). Thunderbird uses this code for finding valid certificates with a matching email-address for email encryption. This strategy does only work for Thunderbird if CERT_FindCertByNicknameOrEmailAddr() only returns valid certificates: If the first certificate that is found is invalid, and there are more certificates for the email address that are valid but have a different subject-DN, they are not found. But CERT_FindCertByNicknameOrEmailAddr() also returns invalid certificates (verified with certutil). So either CERT_FindCertByNicknameOrEmailAddr() has a bug, or thunderbird makes invalid assumptions about CERT_FindCertByNicknameOrEmailAddr(). If this is not an NSS bug: Which Component/Product does mozilla/security/manager/ssl/ belong to? Thanks for your patience :-)
(In reply to comment #14) > But CERT_FindCertByNicknameOrEmailAddr() also returns invalid certificates > (verified with certutil). So either CERT_FindCertByNicknameOrEmailAddr() has a > bug, or thunderbird makes invalid assumptions about > CERT_FindCertByNicknameOrEmailAddr(). > > If this is not an NSS bug: Which Component/Product does > mozilla/security/manager/ssl/ belong to? Core / Security: PSM (or for this specific issue, Security: SMIME). See bug 558337 for another problem with nsNSSCertificateDB::FindCertByEmailAddress(). And greetings to Hamburg, BTW ;-)
> such as NSC_CreateObject or > NSC_SetAttributeValue. That function, and the functions it calls, are the > code ultimately responsible for ensuring that the rules about nicknames > (which are encoded as CKA_LABEL attributes) and subject names are followed. > Apparently, under some circumstances, that code fails to do its job, and you > have now provided a very simple test case, so it should not be too difficult > to "get to the bottom" of this problem now. Actually no, The upperlevel of NSS needs to enforce the guarantees. PKCS #11 modules are not required to have this semantic, so we need to make sure it's enforces at the NSS level. (Fixing softoken only fixes it part of the time). One wonders how this can happen the DBM level itself. Nicknames are stored in the cert db as keys that contain subject entries. We can't physically store more than one nickname for a cert > My original reason for opening this bug was a situation where thunderbird was > not able to find a valid email receipient certificate in a certdb. This is the real problem that needs to be fixed. Event though nicknames map one-for-one in Subjects, email addresses are allowed to map to more than one certificate. If thunderbird can't find them with the existing calls, We need to provide new ways to find them. bob
(In reply to comment #15) > Core / Security: PSM (or for this specific issue, Security: SMIME). See bug > 558337 for another problem with nsNSSCertificateDB::FindCertByEmailAddress(). Thanks Kaspar for the pointer to that bug! The issue I'm describing really seems to be just another symptom of what is already reported in bug 558337 or bug 337430 (dating back to 2006). I think that bug 337433 could solve the issue if the proposed CERT_FindCertByNicknameOrEmailAddrForUsage() only returns valid certificates. Otherwise Thunderbird would still not be able to find valid certificates if an email recipient has invalid and valid certificates in the certdb. > And greetings to Hamburg, BTW ;-) Greetings back!
(In reply to comment #17) > I think that bug 337433 could solve the issue if the proposed > CERT_FindCertByNicknameOrEmailAddrForUsage() only returns valid certificates. Correction: I think that bug 337433 could solve the issue if the proposed CERT_FindCertByNicknameOrEmailAddrForUsage() always returns a valid certificates if there is a mixture of expired and valid certificates for a given emailaddress/nickname n the database.
(In reply to comment #18) > if the proposed > CERT_FindCertByNicknameOrEmailAddrForUsage() always returns a valid > certificates if there is a mixture of expired and valid certificates for a > given emailaddress/nickname n the database. With the implementation proposed in attachment 467521 [details] [diff] [review], this would indeed be the case. In theory, the code which tries to figure out the best cert from a list of candidate certs is already in place (and the NSSCryptoContext_FindBestCertificateBy* functions will prefer valid over expired certs, and certs with a later notAfter date over others - see http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/pki/pkibase.c&rev=1.33&mark=518-539#515 and http://mxr.mozilla.org/mozilla/ident?i=CERT_IsNewer). However, my guess is that many of these problems would go away if there was actually an implementation of CERT_FindCertByEmailAddr, and nsNSSCertificateDB::FindCertByEmailAddress could use that function (instead of CERT_FindCertByNicknameOrEmailAddr). When looking for an e-mail cert, the nickname is more or less irrelevant - and in certain situations it even has adverse effects on the search results, as this bug illustrates. (In reply to comment #16) > If thunderbird can't find them with the existing calls, We need to > provide new ways to find them. I would therefore favor a smart implementation of CERT_FindCertByEmailAddress (http://mxr.mozilla.org/mozilla/ident?i=CERT_FindCertByEmailAddr - even its signature can still be changed, e.g. by adding a keyUsage argument, as it hasn't been exported yet). Adding Kai to the Cc, as he has spent quite some time on code in this area in the past.
OS: Linux → All
Hardware: x86 → All
This bug report seems to have touched on several separate and distinct issues. Separate bug reports should be filed for each one. They include 1) Softoken should not allow multiple certs with different subject names to share a nickname (this bug). 2) Higher layers of NSS, outside of (above) softoken, should also enforce this, (See comment 16). 3) NSS should provide a way to find ALL valid certs for an email address (also from comment 16). 4) PSM should make use of that method to find email certs. Please file separate bugs for items 2, 3 and 4. Thanks. In comments below, I will specifically be addressing the first of these issues.
Attachment #473958 - Attachment mime type: text/x-csrc → text/plain
Attached patch certutil modification as a patch (obsolete) — Splinter Review
I converted the above attached modified certutil.c source file into a patch against the NSS_3_12_7_RTM tag, in hopes that it would apply to the tip of the trunk. Here is that patch. I have not yet tested it.
Jürgen, what are the exact certutil commands you use to add the three test certs to a test DB to reproduce this problem?
I ran NSS's database checker program on the database attached to this bug (first attachment), and it confirmed that the DB is self-inconsistent (which we call "corrupt"). Here's a bit of background on the Berkeley (cert7, cert8) DB schema. The DB has effectively 4 different tables. Each table has a single column taht is the "unique key" or index for that table, and one or more other data columns. Each table is named after the index column/field for that table. They are listed below. The information given here is not complete in every detail. DER Issuer Name and Serial Number table: Index: DER Issuer Name and Serial Number of a cert Data fields: DER cert (entire cert) DER Subject Name table index DER Subject Name table: Index: DER Subject Name: Data fields: 1) A list of (DER Issuer Name and Serial Number) table index values 2) a nickname table index OR SMIME table index value Nickname table Index: UTF8 Nickname Data Field: DER Subject Name SMIME table: (also sometimes called the email address table): Index: RFC 822 email address: Data Fields: 1) DER subject Name 2) S/MIME capabilities structure from a signed message Every Subject Name record must have zero or one nickname table records pointing to it, and zero or more email address records pointing to it. It must have AT LEAST ONE nickname or email address record pointing to it. Originally, it was the intent that a subject name record would always have exactly one record pointing to it, either a nickname OR an email address record. But I have reason to believe that we have not prevented a subject name record from having both at once. A nickname record can have an RFC 822 email address as the nickname. This may be confusing. For most purpsoes, SMIME records are treated just like nicknames. Most searches by nickname actually search both the nickname table and the SMIME table for a match. Which each table has at most one entry of any value, it is possible for both tables to have an entry with the same value, which is an RFC 822 email address. In the case of the DB attached above, there are 3 IS+SN records, 3 subject name records, one nickname record and one SMIME record. Visually it is: Cert 0 ----> Subject 0 ----> Nickname 0 Cert 1 ----> Subject 2 ----> S/MIME 0 Cert 2 ----> Subject 1 ----> **********
Here's a MUCH smaller patch to certutil that, I believe, produces the same series of database updates as the previous patch did. It only changes the behavior of certutil's -E command. The commands I used were: mkdir 594297 certutil -N -d 594297 certutil -A -d 594297 -n "Test-PKI Root" -t ",C," -a -i /tmp/594297c.pem certutil -E -d 594297 -n test@example.com -t "" -a -i /tmp/594297d.pem certutil -E -d 594297 -n test@example.com -t "" -a -i /tmp/594297e.pem where the c, d and e filres are the 3 certificates given as attachments above. The result was a set of files that has similar properties as reported above: 3 certs, 3 subject records, 1 nickname and 1 SMIME record, and one cert and subject record for which there was no SMIME record. But I'm not sure I reproduced exactly the original report, because in my case the orphaned cert was the last one added, and as I understand it, in the reported case, the orphaned cert was the second one, not the last one, added.
Attachment #473958 - Attachment is obsolete: true
Attachment #474579 - Attachment is obsolete: true
I began debugging the execution of the last of the above certutil commands. Suspicious observation #1) At this point in the stack: __CERT_AddTempCertToPerm(cert=0x0c90010, nickname=0x0, trust=0x0) Line 307 CERT_AddTempCertToPerm(cert=0x0c90010, nickname=0x0, trust=0x0) Line 358 CERT_ImportCerts(certdb=0x0c78018, usage=certUsageEmailRecipient, ncerts=0x01, derCerts=0x012fe24, retCerts=0x0, keepCerts=0x01, caOnly=0x0, nickname=0x0) Line 2578 AddCert(slot=0x0c05400, handle=0x0c78018, name=0x0c19680, trusts=0x0c260f6, inFile=0x0c19640, ascii=0x01, emailcert=0x01, pwdata=0x012fef4) Line 184 certutil_main(argc=0x0b, argv=0x0c26060, initialize=0x01) Line 2869 main(argc=0x0b, argv=0x0c26060) Line 3026 Immediately following these lines of code: if (!stanNick && nickname) { stanNick = nssUTF8_Duplicate((NSSUTF8 *)nickname, c->object.arena); } stanNick is NULL. :-/
At this point in the stack: sftkdb_checkConflicts(db=0x0c1e150, objectType=0x01, ptemplate=0x0c89010, len=0x0c, sourceID=0x0) Line 922 sftkdb_write(handle=0x0c1a380, object=0x0c92000, objectID=0x0c9200c) Line 1204 sftk_handleCertObject(session=0x0c1e4c0, object=0x0c92000) Line 721 sftk_handleObject(object=0x0c92000, session=0x0c1e4c0) Line 1503 NSC_CreateObject(hSession=0x01000003, pTemplate=0x012fcb8, ulCount=0x0a, phObject=0x012fc6c) Line 3838 import_object(tok=0x0c54018, sessionOpt=0x0, objectTemplate=0x012fcb8, otsize=0x0a) Line 248 nssToken_ImportCertificate(tok=0x0c54018, sessionOpt=0x0, certType=NSSCertificateType_PKIX, id=0x0c8a070, nickname=0x0, encoding=0x0c8a078, issuer=0x0c8a080, subject=0x0c8a088, serial=0x0c8a090, email=0x0c91070, asTokenObject=0x01) Line 588 __CERT_AddTempCertToPerm(cert=0x0c90010, nickname=0x0, trust=0x0) Line 323 CERT_AddTempCertToPerm(cert=0x0c90010, nickname=0x0, trust=0x0) Line 358 CERT_ImportCerts(certdb=0x0c78018, usage=certUsageEmailRecipient, ncerts=0x01, derCerts=0x012fe24, retCerts=0x0, keepCerts=0x01, caOnly=0x0, nickname=0x0) Line 2578 AddCert(slot=0x0c05400, handle=0x0c78018, name=0x0c19680, trusts=0x0c260f6, inFile=0x0c19640, ascii=0x01, emailcert=0x01, pwdata=0x012fef4) Line 184 certutil_main(argc=0x0b, argv=0x0c26060, initialize=0x01) Line 2869 main(argc=0x0b, argv=0x0c26060) Line 3026 We read this comment: /* Currently the only conflict is with nicknames pointing to the same * subject when creating or modifying a certificate. */ I think SMIME address conflicts may have been overlooked.
Here's another place where we have a nickname conflict check, and no corresponding SMIME entry conflict check. nsslowcert_CertNicknameConflict(nickname=0x0, derSubject=0x0c0f038, handle=0x0c3da40) Line 3977 nsslowcert_UpdatePermCert(dbhandle=0x0c3da40, cert=0x0c0f000, nickname=0x0, trust=0x012fb28) Line 4569 nsslowcert_AddPermCert(dbhandle=0x0c3da40, cert=0x0c0f000, nickname=0x0, trust=0x012fb28) Line 4605 lg_createCertObject(sdb=0x0c1e150, handle=0x0c9200c, templ=0x0c89010, count=0x0c) Line 130 lg_CreateObject(sdb=0x0c1e150, handle=0x0c9200c, templ=0x0c89010, count=0x0c) Line 988 sftkdb_CreateObject(arena=0x0c1d880, handle=0x0c1a380, db=0x0c1e150, objectID=0x0c9200c, template=0x0c89010, count=0x0c) Line 614 sftkdb_write(handle=0x0c1a380, object=0x0c92000, objectID=0x0c9200c) Line 1214 sftk_handleCertObject(session=0x0c1e4c0, object=0x0c92000) Line 721 sftk_handleObject(object=0x0c92000, session=0x0c1e4c0) Line 1503 NSC_CreateObject(hSession=0x01000003, pTemplate=0x012fcb8, ulCount=0x0a, phObject=0x012fc6c) Line 3838 import_object(tok=0x0c54018, sessionOpt=0x0, objectTemplate=0x012fcb8, otsize=0x0a) Line 248 nssToken_ImportCertificate(tok=0x0c54018, sessionOpt=0x0, certType=NSSCertificateType_PKIX, id=0x0c8a070, nickname=0x0, encoding=0x0c8a078, issuer=0x0c8a080, subject=0x0c8a088, serial=0x0c8a090, email=0x0c91070, asTokenObject=0x01) Line 588 __CERT_AddTempCertToPerm(cert=0x0c90010, nickname=0x0, trust=0x0) Line 323 CERT_AddTempCertToPerm(cert=0x0c90010, nickname=0x0, trust=0x0) Line 358 CERT_ImportCerts(certdb=0x0c78018, usage=certUsageEmailRecipient, ncerts=0x01, derCerts=0x012fe24, retCerts=0x0, keepCerts=0x01, caOnly=0x0, nickname=0x0) Line 2578 AddCert(slot=0x0c05400, handle=0x0c78018, name=0x0c19680, trusts=0x0c260f6, inFile=0x0c19640, ascii=0x01, emailcert=0x01, pwdata=0x012fef4) Line 184 certutil_main(argc=0x0b, argv=0x0c26060, initialize=0x01) Line 2869 main(argc=0x0b, argv=0x0c26060) Line 3026
There's a block of code in pcertdb.c after a (false) comment that reads; /* make a new subject entry - this case is only used when updating * an old version of the database. This is OK because the oldnickname * db format didn't allow multiple certs with the same subject. Perhaps it should not allow subject entries to be written with NULL nicknames.
In function lg_createCertObject (see stack above), at about lines 143-158, having just added the cert to the IN+SN and Subject Name tables, the code looks in the DB for an existing SMIME record for the email address in the cert. If it finds a matching SMIME record, it does not update the SMIME record (which is probably a good thing), but the result is that the newly created IN+SN and Subject Name table entries are orphans at birth.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to comment #22) > Jürgen, what are the exact certutil commands you use to add the three test > certs to a test DB to reproduce this problem? Attachment 472980 [details] was created by thunderbird, not certutil. I deleted the original cert8.db and key.db from the thunderbird profile directory, and used thunderbirds certificate manager to manually import attachment 473425 [details], attachment 473427 [details] and attachment 473428 [details]. My experiments with the modified certutil were conducted with the same series of steps that you did: certutil -N certutil -A -d db -n "Test-PKI Root" -t ",C," -a -i /tmp/594297c.pem certutil -E -d db -n test@example.com -t "" -a -i /tmp/594297d.pem certutil -E -d db -n test@example.com -t "" -a -i /tmp/594297e.pem Your patch and my modification give different binary representations of cert8.db, but I think that is due to my modification using CERT_ImportCerts(handle, certUsageEmailRecipient,...) for the root certificate as well. So that is probably not relevant.
Thanks Nelson for structuring this bug! (In reply to comment #20) > This bug report seems to have touched on several separate and distinct issues. > Separate bug reports should be filed for each one. They include > 1) Softoken should not allow multiple certs with different subject names to > share a nickname (this bug). > 2) Higher layers of NSS, outside of (above) softoken, should also enforce > this, (See comment 16). I don't know how to describe the problem sufficiently to create a bug report, so you should not expect me to do that. :-) > 3) NSS should provide a way to find ALL valid certs for an email address > (also from comment 16). I created Bug 596215 > 4) PSM should make use of that method to find email certs. I created Bug 596221 > Please file separate bugs for items 2, 3 and 4. Thanks.
Severity: normal → S3
Assignee: rrelyea → nobody
Priority: P1 → P3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: