New issue
Advanced search Search tips

Issue 889994 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature
Team-Security-UX



Sign in to add a comment

Show actionable advice for obsolete TLS settings in DevTools

Project Member Reported by davidben@chromium.org, Sep 27

Issue description

Filing a bug so I have a place to staple the CL.

See DevTools section of https://docs.google.com/document/d/14JVhCF1g--AAdL5grVmmPJpqSAygwCJj2G_JDGNzleQ/edit?ts=5bad0516
 
Labels: M-72
Status: Started (was: Assigned)
https://chromium-review.googlesource.com/c/chromium/src/+/1250164/

Per review comment, also including the description of the change here:

The security panel currently points out obsolete features in the form of a long
cumbersome sentence like:

    The connection to this site uses TLS 1.2 (a strong protocol), ECDHE_RSA
    with P-256 (a strong key exchange), and AES_256_CBC with HMAC-SHA-1 (an
    obsolete cipher).

It is hard to pick out what is wrong, and it does not say what to use instead.
It also does not allow for new fields without being even more cumbersome. (The
server signature is missing.)

Replace this with *actionable* advice for the site administrator.
Implementation-wise this does the following:

- Replace "secure (strong TLS 1.2)" with "secure connection settings" to mirror
  the existing "obsolete connection settings" text.

- Extend SecurityStyleExplanation with a list of recommendations. These show as
  a list under the "obsolete" heading.

- With the secure/obsolete details moved to the recommendation list. Remove the
  "(a strong protocol)" and "(an obsolete cipher)" parentheses and simplify the
  top-level sentence.

- And with the cumbersome sentence fixed, *finally* unblock incorporating the
  server signature into the secure/obsolete status.

Screenshots: https://drive.google.com/open?id=1-JTy2aMWvKImrZSKrOWlUAG7WU2KKarg

Note the thing with the ordering jumping around is existing behavior. The contrast is also not great, but that is being addressed in https://chromium-review.googlesource.com/c/chromium/src/+/1255895/
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 25

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

commit e7e073ef030ef6d51ea77878bbe9e46cd2007c41
Author: David Benjamin <davidben@chromium.org>
Date: Thu Oct 25 01:26:06 2018

Show actionable advice for obsolete TLS settings in DevTools

The security panel currently points out obsolete features in the form of a long
cumbersome sentence like:

    The connection to this site uses TLS 1.2 (a strong protocol), ECDHE_RSA
    with P-256 (a strong key exchange), and AES_256_CBC with HMAC-SHA-1 (an
    obsolete cipher).

It is hard to pick out what is wrong, and it does not say what to use instead.
It also does not allow for new fields without being even more cumbersome. (The
server signature is missing.)

Replace this with *actionable* advice for the site administrator.
Implementation-wise this does the following:

- Replace "secure (strong TLS 1.2)" with "secure connection settings" to mirror
  the existing "obsolete connection settings" text.

- Extend SecurityStyleExplanation with a list of recommendations. These show as
  a list under the "obsolete" heading.

- With the secure/obsolete details moved to the recommendation list. Remove the
  "(a strong protocol)" and "(an obsolete cipher)" parentheses and simplify the
  top-level sentence.

- And with the cumbersome sentence fixed, *finally* unblock incorporating the
  server signature into the secure/obsolete status. This CL thus additionally
  routes the server signature out of //net and into the SecurityStyleExplanation
  logic.

Screenshots: https://drive.google.com/open?id=1-JTy2aMWvKImrZSKrOWlUAG7WU2KKarg

This CL does not change the styling or other details of how the security style
explanations are presented, though it's been noted the contrast is too low, so
we may wish to fix the styling either as a follow-up or in this CL.

Bug:  889994 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I913679f77fd5fb0cf018c4a20c2f098effa32674
Reviewed-on: https://chromium-review.googlesource.com/c/1250164
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: Adrienne Porter Felt <felt@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602557}
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/chrome/browser/ssl/security_state_tab_helper_browsertest.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/components/security_state/content/content_utils.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/components/security_state/content/content_utils_unittest.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/components/security_state/core/security_state.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/components/security_state/core/security_state.h
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/components/security_state_strings.grdp
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/content/browser/devtools/protocol/security_handler.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/content/browser/loader/resource_loader.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/content/common/common_param_traits_unittest.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/content/public/browser/security_style_explanation.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/content/public/browser/security_style_explanation.h
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/content/public/browser/ssl_status.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/content/public/browser/ssl_status.h
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/net/http/http_response_info.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/net/http/http_response_info_unittest.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/net/quic/quic_chromium_client_session.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/net/ssl/ssl_cipher_suite_names.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/net/ssl/ssl_cipher_suite_names.h
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/net/ssl/ssl_cipher_suite_names_unittest.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/net/ssl/ssl_info.h
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/services/network/public/cpp/net_ipc_param_traits.cc
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/third_party/blink/renderer/core/inspector/browser_protocol.pdl
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/third_party/blink/renderer/devtools/front_end/security/SecurityPanel.js
[modify] https://crrev.com/e7e073ef030ef6d51ea77878bbe9e46cd2007c41/third_party/blink/renderer/devtools/front_end/security/mainView.css

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 25

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

commit 343b3cf56e929c08b2797b2dcf75a704fc4a5ce1
Author: David Benjamin <davidben@chromium.org>
Date: Thu Oct 25 19:45:42 2018

Improve contrast in the DevTools security panel.

The current coloring is:

Background: #fff or #f3f3f3 (DevTools background)
Title text: rgb(48, 57, 66) or #303942 (DevTools default)
Heading text: rgb(90, 90, 90) or #5a5a5a
Explanation text: #8c8c8c

Obsolete settings end up putting #8c8c8c over #f3f3f3 which rather low
contrast. The last two colors are only used in the security panel, so
they don't appear to come from some overall DevTools palette.

Darken everything up a notch. Match the heading text with the title and
DevTools default. (The title font is already much larger than the
heading font, so we retain visual hierachy.) That frees rgb(90, 90, 90)
for use with the explanation text, which is more readable over #f3f3f3
and is still lighter for visual hierarchy.

Screenshots: https://drive.google.com/open?id=1VQV2xVo1sVKJD0sIgj48W-d_SRJCZHpG

Bug:  889994 
Change-Id: I539088f3026db4db2d50a5a27a3f6abf30da3e4f
Reviewed-on: https://chromium-review.googlesource.com/c/1255895
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602832}
[modify] https://crrev.com/343b3cf56e929c08b2797b2dcf75a704fc4a5ce1/third_party/blink/renderer/devtools/front_end/security/mainView.css

Status: Fixed (was: Started)

Sign in to add a comment