User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b4) Gecko/20090426 SUSE/3.5b4-1.1 Firefox/3.5b4 Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b4) Gecko/20090426 SUSE/3.5b4-1.1 Firefox/3.5b4 Problem appeared together with the change to FF 3.5 (worked in the same setup flawless on 3.0.10) - Corporate environment. Proxy settings distributed by PAC file (http://proxy.foo.com/proxy.pac) the PAC file returns by default two proxy entries and contains exceptions for all internal websites (sites in the domain foo.com). The Proxy is a Novell Border Manager, which requires authentication. so on first 'web site hit', the browser is redirected to a BM login page: - server.foo.com:444/BM-Login/ With FF 3.5, this redirection does not succeed and I get an error 'a proxy was configured which refused connection'. Manually browsing to the login page, logging in and then browsing the web works (as the proxy no longer redirects to a login page). Reproducible: Always Steps to Reproduce: Might be tricky, as it's a rather specific setup 1. configure proxy settings 2. have a proxy that redirects to a login page 3. Actual Results: the login page can't be reached Expected Results: The login page appears on screen, you login and then browsing is possible

I cannot triage easily but if it's valid (no reason for me to doubt that) it's rather critical. So getting on people's radar by asking for blocking.

Additional info: it makes a difference if the first page is http or https: on a http:// request, the HTTP/302 is followed, apparently, on a https:// it is not followed to the proxy login page.

jduell: was this perhaps caused by bug 479880? Dominique: does it work with Firefox 3.1 beta 3?

> jduell: was this perhaps caused by bug 479880? It seems very like that it was. I'm reopening 479880. Sigh...

(In reply to comment #3) > Dominique: does it work with Firefox 3.1 beta 3? Wolfgang: do you have anywhere a package I could install for this test? (OSS Factory, x86_64). This would be very helpful for me.

Sounds like we need to block on this for 1.9.0.11 (or maybe .12?).

We have to be careful when following redirects because of this case: <script src="https://www.paypalobjects.com/foo.js"></script> If we follow redirects, then an active network attacker can redirect the request and supply his or her own script.

> jduell: was this perhaps caused by bug 479880? That doesn't match comment 0. That bug is fixed in Firefox 3.0.10, and this bug is claimed to not appear in that version. Or is comment 0 wrong? In any case, the right place to address this bug is right here, not by reopening other bugs. Dominique, do you think you can try a Firefox 3.0.11 candidate build and see whether the problem appears there too? Also, I assume the proxy is doing the redirect via a 3xx response, right? Or is it using some other mechanism? To answer Jason's question in bug 479880, if the redirect is done via HTTP status code, then it's quite possible to follow it in this case. Just add the relevant status codes to the if condition that tests for 407 in ProcessResponse, then in the relevant cases error out if redirection fails (again, just like we handled 407). If the server is using something like a <meta> tag, of course, that won't work.

For comment 7, we could just check whether the request is a document request, I think. Adam, that seems like it would address your concerns, since data from document requests doesn't get imported into someone else's security context.

That might work, but then you have to worry about iframe injection: <iframe src="https://example.com/menu.html"></iframe> Maybe it's safe to follow redirects for the main frame?

Dominique, If you could grab a network trace of the interaction between the browser and your proxy (using a tool like Wireshark), and attach it to this bug, that would be helpful.

> That might work, but then you have to worry about iframe injection: You mean worry in terms of phishing stuff, right? We could restrict it to toplevel frames, I suppose.

Yeah. Consider this page for example: https://www.google.com/analytics/reporting/login?ctu=https://www.google.com/analytics/settings/%3Fet%3Dreset%26hl%3Den-US The password field is in an HTTPS iframe. If you let the attacker redirect the iframe load to https://attacker.com, you'd have no way to know that you were sending your password off to disaster.

Actually, even top-level loads can be a problem if the request is a POST. Consider this page: https://www.google.com/accounts/ If you let the attacker inject a 307 redirect to https://attacker.com, the POST data containing the password gets sent to the attacker. (Note that the load is targeted at the main frame.)

some description for the traffic you see: The Proxy is on 10.3.0.4, port 2323 my machine is 10.25.3.51 the UDP requests from / to port 3024 is the BorderManager Client Trust single sign on check. This does not work properly on Linux (no client available on x86_64), so this one fails. (gives a timeout... but that's fine). the site I try to connect to (intentionally) is https://webmail.leuenberger.net the http redirect is in packet No. 167, HTTP/1.1 302 Moved Temporarily

As far as comment 14 goes, the only time we send POST data along after a 3xx redirect is if a 307 is used, and then we prompt the user before doing so. That said, we don't tell the user _where_ the data will be posted, I guess...

Dominique: Just to make sure... In comment 0 you said you didn't see this issue using Firefox 3.0.10. Can you confirm that? If this is a regression from bug 479880, it'd likely appear in 3.0.10 but not 3.0.9, unless it was interacting with something else in the Firefox 3.5 betas.

OK, I've implemented BZ's suggestion to just let the 3xx response codes through, and then jump to ProcessFailedSSLConnect() if the redirect fails. I've verified that the body of the 3xx response is not parsed in either case (i.e no <script> risks). I tried a batch of different test cases, which I'll also attach. I don't know about enough about the document request && iframes stuff to say whether there's still some additional risk here. Bz/Adam?

So the last test I attached exhibits some possibly weird behavior (that is if anything a separate bug): I redirect the user with a Location header to a bogus protocol ("Location: bogus_protocol://whatevar"). The redirect fails, so we're still safe from <script>s in the response body. But the URL displayed in Firefox is "https://www.mozilla.org/bogus_protocol://whatevar" (the original URL requested was https://www.mozilla.org/, so we're treating the bad protocol as a relative link: is that kosher?).

If I'm understanding this patch correctly, it's still wrong. You seem to be following the redirect for <script> tags. That means an active network attacker can XSS https://www.paypal.com/ by redirect requests to https://www.paypalobjects.com to https://attacker.com.

Yeah. What you probably want to do in the 3xx case is to still fall through to ProcessFailedSSLConnect unless all of the following are true: 1) The channel has the LOAD_DOCUMENT_URI flag. 2) The channel's nsILoadContext has an associatedWindow and that window is the same as the load context topWindow. 3) The status code is not 307 or the channel has no POST data. We could tighten up #3 to "the channel has no POST data" for consistency, if desired.

