Closed
Bug 139522
Opened 23 years ago
Closed 23 years ago
SSL Pages are incorrectly displayed as being not encrypted
Categories
(Core Graveyard :: Security: UI, defect, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
psm2.3
People
(Reporter: MMx, Assigned: KaiE)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [adt1] [m5+])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
KaiE
:
review+
alecf
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0rc1) Gecko/20020417 BuildID: 2002041711 Same Problem as in Bug #139487, just in Win98. The Problem occurs only if I launched Mozilla via Mozilla Mail, check my Mail (starting PSM due to encrypted PWDs) and THEN open an SSL page. Because this SSL page is a Homebanking-Site I would classify this bug as CRITICAL (or worse) Reproducible: Always Steps to Reproduce: 1. exit Mozilla (if open) 2. Launch Mozilla Mail (using RC1 build) 3. Check Mail (have to enter password for PSM) 4. open Navigator 5. open URL "http://www.postbank-banking.de" (in German, but one should at least see that ist's not encrypted as it is supposed to be) Actual Results: The browser switches to https:// ... but the site is NOT encrypted like it is supposed to be. Expected Results: encrypt the page like it does when I launch Mozilla via Navigator.
Updated•23 years ago
|
Assignee: mstoltz → ssaux
Status: UNCONFIRMED → NEW
Component: Security: General → Client Library
Ever confirmed: true
Product: Browser → PSM
QA Contact: bsharma → junruh
Version: other → 2.0
Comment 2•23 years ago
|
||
Kai, can you take a look at this asap?
Assignee: ssaux → kaie
Priority: -- → P1
Target Milestone: --- → 2.3
Assignee | ||
Comment 3•23 years ago
|
||
I can reproduce the reported behaviour.
Status: NEW → ASSIGNED
OS: Windows 98 → All
Hardware: PC → All
Assignee | ||
Comment 5•23 years ago
|
||
This is a major security regression.
Keywords: mozilla1.0,
nsbeta1+
Whiteboard: [adt1]
Assignee | ||
Comment 6•23 years ago
|
||
More facts about the bug: - it is irrelevant whether you try to fetch mail or not. The fact that the mail window is the first opened Mozilla window is sufficient. - The bug is limited to the first opened navigator window. When you later open additional windows, they behave correctly. I traced the code. I can see that the instance of nsSecureBrowserUIImpl does get instantiated when the navigator window opens. However, nsSecureBrowserUIImpl::Init never gets called, and therefore we can't register ourselves for receiving web progress. Therefore we never receive any notifications via OnStateChange and are unable to track security state or update the lock icon.
Summary: SSL Pages are not encrypted → SSL Pages are incorrectly displayed as being not encrypted
Assignee | ||
Comment 7•23 years ago
|
||
It looks like the code in xpfe/global/resources/content/bindings/browser.xml does both creation of the object and calling the init method. But both statements appear unconditionally after another, I don't understand how the first statement could be executed, but not the second. Either we have a strange bug in JavaScript, or there is another source location which creates the instance, but does not call init. I'm searching the source for other instance creation statements.
Assignee | ||
Comment 8•23 years ago
|
||
I can confirm that it is indeed the following two lines trigger creation of the nsSecureBrowserUIImpl instance, but the C++ init method gets never called. this.securityUI = Components.classes[SECUREBROWSERUI_CONTRACTID] .createInstance(Components.interfaces.nsISecureBrowserUI); this.securityUI.init(this.contentWindow); I traced into jsinterp.c, and activated tracefp. I saw the following output after instance creation returned: 25: 00259: setprop "securityUI" inputs: [object XULElement @ 0x8992170], Components.classes[SECUREBROWSERUI_CONTRACTID].createInstance(Components.interfaces.nsISecureBrowserUI) @ 2 output: this.securityUI @ 1 stack: {}
Assignee | ||
Comment 9•23 years ago
|
||
For more tracing, I changed the simple init call in browser.xml to do some debug output: if (!this.securityUI) { dump("==> unable to create instance\n"); } else { dump("==> I have a secure browser ui instance, calling init\n"); } this.securityUI.init(this.contentWindow); dump("==> Init has been called\n"); } } catch (e) { dump(e); } When the above get's executed, I get the following output: ==> I have a secure browser ui instance, calling init TypeError: this.docShell has no properties I know for sure that the "TypeError:" line comes from an exception thrown when trying to call this.securityUI.init(). The output "Init has been called" is not shown. What has this.docShell to do with the init call? Is this a bug in the JavaScript engine?
Comment 10•23 years ago
|
||
> What has this.docShell to do with the init call?
The init call takes |this.contentWindow| as the param. browser.xml has:
<property name="contentWindow"
readonly="true"
onget="return
this.docShell.QueryInterface(Components.interfaces.nsIInterfaceRequestor).getInterface(Components.interfaces.nsIDOMWindow);"/>
So if this.docShell has no properties, the getter for this.contentWindow throws
an exception....
Comment 11•23 years ago
|
||
cc Scott Putterman. This is a major regression. Setting as blocker. Kai, who else should we cc? Javascript engine experts?
Severity: critical → blocker
Comment 12•23 years ago
|
||
cc Jaime. We're not getting any traction on that. Who volunteers to look at Kai's analysis and comment?
Updated•23 years ago
|
Whiteboard: [adt1] → [adt1] [m5+]
Comment 13•23 years ago
|
||
Bah, I had commented on this, apparently when I submitted I got the login page and didn't notice, so my comment was never added. Basically see bug 113076 comment #14 and #15 for what's going on here. The real fix is hard, I'm attaching a work-around.
Comment 14•23 years ago
|
||
Comment 15•23 years ago
|
||
Actually, we probably don't have to create another instance of the secure UI state object, we'll only have to init it. Kai will attach a new patch if that works.
Assignee | ||
Comment 17•23 years ago
|
||
Comment on attachment 81015 [details] [diff] [review] Cheaper work-around I tested the patch, it fixes the problem. Suggested workaround looks good to me. r=kaie
Attachment #81015 -
Flags: review+
Comment 19•23 years ago
|
||
Comment on attachment 81015 [details] [diff] [review] Cheaper work-around sr=alecf with comment discussed
Attachment #81015 -
Flags: superreview+
Comment 20•23 years ago
|
||
Added the following line to the comment above this code: // The same problem caused bug 139522, also worked around below. Checked in. Added adt1.0.0 keyword.
Comment 21•23 years ago
|
||
adding adt1.0.0+. Please check this into the branch after getting drivers approval and add the fixed1.0.0 keyword.
Comment 22•23 years ago
|
||
Comment on attachment 81015 [details] [diff] [review] Cheaper work-around a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #81015 -
Flags: approval+
Comment 24•23 years ago
|
||
verified1.0.0
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.0 → verified1.0.0
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•