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

Issue 647822 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature
Team-Security-UX

Blocking:
issue 646545
issue 647754



Sign in to add a comment

Update SSLStatus when passwords are detected in a page [iOS]

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

Issue description

Create a new class, IOSHttpDowngradeHandler:
This class will be responsible for handling events, checking whether the top-level frame is displaying a non-secure origin, and (if so) updating the SecurityStateModel.
It will implement OnPasswordFormsRendered.
It will require access to a WebState.

PasswordController is the Bling equivalent of ContentPasswordManagerDriver.
It should create a IOSHttpDowngradeHandler, passing it a WebState. (PasswordController has a webStateObserverBridge, from which you can retrieve a web_state().)
You should hook didFinishPasswordFormExtraction to identify when password forms are rendered. When that occurs, call ios_http_downgrade_handler_->OnPasswordFormsRendered.

The detection code should record the event on the NavigationItem’s SSLStatus.
Extend web::SSLStatus to be aware of the HTTP bad detection events. Specifically, add new flags to the ContentStatusFlags enum: DISPLAYED_PASSWORD_FIELD and DISPLAYED_CREDIT_CARD_FIELD. These flags should be set on the current NavigationItem’s SSLStatus whenever the detection logic identifies a password or credit card form field.

The detection code should trigger a refresh of the security UI elements by calling webControllerDidUpdateSSLStatusForCurrentNavigationItem. This will cause the security UI elements to request updated security information from the SSM.

https://docs.google.com/document/d/1xno6g6OnA7strcyzE-o_drevW8L0Mb6ZBEkjsiwa6x0/edit#heading=h.zax2c3mff695
 

Comment 1 by est...@chromium.org, Sep 21 2016

Updated plan from the doc below. The main difference is that we don't need a separate HttpDowngradeHandler class, we can just make a method on the WebState to update the SSLStatus.

Create a new WebStateImpl method, OnSensitiveInputShownOnHttp, which parallels the WebContentsImpl method.
This method should be called whenever a top-level frame is displaying a non-secure origin and displays a password or credit card input.
When called, this method should set the relevant SSLStatus flags on the visible NavigationItem’s SSLStatus.
PasswordController is the Bling equivalent of ContentPasswordManagerDriver.
PasswordController has a webStateObserverBridge, from which you can retrieve a web_state().
didFinishPasswordFormExtraction is called when password forms are rendered. When that occurs, PasswordController should call OnSensitiveInputShownOnHttp on the WebState.
Components: -Security>UX Internals>PageSecurityState
Components: Internals>Permissions>CrowdConsent
Components: -Internals>Permissions>CrowdConsent
Blocking: 646545
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/ca43dc029a86dc54f33777efc21823b7bb5029ad

commit ca43dc029a86dc54f33777efc21823b7bb5029ad
Author: lgarron <lgarron@google.com>
Date: Wed Dec 07 21:39:50 2016

Comment 7 by est...@chromium.org, Dec 13 2016

Status: Started (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 15 2016

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

commit 5f03ba5ae3b6fa1059da663fa8c86f8846b16e31
Author: lgarron <lgarron@chromium.org>
Date: Thu Dec 15 03:05:43 2016

iOS: Mark HTTP pages with password fields with an omnibox icon.

BUG= 647822 
================================
TEST=Use an iPhone, not an iPad. First, enable the proper flag:
--------------------------------
1. Open the Settings app
2. Scroll to Chrome Beta/Dev/Canary and press
3. Scroll down to Experimental Settings and press
4. Scroll to EXTRA FLAGS (ONE PER LINE)
5. Toggle "Append Extra Flags" to ON
6. Set Flag1 to "--mark-non-secure-as=show-non-secure-passwords-cc-ui" (without the quotes)
--------------------------------
Test 3 URLs:
1) Visit https://badssl.com/input/login/ and verify that the omnibox security has a green lock security indicator to the left of the URL.
2) Visit http://http-login.badssl.com/ and verify that the omnibox security has a grey info icon ⓘ security indicator to the left of the URL.
3) Visit http://http.badssl.com/ and verify that the omnibox does *not* have a security indicator to the left of the URL.
--------------------------------
4) Set Flag1 (see above) to "--mark-non-secure-as=neutral" (without the quotes) and check that http://http-login.badssl.com/ does *not* have a security indicator to the left of the URL.
================================

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

[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/chrome/browser/passwords/password_controller.mm
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/chrome/browser/passwords/password_controller_unittest.mm
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/chrome/browser/ssl/ios_security_state_tab_helper.mm
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/web/public/ssl_status.h
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/web/public/test/test_web_state.h
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/web/public/web_state/web_state.h
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/web/web_state/ui/crw_web_controller_unittest.mm
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/5f03ba5ae3b6fa1059da663fa8c86f8846b16e31/ios/web/web_state/web_state_impl.mm

Labels: M-57
Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 15 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/b09d0ee00472b77b3e752c51367423bb4fe091f7

commit b09d0ee00472b77b3e752c51367423bb4fe091f7
Author: lgarron <lgarron@google.com>
Date: Thu Dec 15 05:01:37 2016

Status: Verified (was: Fixed)
Verified in 57.0.2964.0 canary, iPhone 6S iOS 10.1, iPhone iOS 9.3.5
followed steps from comment #8, looks good.

Sign in to add a comment