OK, I think I've incorporated BZ's additional requirements. I had to REQUIRE docshell to do it (does this harm necko standalone-ness? Do we care?).

Oh--I forbid all POST redirects, because this is really only going to be used by proxies that want to redirect users to a login page. I figured it's exceedingly unlikely that such a login page is going to pass-through a POST after login.

This HTML page contains tests for inline <scripts>, <iframes>, and <images>, as well as testing that top-level page redirects work. Step-by-step instructions included.

Marking as secure. Undo if I'm wrong, but we've got the same info in here at bug 479880, which was secure.

So.. that really should be using nsILoadContext if we do it this way. But the point about REQUIRES is a good one. We don't want to make that change, I don't think. I clearly spend too much of my time in code that already depends on docshell. Can we cheat and use "mURI == mDocumentURI" (plus the LOAD_DOCUMENT_URI flag) as the test for a toplevel load? We sort of do already in SetupReplacementChannel....

Oh, and you can just use mLoadFlags directly I think. And instead of the if (x) return PR_TRUE; return PR_FALSE; pattern, just |return x|?

> Can we cheat and use "mURI == mDocumentURI" Yes, very nice. Other suggestions were also good, and incorporated. Patch tested with HTML test I previous attached. Should be ready to go.

Blocking, we should fix this regression for 3.5.

Comment on attachment 377620 [details] [diff] [review] Eliminate DocShell dependency >+nsHttpChannel::ShouldSSLProxyResponseContinue(PRUint32 httpStatus) >+ return (mLoadFlags & nsIChannel::LOAD_DOCUMENT_URI >+ && mURI == mDocumentURI >+ && mRequestHead.Method() != nsHttp::Post); Two nits here. Please put a pair of parens around the '&': while '&' does have higher precedence than '&&', having to remember that or look it up is a huge pain. Also, please put the '&&' at the end of the previous line, not the beginning of the next one. > nsHttpChannel::ProcessResponse() >+ if (mTransaction->SSLConnectFailed()) >+ if (!ShouldSSLProxyResponseContinue(httpStatus)) Some use of && here would be better, I think. ;) >+++ b/netwerk/protocol/http/src/nsHttpChannel.h >+ PRBool ShouldSSLProxyResponseContinue(PRUint32 httpStatus); Document, please. With those nits, looks good.

I've made all the changes except the documentation request, since I didn't understand it (what's wrong with the comment at the beginning of ShouldSSLProxyResponseContinue()? We don't describe any functions in the .h file. So I'm not sure where or what you want me to write). I made all of the formatting changes, though I don't actually agree with any of them :). (ANDs are more natural as the beginning of a clause, and would lessen the need to put 'x & y' in parens.) We want to get this into the Wednesday release; feel free to modify my mods as you like if it moves things along. I'm happy to add more docs if you tell me where.

> I've made all the changes except the documentation request Ok, I guess. I sort of like the purpose of functions documented in the header (with the implementation documented in the body as needed), but this one's name might be good enough for the purpose... I'll land that last patch as-is. Thanks for fixing the other issues!

Pushed http://hg.mozilla.org/mozilla-central/rev/b7f9b3f6799f

Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/43f69cd368c4 Jason, thanks for indulging my whims. ;)

Since it's not clear that this regressed on 1.9.0, we're pushing it out past 1.9.0.11. It'd be good to know if we need to take this in 1.9.0.12 though...

