Issue metadata
Sign in to add a comment
|
Explanation has wrong severity for SHA-1 in Security Panel |
||||||||||||||||||||||||
Issue descriptionChrome Version: 59.0.3053.0 What steps will reproduce the problem? (1) Visit https://sha1-2017.badssl.com/ (2) Open DevTools Security panel (3) Observe: "Sha-1 certificate" explanation shown with Neutral treatment rather than Broken treatment. This: if (security_info.sha1_in_chain) { security_style_explanations->neutral_explanations.push_back( ... should be if (security_info.sha1_in_chain) { security_style_explanations->insecure_explanations.push_back( ... if (security_info.cert_status & WEAK_SIGNATURE_ALGORITHM)
,
Sep 13 2017
Carlos is working on this one
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/232281fc945e0318ac59421a6a5dd0097f2717f5 commit 232281fc945e0318ac59421a6a5dd0097f2717f5 Author: Carlos IL <carlosil@google.com> Date: Wed Sep 13 16:25:57 2017 SHA1 Certificate explanation in security panel is now 'Broken' Websites with Security errors due to SHA1 signed certificates in the chain now show up with the broken (red triangle) indicator in the DevTools Security panel, instead of the neutral (red circle) indicator. R=estark@chromium.org Bug: 705671 Change-Id: I400936a3221ddeea65897a83f4ad04e6683e2d22 Reviewed-on: https://chromium-review.googlesource.com/664343 Reviewed-by: Emily Stark <estark@chromium.org> Commit-Queue: Carlos Joan Rafael Ibarra Lopez <carlosil@google.com> Cr-Commit-Position: refs/heads/master@{#501656} [modify] https://crrev.com/232281fc945e0318ac59421a6a5dd0097f2717f5/chrome/browser/ssl/security_state_tab_helper_browser_tests.cc [modify] https://crrev.com/232281fc945e0318ac59421a6a5dd0097f2717f5/components/security_state/content/content_utils.cc
,
Sep 13 2017
FWIW, the proposal here was to also check that security_info.cert_status & WEAK_SIGNATURE_ALGORITHM to handle the corner case wherein the user's on a page signed with an Enterprise SHA1 certificate with the enablesha1forlocalanchors group policy set. https://cs.chromium.org/chromium/src/net/cert/cert_verify_proc.cc?l=622&rcl=232281fc945e0318ac59421a6a5dd0097f2717f5 (It's probably a puntable corner case, but noting for posterity).
,
Sep 20 2017
Will start working on that corner case, just making sure I understand it correctly, in the case where the policy is set and the certificate is an Enterprise one, we should treat it as secure? or as neutral?
,
Sep 20 2017
When the policy is set and the certificate chains to a local anchor, the /net/cert code skips setting CERT_STATUS_WEAK_SIGNATURE_ALGORITHM on the result. The code in security_state uses the presence of SHA1 without CERT_STATUS_WEAK_SIGNATURE_ALGORITHM set to change the security state to NONE (aka neutral): https://cs.chromium.org/chromium/src/components/security_state/core/security_state.cc?l=197&rcl=170f5d42dab16c6733cadad4a8124068e5341c76 In contrast, when the policy isn't set or the root isn't a local root, the /net/cert code sets CERT_STATUS_WEAK_SIGNATURE_ALGORITHM and the security_state code sets the state to DANGEROUS (aka insecure): https://cs.chromium.org/chromium/src/components/security_state/core/security_state.cc?l=149&rcl=170f5d42dab16c6733cadad4a8124068e5341c76
,
Sep 20 2017
,
Sep 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/56136aecba7e8d5c01bf40e44b2afb0655181396 commit 56136aecba7e8d5c01bf40e44b2afb0655181396 Author: Carlos IL <carlosil@chromium.org> Date: Mon Sep 25 18:20:29 2017 Changed content_utils to handle case where SHA1 certificates are allowed Changed ExplainCertificateSecurity so that when there is a SHA1 signature in the chain it now checks that cert_status is CERT_STATUS_WEAK_SIGNATURE_ALGORITHM, if so it adds the explanation as insecure, if not (in the case where enablesha1forlocalanchoses is set and the certificiate is an Enterprise SHA1 one) it adds the explanation as neutral. Also changed a test in security_state_tab_helper_browser_tests so it matches the new behavior. Bug: 705671 Change-Id: I3cf775324a8df856fbc67168982e26a24c7eb6e1 Reviewed-on: https://chromium-review.googlesource.com/676198 Commit-Queue: Carlos IL <carlosil@chromium.org> Reviewed-by: Eric Lawrence <elawrence@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Cr-Commit-Position: refs/heads/master@{#504106} [modify] https://crrev.com/56136aecba7e8d5c01bf40e44b2afb0655181396/chrome/browser/ssl/security_state_tab_helper_browser_tests.cc [modify] https://crrev.com/56136aecba7e8d5c01bf40e44b2afb0655181396/components/security_state/content/content_utils.cc
,
Sep 25 2017
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by est...@chromium.org
, Jun 7 2017