997370 - nsIX509Cert idl incorrect documentation of sha1fingerprint and md5fingerprint

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Camilo Viecco (:cviecco)]

While helping write some heartbleed bugs it was discovered that the idl documentation in nsIX509Cert.idl for the sha1Fingerprint and md5Fingerprint do not match the impleeentation.

The documentation says:
The fingerprint of the certificate's public key,
calculated using the $CHOICE algorithm

and it should be

The fingerprint of the certificate's DER encoding,
calculated using the $CHOICE algorithm

Comment 2

[Mike Hoye [:mhoye]]

Switching the whiteboard tag to [good next bug] . Even if it's docs, nothing that touches crypto gets to be a good first bug. Camilo, would you be willing to mentor a contributor through this?

Comment 3

[Camilo Viecco (:cviecco)]

Yes

Comment 4

[hpathak]

Attachment: Updated comment.

Comment 5

[Camilo Viecco (:cviecco)]

Comment on attachment 8435946 [details] [diff] [review]
Updated comment.

Review of attachment 8435946 [details] [diff] [review]:
-----------------------------------------------------------------

very good. r- for not compiling (should be an easy fix). For pushing to try see https://wiki.mozilla.org/ReleaseEngineering/TryServer#Try_Server

::: security/manager/ssl/src/nsNSSCertificateDB.cpp
@@ +1,1 @@
> +SetCertTrust/* This Source Code Form is subject to the terms of the Mozilla Public

Opps, this will not compile. Do a try run to see it :)

Comment 6

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8435946 [details] [diff] [review]
Updated comment.

Review of attachment 8435946 [details] [diff] [review]:
-----------------------------------------------------------------

Also, patch formatting should have 8 lines of unified context - see the section on writing a ~/.hgrc file here: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ
Finally, the patch comment should have "bug 997370 - " at the beginning. When you get an r+, you'll add r=<the reviewer's irc handle> at the end.

Comment 7

[hpathak]

Attachment: Bug997370_comment_fix.diff

https://tbpl.mozilla.org/?tree=Try&rev=5113b5eabf97. 

Hope this goes through.

Comment 8

[hpathak]

Attachment: Bug997370_comment_fix.diff

Received r+ from Camilo.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8437070 [details] [diff] [review]
Bug997370_comment_fix.diff

In the future, you can just use the checkin-needed bug keyword :)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4f9febf724b0

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4f9febf724b0