Implement 'Not secure' warning for non-secure pages in Incognito mode

chrome/browser/ssl/security_state_tab_helper.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ssl/security_state_tab_helper.h"
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
 #include "components/prefs/pref_service.h"
 #include "components/security_state/content/content_utils.h"
 #include "components/ssl_config/ssl_config_prefs.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/origin_util.h"
 #include "net/base/net_errors.h"
 #include "net/cert/x509_certificate.h"
 #include "net/ssl/ssl_cipher_suite_names.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "third_party/boringssl/src/include/openssl/ssl.h"
@@ skipping 79 matching lines @@
   // After recording the histogram, clear the time of the warning. A
   // timing histogram will not be recorded again on this page, because
   // the time is only set the first time the HTTP-bad warning is shown
   // per page.
   time_of_http_warning_on_current_navigation_ = base::Time();
 }
 
 void SecurityStateTabHelper::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsInMainFrame() &&
       !navigation_handle->IsSameDocument()) {

[estark, 2017/06/09 05:09:38]

I think this needs a `&& navigation_handle->HasCommitted()` + test as well.
(sorry, existing bug, but might as well fix it here since it'll apply to the
incognito case)

[elawrence, 2017/06/13 15:31:35]

Done and test added.

The console clears when a navigation begins; if the navigation doesn't commit
(e.g. goes to a download or a 204) then the console message will be lost and
won't reflect the present state of the page. That's in contrast to the Security
panel, which refreshes itself with the current page's data in this scenario. But
I suppose that is in line with how the console works otherwise.

     // Only reset the console message flag for main-frame navigations,
     // and not for same-document navigations like reference fragments and
     // pushState.
     logged_http_warning_on_current_navigation_ = false;
+
+    security_state::SecurityInfo security_info;
+    GetSecurityInfo(&security_info);
+    if (security_info.is_incognito &&
+        security_info.security_level == security_state::HTTP_SHOW_WARNING) {
+      web_contents()->GetMainFrame()->AddMessageToConsole(
+          content::CONSOLE_MESSAGE_LEVEL_WARNING,
+          "This page was loaded non-securely in an incognito mode browser. A "
+          "warning has been added to the URL bar. For more information, see "
+          "https://goo.gl/y8SRRv.");
+    }
   }
 }
 
 void SecurityStateTabHelper::WebContentsDestroyed() {
   if (time_of_http_warning_on_current_navigation_.is_null()) {
     return;
   }
   // Record how quickly the tab is closed after a user encounters an
   // HTTP-bad warning. This histogram will only be recorded if the
   // WebContents is destroyed before another navigation begins.
@@ skipping 60 matching lines @@
 }
 
 std::unique_ptr<security_state::VisibleSecurityState>
 SecurityStateTabHelper::GetVisibleSecurityState() const {
   auto state = security_state::GetVisibleSecurityState(web_contents());
 
   // Malware status might already be known even if connection security
   // information is still being initialized, thus no need to check for that.
   state->malicious_content_status = GetMaliciousContentStatus();
 
+  if (!state->certificate &&

[estark, 2017/06/09 05:09:37]

Curious why check the certificate here? You could check
content::IsOriginSecure(web_contents()->GetController().GetVisibleEntry()->GetURL())
to be consistent with Phase 1
(https://cs.chromium.org/chromium/src/components/password_manager/content/brow...)

[elawrence, 2017/06/13 15:31:35]

It was intended as a simple optimization to avoid performing more expensive work
in the common case where the page is HTTPS.

I'll revise this section to make it simpler.

+      security_state::IsHttpWarningForIncognitoEnabled()) {

[estark, 2017/06/09 05:09:38]

IIRC, the reason for checking the field trial here was to avoid checking the
field trial on every call, but it doesn't look like you're caching the result of
the field trial check anywhere. I suggest either:

- Check security_state::IsHttpWarningForIncognitoEnabled() once in the
SecurityStateTabHelper constructor and remember the result as a data member, and
rename VisibleSecurityState::is_incognito to |is_incognito_warning_enabled| or
something to indicate that it might be false even when we are in incognito.

- OR decide that we don't care about the cost of checking the field trial (I
think that would probably be reasonable given that it hasn't been an issue so
far). In that case I'd make VisibileSecurityState::is_incognito do what it says
on the tin (set it to true if and only if we are in incognito mode) and do the
field trial check inside security_state.cc where we do the other field trial
checks. 

[elawrence, 2017/06/13 15:31:35]

The is_incognito flag now does what it says on the tin.

+    content::BrowserContext* context = web_contents()->GetBrowserContext();
+    if (context->IsOffTheRecord() &&
+        !Profile::FromBrowserContext(context)->IsGuestSession()) {

[estark, 2017/06/09 05:09:38]

If it's not too hard, could you add a test that the warning isn't shown in guest
mode?

[elawrence, 2017/06/13 15:31:35]

Added SecurityStateTabHelperTest.SecurityLevelNotDowngradedForHTTPInGuestMode.

+      state->is_incognito = true;
+    }
+  }
   return state;
 }