> Since it's not clear that this regressed on 1.9.0 Sorry if I didn't make that clear: this is a regression om 1.9.0. Proxy HTTPS redirects (which previously didn't fail in Fx 3.0.9 and earlier) fail in 3.0.10. That's the regression, and what the bug was opened on. The other security risks from proxy replies are present in 3.0.9 and earlier, and fixed in 3.0.10 Both issues are fixed on trunk and 1.9.1.

Renominating per comment 38 (though I can understand if we still want to push this to 1.9.0.12).

We're already code frozen for 1.9.0.11 and Dan and I discussed and decided this can wait for 1.9.0.12. I'd like it to bake anyway. We'll get it fixed in Firefox 3.0.12 (and please be sure to request approval on an appropriate patch).

Jason, want to spin up a 1.9.0 patch here? Or does that last patch apply to 1.9.0 cleanly?

Just needed one line change in .h file.

Comment on attachment 378526 [details] [diff] [review] Patch for 1.9.0 branch Approved for 1.9.0.12, a=dveditz for release-drivers

So it turns out that all other major browsers (IE 8, Safari, Chrome, Opera) no longer honor 3xx replies from HTTPS proxies. This is in response to the script/iframe vulnerabilities mentioned in the paper cited in bug 479880 (see section III.B: it's basically what we already describe in this bug report). We're handling the exploits in the paper correctly. But since we do support top-level redirects, an evil interloper could still redirect a user to a bogus site, and they might be vulnerable if they don't look at the location bar. I think it was worth supporting top-level redirects when it looked like some existing proxies relied on them for logins, but if all other browsers have now broken that use case, that's moot, and the main "use" for 3xx replies going forward might be the malicious scenario. I suspect we should back the patches out and change to WONTFIX. Locking down bug while we discuss.

> might be vulnerable if they don't look at the location bar That's always the case, no? Have we talked about this with the other browser vendors, by any chance? Maybe it's just that no one reported a bug to them about this case yet...

Checking in nsHttpChannel.h; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.h,v <-- nsHttpChannel.h new revision: 1.99; previous revision: 1.98 done Checking in nsHttpChannel.cpp; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v <-- nsHttpChannel.cpp new revision: 1.338; previous revision: 1.337 done

From the description in the paper, it sounds like IE and Chrome have not supported 30x replies from proxy servers for a while. But if we think we're handling them safely, we might as well keep them, just in case some proxy authors want to special-case and give us a redirect to a custom error page. It's better than not giving them any way to deliver their own content. Is there a well-known good way to talk about this with other browser vendors? I think I'll leave filing bug reports with apple, etc., to the proxy vendors. Making bug public again. Re: a 1.8 patch: I'm taking a PTO day today. If I see my 1.8 patch for 479880 land before Tuesday, I'll submit a separate patch for this bug; otherwise I'll roll them together (which bug would I attach a combo patch for? I assume 479880).

I landed bug 479880 on the 1.8 branch.

Comment on attachment 381170 [details] [diff] [review] Patch for 1.8 sr=dveditz -- this code is so little changed it doesn't need a full re-review.

Comment on attachment 381170 [details] [diff] [review] Patch for 1.8 Approved for 1.8.1.22. a=ss

Checked into 1.8(.1) branch. Checking in nsHttpChannel.cpp; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v <-- nsHttpChannel.cpp new revision: 1.256.2.24; previous revision: 1.256.2.23 done Checking in nsHttpChannel.h; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.h,v <-- nsHttpChannel.h new revision: 1.70.4.7; previous revision: 1.70.4.6 done

(In reply to comment #26) > Created an attachment (id=377596) [details] > Manual test HTML page for this bug > > This HTML page contains tests for inline <scripts>, <iframes>, and <images>, as > well as testing that top-level page redirects work. Step-by-step instructions > included. I used this in 1.8.1.22 testing with Seamonkey. Everything is fixed in the latest nightly builds except for step 14 ("Click here. You should be redirected to Mozilla.org."). This redirect fails with both the nightlies and with the shipped 1.1.16. I assume that the expected behavior, post-fix, is that the redirect works?

Yes, the last redirect to https://www.mozilla.org should work. But, did you follow step #1 (Make an exception for ".mozilla.com" in your proxy settings)? Oh, actually, there's a typo in my instructions--the exception should be for ".mozilla.org", because that's where the link goes to, not mozilla.com. Sigh. Sorry! Anyway, change the exception to use mozilla.org, and you should be fine.

Jason, yeah, I followed the instructions (this time) and tried that alternate, which did work. I was wondering if something was off. Ok, this is verified then. Verified1.8.1.22

Since there were some lingering sec concerns about allows HTTPS redirects with CONNECT, and other browsers don't support them, we've re-blocked them in bug 599295. So this will be WONTFIX as of Firefox 10 (i.e. whenever bug 599295 lands).

This problem could be fixed cleanly by handling HTTP error 511 Network Authentication Required as defined by the ietf httpbis workgroup (see bug #728658)