certutil and PP print bogus cert fingerprints

RESOLVED FIXED in 3.9

Status

NSS
Tools
P1
normal
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Nelson Bolyard (seldom reads bugmail))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

All recent versions of certutil and PP print the same exact MD5 and SHA1
fingerprint values for ALL certificates, regardless of their content.

I believe this bug is a regression that was introduced by the fix for bug 45303,
which was checked in in the summer of 2000.  Likely all versions of NSS 3.x
beginning with 3.1 or 3.2 have had this bug.  

The fix is tiny.  Patch to follow.
(Assignee)

Comment 1

14 years ago
Marking P1 for 3.9.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → 3.9
(Assignee)

Comment 2

14 years ago
Created attachment 131924 [details] [diff] [review]
patch v1

This patch keeps the output format the same as it was in NSS 3.8, but fixes
the computed fingerprint values.  

But to be honest, I think the fingerprint should be displayed after the
signature, not before.	If we aren't required to preserve the format of the
output exactly,
(and I think we are not) then I would rather move the Fingerprints down.
(Assignee)

Comment 3

14 years ago
Created attachment 131925 [details] [diff] [review]
patch v2

This alternative patch causes the fingerprints to appear immediately after
the signature, not before it.  I invite review comments on either patch.
(Assignee)

Comment 4

14 years ago
Before the changes for bug 45303, certutil printed the fingerprints last,
after the cert and the trust flags, and pp printed neither fingerprints nor
trust flags.  With my patch v2, both certutil and pp print the fingerprints
immediately after the signature, and certutil additionally prints the trust
flags.  

I will checkin patch v2 shortly.
(Assignee)

Comment 5

14 years ago
Checked in patch v2.  Marking fixed.  
Some question remains about whether NSS fingerprints will now match OpenSSL
fingerprints.  If they do not, I will reopen this bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 6

14 years ago
Comment on attachment 131925 [details] [diff] [review]
patch v2

>@@ -2386,6 +2385,7 @@
>     SECU_PrintAsHex(out, &sd->signature, "Signature", level+1);
> loser:
>     PORT_FreeArena(arena, PR_FALSE);
>+    SECU_PrintFingerprints(out, der, "Fingerprint", level+1);
>     return rv;

Would it be more correct to print the fingerprints before the "loser"
label?

Comment 7

14 years ago
Comment on attachment 131924 [details] [diff] [review]
patch v1

>@@ -2377,6 +2376,7 @@
> 
>     SECU_Indent(out, level); fprintf(out, "%s:\n", m);
>     rv = (*inner)(out, &sd->data, "Data", level+1);
>+    SECU_PrintFingerprints(out, der, "Fingerprint", level+1);
>     if (rv) 
> 	goto loser;

Similar comment: I think it would be better to move
SECU_PrintFingerprints() after "goto loser;".
(Assignee)

Comment 8

14 years ago
The print functions should attempt to print all they can.  
They should NOT "bail out" on the first error, but rather go on and 
attempt to print whatever they can.
In this case, even if the attempt to parse the signed portion of the
certificate fails, it is still possible to compute and display the 
fingerprint of the der cert, and that's what these patches do.
You need to log in before you can comment on or make changes to this bug.