Bug 40138 - Authorization header is sent from an HTTP Auth protected site on redirect
Summary: Authorization header is sent from an HTTP Auth protected site on redirect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-06-03 13:57 PDT by Tosh
Modified: 2010-12-15 09:01 PST (History)
5 users (show)

See Also:


Attachments
Screen shot of Webkit Nightly Build after 302 redirection from Basic Auth page to no AUTH page (339.29 KB, image/png)
2010-07-22 15:27 PDT, Tosh
no flags Details
Patch v1 (10.89 KB, patch)
2010-12-14 16:59 PST, Brady Eidson
ap: review+
beidson: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tosh 2010-06-03 13:57:37 PDT
If I am on a Basic Auth protected website and click a link to download a file which sends the browser a 302 redirect to a new site hosting
Comment 1 Tosh 2010-06-03 14:03:02 PDT
ARG!!  Hit ENTER at the wrong time...

If I am on a Basic Auth protected website and click a link to download a file which sends the browser a 302 redirect to a new site hosting the file to be downloaded, then the Authorization header is sent to the new site, like this:

mysite.com/protected/download?fileID=123

Returns a 302 redirect to:

newSite.com/notProtected/download?fileID=123

The above URL will be sent the Authorization header from the original site.

Normally this is probably not a problem, but some web services these days accept either the option of Authorization headers or URL tokens to access their resources, and when BOTH are sent this causes errors.

I would expect Authorization headers to be sent ONLY to sites the browser knows are requesting them.

I hope this was a coherent bug report.
Comment 2 Alexey Proskuryakov 2010-06-03 19:45:24 PDT
Are you seeing this with nightly builds from <http://nightly.webkit.org>?
Comment 3 Tosh 2010-06-04 07:05:00 PDT
(In reply to comment #2)
> Are you seeing this with nightly builds from <http://nightly.webkit.org>?

I can't even be certain it's a webKit bug as it only happens with Safari 4.0.5 on OSX 10.5+ and 10.6+.  On 10.4+ the above behaviour is not exhibited.

I've also reported the same bug to Apple in case that matters.
Comment 4 Alexey Proskuryakov 2010-06-04 10:47:21 PDT
Can you please test this with a nightly build? Since there is no test case or example site for me to easily reproduce the problem, I'd like to know the results of your testing.

Nightly builds don't permanently replace anything in your OS or Safari installation, so there is no danger in testing them.
Comment 5 Tosh 2010-07-22 15:27:09 PDT
Created attachment 62349 [details]
Screen shot of Webkit Nightly Build after 302 redirection from Basic Auth page to no AUTH page

Screen shot of Webkit Nightly Build after 302 redirection from Basic Auth page to no AUTH page.

You should note:
Referer:http://www.filistics.com/tosh/files.pl?projectID=30

The above URL is Basic AUTH protected.  It sets a 302 redirect to a resource on S3, with the AUTH info contained in the URL.

But you will also see in the headers:
Authorization:Basic dG9zaDp0b3No

S3 doesn't like having TWO AUTH possibilities since both methods could be valid, so it throws an error.

It seems WebKit it passing along the Basic Auth status when it shouldn't be.
Comment 6 Tosh 2010-07-22 15:28:53 PDT
Damn that's kinda incoherent, let me know what I need to clarify, sorry.

Tosh
Comment 7 Alexey Proskuryakov 2010-07-22 16:23:59 PDT
<rdar://problem/8225016>
Comment 8 Brady Eidson 2010-12-14 16:53:43 PST
Firefox seems to automatically send credentials if the redirect is to a page in the same security origin, but strips them if the page is not.  We should probably match.
Comment 9 Brady Eidson 2010-12-14 16:59:05 PST
Created attachment 76595 [details]
Patch v1
Comment 10 Alexey Proskuryakov 2010-12-14 17:28:02 PST
Comment on attachment 76595 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=76595&action=review

> WebCore/platform/network/cf/ResourceHandleCFNet.cpp:485
> +    if (!protocolHostAndPortAreEqual(request.url(), redirectResponse.url()))
> +        request.clearHTTPAuthorization();

So, we're preserving the authorization header in more cases than Firefox? This doesn't seem great, although I can't imagine a practical situation where this would be a problem.

> LayoutTests/http/tests/loading/authentication-sent-to-redirect-expected.txt:8
> +frame "<!--framePath //<!--frame0-->-->" - didReceiveServerRedirectForProvisionalLoadForFrame
> +frame "<!--framePath //<!--frame0-->-->" - didCommitLoadForFrame

Does this test need to dump these? All this logging is good for is making tests flaky.
Comment 11 Brady Eidson 2010-12-14 17:32:36 PST
(In reply to comment #10)
> (From update of attachment 76595 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76595&action=review
> 
> > WebCore/platform/network/cf/ResourceHandleCFNet.cpp:485
> > +    if (!protocolHostAndPortAreEqual(request.url(), redirectResponse.url()))
> > +        request.clearHTTPAuthorization();
> 
> So, we're preserving the authorization header in more cases than Firefox? This doesn't seem great, although I can't imagine a practical situation where this would be a problem.

I *think* we're matching Firefox.

See my comment earlier:
"Firefox seems to automatically send credentials if the redirect is to a page in the same security origin, but strips them if the page is not."

This code matches that by stripping the auth headers only when the security origin of the two involved URLs differ.

> 
> > LayoutTests/http/tests/loading/authentication-sent-to-redirect-expected.txt:8
> > +frame "<!--framePath //<!--frame0-->-->" - didReceiveServerRedirectForProvisionalLoadForFrame
> > +frame "<!--framePath //<!--frame0-->-->" - didCommitLoadForFrame
> 
> Does this test need to dump these? All this logging is good for is making tests flaky.

It gets these simply by being an http/loading test.  You're right, they're not needed (though I don't forsee flakiness in this case), so I'll move them to a different directory.

Thanks for the review!
Comment 12 Brady Eidson 2010-12-14 17:50:48 PST
http://trac.webkit.org/changeset/74084
Comment 13 WebKit Review Bot 2010-12-14 18:18:36 PST
http://trac.webkit.org/changeset/74084 might have broken Qt Linux Release
The following tests are not passing:
http/tests/misc/authentication-sent-to-redirect.html
Comment 14 Brady Eidson 2010-12-15 09:01:43 PST
(In reply to comment #13)
> http://trac.webkit.org/changeset/74084 might have broken Qt Linux Release
> The following tests are not passing:
> http/tests/misc/authentication-sent-to-redirect.html

I don't see this failure on their bot anymore, but their bot isn't in great shape ATM.

It's quite possible Qt's networking had the same bug the Mac and Windows ports had, and they probably should fix it in the same manner.