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

Issue 736183 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug
Team-Security-UX

Blocking:
issue 657299
issue 744601



Sign in to add a comment

Move mixed content string calculation from DevTools to GetSecurityStyle() in content_utils

Project Member Reported by lgar...@chromium.org, Jun 23 2017

Issue description

This calculation needs to be shared with the other security bullet calculations in https://crbug.com/657299

- Create mixed content explanations in GetSecurityStyle() instead of DevTools.
- Add bool describes_subresource_security to SecurityStyleExplanation 
- DevTools: Replace custom string logic for the mixed content bullet with the describes_subresource_security explanation.
  - The “View 1 request in Network Panel” link should still be calculated in DevTools, but attached to the explanation with describes_subresource_security

 
Also: stop sending parts of SecurityStateExplanations that DevTools will not need anymore.
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 6 2017

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

commit 68fe95883c015e32cdc96f107a2442630b14595c
Author: Lucas Garron <lgarron@chromium.org>
Date: Thu Jul 06 03:56:52 2017

Add a mixed content type field to SecurityStyleExplanation and send it to DevTools.

This prepares us for a future where mixed content explanations are calculated in the browser.

This change also refactors the DevTools protocol to share the MixedContentType
definition across the Security and Network protocols. (Previously, there was
only an inline definition in Network.Resource).

Bug: 657299,  736183 
Change-Id: I5a53570114f4eff2e95865da015e078b01db518d
Reviewed-on: https://chromium-review.googlesource.com/550854
Commit-Queue: Lucas Garron <lgarron@chromium.org>
Reviewed-by: Blaise Bruer <allada@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484459}
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/components/security_state/DEPS
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/components/security_state/content/content_utils.cc
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/components/security_state/content/content_utils_unittest.cc
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/content/browser/devtools/protocol/security_handler.cc
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/content/public/browser/security_style_explanation.h
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/core/inspector/browser_protocol.json
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/devtools/front_end/sdk/NetworkManager.js
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/devtools/front_end/sdk/NetworkRequest.js
[modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js

Blocking: 740247
Blocking: -740247
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 12 2017

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

commit 67060f4406843ceb42a8a23cec7b14a757d2d974
Author: Lucas Garron <lgarron@chromium.org>
Date: Wed Jul 12 00:16:57 2017

Refactor GetSecurityStyle() in content_utils into clear helper functions.

No explanations are added or removed in this CL.

The code for Explain*Security has been moved directly from GetSecurityStyle, so
the set of elements in the lists of returned explanations should not change.
However, the the explanations for a given severity are now consistently by the same topic order (HTTP/certificate/connection/content security).

Bug:  736183 
Change-Id: Ic23f2cad22fb9934e8501b93c5c413a911be9585
Reviewed-on: https://chromium-review.googlesource.com/564217
Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: Lucas Garron <lgarron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485740}
[modify] https://crrev.com/67060f4406843ceb42a8a23cec7b14a757d2d974/components/security_state/content/content_utils.cc

Blocking: 744601
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 25 2017

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

commit bb87a597a382716b402cc20d3593f0b066cd9940
Author: Lucas Garron <lgarron@chromium.org>
Date: Tue Jul 25 17:35:45 2017

Move mixed content explanation calculation from DevTools to GetSecurityStyle().

# Layout Test Updates

## active-and-passive-subresources-with-cert-errors.html

- Removed.
  - Now tested by `SecurityStateContentUtilsTest.CertErrorContentExplanations`.

## active-subresource-with-cert-errors.html

- Removed.
  - Now tested by `SecurityStateContentUtilsTest.CertErrorContentExplanations`.

## blocked-mixed-content-and-subresources-with-cert-errors.html

- "subresources-with-cert-errors" part removed.
  - Now handled by `SecurityStateContentUtilsTest.CertErrorContentExplanations`.
- Renamed to `blocked-mixed-content.html`

## mixed-content-active-and-passive-reload.html

- Explicitly pass explanations for passive mixed content (neutral) and active
  mixed content (insecure) into the SecurityStateChanged calls.
- Add before/after refresh markers.

## mixed-content-and-subresources-with-cert-errors.html

- Removed.
  - Now tested by
    `SecurityStateContentUtilsTest.MixedContentAndCertErrorExplanations`.

## mixed-content-reload.html

- Explicitly pass explanation for passive mixed content (neutral) into the
  SecurityStateChanged calls.
- Add before/after refresh markers.

## mixed-form.html

- Removed
  - Mixed forms tested by
    `SecurityStateContentUtilsTest.MixedContentExplanations`.

## passive-subresource-with-cert-errors.html

- Removed.
  - Now tested by `SecurityStateContentUtilsTest.CertErrorContentExplanations`.

## security-all-resources-secure.html

- Removed.
  - Now tested by `SecurityStateContentUtilsTest.SecureContentExplanation`.

## security-blocked-mixed-content-and-malicious.html

- Removed.
  - Mixed content explanation test redundant with `security-blocked-mixed-
    content.html`
  - Created `security-summary.html` to test the incidental security summary
    "override" previously in this test.

## security-blocked-mixed-content.html

- Removed secure content explanation (not handled in DevTools anymore).

## security-explanation-ordering.html

- Added `Secure resources` explanation to the list used for this test.

## security-secure-but-malicious.html

- Removed.
  - Summary override handled by new `security-summary.html` test.
  - Safe Browsing state was already being calculated in browser. Added a
    `SecurityStateContentUtilsTest.GetSecurityStyleForSafeBrowsing` test for it.

Bug:  736183 , 657299
Change-Id: I63738fa85b38302ce6024711be083ab60bde0bc6
Reviewed-on: https://chromium-review.googlesource.com/557427
Commit-Queue: Lucas Garron <lgarron@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489355}
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/chrome/browser/ssl/security_state_tab_helper_browser_tests.cc
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/components/security_state/content/BUILD.gn
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/components/security_state/content/content_utils.cc
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/components/security_state/content/content_utils_unittest.cc
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/components/security_state_strings.grdp
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-and-passive-subresources-with-cert-errors-expected.txt
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-and-passive-subresources-with-cert-errors.html
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-subresource-with-cert-errors-expected.txt
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-subresource-with-cert-errors.html
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/blocked-mixed-content-and-subresources-with-cert-errors-expected.txt
[add] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/blocked-mixed-content-expected.txt
[rename] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/blocked-mixed-content.html
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-active-and-passive-reload-expected.txt
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-active-and-passive-reload.html
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-and-subresources-with-cert-errors-expected.txt
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-and-subresources-with-cert-errors.html
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-reload-expected.txt
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-reload.html
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-form-expected.txt
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-form.html
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/passive-subresource-with-cert-errors-expected.txt
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/passive-subresource-with-cert-errors.html
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-all-resources-secure-expected.txt
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-all-resources-secure.html
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content-and-malicious-expected.txt
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content-and-malicious.html
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content-expected.txt
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-explanation-ordering.html
[delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-secure-but-malicious-expected.txt
[add] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-summary-expected.txt
[rename] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-summary.html
[modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js

Status: Fixed (was: Started)

Sign in to add a comment