Closed
Bug 59827
Opened 24 years ago
Closed 24 years ago
Mozilla doesn't warn or change "padlock" icon when redirected from a secure site to an insecure one
Categories
(Core :: DOM: Navigation, defect, P3)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: sgifford+mozilla-old, Assigned: adamlock)
References
()
Details
Attachments
(3 files)
1.31 KB,
patch
|
Details | Diff | Splinter Review | |
1.62 KB,
patch
|
Details | Diff | Splinter Review | |
1.61 KB,
patch
|
Details | Diff | Splinter Review |
When I visit https://www.c2.net/ I am (eventually) redirected to http://www.c2.net/products/sh3/index.php3 This is not a secure page, either according to its URL, or according to Netscape. But I am not given any kind of warning that I've transitioned to an insecure page, and the little "padlock" icon in the lower-right corner still appears locked with an orange background. When I click on the padlock, it displays information about the secure connection I was redirected from. This could be an actual security risk if the page I was being redirected from comtained sensitive information, such as credit card numbers. I did not check to verify that this actually happens when the URL redirected from is the result of a (GET or POST) form submission. Please move this around to the right component; I wasn't quite sure where to put it.
Comment 1•24 years ago
|
||
Clicking on the link does not exhibit this behavior, but typing the URL in the URL bar does. I see this with linux trunk build 2000-11-10-21, PSM 1.4 I can also repoduce this with the 2000-11-10-09 linux branch build! Confirming. Nominating for rtm because this bug is present in the most recent branch build and represents a serious security problem.
Comment 2•24 years ago
|
||
The opposite behaviour (going from a non-secure site to a secure one) is also like this: the padlock icon remains broken and the page info shows information about the page not being encrypted, although you are in a https server.
changing QA contact to junruh@netscape.com
QA Contact: nitinp → junruh
Comment 4•24 years ago
|
||
This definitely seems to be a bug in necko, with it sending the wrong types of notifications to the nsIWebProgressListener::OnStateChange() listener.
Comment 5•24 years ago
|
||
1024[806b900]: SecureUI:86274c8: OnStateChange: 70001 :https://www.c2.net/ 1024[806b900]: SecureUI:86274c8: OnStateChange: 10001 :http://www.c2.net/products/sh3/index.php3 1024[806b900]: SecureUI:86274c8: OnStateChange: 10010 :https://www.c2.net/ 1024[806b900]: SecureUI:86274c8: OnStateChange: 30004 :http://www.c2.net/products/sh3/index.php3 1024[806b900]: SecureUI:86274c8: OnStateChange: 10001 :about:layout-dummy-request 1024[806b900]: SecureUI:86274c8: OnStateChange: 10010 :http://www.c2.net/products/sh3/index.php3 [...] 1024[806b900]: SecureUI:86274c8: OnStateChange: 10010 :http://www.c2.net/images/horizontal.gif 1024[806b900]: SecureUI:86274c8: OnStateChange: 60010 :https://www.c2.net/ Note the following problems: * There is no 30004 notification for https://www.c2.net/ There should be one before the 10001 notification for http://www.c2.net/products/sh3/index.php3 * Nothing in the entire code ever sends a STATE_REDIRECTING notification. I would expect a 30002 notification to index.php3 before or concurrent with the 30004 notification.
Comment 6•24 years ago
|
||
Comment 7•24 years ago
|
||
I'm attaching a patch which causes a nsIWebProgress::STATE_REDIRECTING to be issued when the document url is a redirect. This should fix the problems John mentioned about the secure browser UI not getting the redirect notification. However, I don't think it will fix this bug until we tweak the secure UI implementation to honor this redirected flag.
Comment 8•24 years ago
|
||
The proposed fix plus a tweaked secure UI implementation helps www.c2.net. I'm getting odd behavior out of https://scopus/bugsplat though. I don't get a subsequent 30004 notification for the page it redirects to.
Comment 9•24 years ago
|
||
I did a similar tweak to the securebrowserUIImpl and this test case works for me now too. I'm not sure why you aren't seeing the notification for https://scopus/bugsplat. I just set a break point in nsDocLoader::OnRedirect and watched it correctly fire the notification for the redirect. So it is going out. I'll keep digging.
Comment 10•24 years ago
|
||
I misread that you are missing the 30004 notification on the subsequent document and not the redirect. I'll look into that right now
Comment 11•24 years ago
|
||
I am missing the 30004 on the subsequent document. I don't expect it for the redirect, the 70002 serves that purpose. On a different note, I probably also want 10002 notifications for redirected images, so I can report mixed content for a secure page with an insecure image redirecting to a secure page.
Comment 12•24 years ago
|
||
I agree you should get redirect notifications for requests within the document as well. I'll post a fix with that additional change.
Comment 13•24 years ago
|
||
When I pull up http://junruh.mcom.com/tests.html and click on "secure to insecure" I don't get the redirect notification for https://junruh.mcom.com/redirect-to-insecure.html I expect.
Comment 14•24 years ago
|
||
Adding my name to the cc: list
Comment 15•24 years ago
|
||
MScott, can you post the fix yet?
Comment 16•24 years ago
|
||
Comment 17•24 years ago
|
||
Here's the patch I promised the other day. It should issue the redirecting notification for sub requests inside of the main document (i.e. images that are redirects in the page). I don't have a full build at my current location so I haven't been able to verify this fix yet but I thought I would post it so others could at least see it.
Comment 18•24 years ago
|
||
When I pull up http://junruh.mcom.com/tests.html and click on "secure to insecure" I still don't get the redirect notification for https://junruh.mcom.com/redirect-to-insecure.html I expect.
Whiteboard: awaiting new patch
Comment 19•24 years ago
|
||
John, the reason you aren't getting a redirect notification in the secure to insecure case is because there is no redirect on that page. I'm not sure how junruh has this particular test case set up but I just generated an http log for this link and didn't see any redirect notification. Here's a snippet of the log. Junruh, do you know how this particular page is set up? I didn't see a refresh header either so that's not it. 0[a42ca0]: Creating nsHTTPResponseListener [this=5098180] for URI: https://junruh.mcom.com/redirect-to-insecure.html. 0[a42ca0]: Creating nsHTTPServerListener [this=5098180]. 0[a42ca0]: nsHTTPPipelinedRequest::WriteRequest()[5018c80], mOnStopDone=1, mTransport=501f5c4 0[a42ca0]: nsHTTPServerListener::OnStartRequest [this=5098180]. 0[a42ca0]: nsHTTPServerListener::OnDataAvailable [this=5098180]. stream=50987a0. offset=0. length=1458. 0[a42ca0]: nsHTTPServerListener::ParseStatusLine [this=5098180]. aLength=1458 0[a42ca0]: ParseStatusLine [this=5098180]. Got Status-Line:HTTP/1.1 200 OK 0[a42ca0]: ParseStatusLine [this=509a7d0]. HTTP-Version: HTTP/1.1 0[a42ca0]: ParseStatusLine [this=509a7d0]. Status-Code: 200 0[a42ca0]: ParseStatusLine [this=509a7d0]. Reason-Phrase: OK 0[a42ca0]: OnDataAvailable [this=5098180]. Parsing Headers 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string:Server: Netscape-Enterprise/4.0 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string:Date: Fri, 01 Dec 2000 02:12:18 GMT 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string:Content-type: text/html 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string:Etag: "0-0-4b4-38c5906d" 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string:Last-modified: Tue, 07 Mar 2000 23:27:41 GMT 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string:Content-length: 1204 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string:Accept-ranges: bytes 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string:Connection: keep-alive 0[a42ca0]: ParseHTTPHeader [this=5098180]. Got header string: 0[a42ca0]: nsHTTPChannel::FinishedResponseHeaders [this=5090b80]. 0[a42ca0]: ProcessStatusCode [this=5090b80]. Status - Successful: 200. 0[a42ca0]: nsHTTPFinalListener::OnStartRequest [this=5095c20], mOnStartFired=0 0[a42ca0]: Deleting nsHTTPChannel [this
Comment 20•24 years ago
|
||
This is the way the redirect is setup at https://junruh.mcom.com/redirect-to-insecure.html this.location = "http://junruh/tests.html"; Is there a better way to set this up?
Comment 21•24 years ago
|
||
I would expect to get either a STATE_TRANSFERRING or a STATE_REDIRECTING notification for redirect-to-insecure.html. I'm getting neither.
Comment 22•24 years ago
|
||
With 4.76, click on the security button, then on Navigator, and check all of the boxes. Then retry the tests on http://junruh.mcom.com/tests.html. You'll always get warnings when going from secure to insecure and vice-versa.
Comment 23•24 years ago
|
||
I don't think you should get a redirecting state change in this case because we are loading a page whose on load handler loads another url (tests.html). There is no redirect happening here. I'll look into sending a STATE_TRANSFERING in this case.....It looks like we currently only set this state flag when we first start reading bytes for the document body. In this test case the body doesn't have any bytes so we never have any progress to set. Hence the lack of a state_TRANSFERING flag.
Comment 24•24 years ago
|
||
The body does have bytes. Turn off javascript and view the source.
Comment 25•24 years ago
|
||
btw, I just checked inthe doc loader patch (sr=rpotts). John, do you mind if I land your patch for 36827 to along with this on the trunk? Also, the problem with this one particular test case isn't related to our changes. There's a bug somewhere in http that's exposed by the way junruh is loading the redirected url in the onload handler for the secure document. We aren't calling a progress notification on the document and that's why you don't get a STATE_IS_TRANSFERING notification for https://junruh.mcom.com/redirect-to-insecure.html
Assignee: ddrinan → mscott
Comment 26•24 years ago
|
||
Feel free to land my 36827 patch for me.
Updated•24 years ago
|
Whiteboard: awaiting new patch
Comment 27•24 years ago
|
||
Oops, that's bug 31982, not 36827
Comment 28•24 years ago
|
||
This has been checked into the tip. Marking fixed. The remaining problem has been branched out into Bug #63415.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Updated•24 years ago
|
Status: RESOLVED → VERIFIED
Comment 29•24 years ago
|
||
Verified with the 121/21 WinNT commercial trunk build.
Updated•24 years ago
|
Whiteboard: [build-accept] → [branch accept]
Comment 31•24 years ago
|
||
patch landed on the branch. I'm going to re-attach the exact patch that I landed on the branch. mscott & jgmeyers, please double check that I did the right thing.
Comment 32•24 years ago
|
||
Comment 33•24 years ago
|
||
adding myself to the cc list. sorry jgmyers, I keep misspelling your name.
Comment 34•24 years ago
|
||
junruh - pls verify on the branch (mtest) builds.
Comment 35•24 years ago
|
||
Verified on the latest MTEST builds on Win, Mac and Linux.
Updated•24 years ago
|
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Comment 36•24 years ago
|
||
This has regressed in recent builds, at least for http://scopus/bugsplat The OnStateChange listener is no longer getting STATE_IS_DOCUMENT notifications for the destination page.
Comment 37•24 years ago
|
||
Has this regressed on the BRANCH or the TRUNK?
Comment 38•24 years ago
|
||
This has regressed on the trunk. I don't have a branch build to test.
Comment 39•24 years ago
|
||
This is also a regression on the branch. Tested with the latest Win32 and Mac mtest builds.
Comment 40•24 years ago
|
||
shouldn't 63415 be re-opened and not this one? this bug is still working for me. The bugsplat problems was the same thing as the problem in 63415 (or so I thought)
Comment 41•24 years ago
|
||
63415 was that we weren't getting STATE_IS_TRANSFERING notification for the original document. Now we aren't getting STATE_IS_TRANSFERING|STATE_IS_DOCUMENT notifications for the *destination* document.
Comment 42•24 years ago
|
||
I'm wondering if we ever got that notification. I'm not seeing any changes in any of the files that generate the notifications since we initially checked in changes to get the padlock icon working (31982). Still poking but I'm pretty sure this is a problem we don't need to hold the branch train for. Especially since we (at least I) don't have a good understanding of what's going wrong and what we can do to fix it. Angela, I recommend clearing the branch accept for this bug......
Comment 43•24 years ago
|
||
As you can see in the 12-08 15:22 comment, we used to get a 30004 notification. This is indeed a regression.
Comment 44•24 years ago
|
||
Removing branch accept for the status whiteboard. .Angela...
Whiteboard: [branch accept]
Reporter | ||
Comment 45•24 years ago
|
||
I'm not seeing this bug anymore on 2001032614. Unless somebody has an objection, I'll go ahead and resolve this bug in a few days. Why was it reopened?
Comment 46•24 years ago
|
||
I still see the problem on http://scopus/bugsplat in recent builds. I'm getting a 50002 notification, but not the STATE_IS_DOCUMENT notification I'm expecting.
Comment 47•24 years ago
|
||
... That is I'm not seeing the STATE_IS_DOCUMENT|STATE_TRANSFERRING notification I exepct on the destination document.
Comment 48•24 years ago
|
||
Can someone explain the difference between STATE_IS_DOCUMENT and STATE_IS_NETWORK?
Comment 49•24 years ago
|
||
Nominate for 0.9. This bug could cause a change to the nsIWebProgressListener interface.
Keywords: mozilla0.9
Comment 50•24 years ago
|
||
Changing product from Browser:Security:Crypto --> PSM 2.0
Component: Security: Crypto → Client Library
Product: Browser → PSM
Version: other → 2.0
Comment 51•24 years ago
|
||
Changing from PSM/2.0 to Browser/Embedding: Docshell
Assignee: mscott → adamlock
Status: REOPENED → NEW
Component: Client Library → Embedding: Docshell
Product: PSM → Browser
QA Contact: junruh → adamlock
Version: 2.0 → other
Comment 52•24 years ago
|
||
This worksforme with the 2001043004 WinNT Netscape 6 build.
Status: NEW → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•