Closed Bug 475454 Opened 16 years ago Closed 16 years ago

vfychain requireFreshInfo incorrectly claims "revoked"

Categories

(NSS :: Libraries, defect)

3.12.3
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
3.12.3

People

(Reporter: KaiE, Assigned: alvolkov.bgs)

Details

(Whiteboard: [bug description see comment 9] PKIX)

Attachments

(11 files)

I will attach all the required files to reproduce this bug.
Attached file intermediate cert 1
This cert issued the above attachment.
Attached file intermediate cert 2
this intermediate issued the above attachment
this cert issued intermediate cert 2 either import this root into your database explicitly, with trust set to TC,C,C or make sure the NSS roots module gets loaded while you perform the tests
Attached file CRL for leaf cert
That's the binary CRL which is listed in the server cert, downloaded from http://crl.globalsign.net/ExtendVal1.crl
pp -a -i addons.mozilla.org -t certificate | grep -A1 -iw serial Serial Number: 01:00:00:00:00:01:1c:7b:96:3a:0b crlutil -L -d . -n globev |grep -i 01:00:00:00:00:01:1c:7b:96:3a:0b The cert's serial number is NOT contained in the CRL.
Note I'm using NSS_DEFAULT_DB_TYPE="sql" that's why this zip file has key4.db and cert9.db I have imported both intermediates and the root cert, and also the crl into this database. $ certutil -d . -L Certificate Nickname Trust Attributes SSL,S/MIME,JAR/XPI globev ,, glob ,, globroot CT,C,C $ crlutil -d . -L CRL names CRL Type globev CRL
This comment is about a SUCCESS (not a bug), for comparison. Using the above database, I ran the following, which gives me the expected good result: $ vfychain -d . -f -pp -a -u 1 -v -g leaf -m crl addons.mozilla.org Chain is good! Root Certificate Subject:: "CN=GlobalSign Root CA,OU=Root CA,O=GlobalSign nv- sa,C=BE" Certificate 1 Subject: "CN=addons.mozilla.org,O=Mozilla Corporation,OU=Mozill a Add-ons,OID.2.5.4.9=1981 Landings Dr.,L=Mountain View,ST=California,C=U S,OID.1.3.6.1.4.1.311.60.2.1.2=California,OID.1.3.6.1.4.1.311.60.2.1.3=US ,serialNumber=C2759208,OID.2.5.4.15="V1.0, Clause 5.(b)"" Certificate 2 Subject: "CN=GlobalSign Extended Validation CA,O=GlobalSign,OU= Extended Validation CA" Certificate 3 Subject: "CN=GlobalSign,O=GlobalSign,OU=GlobalSign Root CA - R2"
This comment describes the bug. Using the above database, I ran the following, which gives me a FAILURE. I think the CRL is present. I think vfychain is wrong to say "revoked". I hope this is the same bug that causes https://addons.mozilla.org to show without EV in Firefox (even with CRLs available): $ vfychain -d . -f -pp -a -u 1 -v -g leaf -h requireFreshInfo -m crl addons.mozilla.org Chain is bad, -8180 = Peer's Certificate has been revoked. PROBLEM WITH THE CERT CHAIN: CERT 3. globroot [Certificate Authority]: ERROR -8180: Peer's Certificate has been revoked. ERROR -8180: Peer's Certificate has been revoked.
Whiteboard: [bug description see comment 9]
Attachment #358960 - Attachment description: Prepared cert database (zip file) → Prepared cert database (zip file) (db=sql)
Alexei, you told me that you can reproduce the problem. But you said, maybe it is because of the new shared database. I repeated my commands with the old database mode. The previous attachment is a database with the same contents as the other one, but it uses the classic key3.db and cert8.db I get exactly the same results as in comment 8 and comment 9. I conclude the problem is not related to shared db.
Attached patch LibPkix patch v1Splinter Review
Alexei has given me a patch that solves this problem in vfychain. Alexei, is it right to ask Nelson for review?
Attachment #359228 - Flags: review?(nelson)
Some background for this bug, and why this problem is not yet solved for me: I originally saw a problem in Firefox: Site addons.mozilla.org is no longer shown as EV. I tried to reproduce the problem with vfychain. Up to comment 12, that's what this bug is about. The attached "Alexei's patch v1" fixes this problem in vfychain. Unfortunately, even with this patch applied, Firefox still does not work :-(
After a lot of stupid code disabling, trying to analyze the differences between Firefox code and vfychain, I found a sideeffect bug. I will attach two test patches, that modify vfychain. In my opinion, both changes to vfychain should be "safe" and "allowed". However, both changes cause vfychain to fail again, as described in comment 9. The first patch will remove input parameter cert_pi_date. In my opinion, this parameter is optional. Firefox PSM does NOT provide this parameter. If I change vfychain in the same way, then vfychain fails, too. First Additional Bug complaint: =============================== Please make sure that CERT_PKIXVerifyCert does not require input parameter cert_pi_date. The API description in certt.h says, this parameter is optional (because it has a default value of 0 == now). With NSS 3.12.2, it was not necessary to specify the parameter. Now with NSS 3.12.3 beta it seems necessary. Now, during my testing, I found another side effect. Currently it's not sufficient to specify the cert_pi_date parameter! It's even necessary that it's specified before parameter cert_pi_revocationFlags! In other words: - currently, vfychain gives input parameter date, then revocationFlags - if you revert that order, vfychain fails in the same way as in comment 9 This is what the second patch will show: Second Additional Bug complaint: ================================ Please make sure that the order of input parameters does not have different effects. I guess the code that reads cert_pi_revocationFlags currently depends on having seen cert_pi_date already, but it should not depend on it.
Alexei, do you agree with the bug description in comment 14, 15 and 16?
(In reply to comment #12) > Created an attachment (id=359228) [details] > Alexei, is it right to ask Nelson for review? It is isolated problem that this patch fixes. OK to review it.
Kai, thank you for you help identifying the problem. The problem was that the date object (that contains requested validation time that used for revocation check) was accessed before it was set, resulting in revocation check been done on the null date object, that in conjunction with requirement of having fresh revocation information were giving "revoked" status for the certificate. The fix is to obtain validation time late and make sure it is not NULL. For that processing params constructor is modified to set time to pr_now at the object creating. Later user can modify the time with SetDate call. And modify revocation checker to use validation time(from processing params) during cert revocation check.
Attachment #359397 - Flags: review?(nelson)
Attachment #359228 - Attachment description: Alexei's patch v1 → LibPkix patch v1
Attachment #359228 - Flags: review?(nelson) → review+
Comment on attachment 359228 [details] [diff] [review] LibPkix patch v1 r=nelson
Comment on attachment 359397 [details] [diff] [review] LibPkix patch v2 - use date provided by processing params object for revocation check r=nelson
Attachment #359397 - Flags: review?(nelson) → review+
Whiteboard: [bug description see comment 9] → [bug description see comment 9] PKIX
OS: Linux → All
Hardware: x86 → All
Patches have been integrated.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
I see you still "always" add the date parameter. I propose: - by default, do NOT add the data parameter - ONLY if parameter -b was given add the date parameter
Target Milestone: --- → 3.12.3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: