[Cronet] Public key pinning for Java API

components/cronet/android/cronet_url_request_context_adapter.cc

 // Copyright 2014 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 "components/cronet/android/cronet_url_request_context_adapter.h"
 
 #include <map>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/bind.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_filter.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/pref_service_factory.h"
 #include "base/single_thread_task_runner.h"
@@ skipping 283 matching lines @@
     scoped_ptr<net::HttpServerPropertiesManager> http_server_properties_manager(
         new net::HttpServerPropertiesManager(pref_service_.get(),
                                              kHttpServerProperties,
                                              GetNetworkTaskRunner()));
     http_server_properties_manager->InitializeOnNetworkThread();
     http_server_properties_manager_ = http_server_properties_manager.get();
     context_builder.SetHttpServerProperties(
         http_server_properties_manager.Pass());
   }
 
+// Explicitly disable the persister for Cronet to avoid persistence
+// of dynamic HPKP. This is a safety measure in case if somebody will
+// enable the persistence by specifying transport_security_persister_path
+// in the future.
+#if DCHECK_IS_ON()

[mef, 2015/11/06 18:02:17]

DCHECK_IS_ON() in debug, but not in the release.
Why is it only needed in debug?

[kapishnikov, 2015/11/06 19:45:08]

The intention was to execute it in debug mode only to avoid production code
changes since this is only a safety check for the situation when we enable the
persistence in the future. I assume that the developer who enables the
persistence in Cronet would write unit tests that will fail because of this
DCHECK. On the other hand, if persistence is enabled but there is no enough test
coverage, there is a risk that we may miss a bug during development and discover
it in release only. I have removed DCHECK to avoid situation with undiscovered
bug; however, both arguments have their merits.

+  context_builder.set_transport_security_persister_path(base::FilePath());
+#endif
+
   context_ = context_builder.Build().Pass();
 
   default_load_flags_ = net::LOAD_DO_NOT_SAVE_COOKIES |
                         net::LOAD_DO_NOT_SEND_COOKIES;
   if (config->load_disable_cache)
     default_load_flags_ |= net::LOAD_DISABLE_CACHE;
 
   if (config->enable_sdch) {
     DCHECK(context_->sdch_manager());
     sdch_owner_.reset(
@@ skipping 40 matching lines @@
                                                  quic_hint.port);
       net::AlternativeService alternative_service(
           net::AlternateProtocol::QUIC, "",
           static_cast<uint16>(quic_hint.alternate_port));
       context_->http_server_properties()->SetAlternativeService(
           quic_hint_host_port_pair, alternative_service, 1.0f,
           base::Time::Max());
     }
   }
 
+  // Iterate through HPKP configuration for every host.
+  for (auto hpkp_itr = config->hpkp_list.begin();
+       hpkp_itr != config->hpkp_list.end(); ++hpkp_itr) {
+    const URLRequestContextConfig::Hpkp& hpkp = **hpkp_itr;
+
+    // Convert the vector of hash strings from the config to
+    // a vector of HashValue objects.
+    net::HashValueVector hash_value_vector;
+    for (const auto& hash : hpkp.pin_hashes) {
+      auto hash_value = net::HashValue(net::HASH_VALUE_SHA256);
+      bool good_hash = hash_value.FromString(*hash);
+      if (good_hash) {
+        hash_value_vector.push_back(hash_value);
+      } else {
+        LOG(WARNING) << "Unable to add hash value " << *hash;
+      }
+    }
+

[mef, 2015/11/06 18:02:17]

maybe add DCHECK here that
context->transport_security_state()->transport_security_persister_ is NULL? (I
don't see an accessor, so it could be easier said than done).

[kapishnikov, 2015/11/06 19:45:08]

Yes, the problem is that there is no accessor. We decided not to introduce one.
We could also check context_builder.set_transport_security_persister_ before
constructing the context.

+    // Add the host pinning.
+    context_->transport_security_state()->AddHPKP(
+        hpkp.host, hpkp.expiration_date, hpkp.include_subdomains,
+        hash_value_vector, GURL());

[mef, 2015/11/06 18:02:17]

GURL() -> GURL::EmptyGURL();

[kapishnikov, 2015/11/06 19:45:09]

Done.

+  }
+
   JNIEnv* env = base::android::AttachCurrentThread();
   jcronet_url_request_context_.Reset(env, jcronet_url_request_context.obj());
   Java_CronetUrlRequestContext_initNetworkThread(
       env, jcronet_url_request_context.obj());
 
 #if defined(DATA_REDUCTION_PROXY_SUPPORT)
   if (data_reduction_proxy_)
     data_reduction_proxy_->Init(true, GetURLRequestContext());
 #endif
   is_context_initialized_ = true;
@@ skipping 147 matching lines @@
 static jint SetMinLogLevel(JNIEnv* env,
                            const JavaParamRef<jclass>& jcaller,
                            jint jlog_level) {
   jint old_log_level = static_cast<jint>(logging::GetMinLogLevel());
   // MinLogLevel is global, shared by all URLRequestContexts.
   logging::SetMinLogLevel(static_cast<int>(jlog_level));
   return old_log_level;
 }
 
 }  // namespace cronet