Added NetworkingPrivateCrypto and its unit test.

chrome/browser/extensions/api/networking_private/networking_private_crypto.cc

Index: chrome/browser/extensions/api/networking_private/networking_private_crypto.cc
diff --git a/chrome/browser/extensions/api/networking_private/networking_private_crypto.cc b/chrome/browser/extensions/api/networking_private/networking_private_crypto.cc
new file mode 100644
index 0000000000000000000000000000000000000000..77ddb55a743a6d23270d024708eeb23a6c9fdcee
--- /dev/null
+++ b/chrome/browser/extensions/api/networking_private/networking_private_crypto.cc
@@ -0,0 +1,216 @@
+// Copyright 2013 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 "chrome/browser/extensions/api/networking_private/networking_private_crypto.h"
+
+#include <cert.h>
+#include <cryptohi.h>
+#include <keyhi.h>
+#include <keythi.h>
+#include <pk11pub.h>
+#include <sechash.h>
+#include <secport.h>
+
+#include "base/base64.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "crypto/nss_util.h"
+#include "crypto/rsa_private_key.h"
+#include "crypto/scoped_nss_types.h"
+#include "net/cert/pem_tokenizer.h"
+#include "net/cert/x509_certificate.h"
+
+const char kTrustedCAPublicKeyDER[] =

[Greg Spencer (Chromium), 2013/08/30 18:38:17]

What is the origin of this CA key, and why is it hard coded here?

