240496 - [FIXr]Signed jar file on https site shows as unsecured/unverified

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Bryan White]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040316
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040316

A signed jar file served from an https site does not show the padlock icon in
the locked position and the Page Info "Security" tab indicates the connextion is
no secured.



Reproducible: Always
Steps to Reproduce:
1. Open URL
2. The jar file will display a Hello World alert
3: Click on the padlock icon to view the security page info.

Actual Results:  
Web Site Identity Not Verified
--
Connection Not Encrypted
 

Expected Results:  
Web Site Identity Verified
--
Connection Encripted...

Comment 1

[Boris Zbarsky [:bzbarsky]]

So are we keying off the URL scheme somewhere instead of keying off the
nsIPrincipal?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Confirming.  At first blush, the problem is:

464 nsJARChannel::GetSecurityInfo(nsISupports **aSecurityInfo)
465 {
466     *aSecurityInfo = nsnull;
467     return NS_OK;
468 }

We probably need to keep around the security info from the load in nsJAR or
something...  I'm not familiar with how that part of the code works, really...
anyone know enough to quickly do this?

Comment 3

[Alec Flett]

not off hand..

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Wasn't too bad after all, since we don't use the jar cache here....

Comment 5

[Darin Fisher]

Comment on attachment 146541 [details] [diff] [review]
Patch

looks good, except i would forward the "context" parameter from OnStopRequest
as well as the request.

i think you can change your XXXbz to a NOTE that says that we indeed do not
need to worry about mSecurityInfo in that case b/c that case applies to any URI
that doesn't need to be downloaded in order to provide the resource as a
nsIFile.

r+sr=darin with that change.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: With those changes

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 146549 [details] [diff] [review]
With those changes

Could you give this a once-over?

Comment 8

[Darin Fisher]

Comment on attachment 146549 [details] [diff] [review]
With those changes

>+    if (channel) {
>+        channel->GetSecurityInfo(getter_AddRefs(mSecurityInfo));
>+    }

prevailing style in this file is no brackets when not needed.

r=darin

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: With that change

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 146690 [details] [diff] [review]
With that change

Checked in on the trunk.  I think we should take this on the 1.7 branch...

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 146690 [details] [diff] [review]
With that change

a=asa (on behalf of drivers) for checkin to 1.7

Comment 12

[Boris Zbarsky [:bzbarsky]]

Checked in on the branch.