Implementation for chrome.certificateProvider.requestPin/stopPinRequest

chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc

Index: chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc
diff --git a/chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc b/chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc
index eec8b3bf2483b30e2f303a1158e9857ef021862d..11ffe2855478dd4c728f34ff3e5586a1ca86919c 100644
--- a/chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc
+++ b/chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc
@@ -14,6 +14,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
+#include "base/rand_util.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
@@ -112,6 +113,15 @@ bool DefaultDelegate::DispatchSignRequestToExtension(
     return false;
 
   api_cp::SignRequest request;
+  // Generate a random request id so that a malicious extension can not guess a
+  // valid id and spam the user.
+  uint64_t sign_request_id = base::RandUint64();

[emaxx, 2016/09/06 15:02:10]

I'm afraid we can put the client extensions into a bad situation by generating
the number that cannot be stored as a JavaScript number precisely.

My understanding is that the JavaScript number can store precisely 53-bit
integers, but I may be wrong here (and in any way hardcoding this here would be
quite strange).

So maybe, for simplicity reasons, we should generate the numbers that fit into
the usual 4-byte int range.

[emaxx, 2016/09/06 15:02:10]

There is already another "sign_request_id" in the existing code that is
generated by CertificateProviderService and passed to this function - here it is
named as "request_id". Can we reuse it instead of introducing a new one
identifier?

There is one difference between them: the existing ids are generated simply by
increasing a counter instead of the random used here (see the generation code
here:
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/certificate_prov...
). But I think it should be fine to modify the SignRequests class implementation
accordingly.

[igorcov, 2016/09/07 09:12:28]

Done.

[igorcov, 2016/09/07 09:12:28]

Done.

+  // If we hit the improbable case when generated ID exists already, we
+  // generate another ID.
+  while (!service_->pin_dialog_manager()->AddSignRequestId(sign_request_id)) {
+    sign_request_id = base::RandUint64();
+  }
+  request.sign_request_id = sign_request_id;
   switch (hash) {
     case net::SSLPrivateKey::Hash::MD5_SHA1:
       request.hash = api_cp::HASH_MD5_SHA1;