Issue metadata
Sign in to add a comment
|
Show actionable advice for obsolete TLS settings in DevTools |
||||||||||||||||||||||||
Issue descriptionFiling 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
,
Oct 23
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/
,
Oct 23
Screenshots of contrast fix: https://drive.google.com/open?id=1VQV2xVo1sVKJD0sIgj48W-d_SRJCZHpG
,
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
,
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
,
Oct 25
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by davidben@chromium.org
, Oct 16