...and just a reminder that this code review is publicly viewable (which should
only really matter if we're doing it wrong in the first place :-) ).

[mef, 2013/08/30 20:07:17]

Actually I'm not sure. I took it from original code, but I have no idea how it
got there.

+    "MIIBCgKCAQEAvCKAvYD2OiEAO652XjV/PcNkXFWUhjQvBYcozfdpjBezUKe4gvrfx0Mt1n6roG"
+    "+3E3KApEcVwSCZUM3sFGIJW6SYzdJBtjZO/+guMjBKgahCo2ybM27KsvVTZuAnU4YahR6nOT9K"
+    "d477VGZm+1hUwF45x/VQBgvgitTO4WpVH4sXAOZpoyfmCCVpPBKdjQUs1i6iMd60UlDWIEneca"
+    "D5rSBAEvHdJevV5rg29NaPf8pD3NcQW+Y/UYqFs/P/9gMtyyNPnK0Y55MFjKxSmvdM6Zl6vm5+"
+    "TQrjxhypk/o6pZFdHL1m68xg3IZ0ys/4khyYfVf6YUeeq4C35EiAKpLFGwIDAQAB";
+
+namespace {

[Ryan Sleevi, 2013/08/30 18:40:26]

nit: line break between 33/34.

[mef, 2013/08/30 20:07:17]

Done.

+bool GetDERFromPEM(const std::string& pem_data,

[Greg Spencer (Chromium), 2013/08/30 18:38:17]

Document what this function does, and what the parameters are.

[Ryan Sleevi, 2013/08/30 18:40:26]

nit: Add comment.

// Parses |pem_data| for a PEM block of |pem_type|.
// Returns true if a |pem_type| block is found, storing the decoded result in
|der_output|.

[mef, 2013/08/30 20:07:17]

Done.

[mef, 2013/08/30 20:07:17]

Done.

+                   const std::string& pem_type,
+                   std::string* der_output) {
+  std::vector<std::string> headers;
+  headers.push_back(pem_type);
+  net::PEMTokenizer pem_tok(pem_data, headers);
+  if (!pem_tok.GetNext()) {
+    return false;
+  }
+
+  *der_output = pem_tok.data();
+  return true;
+}
+
+}  // namespace
+
+namespace crypto {
+typedef scoped_ptr_malloc<

[Greg Spencer (Chromium), 2013/08/30 18:38:17]

Document what this typedef is used for, and why it's in the crypto namespace.

[mef, 2013/08/30 20:07:17]

Removed per Ryan's suggestion.

+    CERTCertificate,
+    crypto::NSSDestroyer<CERTCertificate,

[Greg Spencer (Chromium), 2013/08/30 18:38:17]

If this is in the crypto namespace, you don't need to have crypto:: here.

[mef, 2013/08/30 20:07:17]

Done.

+                         CERT_DestroyCertificate> >
+    ScopedCERTCertificate;
+}  // namespace crypto

[Ryan Sleevi, 2013/08/30 18:40:26]

Don't stick this into crypto - you don't need to. This can just go in line
47/48.

[mef, 2013/08/30 20:07:17]

Done.

+
+NetworkingPrivateCrypto::NetworkingPrivateCrypto() {}
+
+NetworkingPrivateCrypto::~NetworkingPrivateCrypto() {}
+
+bool NetworkingPrivateCrypto::VerifyCredentials(
+    const std::string& certificate,
+    const std::string& signature,
+    const std::string& data,
+    const std::string& connected_mac) {
+  crypto::EnsureNSSInit();
+
+  std::string cert_data;
+  if (!GetDERFromPEM(certificate, "CERTIFICATE", &cert_data)) {
+    LOG(ERROR) << "Failed to parse certificate.";
+    return false;
+  }
+  SECItem der_cert = {
+      siDERCertBuffer,
+      reinterpret_cast<unsigned char*>(const_cast<char*>(cert_data.c_str())),
+      cert_data.length()};
+  // Parse into a certificate structure.
+  crypto::ScopedCERTCertificate cert(CERT_NewTempCertificate(
+      CERT_GetDefaultCertDB(), &der_cert, NULL, PR_FALSE, PR_TRUE));
+  if (!cert.get()) {
+    LOG(ERROR) << "Failed to parse certificate.";
+    return false;
+  }
+  // Check that the certificate is signed by trusted CA.

[Ryan Sleevi, 2013/08/30 18:40:26]

A little judicious use of vertical whitespace would help here. 84/85 and 77/78.

+  std::string ca_key_der;
+  base::Base64Decode(kTrustedCAPublicKeyDER, &ca_key_der);

[Greg Spencer (Chromium), 2013/08/30 18:38:17]

Couldn't you just do this once, and store it for next time?

[Ryan Sleevi, 2013/08/30 18:40:26]

Since this is a fixed constant, it'd be much better just to store it in DER form
in the constant - eg: don't base64 it.

Be sure to update the type to a 'const unsigned char kTrustedCAPublicKeyDER[]',
otherwise you'll get the compilers yakking at you for unsigned vs signed char
constants.

[mef, 2013/08/30 20:07:17]

I was thinking about that, but couldn't think of an easy way to represent byte
data. 
Should I use python to base64 decode and generate array of bytes like { 0x01,
0xab, etc } or is there a better way?

[mef, 2013/08/30 20:07:17]

I could, but realistically this is not performance-bound function, and it is not
going to be called more than 5 times during the session.
Storing it somewhere would require a singleton, which live forever, which is not
that great either.

[Greg Spencer (Chromium), 2013/08/30 20:11:17]

I like Ryan's idea better of just storing the DER encoded form in the first
place.

[mef, 2013/08/30 20:37:00]

Done.

+  SECItem trusted_ca_key_der_item = {
+      siDERCertBuffer,
+      reinterpret_cast<unsigned char*>(const_cast<char*>(ca_key_der.c_str())),
+      ca_key_der.size()};

[Ryan Sleevi, 2013/08/30 18:40:26]

style nit:  this style of initializing a SECItem is pretty uncommon. We normally
go for explicit intialization of the fields.

Same throughout this file.

[mef, 2013/08/30 20:07:17]

D'Oh, I've found this style of initializing and converted all other cases to it.
:-(

+  crypto::ScopedSECKEYPublicKey ca_public_key(
+      SECKEY_ImportDERPublicKey(&trusted_ca_key_der_item, CKK_RSA));
+  SECStatus verified = CERT_VerifySignedDataWithPublicKey(
+      &cert->signatureWrap, ca_public_key.get(), NULL);
+  if (verified != SECSuccess) {
+    LOG(ERROR) << "Certificate is not issued by trusted CA.";

[Greg Spencer (Chromium), 2013/08/30 18:38:17]

"by the trusted CA"?

[mef, 2013/08/30 20:07:17]

Done.

+    return false;
+  }
+
+  // Check that the device listed in the certificate is correct.
+  // Something like evt_e161 001a11ffacdf
+  char* common_name = CERT_GetCommonName(&cert->subject);
+  if (!common_name) {
+    LOG(ERROR) << "Certificate does not have common name.";
+    return false;
+  }
+
+  std::string subject_name(common_name);
+  PORT_Free(common_name);
+  std::string translated_mac;
+  RemoveChars(connected_mac, ":", &translated_mac);
+  if (!EndsWith(subject_name, translated_mac, false)) {
+    LOG(ERROR) << "MAC addresses don't match.";
+    return false;
+  }
+
+  // Make sure that the certificate matches the unsigned data presented.
+  // Verify that the |signature| matches |data|.
+  crypto::ScopedSECKEYPublicKey public_key(CERT_ExtractPublicKey(cert.get()));
+  if (!public_key.get()) {
+    LOG(ERROR) << "Unable to extract public key from certificate.";
+    return false;
+  }
+  SECItem signature_item = {
+      siBuffer,
+      reinterpret_cast<unsigned char*>(const_cast<char*>(signature.c_str())),
+      static_cast<unsigned int>(signature.size())};
+  verified = VFY_VerifyDataDirect(reinterpret_cast<unsigned char*>(
+      const_cast<char*>(data.c_str())), data.size(),
+      public_key.get(), &signature_item, SEC_OID_PKCS1_RSA_ENCRYPTION,
+      SEC_OID_SHA1, NULL, NULL);
+  if (verified != SECSuccess) {
+    LOG(ERROR) << "Signed blobs did not match.";
+    return false;
+  }
+  return true;
+}
+
+bool NetworkingPrivateCrypto::EncryptByteString(const std::string& pub_key_der,
+                                                const std::string& data,
+                                                std::string* encrypted_output) {
+  crypto::EnsureNSSInit();
+
+  SECItem pub_key_der_item = {
+      siDERCertBuffer,
+      reinterpret_cast<unsigned char*>(const_cast<char*>(pub_key_der.c_str())),
+      pub_key_der.size()};
+  crypto::ScopedSECKEYPublicKey public_key(SECKEY_ImportDERPublicKey(
+      &pub_key_der_item, CKK_RSA));
+  if (!public_key.get()) {
+    LOG(ERROR) << "Failed to parse public key.";
+    return false;
+  }
+
+  size_t encrypted_length = SECKEY_SignatureLen(public_key.get());
+  if (encrypted_length < data.size() + 13) {

[Ryan Sleevi, 2013/08/30 18:40:26]

Why are you using "13" here. This deserves a comment.

Also note that RSAES is defined as operating on messages up to a length of k -
11, where k is the octet length of the RSA modulus (SECKEY_PublicKeyStrength -
which returns the length in *bytes*)

RFC 3447 7.2.1 defines the inputs.

[mef, 2013/08/30 20:07:17]

Done.

+    LOG(ERROR) << "Too much data to encrypt.";
+    return false;
+  }
+
+  scoped_ptr<unsigned char[]> rsa_output(new unsigned char[encrypted_length]);
+  SECStatus encrypted = PK11_PubEncryptPKCS1(
+      public_key.get(),
+      rsa_output.get(),
+      reinterpret_cast<unsigned char*>(const_cast<char*>(data.data())),
+      data.length(),
+      NULL);
+  if (encrypted != SECSuccess) {
+    LOG(ERROR) << "Error during encryption.";
+    return false;
+  }
+  encrypted_output->assign(reinterpret_cast<char*>(rsa_output.get()),
+                           encrypted_length);
+  return true;
+}
+
+bool NetworkingPrivateCrypto::DecryptByteString(
+    const std::string& private_key_pem,
+    const std::string& encrypted_data,
+    std::string* decrypted_output) {
+  crypto::EnsureNSSInit();
+
+  std::string private_key_der;
+  if (!GetDERFromPEM(private_key_pem, "PRIVATE KEY", &private_key_der)) {
+    LOG(ERROR) << "Failed to parse private key PEM.";
+    return false;
+  }
+  std::vector<uint8> private_key_data(private_key_der.begin(),
+                                      private_key_der.end());
+  scoped_ptr<crypto::RSAPrivateKey> private_key(
+      crypto::RSAPrivateKey::CreateFromPrivateKeyInfo(private_key_data));
+  if (!private_key || !private_key->public_key()) {
+    LOG(ERROR) << "Failed to parse private key DER.";
+    return false;
+  }
+
+  size_t encrypted_length = SECKEY_SignatureLen(private_key->public_key());
+  scoped_ptr<unsigned char[]> rsa_output(new unsigned char[encrypted_length]);
+  unsigned int output_length = 0;
+  SECStatus decrypted =
+      PK11_PrivDecryptPKCS1(private_key->key(),
+                            rsa_output.get(),
+                            &output_length,
+                            encrypted_length,
+                            reinterpret_cast<unsigned char*>(
+                                const_cast<char*>(encrypted_data.data())),
+                            encrypted_data.length());
+  if (decrypted != SECSuccess) {
+    LOG(ERROR) << "Error during decryption.";
+    return false;
+  }
+  decrypted_output->assign(reinterpret_cast<char*>(rsa_output.get()),
+                           output_length);
+  return true;
+}