1229013 - Cannot verify sender cert in smime signed received email since nsIPKIParamBlock went away

Status: RESOLVED FIXED

(Thunderbird :: Security, defect)

Description

[Mike Cloaked]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.86 Safari/537.36

Steps to reproduce:

Receive an smime signed email and see the small envelope icon with the red spot at top right of the preview pane. Try to click the envelope to check smime certificate information.


Actual results:

Clicking the smime icon envelope does nothing.


Expected results:

Clicking the smime icon envelope should open a pop up with certificate information.

Comment 1

[Mike Cloaked]

Build ID is 

Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Thunderbird/44.0a2 ID:20151106004034 CSet: 667801dbe2a1de6cf9d8e3e70f2d4e9da2633d3d

I was not checking this in recent nightly versions so I don't know when this regression happened.

Comment 2

[gene c]

I confirm same issue in nightly builds. Problem is for both the signed (seal envelope icon) and encrypted mail (the padlock icon) - neither open with additional information anymore for s/mime secured email.

User Agent:   Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0a1
Build Id  :   20151129030241

Gene

Comment 3

[Magnus Melin [:mkmelin]]

Timestamp: 03.12.2015 23.39.57
Error: TypeError: Components.classes['@mozilla.org/security/pkiparamblock;1'] is undefined
Source File: chrome://messenger-smime/content/msgReadSMIMEOverlay.js
Line: 54

Comment 4

[Kent James (:rkent)]

We should probably determine what replaced this usage in m-c and do the same changes in c-c. There were no more uses of this in m-c so they removed it.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Basically, you can use nsIDialogParamBlock everywhere nsIPKIParamBlock is being used. Hopefully https://hg.mozilla.org/mozilla-central/rev/23f0dcfdcce1#l5.31 and https://hg.mozilla.org/mozilla-central/rev/23f0dcfdcce1#l11.23 and so on will be helpful.

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: bug1229013_smime_nsIDialogParamBlock_conversion.patch

The fix

Comment 7

[Kent James (:rkent)]

Comment on attachment 8702082 [details] [diff] [review]
bug1229013_smime_nsIDialogParamBlock_conversion.patch

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

Trying to move forward blockers, I reviewed this code and tested that it works. If you would prefer a review my jcranmer, reset the review field.

Comment 8

[Magnus Melin [:mkmelin]]

Thx for the review!

https://hg.mozilla.org/comm-central/rev/c25d8e4d9e67 -> FIXED

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 8702082 [details] [diff] [review]
bug1229013_smime_nsIDialogParamBlock_conversion.patch

[Approval Request Comment]
Regression caused by (bug #): 1217602 
User impact if declined: inability to verify sender s/mime cert 
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky): not risky, it's completely broken as is

Comment 10

[Mike Cloaked]

Excellent - thank you.

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 8702082 [details] [diff] [review]
bug1229013_smime_nsIDialogParamBlock_conversion.patch

(In reply to Magnus Melin from comment #9)
> Risk to taking this patch (and alternatives if risky): not risky, it's
> completely broken as is
Very funny and very convincing ;-)

Comment 12

[Jorg K (CEST = GMT+2)]

Aurora:
https://hg.mozilla.org/releases/comm-aurora/rev/ed9083131720

Comment 13

[Jorg K (CEST = GMT+2)]

Magnus, do you want to land this on TB 44 beta given that TB 45 beta should be coming out soon?

My preference would be to forget about TB 44 beta (since all the good stuff is in TB 45). If you agree, please clear the flag and set status-thunderbird44 to wontfix.

Comment 14

[Magnus Melin [:mkmelin]]

Well, I think SeaMonkey is releasing from that, so they would want it.

Comment 15

[Jorg K (CEST = GMT+2)]

Comment on attachment 8702082 [details] [diff] [review]
bug1229013_smime_nsIDialogParamBlock_conversion.patch

Sorry, I'm clearing the beta approval since the beta (TB 44) is now gone.
TB 45 in now in beta and it was landed on there when it was still in Aurora.