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,

[Ryan Sleevi, 2015/02/03 01:44:49]

Needs documentation about expected behaviours (you've got it on 98/99, but
that's a *use* of this enum rather than an explanation about what this could
mean)

[pneubeck (no reviews), 2015/02/05 10:41:57]

Done.

   HASH_ALGORITHM_SHA1,
   HASH_ALGORITHM_SHA256,
   HASH_ALGORITHM_SHA384,
   HASH_ALGORITHM_SHA512
 };
 
 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;
 };
 
+struct PublicKeyInfo {

[Ryan Sleevi, 2015/02/03 01:44:50]

NEeds documentation

[pneubeck (no reviews), 2015/02/03 20:15:00]

changed the name to SPKI and documented it.

+  PublicKeyInfo();
+  ~PublicKeyInfo();
+
+  std::vector<char> public_key_spki_der;

[Ryan Sleevi, 2015/02/03 01:44:50]

Why |char| and not |uint8_t| ?

[pneubeck (no reviews), 2015/02/03 10:48:18]

Generated API code uses vector<char>, the code that I used from net/ mostly
std::string.
I'll change this back to std::string which is more consistent with the rest of
this file.

+  net::X509Certificate::PublicKeyType key_type;
+
+  // For RSA a public exponent of 65537 is assumed, so there is no member for
+  // that.

[Ryan Sleevi, 2015/02/03 01:44:50]

This doesn't always hold true

[pneubeck (no reviews), 2015/02/03 10:48:18]

Is it acceptable to restrict it to 65537 if I implement the verification of
that?
Or should we allow usage (but not generation) of such certs/keys?

+
+  // Set if |key_type| equals kPublicKeyTypeRSA.
+  unsigned int modulus_length_bits;

[Ryan Sleevi, 2015/02/03 01:44:50]

This is a weird interface then, as ec keys have a strength/public key length.

[pneubeck (no reviews), 2015/02/03 20:15:00]

changed the name to key_size_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.
+// If |hash_algorithm| is HASH_ALGORITHM_NONE, |data| will not be hashed before
+// signing. PKCS#1 v1.5 padding will still be applied.

[Ryan Sleevi, 2015/02/03 01:44:50]

OK, so from a documentation standpoint, it's a little weird.

Consider
- If this API is meant to take EC keys, what would HASH_ALGORITHM_NONE mean?
- The maximum size of |data| is limited in the case of PKCS#1 v1.5 signing with
an RSA key (to modulus_len - 11, IIRC).

Maybe this should be split into two APIs - Sign() and SignDirectPKCS1Padded()
[since lines 91-92 no longer apply, and it's not exactly Direct signing], with a
shared under the hood impl?

[pneubeck (no reviews), 2015/02/03 10:48:18]

This function is in namespace subtle and an implementation details of the
PlatformKeysService::Sign (more a SignRSA currently).
PlatformKeysService::Sign itself has some amount of implementation which I'd
hate to duplicate.

The question is how to pass around the Sign parameters "Sign(HashAlgorithm) or
DirectSign".
I got annoyed already by the huge number of arguments of this function that I'd
consider putting them into a struct including a new 'bool direct_sign':

struct SignParams { // or SignRSAParams
  std::string token_id;
  std::string public_key;
  HashAlgorithm hash_algorithm;  // Ignored if direct_sign==true.
  bool direct_sign;
  std::string data;
};

[pneubeck (no reviews), 2015/02/05 10:41:57]

done.
I addressed your comments by clearer documentation ('optionally digest...',
'hash algorithm ignored if direct_sign_pkcs_padded is true...') and by renaming
the Sign function to SignRSA.

 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);
 
 // If the certificate request could be processed successfully, |matches| will
 // contain the list of matching certificates (maybe 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,
+                  PublicKeyInfo* 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_