Closed
Bug 475454
Opened 16 years ago
Closed 16 years ago
vfychain requireFreshInfo incorrectly claims "revoked"
Categories
(NSS :: Libraries, defect)
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)
1.94 KB,
text/plain
|
Details | |
1.55 KB,
text/plain
|
Details | |
1.45 KB,
text/plain
|
Details | |
1.25 KB,
text/plain
|
Details | |
1.60 KB,
application/octet-stream
|
Details | |
7.68 KB,
application/octet-stream
|
Details | |
4.40 KB,
application/octet-stream
|
Details | |
971 bytes,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
1004 bytes,
patch
|
Details | Diff | Splinter Review | |
1.23 KB,
patch
|
Details | Diff | Splinter Review | |
9.04 KB,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
I will attach all the required files to reproduce this bug.
Reporter | ||
Comment 1•16 years ago
|
||
Reporter | ||
Comment 2•16 years ago
|
||
This cert issued the above attachment.
Reporter | ||
Comment 3•16 years ago
|
||
this intermediate issued the above attachment
Reporter | ||
Comment 4•16 years ago
|
||
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
Reporter | ||
Comment 5•16 years ago
|
||
That's the binary CRL which is listed in the server cert, downloaded from http://crl.globalsign.net/ExtendVal1.crl
Reporter | ||
Comment 6•16 years ago
|
||
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.
Reporter | ||
Comment 7•16 years ago
|
||
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
Reporter | ||
Comment 8•16 years ago
|
||
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"
Reporter | ||
Comment 9•16 years ago
|
||
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.
Reporter | ||
Updated•16 years ago
|
Whiteboard: [bug description see comment 9]
Reporter | ||
Updated•16 years ago
|
Attachment #358960 -
Attachment description: Prepared cert database (zip file) → Prepared cert database (zip file) (db=sql)
Reporter | ||
Comment 10•16 years ago
|
||
Reporter | ||
Comment 11•16 years ago
|
||
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.
Reporter | ||
Comment 12•16 years ago
|
||
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)
Reporter | ||
Comment 13•16 years ago
|
||
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 :-(
Reporter | ||
Comment 14•16 years ago
|
||
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.
Reporter | ||
Comment 15•16 years ago
|
||
Reporter | ||
Comment 16•16 years ago
|
||
Reporter | ||
Comment 17•16 years ago
|
||
Alexei, do you agree with the bug description in comment 14, 15 and 16?
Assignee | ||
Comment 18•16 years ago
|
||
(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.
Assignee | ||
Comment 19•16 years ago
|
||
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)
Assignee | ||
Updated•16 years ago
|
Attachment #359228 -
Attachment description: Alexei's patch v1 → LibPkix patch v1
Updated•16 years ago
|
Attachment #359228 -
Flags: review?(nelson) → review+
Comment 20•16 years ago
|
||
Comment on attachment 359228 [details] [diff] [review]
LibPkix patch v1
r=nelson
Comment 21•16 years ago
|
||
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+
Assignee | ||
Updated•16 years ago
|
Whiteboard: [bug description see comment 9] → [bug description see comment 9] PKIX
Assignee | ||
Updated•16 years ago
|
OS: Linux → All
Hardware: x86 → All
Assignee | ||
Comment 22•16 years ago
|
||
Patches have been integrated.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 23•16 years ago
|
||
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
Updated•16 years ago
|
Target Milestone: --- → 3.12.3
You need to log in
before you can comment on or make changes to this bug.
Description
•