certificate trust flags don't persist properly in legacy database

NEW
Unassigned

Status

defect
5 years ago
5 years ago

People

(Reporter: keeler, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Posted file certdbtest.c (obsolete) —
STR:
* import a certificate (add it to the database permanently)
* set its trust flags to something like CERTDB_VALID_CA
* call CERT_DestroyCertificate on all in-memory copies (thus ensuring that the only information we have on it is in the database)
* re-import the same certificate
* set its trust flags to CERTDB_TERMINAL_RECORD
* CERT_DestroyCertificate again
* re-import the certificate

expected results: its trust flags should be CERTDB_TERMINAL_RECORD
actual results: its trust flags are CERTDB_VALID_CA

I've attached a proof-of-concept that should illustrate the issue.
Do we even allow you to re-import the same certificate? The database is keyed by issuer and serial number, and the "new" one is going to match.
[stupid bugzilla, stray focus submits early] I meant to suggest that you watch for errors (NSPR logging? Debug?) that indicate you're not really doing anything with the middle steps, resulting in perfectly understandable behavior.

You might have to explicitly remove it from the database between steps.
My understanding is if the exact same certificate is imported, NSS will essentially say, "Oh, yeah, I already know what that is" and re-load it from the internal database.

In any case, I think I've found what might explain this:

lg_SetTrustAttribute in lib/softoken/legacydb/lgattr.c does the following:

It looks up the current trust:

1660     cert = lg_getCert(obj, certHandle);
...
1665     dbTrust = *cert->trust;

then, depending on the type, it sets the flags:

1667     switch (attr->type) {
1668     case CKA_TRUST_EMAIL_PROTECTION:
1669 	dbTrust.emailFlags = flags |
1670 		(cert->trust->emailFlags & CERTDB_PRESERVE_TRUST_BITS);
1671 	break;
...

and then saves the result in the database:

1689     rv = nsslowcert_ChangeCertTrust(certHandle, cert, &dbTrust);

However, note line 1670. CERTDB_PRESERVE_TRUST_BITS is (CERTDB_USER | CERTDB_NS_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_INVISIBLE_CA | CERTDB_GOVT_APPROVED_CA). Thus, if one of those bits ever gets set, they can never be un-set (which is exactly the behavior I'm seeing).

Bob, from what I can tell, this is part of a large patch you wrote for bug 217538. Is this intended behavior? What's the right way to clear these flags if they've been set? Thanks.
Flags: needinfo?(rrelyea)
Hmm, so I know the CERTDB_VALID bit doesn't mean one most people think it means. It doesn't mean 'this cert is valid', it means, the trust record is valid (that is authoritative). That is why I renamed it to CERTDB_TERMINAL_RECORD.

Now the CERTDB_VALID_CA bit is different. NSS itself doesn't use it, I think PSM uses it to mark a certificate as a CA. 

> However, note line 1670. CERTDB_PRESERVE_TRUST_BITS is (CERTDB_USER | CERTDB_NS_TRUSTED_CA | 
> CERTDB_VALID_CA | CERTDB_INVISIBLE_CA | CERTDB_GOVT_APPROVED_CA).

So these don't have mappings through the PKCS #11 trust interface. 
CERTDB_USER is set based on finding the associated private key.
CERTDB_GOVT_APPROVED_CA is set based on a different PKCS #11 attribute. It's no longer used by NSS.
CERTDB_NS_TRUSTED_CA isn't used either.

I'm not sure if CERTDB_VALID_CA or CERTDB_INVISIBLE_CA are even stored anymore. I know NSS doesn't actually use them.

bob
Flags: needinfo?(rrelyea)
BTW the trust value inside softoken (for the legacy database) and the trust value on the cert are not the same. There is a translation process that goes on as the trust values are translated into PKCS #11 attributes. The mapping can be a bit convoluted because multiple pkcs #11 trust records from multiple different PKCS #11 modules are merged into a single trust value for the cert. One thing we are missing is access to administer the individual PKCS #11 attributes in the databases. Things like orphaned trust records are basically invisible until the certificate shows up. It might be a good idea to have an administrative interface to examine the actual trust records stored in the various databases/tokens.

bob
Thanks Bob. 

Could you please clarify a bit more regarding couple of things .

1. So is the behavior illustrated by the test a bug or was it designed that way ? What could be a possible fix?

2. Basically we want to clear the certdb_valid_ca flag so that we can push CA revocations without having them to be built-in/pre-compiled manually. Can it be reset without having to modify the CERTDB_PRESERVE_TRUST_BITS macro or the checks in the lg_SetTrustAttribute function ?
Flags: needinfo?(rrelyea)
Changed the macro so that CA certificate's flag changes persist. I ran the NSS test suite with this change and nearly all of them passed. The ones which failed were due to "selfserv process not detectable " , which i think is a known issue due to multiple threads accessing the same port.
Attachment #8468108 - Flags: review?(rrelyea)
Hpathak, I think you missed the point.  The 'trust' structure at the NSS level is not connected to the 'trust' structure at the softoken level. There isn't a mapping, so changing the macro of what winds up in the database (or more exactly what gets read out of it) will still not persist the bits you want to persist.

If the bits persist using the new database, then there is a mapping, and your change would be appropriate. If there isn't a mapping, then we need more code to add flags into the trust interface to express those bits.


bob
Flags: needinfo?(rrelyea)
Posted file certdbtest.c
Bob, here's a small standalone program that I think illustrates the issue. If you compile this (Makefile coming soon), create a database (`certutil -N -d .` with an empty password), and run it once, this is the output:

permanently deleting certificate to start clean.
attempting to get cert trust
CERT_GetCertTrust failed (maybe no record?): security library: bad database.
making cert permanent with no particular trust flags
read back flags 0 0 0
attempting to change cert trust to CERTDB_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_TERMINAL_RECORD(25) 0 0
read back flags 25 0 0
destroying certificate
recreating certificate
read back flags 25 0 0
attempting to change cert trust to CERTDB_VALID_CA | CERTDB_TERMINAL_RECORD(9) 0 0
read back flags 9 0 0
destroying certificate
recreating certificate
read back flags 9 0 0

This appears to work as expected (i.e. the fact that that cert trust has the flag CERTDB_TERMINAL_RECORD is persisted at the end). However, subsequent runs output the following:

permanently deleting certificate to start clean.
attempting to get cert trust
CERT_GetCertTrust failed (maybe no record?): security library: bad database.
making cert permanent with no particular trust flags
read back flags 0 0 0
attempting to change cert trust to CERTDB_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_TERMINAL_RECORD(25) 0 0
read back flags 25 0 0
destroying certificate
recreating certificate
read back flags 24 0 0
attempting to change cert trust to CERTDB_VALID_CA | CERTDB_TERMINAL_RECORD(9) 0 0
read back flags 9 0 0
destroying certificate
recreating certificate
read back flags 8 0 0

Note that the cert trust read back from the database does not include the CERTDB_TERMINAL_RECORD flag. The question is, is this expected behavior, or is there a bug? In general, how can we ensure than when we set the CERTDB_TERMINAL_RECORD flag on a certificate trust that it will still be there when we need it?
Thanks for having a look.
Attachment #8464322 - Attachment is obsolete: true
Attachment #8468108 - Attachment is obsolete: true
Attachment #8468108 - Flags: review?(rrelyea)
Flags: needinfo?(rrelyea)
If you run your test program with NSS_DEFAULT_DB set to 'sql', are the bits preserved?

bob
Flags: needinfo?(rrelyea)
Here's the output from using the sql database:

(first run)
permanently deleting certificate to start clean.
attempting to get cert trust
CERT_GetCertTrust failed (maybe no record?): Certificate extension not found.
making cert permanent with no particular trust flags
read back flags 0 0 0
attempting to change cert trust to CERTDB_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_TERMINAL_RECORD(25) 0 0
read back flags 25 0 0
destroying certificate
recreating certificate
read back flags 25 0 0
attempting to change cert trust to CERTDB_VALID_CA | CERTDB_TERMINAL_RECORD(9) 0 0
read back flags 9 0 0
destroying certificate
recreating certificate
read back flags 9 0 0

(subsequent runs)
permanently deleting certificate to start clean.
attempting to get cert trust
CERT_GetCertTrust failed (maybe no record?): Certificate extension not found.
making cert permanent with no particular trust flags
read back flags 0 0 0
attempting to change cert trust to CERTDB_TRUSTED_CA | CERTDB_VALID_CA | CERTDB_TERMINAL_RECORD(25) 0 0
read back flags 25 0 0
destroying certificate
recreating certificate
read back flags 25 0 0
attempting to change cert trust to CERTDB_VALID_CA | CERTDB_TERMINAL_RECORD(9) 0 0
read back flags 9 0 0
destroying certificate
recreating certificate
read back flags 1 0 0

So, while it's different, I'm not sure it's necessarily more correct (it does appear to preserve the terminal record bit, though).
You need to log in before you can comment on or make changes to this bug.