[Android WebView] Propagate Java exceptions thrown in OnReceivedSslError

content/browser/ssl/ssl_manager.cc

 // Copyright (c) 2012 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 "content/browser/ssl/ssl_manager.h"
 
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 107 matching lines @@
     }
     return;
   }
 
   NavigationControllerImpl* controller =
       static_cast<NavigationControllerImpl*>(&web_contents->GetController());
   controller->SetPendingNavigationSSLError(true);
 
   SSLManager* manager = controller->ssl_manager();
   manager->OnCertError(std::move(handler));
+  // On Android, OnCertError can cause a Java exception to be thrown - in such a
+  // case we cannot allow calls back into Java here. If adding any additional
+  // code here, make sure it cannot call into Java.
 }
 
 }  // namespace
 
 // static
 void SSLManager::OnSSLCertificateError(
     const base::WeakPtr<SSLErrorHandler::Delegate>& delegate,
     const ResourceType resource_type,
     const GURL& url,
     const base::Callback<WebContents*(void)>& web_contents_getter,
@@ skipping 165 matching lines @@
     case net::ERR_CERT_NAME_CONSTRAINT_VIOLATION:
     case net::ERR_CERT_VALIDITY_TOO_LONG:
     case net::ERR_CERTIFICATE_TRANSPARENCY_REQUIRED:
       if (!handler->fatal())
         options_mask |= OVERRIDABLE;
       else
         options_mask |= STRICT_ENFORCEMENT;
       if (expired_previous_decision)
         options_mask |= EXPIRED_PREVIOUS_DECISION;
       OnCertErrorInternal(std::move(handler), options_mask);
+      // On Android, OnCertErrorInternal can cause a Java exception to be thrown
+      // - in such a case we cannot allow calls back into Java here. If adding
+      // any additional code here, make sure it cannot call into Java.

[estark, 2017/05/11 00:50:56]

Hmm, I don't think I understand this. Is it a problem if a caller of
OnCertError() calls back into Java?

[gsennton, 2017/05/11 12:17:17]

Yes, so what happens is this:
OnCertErrorInternal() calls into some code in Android WebView which in turn uses
JNI to call into Java. This call into Java ends up calling a callback,
implemented by an app-developer, which happens to throw a run-time exception.
When an exception is thrown (and never caught) on the Java-side of a JNI call
that exception is seen as pending when the call returns back to native.

According to the JNI documentation it is not safe to call back into Java from
native when there exists a pending Java exception.

The way we (Chrome on Android / Android WebView) normally deal with Java
exceptions thrown during JNI calls is to check whether there exists a pending
exception just after returning from Java to native, and if so, log the
exception, and explicitly cause a native crash instead (so that we can't
continue executing in a state where we don't really how to deal with what
happened on the Java side).
However, this way of dealing with Java exceptions is a bit messy in terms of
crash reporting - seeing only the native crash doesn't necessarily help us debug
the original Java exception, since often we don't get the log output containing
the Java crash that was the actual cause.

In Android WebView, because some of our Java exceptions are caused by
app-developers rather than by us (WebView/Chrome developers), the impact of
reporting these native crashes instead of the original java exceptions is even
higher - most app-developers will never see the native crashes because they
aren't forwarded to app developers, and even if they were, the native crash
information wouldn't make much sense to them.

So what we want to do in this code instead is to keep the Java exception
pending, and return out of our native call stack - up until we reach back into
Android (Java). At that point Android will handle the pending Java exception
through an UnhandledExceptionHandler and properly report that exception to the
app-developer.
But the important point here is that, because of the JNI documentation stating
that it's not safe to call back into Java with a pending Java exception, we have
to ensure that when returning out of our native call stack we never call back
into Java.

       break;
     case net::ERR_CERT_NO_REVOCATION_MECHANISM:
       // Ignore this error.
       handler->ContinueRequest();
       break;
     case net::ERR_CERT_UNABLE_TO_CHECK_REVOCATION:
       // We ignore this error but will show a warning status in the location
       // bar.
       handler->ContinueRequest();
       break;
     case net::ERR_CERT_CONTAINS_ERRORS:
     case net::ERR_CERT_REVOKED:
     case net::ERR_CERT_INVALID:
     case net::ERR_SSL_WEAK_SERVER_EPHEMERAL_DH_KEY:
     case net::ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN:
       if (handler->fatal())
         options_mask |= STRICT_ENFORCEMENT;
       if (expired_previous_decision)
         options_mask |= EXPIRED_PREVIOUS_DECISION;
       OnCertErrorInternal(std::move(handler), options_mask);
+      // On Android, OnCertErrorInternal can cause a Java exception to be thrown
+      // - in such a case we cannot allow calls back into Java here. If adding
+      // any additional code here, make sure it cannot call into Java.
       break;
     default:
       NOTREACHED();
       handler->CancelRequest();
       break;
   }
 }
 
 void SSLManager::DidStartResourceResponse(const GURL& url,
                                           bool has_certificate,
@@ skipping 43 matching lines @@
   protocol::SecurityHandler* security_handler =
       protocol::SecurityHandler::FromAgentHost(agent_host);
   if (!security_handler ||
       !security_handler->NotifyCertificateError(
           cert_error, request_url,
           base::Bind(&OnAllowCertificateWithRecordDecision, false, callback))) {
     GetContentClient()->browser()->AllowCertificateError(
         web_contents, cert_error, ssl_info, request_url, resource_type,
         overridable, strict_enforcement, expired_previous_decision,
         base::Bind(&OnAllowCertificateWithRecordDecision, true, callback));
+    // On Android, AllowCertificateError can cause a Java exception to be thrown
+    // - in such a case we cannot allow calls back into Java here. If adding any
+    // additional code here, make sure it cannot call into Java.
   }
 }
 
 void SSLManager::UpdateEntry(NavigationEntryImpl* entry,
                              int add_content_status_flags,
                              int remove_content_status_flags) {
   // We don't always have a navigation entry to update, for example in the
   // case of the Web Inspector.
   if (!entry)
     return;
@@ skipping 48 matching lines @@
   SSLManagerSet* managers =
       static_cast<SSLManagerSet*>(context->GetUserData(kSSLManagerKeyName));
 
   for (std::set<SSLManager*>::iterator i = managers->get().begin();
        i != managers->get().end(); ++i) {
     (*i)->UpdateEntry((*i)->controller()->GetLastCommittedEntry(), 0, 0);
   }
 }
 
 }  // namespace content