This bug was originally reported by Kai Engert in bug 406755 comment 37. He reports that he called CERT_PKIXVerifyCert passing it a set of one or more certificates to be used as explicit trust anchors, and a policy OID, and a cert from a particular server. He expected the function call to either a) succeed and return a chain that was anchored by one of the roots he supplied or b) fail Instead, he reports that it succeeded, but the root CA cert that it returned as being the matching trust anchor was NOT one of the certs he had passed in to the call, but rather was one of NSS's built-in trusted root certs. So, it appears that NSS did not use the caller-supplied list of anchors to the exclusion of all the other usual trust anchors, but merely in addition to the usual trust anchors. This is bad news for EV. I think we should try to reproduce this with vfychain or vfyserv. I will attempt to attach to this bug a zip file containing the certs sent by the server in the TLS handshake. I don't know exactly how many root CA certs Kai passed to CERT_PKIXVerifyCert, but I believe that one of them was the cert found at http://crt.comodoca.com/COMODOCertificationAuthority.crt and the EV policy OID used with it was 220.127.116.11.4.1.6418.104.22.168.5.1 Kai, can you tell us if there were any other certs passed as trust anchors?
Created attachment 309330 [details] zip file of the 5 certs sent by secure.comodo.com:443 In the attached zip file, the 5 certs sent by the server are in files named cert.001 ... cert.005. The server cert, in cert.001, bears the policy OID 22.214.171.124.4.1.64126.96.36.199.5.1 The file cert.005 appears to bear the (er, a) root CA cert.
The cert in file cert.005 is the root CA cert that (I believe) Kai supplied to CERT_PKIXVerifyCert in his test. I was able to reproduce the bug with this test command: vfychain -ppvv -o OID.188.8.131.52.4.1.64184.108.40.206.5.1 -u 1 \ cert.001 cert.002 cert.003 cert.004 cert.005 -t cert.005 Just as Kai reported in bug 406755 comment 37, the root cert returned by the call to CERT_PKIXVerifyCert was a cert from nssckbi, namely: Version: 3 (0x2) Serial Number: 44:be:0c:8b:50:00:21:b4:11:d3:2a:68:06:a9:ad:69 Signature Algorithm: PKCS #1 SHA-1 With RSA Encryption Issuer: "CN=UTN - DATACorp SGC,OU=http://www.usertrust.com,O=The USER TRUST Network,L=Salt Lake City,ST=UT,C=US" Validity: Not Before: Thu Jun 24 18:57:21 1999 Not After : Mon Jun 24 19:06:30 2019 Subject: "CN=UTN - DATACorp SGC,OU=http://www.usertrust.com,O=The USE RTRUST Network,L=Salt Lake City,ST=UT,C=US"
Created attachment 311894 [details] [diff] [review] Patch v1 Trust only certs from user defined trust anchor list if it was specified.
Comment on attachment 311894 [details] [diff] [review] Patch v1 This patch appears to make 3 unrelated changes (plus adding and removing a bunch of blank lines). One of the changes is the subject of this bug. r+ One fixes a double-free of an arena. r+ One changes the behavior of the output parameter cert_po_errorLog. r? I have a question about that errorLog change. Q: Is it true that, in case the cert chain is valid and no error occurs, this "errorlog" parameter outputs the validated chain? If not, is there any means defined whereby this function outputs the entire validated cert chain (or chains)? If so, is that implemented?
(in reply to comment #3) > Patch v1 Thanks Alexei and Nelson. I've just tested this patch with Firefox 3 CVS + NSS HEAD, and it does appear to resolve the issues I was concerned about in Bug #406755 Comments #33, #34 and #35. i.e. I now get the EV UI for https://secure.comodo.com with only the "COMODO Certification Authority" Root added locally to myTrustedEVInfos. (further to Bug #406755 Comment #39) > Kai, > I'm pretty sure that the issue with certs in the temp cert db is a real > issue... Nelson, Kai, I'm following this up on this point here, since Bug #406755 is now RESOLVED FIXED. With Alexei's patch + just the "COMODO Certification Authority" added locally to myTrustedEVInfos, I notice that (for https://secure.comodo.com) the Firefox Certificate Viewer still shows the certificate chain up to "AddTrust External CA Root", even though PSM/NSS must be using the chain up to "COMODO Certification Authority" to do the is-valid-for-EV check. Is this behaviour due to "the issue with certs in the temp cert db" ? Would Kai's patch "to do the EV evaluation in the auth-cert callback" change this behaviour, so that the same certificate chain is used in both places? This is a relatively minor issue for me, but am I interested to know what your plans are. Thanks.
In reply to Rob: Plan: - fix this bug - test again I appreciate your attempts to fix early. I attached the patch I used to PSM in bug 406755 comment 59, in case you want to do additional early testing! Thanks!
(In reply to comment #4) > (From update of attachment 311894 [details] [diff] [review]) > One changes the behavior of the output parameter cert_po_errorLog. r? > > I have a question about that errorLog change. > Q: Is it true that, in case the cert chain is valid and no error > occurs, this "errorlog" parameter outputs the validated chain? This is not true. errorlog outputs a list of error that occurred during validation, the errors that describe why processed certs have not made into the final chain. There is no way to control what gets into errorlog except that a caller can set it to null that outputs no errorlog at all.
(In reply to comment #7) > (In reply to comment #4) > > (From update of attachment 311894 [details] [diff] [review] [details]) > > One changes the behavior of the output parameter cert_po_errorLog. r? > > > > I have a question about that errorLog change. > > Q: Is it true that, in case the cert chain is valid and no error > > occurs, this "errorlog" parameter outputs the validated chain? Sorry, didn't get to the point of the question from the first time :) No, errorLog is not used to pass valid chains. We have another entry (cert_po_certList) in output array that is used for valid chains. Please check attachment 311690 [details] [diff] [review].
patch is integrated.
Bob, Nelson, I just reread Rob's comment (from comment 5). He says, Alexei's patch already fixed the issue! It appears it's no longer necessary to use a PSM patch to keep around certs in the temporary DB for a longer time? Rob, sorry I misread your comment 5 earlier today. (In reply to comment #5) > With Alexei's patch + just the "COMODO Certification Authority" added locally > to myTrustedEVInfos, I notice that (for https://secure.comodo.com) the > Firefox Certificate Viewer still shows the certificate chain up to "AddTrust > External CA Root", even though PSM/NSS must be using the chain up to "COMODO > Certification Authority" to do the is-valid-for-EV check. > > Is this behaviour due to "the issue with certs in the temp cert db" ? No. It's because we still use the classic NSS functionality to find the chain that will be presented in the UI. > Would Kai's patch "to do the EV evaluation in the auth-cert callback" change > this behaviour, so that the same certificate chain is used in both places? No. We'd have to change PSM to use a new API to give us the full chain. Unfortunately, the functionality in NSS that would give out a full chain as a result of a policy verification has not yet been implemented.
Alexei, thanks a lot for your patch and checking it in. Should this bug be marked fixed?
I suspect that Rob now has the intermediate CA certs in his cert DB, and so no longer sees the problem. I suspect that if the experiment is tried again, with a clean/fresh cert DB, the problem will be reproduced. The fundamental issue hasn't changed. Certs sent by the server, but unused in the non-pkix cert validation, will not be around after the SSL handshake is completed, and hence will not be available to complete a cert chain to a different root than the root used during the handshake, UNLESS they are also in the cert DB.
(in reply to common #12) > I suspect that Rob now has the intermediate CA certs in his cert DB, and > so no longer sees the problem. Nelson, I don't think this is the case. At first, the Firefox Certificate Store Viewer doesn't show me any of the relevant Intermediate CA Certificates. Neither does "nsscertutil -L -d ~/.mozilla/firefox/5k7nlhgc.default/". Then I browse to https://secure.comodo.com and I see the EV UI. After that, the AddTrustExternalCARoot->COMODOEVSGCCA Intermediate CA Certificate (which is *not* part of a trusted-for-EV chain) appears in the Certificate Store Viewer and in the output from nsscertutil. I see no sign of the COMODOCertificationAuthority->COMODOEVSGCCA Intermediate CA Certificate (which *is* part of the only relevant trusted-for-EV chain in my local build) being added to the cert db. Am I looking in the right place?
(In reply to comment #12) > The fundamental issue hasn't changed. Certs sent by the server, but unused > in the non-pkix cert validation, will not be around after the SSL handshake > is completed, and hence will not be available to complete a cert chain > to a different root than the root used during the handshake, UNLESS they > are also in the cert DB. > If cert chain was cached, all certs are still going to be in temporary database.
As I understand Rob Stradling's explanation in bug 406755 comment 33, this server is now sending out one server cert followed by 3 different intermediate CA certs, each of which is a "cross issued" cert for the CA that issued the server cert. Each of those intermediate CA certs was issued by a different root. Pictorially, the 4 certs (A, B, C, D) are: +-->B (--> root Rb) / A-+---->C (--> root Rc) \ +-->D (--> root Rd) During the first validation (non-libPKIX) we validate one path, say, A -> B -> Rb which is NOT a path to an EV Root. The certs in that path are held in the "temp cert DB" by PSM, because PSM holds references to those certs. So, when the handshake is over, certs A and B are kept, but certs C and D are discarded. Then later, in the second validation (using libPKIX) for EV, we want to construct and validate a path to (say) root Rc, which is an EV root, but since cert C has been discarded and is no longer available, the path from A to Rc can no longer be constructed, and so the EV validation fails. One proposed solution is to cause the EV validation to take place at essentially the same time as the non-EV validation, that is, before the handshake is over, and before any of the certs sent by the server are discarded. Now, it appears that the second (EV) validation is succeeding, despite the fact that certs C and D were previously discarded. I think the only possible explanations are: 1) For some reason not yet known, the first validation did use the path A->C->Rc, so those certs were not discarded, and the discarded certs were B and D. This raises the question: why did that change? Is that server now sending out a different group of certs than it sent when I investigated bug 406755? 2) Certs C and D were not discarded when they should have been, that is, were leaked, a bug. 3) libPKIX fetched the previously-discarded cert C using the AIA URL. That would be both good news and bad news. Good: cert fetching via AIA works. Bad: we're fetching a cert that we've had in our posession in the last few milliseconds. :( 4) Some other web page fetch validated the chain A->C->Rc causing it to be cached in memory by the time this test was done.
I see that the certificates received from the peer in the Certificate message are not discarded until the SSL connection is over, and the socket is closed. They are not discarded as soon as the handshake is done. So, if a socket & connection stay open (keep alive) for a long time, their certs might stay in the temp cert DB for a long time. But I think our EV logic should not depend on the server using keep-alive for the outer-most page connection.
I declare this bug fixed. :) The issue that we've been discussing in comment 12 - comment 16 above is really the subject of bug 406755, which I will reopen and add comments.