Implement chrome.platformKeys.getKeyPair().

chrome/browser/chromeos/platform_keys/platform_keys.h

 // 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_PLATFORM_KEYS_H_
 #define CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_PLATFORM_KEYS_H_
 
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "net/cert/x509_certificate.h"
 #include "net/ssl/ssl_client_cert_type.h"
 
 namespace content {
 class BrowserContext;
 }
 
-namespace net {
-class X509Certificate;
-typedef std::vector<scoped_refptr<X509Certificate> > CertificateList;
-}
-
 namespace chromeos {
 
 namespace platform_keys {
 
 // A token is a store for keys or certs and can provide cryptographic
 // operations.
 // ChromeOS provides itself a user token and conditionally a system wide token,
 // thus these tokens use static identifiers. The platform keys API is designed
 // to support arbitrary other tokens in the future, which could then use
 // run-time generated IDs.
 extern const char kTokenIdUser[];
 extern const char kTokenIdSystem[];
 
 // Supported hash algorithms.
 enum HashAlgorithm {
+  HASH_ALGORITHM_NONE,  // The value if no hash function is selected.
   HASH_ALGORITHM_SHA1,
   HASH_ALGORITHM_SHA256,
   HASH_ALGORITHM_SHA384,
   HASH_ALGORITHM_SHA512
 };
 
+// Parameters to the SignRSA function.
+struct SignRSAParams {
+  // The data that will be signed.
+  std::string data;
+
+  // Must be the DER encoding of a SubjectPublicKeyInfo.
+  std::string public_key;
+
+  // If true, |data| will not be hashed before signing. Only PKCS#1 v1.5 padding
+  // will be applied before signing.
+  // If false, |hash_algorithm| must be set to a value != NONE.
+  bool sign_direct_pkcs_padded = false;

[Ryan Sleevi, 2015/02/07 02:09:40]

It feels like by moving this into a struct, you're using in class initialization
(
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Initialization
) 

to avoid
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Default_Argum...

At the least, it seems like SignRSA() should take params "as expected", and if
it needs to bundle into a struct, that should be an internal implementation
detail for marshalling, not a public API contract.

[pneubeck (no reviews), 2015/02/08 10:52:00]

The possibility of using this as default arguments wasn't intentional.
Changed to explicit constructors to prevent that.
(With the side effect of 3-times as much code...)

I'm not convinced about the 'should take params "as expected"' as the number of
arguments is already significant and any passing of this number of arguments is
a huge amount of boilerplate that makes the code less readable.
This class is meant to encapsulate these arguments, so that code that just
passes them around is oblivious to its content.
(And I have already code that just passes it around.)

+
+  // Determines the hash algorithm that is used to digest |data| before signing.
+  // Ignored if |sign_direct_pkcs_padded| is true.
+  HashAlgorithm hash_algorithm = HASH_ALGORITHM_NONE;
+};
+
 struct ClientCertificateRequest {
   ClientCertificateRequest();
   ~ClientCertificateRequest();
 
   // The list of the types of certificates requested, sorted in order of the
   // server's preference.
   std::vector<net::SSLClientCertType> certificate_key_types;
 
   // List of distinguished names of certificate authorities allowed by the
   // server. Each entry must be a DER-encoded X.509 DistinguishedName.
   std::vector<std::string> certificate_authorities;
 };
 
+// Holds details about a certificate's (subject) key, i.e. the X.509 Subject
+// Public Key Info (SPKI) of the certificate.
+// As this class supports only specific types of keys (currently only RSA), this
+// does not necessarily mirror the internal structure of a X.509 SPKI.
+struct SubjectPublicKeyInfo {
+  SubjectPublicKeyInfo();
+  ~SubjectPublicKeyInfo();
+
+  std::string public_key_spki_der;
+  net::X509Certificate::PublicKeyType key_type;
+
+  // For RSA a public exponent of 65537 is assumed, so there is no member for
+  // that.
+
+  size_t key_size_bits = 0;
+};

[Ryan Sleevi, 2015/02/07 02:09:40]

Ditto this. I don't really understand this structure much :(

[pneubeck (no reviews), 2015/02/08 10:52:00]

I can only guess what you find problematic here.
As this struct seems not to form a reasonable general concept to you, I changed
it's description/intention to only hold the results of the GetPublicKey
function.
That function happens to return several seemingly unrelated properties because I
didn't want unnecessary many calls to CERT_ExtractPublicKey (because of
potential runtime cost).
If a larger number of calls to CERT_ExtractPublicKey is unproblematic, I can
also split the function into several with more conceptually reasonable results.

[Ryan Sleevi, 2015/02/10 00:59:32]

Sorry I wasn't clearer:

I don't understand why you don't do

bool GetPublicKey(scoped_refptr<X509Certificate> certificate,
                  std::string* spki_der,
                  net::X509Certificate::PublicKeyType* key_type,
                  size_t* key_size_in_bits);

+
 namespace subtle {
 // Functions of this namespace shouldn't be called directly from the context of
 // an extension. Instead use PlatformKeysService which enforces restrictions
 // upon extensions.
 
 typedef base::Callback<void(const std::string& public_key_spki_der,
                             const std::string& error_message)>
     GenerateKeyCallback;
 
 // Generates a RSA key pair with |modulus_length_bits|. |token_id| is currently
 // ignored, instead the user token associated with |browser_context| is always
 // used. |callback| will be invoked with the resulting public key or an error.
 void GenerateRSAKey(const std::string& token_id,
                     unsigned int modulus_length_bits,
                     const GenerateKeyCallback& callback,
                     content::BrowserContext* browser_context);
 
 typedef base::Callback<void(const std::string& signature,
                             const std::string& error_message)> SignCallback;
 
-// Digests |data| with |hash_algorithm| and afterwards signs the digest with the
-// private key matching |public_key|, if that key is stored in the given token.
-// |token_id| is currently ignored, instead the user token associated with
-// |browser_context| is always used. |public_key| must be the DER encoding of a
-// SubjectPublicKeyInfo. |callback| will be invoked with the signature or an
-// error message.
-// Currently supports RSA keys only.
-void Sign(const std::string& token_id,
-          const std::string& public_key,
-          HashAlgorithm hash_algorithm,
-          const std::string& data,
-          const SignCallback& callback,
-          content::BrowserContext* browser_context);
+// Optionally digests |data|, applies PKCS1 padding and afterwards signs the
+// data with the private key matching |params.public_key|. If a non empty token
+// id is provided and the key is not found in that token, the operation aborts.
+// |callback| will be invoked with the signature or an error message.
+void SignRSA(const std::string& token_id,
+             scoped_ptr<SignRSAParams> params,
+             const SignCallback& callback,
+             content::BrowserContext* browser_context);
 
 // If the certificate request could be processed successfully, |matches| will
 // contain the list of matching certificates (which may be empty) and
 // |error_message| will be empty. If an error occurred, |matches| will be null
 // and |error_message| contain an error message.
 typedef base::Callback<void(scoped_ptr<net::CertificateList> matches,
                             const std::string& error_message)>
     SelectCertificatesCallback;
 
 // Returns the list of all certificates that match |request|. |callback| will be
 // invoked with these matches or an error message.
 void SelectClientCertificates(const ClientCertificateRequest& request,
                               const SelectCertificatesCallback& callback,
                               content::BrowserContext* browser_context);
 
 }  // namespace subtle
 
+// Fills |info| with information about the key certified by |certificate|.
+bool GetPublicKey(scoped_refptr<net::X509Certificate> certificate,
+                  SubjectPublicKeyInfo* info);
+
 // If the list of certificates could be successfully retrieved, |certs| will
 // contain the list of available certificates (maybe empty) and |error_message|
 // will be empty. If an error occurred, |certs| will be empty and
 // |error_message| contain an error message.
 typedef base::Callback<void(scoped_ptr<net::CertificateList> certs,
                             const std::string& error_message)>
     GetCertificatesCallback;
 
 // Returns the list of all certificates with stored private key available from
 // the given token. |token_id| is currently ignored, instead the user token
@@ skipping 43 matching lines @@
 // of available tokens is determined, possibly with an error message.
 // Must be called and calls |callback| on the UI thread.
 void GetTokens(const GetTokensCallback& callback,
                content::BrowserContext* browser_context);
 
 }  // namespace platform_keys
 
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_PLATFORM_KEYS_H_