New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 655422 link

Starred by 1 user
Status: Fixed
Owner:
█ ananta@chromium.org
Last visit > 30 days ago
Closed: Nov 2016
Cc:
jam@chromium.org
clamy@chromium.org
scottmg@chromium.org
█ nasko@chromium.org
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

PlzNavigate: Renderer initiated navigations should pass a flag indicating whether the post data contains passwords.

Project Member Reported by ananta@chromium.org, Oct 13 2016 
Renderer initiated navigations in PlzNavigate are handled via RenderFrameImpl::BeginNavigate. This IPC passes in the form post data in the  
CommonNavigationParams structure.

This structure comes back from the browser in the CommitNavigation IPC where we eventually create a WebURLRequest to complete the navigation.

The problem here is that we lost the fact that the post data contains passwords. 
These would end up being serialized. The way this works today is that session 
restore code during shutdown removes the passwords from the page state before it 
is written out.

We need to pass the flag which indicates that the post contains passwords to the 
browser which should pass it back to the renderer via CommitNavigation. This will
ensure that we don't lose that state.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4dbb42fe4765527a3f0ed359caac5f3dce7b1440

commit 4dbb42fe4765527a3f0ed359caac5f3dce7b1440
Author: ananta <ananta@chromium.org>
Date: Sat Oct 15 02:09:27 2016

PlzNavigate: Fix the failing ContinueWhereILeftOffTest.PostWithPassword test.

This test exercises the session restore code path by posting passwords via a top level FORM post and
then exits the browser and restarts it. Assumption being that after restart we won't have the post data
available as it should have been stripped here https://cs.chromium.org/chromium/src/components/sessions/content/content_serialized_navigation_driver.cc?dr&q=GetSanitizedPageSta&sq=package:chromium&l=101
in the sesson restore code path during exit.

This fails in PlzNavigate because the FrameHostMsg_BeginNavigation IPC only sends the post data via the CommonNavigationParams structure.
Hence when the request eventually completes in the renderer via the FrameMsg_CommitNavigation IPC, we lose the fact that the post contains passwords.
This eventually makes it back to the browser via the PageState which also loses the flag.

Fix is to pass the flag which indicates the post sensitive data like passwords in the ResourceRequestBodyImpl structure.

BUG= 655422 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2406053002
Cr-Commit-Position: refs/heads/master@{#425533}

[modify] https://crrev.com/4dbb42fe4765527a3f0ed359caac5f3dce7b1440/content/child/web_url_request_util.cc
[modify] https://crrev.com/4dbb42fe4765527a3f0ed359caac5f3dce7b1440/content/common/page_state_serialization.cc
[modify] https://crrev.com/4dbb42fe4765527a3f0ed359caac5f3dce7b1440/content/common/resource_messages.cc
[modify] https://crrev.com/4dbb42fe4765527a3f0ed359caac5f3dce7b1440/content/common/resource_request_body_impl.cc
[modify] https://crrev.com/4dbb42fe4765527a3f0ed359caac5f3dce7b1440/content/common/resource_request_body_impl.h
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 18 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0bfce19cb62429f32b742ed00c66ba90214821bc

commit 0bfce19cb62429f32b742ed00c66ba90214821bc
Author: ananta <ananta@chromium.org>
Date: Tue Oct 18 23:08:25 2016

PlzNavigate: Enable the ContinueWhereILeftOffTest.PostWithPassword test.

Enable this test.

BUG= 655422 

Review-Url: https://codereview.chromium.org/2431533002
Cr-Commit-Position: refs/heads/master@{#426079}

[modify] https://crrev.com/0bfce19cb62429f32b742ed00c66ba90214821bc/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter

Comment 3 by ananta@chromium.org, Nov 2 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment