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

Issue 647759 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug
Team-Security-UX



Sign in to add a comment

Cert viewer is not hooked up to interstitials properly

Project Member Reported by lgar...@chromium.org, Sep 16 2016

Issue description

Chrome 55.0.2859.0
OSX 10.11.6

What steps will reproduce the problem?
(1) Visit untrusted-root.badssl.com
(2) Open the DevTools Security panel
(3) Click on "View certificate"

What is the expected output?
The cert viewer opens, with a 1-certificate chain.

What do you see instead?
If there was a previous page (e.g. you navigated from badssl.com), the cert viewer opens for that page.
If there was no previous page, Chrome crashes.

On the commandline:
2016-09-16 11:16:47.813 Google Chrome Canary[67536:4129441] Failed to connect (_okButton) outlet from (SFCertificatePanel) to (NSButton): missing setter or instance variable

Bisect range:
https://chromium.googlesource.com/chromium/src/+log/8ccc7ad89e21c857ab2b317a4875c881cf4fcf35..2d25b500542df1129457767aef5ec97b31ca695e

jam@, I'm guessing this one is you:
8ae7cad Stop using CertStore which is not compatible with PlzNavigate. by jam ยท 8 days ago
https://chromium.googlesource.com/chromium/src/+/8ae7cadae586d05e906fa581702e77c32080c4e7

From the symptoms, I'm guessing that the cert viewer action is trying to route the "show certificate" command to the underlying page rather than the interstitial.
The underlying cause is probably Issue 392354 (Interstitials are weird and do weird things), but we need a more superficial fix.
 
> I think GetLastCommittedEntry() should be GetVisibleEntry() here? 

That sounds like the right kind of change. (If it works, it's probably the right change.)

felt@ would probably know more.

Comment 3 by f...@chromium.org, Sep 16 2016

Re #1: Yes, I suspect that's the problem.

Comment 4 by est...@chromium.org, Sep 16 2016

Cc: jam@chromium.org
Owner: est...@chromium.org
I'll send a fix.

Comment 5 by jam@chromium.org, Sep 16 2016

(just saw my bug mail). sorry for the regression and thanks for the fix. if you get tied up with other stuff I'm also happy to fix it.
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 17 2016

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

commit b056baab94111806630ff183834064a461906e42
Author: estark <estark@chromium.org>
Date: Sat Sep 17 01:19:33 2016

Fix DevTools showCertificateViewer to use visible entry

When the Security.showCertificateViewer command is received by the
browser, it should show the certificate for the visible entry, not
necessarily the last committed entry, because there might be a transient
entry for an interstitial overlaying the last committed entry.

BUG= 647759 

Review-Url: https://codereview.chromium.org/2344113003
Cr-Commit-Position: refs/heads/master@{#419359}

[modify] https://crrev.com/b056baab94111806630ff183834064a461906e42/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
[modify] https://crrev.com/b056baab94111806630ff183834064a461906e42/content/browser/devtools/protocol/security_handler.cc

Comment 7 by est...@chromium.org, Sep 18 2016

Status: Fixed (was: Assigned)
Labels: hasbisect-per-revison
Tested per-revision bisecting, which led tot he exact commit: https://chromium.googlesource.com/chromium/src/+log/7e820c3c44dbc74eeb41bd24511b355d2dcfde38..8ae7cadae586d05e906fa581702e77c32080c4e7

:-)

Comment 9 by son...@google.com, Oct 7 2016

Status: Verified (was: Fixed)
Verified on build 8872.0.0
Components: -Security>UX

Sign in to add a comment