Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2333513002:
Fix SSL redirects with PlzNavigate. (Closed)
DescriptionFix SSL redirects with PlzNavigate.
When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests; it's not needed by the stream request job in content/, and it's already been checked/used for the http request earlier.
This fixes:
SSLUITest.ClientRedirectFromMixedContentSSLState
SSLUITest.ClientRedirectToMixedContentSSLState
SSLUITest.TestBadFrameNavigation
SSLUITest.TestUnsafeContents
BUG=504347
Committed: https://crrev.com/5e546b65e53db9fa79ad9b10fba50a902e9f5e1b
Cr-Commit-Position: refs/heads/master@{#417945}
Patch Set 1 #Patch Set 2 : slightly different fix #Messages
Total messages: 17 (13 generated)
The CQ bit was checked by jam@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by jam@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The solution is to adjust the policy for the PlzNavigate stream request so that this referrer checking isn't done. This fixes SSLUITest.ClientRedirectFromMixedContentSSLState and SSLUITest.ClientRedirectToMixedContentSSLState. BUG=504347 ========== to ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests. This fixes SSLUITest.ClientRedirectFromMixedContentSSLState and SSLUITest.ClientRedirectToMixedContentSSLState. BUG=504347 ==========
jam@chromium.org changed reviewers: + clamy@chromium.org
This seems to be a side effect of r374891 There are various ways to fix this, not sure which one is the best one: -patchset 2 just doesn't set the referrer for the stream request, which is fine since it's not used (i.e. the request has already been made) -patchset 1 adjusted the referrer policy -we could update the net layer to consider blob:https:// to be secure (not sure of all the implications with that though) -other ideas?
Description was changed from ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests. This fixes SSLUITest.ClientRedirectFromMixedContentSSLState and SSLUITest.ClientRedirectToMixedContentSSLState. BUG=504347 ========== to ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests. This fixes: SSLUITest.ClientRedirectFromMixedContentSSLState SSLUITest.ClientRedirectToMixedContentSSLState SSLUITest.TestBadFrameNavigation SSLUITest.TestUnsafeContents BUG=504347 ==========
Description was changed from ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests. This fixes: SSLUITest.ClientRedirectFromMixedContentSSLState SSLUITest.ClientRedirectToMixedContentSSLState SSLUITest.TestBadFrameNavigation SSLUITest.TestUnsafeContents BUG=504347 ========== to ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests. This fixes: SSLUITest.ClientRedirectFromMixedContentSSLState SSLUITest.ClientRedirectToMixedContentSSLState SSLUITest.TestBadFrameNavigation SSLUITest.TestUnsafeContents BUG=504347 ==========
I think it makes more sense not to set the referrer at all (patchset 2) since it was already checked when the actual request was made. Version 2 lgtm.
The CQ bit was checked by jam@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests. This fixes: SSLUITest.ClientRedirectFromMixedContentSSLState SSLUITest.ClientRedirectToMixedContentSSLState SSLUITest.TestBadFrameNavigation SSLUITest.TestUnsafeContents BUG=504347 ========== to ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests; it's not needed by the stream request job in content/, and it's already been checked/used for the http request earlier. This fixes: SSLUITest.ClientRedirectFromMixedContentSSLState SSLUITest.ClientRedirectToMixedContentSSLState SSLUITest.TestBadFrameNavigation SSLUITest.TestUnsafeContents BUG=504347 ==========
Message was sent while issue was closed.
Description was changed from ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests; it's not needed by the stream request job in content/, and it's already been checked/used for the http request earlier. This fixes: SSLUITest.ClientRedirectFromMixedContentSSLState SSLUITest.ClientRedirectToMixedContentSSLState SSLUITest.TestBadFrameNavigation SSLUITest.TestUnsafeContents BUG=504347 ========== to ========== Fix SSL redirects with PlzNavigate. When a redirect between HTTPS URLs occurs, the default policy will check that the referrer is only set if it's allowed (i.e. both URLs are secure). However for the PlzNavigate stream request occurs, the destination will be a blob:https scheme and net/ will treat it as insecure and remove the referrer (see URLRequestJob::ComputeReferrerForRedirect). Then URLRequest::StartJob will find a referrer mismatch and cancel the request. The fix is to not tell the net layer about the referrer for the PlzNavigate stream requests; it's not needed by the stream request job in content/, and it's already been checked/used for the http request earlier. This fixes: SSLUITest.ClientRedirectFromMixedContentSSLState SSLUITest.ClientRedirectToMixedContentSSLState SSLUITest.TestBadFrameNavigation SSLUITest.TestUnsafeContents BUG=504347 Committed: https://crrev.com/5e546b65e53db9fa79ad9b10fba50a902e9f5e1b Cr-Commit-Position: refs/heads/master@{#417945} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/5e546b65e53db9fa79ad9b10fba50a902e9f5e1b Cr-Commit-Position: refs/heads/master@{#417945} |
