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)

Product:
Core
Component:
DOM: Navigation
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: sgifford+mozilla-old, Assigned: adamlock)

References

(
URL
)

Details

Attachments

(3 files)

proposed fix to pass a STATE_REDIRECTING flag when the main document is redirected
(deleted), patch
Details | Diff | Splinter Review
untested patch to signal redirects for all requests and not just the main document.
(deleted), patch
Details | Diff | Splinter Review
patch I landed on the branch.
(deleted), 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.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: rtm
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
This definitely seems to be a bug in necko, with it sending the wrong types of 
notifications to the nsIWebProgressListener::OnStateChange() listener.

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.




  

  



    
    

    
  
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. 

  

  



    
    

    
  
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.


  

  



    
    

    
  
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.



  

  



    
    

    
  
I misread that you are missing the 30004 notification on the subsequent document
and not the redirect. I'll look into that right now

  

  



    
    

    
  
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.


  

  



    
    

    
  
I agree you should get redirect notifications for requests within the document
as well. I'll post a fix with that additional change.

  

  



    
    

    
  
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.


  

  



    
    

    
  
Adding my name to the cc: list

  

  



    
    

    
  
MScott, can you post the fix yet?

  

  



    
    

    
  


  

  

  



    
    

    
  
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.

  

  



    
    

    
  
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

    
  
Blocks: 31982

    
    

    
  
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

  

  



    
    

    
  
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?

  

  



    
    

    
  
I would expect to get either a STATE_TRANSFERRING or a STATE_REDIRECTING 
notification for redirect-to-insecure.html.  I'm getting neither.


  

  



    
    

    
  
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.

  

  



    
    

    
  
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. 

  

  



    
    

    
  
The body does have bytes. Turn off javascript and view the source.

  

  



    
    

    
  
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

    
    

    
  
Feel free to land my 36827 patch for me.


  

  



    
  
Whiteboard: awaiting new patch

    
    

    
  
Oops, that's bug 31982, not 36827


  

  



    
    

    
  
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

    
  
Status: RESOLVED → VERIFIED

    
    

    
  
Verified with the 121/21 WinNT commercial trunk build.

  

  



    
    

    
  
Added branch accept to status whiteboard

  

  


Whiteboard: [build-accept]

    
  
Whiteboard: [build-accept] → [branch accept]

    
    

    
  
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.

  

  



    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch I landed on the branch. (deleted)
        — 
          — Splinter Review
      

    


  

  

  



    
    

    
  
adding myself to the cc list.

sorry jgmyers, I keep misspelling your name.


  

  



    
    

    
  
junruh - pls verify on the branch (mtest) builds.

  

  



    
    

    
  
Verified on the latest MTEST builds on Win, Mac and Linux.

  

  



    
  
Status: VERIFIED → REOPENED
Resolution: FIXED → ---

    
    

    
  
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.


  

  



    
    

    
  
Has this regressed on the BRANCH or the TRUNK?

  

  



    
    

    
  
This has regressed on the trunk.  I don't have a branch build to test.


  

  



    
    

    
  
This is also a regression on the branch. Tested with the latest Win32 and Mac 
mtest builds.

  

  



    
    

    
  
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)

  

  



    
    

    
  
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.


  

  



    
    

    
  
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......

  

  



    
    

    
  
As you can see in the 12-08 15:22 comment, we used to get a 30004 notification.  
This is indeed a regression.


  

  



    
    

    
  
Removing branch accept for the status whiteboard.
.Angela...

  

  


Whiteboard: [branch accept]

    
    

    
  
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?




  

  



    
    

    
  
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.


  

  



    
    

    
  
... That is I'm not seeing the STATE_IS_DOCUMENT|STATE_TRANSFERRING notification
I exepct on the destination document.


  

  



    
    

    
  
Can someone explain the difference between STATE_IS_DOCUMENT and
STATE_IS_NETWORK?


  

  



    
    

    
  
Nominate for 0.9.  This bug could cause a change to the nsIWebProgressListener
interface.


  

  


Keywords: mozilla0.9

    
    

    
  
Changing product from Browser:Security:Crypto --> PSM 2.0

  

  


Component: Security: Crypto → Client Library
Product: Browser → PSM
Version: other → 2.0

    
    

    
  
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

    
    

    
  
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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: