New issue
Advanced search Search tips

Issue 750649 link

Starred by 3 users
Status: Fixed
Owner:
est...@chromium.org
Closed: Aug 2017
Cc:
elawrence@chromium.org
jam@chromium.org
est...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression
Team-Security-UX



Sign in to add a comment

Secure content marked as insecure on page reload once security status is lowered

Project Member Reported by ha...@opera.com, Jul 31 2017 
Chrome Version: 60.0.3112.78; also Canary 62.0.3168.0
OS: Win7

If a secure page loads some unsecured content (e.g. an image ) over http:  dynamically, the page's security status isn't reset on reloading it by the reload button or the reload context menu. 

If page reload is done by pressing enter in the address field, the security status is back to secure.

What steps will reproduce the problem?
(1) Load https://mixed-content-test.appspot.com/ ensure that padlock is shown.
(2) Click http image button to load an image over plain http 
(3) Press page reload button or context menu reload.

What is the expected result?
Page should regain the secure status as no unsecured content is loaded.

What happens instead?
Page security status is still lowered (non-secure).

This used to work as in the expected result in M59.

Comment 1 by elawrence@chromium.org, Jul 31 2017

Cc: -pkasting@chromium.org est...@chromium.org
Labels: hasbisect
This also reproduces in Back/Forward navigation (e.g. Step 3, navigate anywhere, then click back).

You are probably looking for a change made after 477816 (known good), but no later than 477837 (first known bad).
 https://chromium.googlesource.com/chromium/src/+log/4cd85bfda2021baba6a044d565323ab88e598398..db06e65dcd8da3f5b8cab60ecb32e9ba89c1598c

My guess is it's this one:
https://chromium.googlesource.com/chromium/src/+/db06e65dcd8da3f5b8cab60ecb32e9ba89c1598c%5E%21/#F0

This, in turn, may be the same root cause for other issues like crbug.com/747095. Fundamentally, the problem is that SSLStatus contains several flags which relate to operations on the page, and if a SSLStatus is reused after back/forward navigation, those SSLStatus flags may be incorrect.

In an upcoming CL, I'll be extracting the DISPLAYED_PASSWORD_FIELD_ON_HTTP and DISPLAYED_CREDIT_CARD_FIELD_ON_HTTP flags from the SSLStatus, but DISPLAYED_INSECURE_CONTENT and RAN_INSECURE_CONTENT are the relevant flags for this bug.

Comment 2 by elawrence@chromium.org, Jul 31 2017 

Confirmed by per-revision bisect.

You are probably looking for a change made after 477836 (known good), but no later than 477837 (first known bad).
https://chromium.googlesource.com/chromium/src/+log/e043e3ceedef5d2ab6eb984d8f0a627989abad3e..db06e65dcd8da3f5b8cab60ecb32e9ba89c1598c

Comment 3 by est...@chromium.org, Jul 31 2017

Cc: -est...@chromium.org
Labels: -Pri-3 M-62 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1
Owner: est...@chromium.org
Status: Assigned (was: Untriaged)
I think the right thing to do here is clear content status flags when a navigation commits, somewhere around here: https://cs.chromium.org/chromium/src/content/browser/ssl/ssl_manager.cc?sq=package:chromium&l=193
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 2 2017 

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

commit 9f784d27459c5a4e1c5fc3a77931870b1b847946
Author: Emily Stark <estark@google.com>
Date: Wed Aug 02 21:25:54 2017

Clear SSLStatus content status flags on navigation

As of https://codereview.chromium.org/2926803002, when navigating to an existing
entry, the new navigation's SSLStatus remains unchanged from the existing
entry. This is incorrect for content status flags, which depend on the content
of the page; navigating to an existing entry does not mean that the content on
the page is the same as it was when we navigated away from the existing
entry. For this reason, this CL clears content status flags when a navigation
commits. Any content status flags that do apply to the new navigation should be
re-added as the content on the page loads.

Bug:  750649 
Change-Id: I41441c90ddeb85b6cbf35e4b102ce322cffb7db9
Reviewed-on: https://chromium-review.googlesource.com/597488
Reviewed-by: Eric Lawrence <elawrence@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491504}
[modify] https://crrev.com/9f784d27459c5a4e1c5fc3a77931870b1b847946/chrome/browser/ssl/ssl_browser_tests.cc
[modify] https://crrev.com/9f784d27459c5a4e1c5fc3a77931870b1b847946/content/browser/ssl/ssl_manager.cc
[modify] https://crrev.com/9f784d27459c5a4e1c5fc3a77931870b1b847946/content/browser/ssl/ssl_manager.h

Comment 5 by est...@chromium.org, Aug 2 2017

Status: Fixed (was: Assigned)

Comment 6 by elawrence@chromium.org, Sep 25 2017

Cc: est...@chromium.org
 Issue 746566  has been merged into this issue.
►

Sign in to add a comment