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..93a37ffb9ce8853effeb3936a1dfaf20c4a9d7d1
--- /dev/null
+++ b/chrome/browser/extensions/api/networking_private/networking_private_crypto.cc
@@ -0,0 +1,278 @@
+// 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 <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 "net/cert/pem_tokenizer.h"
+#include "net/cert/x509_certificate.h"
+
+const char kTrustedCAExponent[] = "00010001";
+
+const char kTrustedCAModulus[] =
+    "BC2280BD80F63A21003BAE765E357F3DC3645C559486342F058728CDF7698C17B350A7B8"
+    "82FADFC7432DD67EABA06FB7137280A44715C1209950CDEC1462095BA498CDD241B6364E"
+    "FFE82E32304A81A842A36C9B336ECAB2F55366E02753861A851EA7393F4A778EFB546666"
+    "FB5854C05E39C7F550060BE08AD4CEE16A551F8B1700E669A327E60825693C129D8D052C"
+    "D62EA231DEB45250D62049DE71A0F9AD204012F1DD25EBD5E6B836F4D68F7FCA43DCD710"
+    "5BE63F518A85B3F3FFF6032DCB234F9CAD18E793058CAC529AF74CE9997ABE6E7E4D0AE3"
+    "C61CA993FA3AA5915D1CBD66EBCC60DC8674CACFF8921C987D57FA61479EAB80B7E44880"
+    "2A92C51B";
+
+namespace {
+bool GetDERFromPEM(const std::string& pem_data,
+                   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
+
+NetworkingPrivateCrypto::NetworkingPrivateCrypto()
+    : ca_public_key_(NULL),
+      cert_public_key_(NULL),
+      cert_(NULL),
+      enc_public_key_(NULL) {}
+
+NetworkingPrivateCrypto::~NetworkingPrivateCrypto() {
+  if (ca_public_key_)
+    SECKEY_DestroyPublicKey(ca_public_key_);
+  if (cert_public_key_)
+    SECKEY_DestroyPublicKey(cert_public_key_);
+  if (cert_)
+    CERT_DestroyCertificate(cert_);
+  if (enc_public_key_)
+    SECKEY_DestroyPublicKey(enc_public_key_);
+}
+
+bool NetworkingPrivateCrypto::VerifyCredentials(
+    const std::string& certificate,
+    const std::string& signed_data,
+    const std::string& unsigned_data,
+    const std::string& connected_mac) {
+  crypto::EnsureNSSInit();
+
+  net::CertificateList cl =
+      net::X509Certificate::CreateCertificateListFromBytes(
+          const_cast<char*>(certificate.c_str()),
+          certificate.size(),
+          net::X509Certificate::FORMAT_AUTO);
+  if (!cl.size()) {
+    LOG(ERROR) << "Failed to parse certificate.";
+    return false;
+  }
+
+  // Check that the device listed in the certificate is correct.
+  std::string subject_name = cl[0]->subject().GetDisplayName();

[mef, 2013/08/28 18:05:59]

Alternatively I can get subject name directly from cert_ below. Would it make
code cleaner?

[Ryan Sleevi, 2013/08/28 18:31:26]

The above is really only intended for display purposes, not for any security
checks (as the class mentions)

My biggest concern here is that it feels that the order of operations here is
incorrect. You should first verify the signature on the certificate (so that you
know you can trust the fields - including subject), and then perform the
remaining sanity checks.

[mef, 2013/08/28 21:28:58]

Done.

+  // Something like evt_e161 001a11ffacdf
+  size_t subject_mac_pos = subject_name.rfind(' ');
+  if (subject_mac_pos == std::string::npos) {
+    LOG(ERROR) << "Badly formatted subject.";
+    return false;
+  }
+
+  std::string translated_mac;
+  RemoveChars(connected_mac, ":", &translated_mac);
+  if (base::strcasecmp(translated_mac.c_str(),
+                       subject_name.substr(subject_mac_pos + 1).c_str())) {

[Ryan Sleevi, 2013/08/28 19:02:37]

why not just use std::string::compare?

[mef, 2013/08/28 21:28:58]

Done.

+    LOG(ERROR) << "MAC addresses don't match.";
+    return false;
+  }
+
+  std::string cert_data;
+  if (!GetDERFromPEM(certificate, "CERTIFICATE", &cert_data)) {

[Ryan Sleevi, 2013/08/28 19:02:37]

Why are you parsing the certificate twice like this? First into the OS handle
(which won't work in the sandbox), and then into an NSS handle.

Pick one format and stick with it.

[mef, 2013/08/28 21:28:58]

Done.

+    LOG(ERROR) << "Failed to parse certificate.";
+    return false;
+  }
+
+  SECItem der_cert;
+  der_cert.data =
+      reinterpret_cast<unsigned char*>(const_cast<char*>(cert_data.c_str()));
+  der_cert.len = cert_data.length();
+  der_cert.type = siDERCertBuffer;
+
+  // Parse into a certificate structure.
+  cert_ = CERT_NewTempCertificate(
+      CERT_GetDefaultCertDB(), &der_cert, NULL, PR_FALSE, PR_TRUE);
+
+  if (!cert_) {
+    LOG(ERROR) << "Failed to parse certificate.";
+    return false;
+  }
+
+  SECStatus verified = SECSuccess;
+
+// TODO(mef): Add exports to third_party/nss/nss/exports_win.def and roll DEPS.
+#if !defined(OS_WIN)
+  crypto::PrivateKeyInfoCodec trusted_ca_key_codec(true);

[mef, 2013/08/28 18:05:59]

crypto::PrivateKeyInfoCodec is causing linking errors on Windows. 
Is there some alternative/easier way to create public key from its modulus and
exponent?

[Ryan Sleevi, 2013/08/28 18:31:26]

Correct, it's not exported, because it's not part of the "public" API.

Instead of storing the CAModulus and CAExponent, just store the CA SPKI. This
makes it easier for all utilities to work with (eg: easy paste into command-line
for use with tools like OpenSSL).

That also fully avoids your dependency here on RSAPrivateKey - you can just
import the DER directly.

[mef, 2013/08/28 21:28:58]

Done.

+  base::HexStringToBytes(kTrustedCAModulus, trusted_ca_key_codec.modulus());
+  base::HexStringToBytes(kTrustedCAExponent,
+                         trusted_ca_key_codec.public_exponent());
+  std::vector<uint8> trusted_ca_key_der;
+  if (!trusted_ca_key_codec.ExportPublicKey(&trusted_ca_key_der)) {
+    LOG(ERROR) << "Failed CA ExportPublicKey";
+    return false;
+  }
+
+  SECItem trusted_ca_key_der_item;
+  trusted_ca_key_der_item.data =
+      const_cast<unsigned char*>(&trusted_ca_key_der.front());
+  trusted_ca_key_der_item.len = trusted_ca_key_der.size();
+
+  ca_public_key_ = SECKEY_ImportDERPublicKey(&trusted_ca_key_der_item, CKK_RSA);
+  verified = CERT_VerifySignedDataWithPublicKey(

[mef, 2013/08/28 18:05:59]

Is it ok to add these to third_party/nss/nss/exports_win.def?

[Ryan Sleevi, 2013/08/28 18:31:26]

Use http://mxr.mozilla.org/nss/source/lib/nss/nss.def#699
(CERT_VerifySignedDataWithPublicKeyInfo) and entirely avoid the import phase.

And yes, it's fine to add either of these functions to nss.def (as they're in
the upstream NSS.def). I would just add the one you need - which should be the
PublicKeyInfo one.

[mef, 2013/08/28 21:28:58]

Done. I did go with CERT_VerifySignedDataWithPublicKey as neither of them is
already exported and I have the SECKEYPublicKey handy. 

+      &cert_->signatureWrap, ca_public_key_, NULL);
+  if (verified != SECSuccess) {
+    LOG(ERROR) << "Certificate is not issued by trusted CA.";
+    return false;
+  }
+#endif
+
+  // Excellent, the certificate checks out, now make sure that the certificate
+  // matches the unsigned data presented.
+  // We're going to verify that hash(unsigned_data) == public(signed_data)
+  cert_public_key_ = CERT_ExtractPublicKey(cert_);

[Ryan Sleevi, 2013/08/28 18:31:26]

Please go through the comments for "we" and look to remove it.

In most cases, it should be very simple, and will make the comments much
cleaner.

https://groups.google.com/a/chromium.org/d/msg/chromium-dev/NH-S6KCkr2M/yv0z3...

[mef, 2013/08/28 21:28:58]

Done.

+
+  if (!cert_public_key_) {
+    LOG(ERROR) << "Unable to extract public key from certificate.";
+    return false;
+  }
+
+  struct rsa_hash {
+    unsigned char asn1[15];
+    unsigned char data[SHA1_LENGTH];
+  } rsa_hash = {{0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2b, 0x0e, 0x03, 0x02,
+                 0x1a, 0x05, 0x00, 0x04, 0x14}, };

[mef, 2013/08/28 18:05:59]

Is there a better way to add ASN1 prefix in front of hash data?

[Ryan Sleevi, 2013/08/28 18:31:26]

Yeah, don't use this API - you're totally using the wrong abstraction :)

VFY_VerifyDigestDirect

[mef, 2013/08/28 21:28:58]

Done.

+
+  SECStatus hashed = HASH_HashBuf(HASH_AlgSHA1,
+                                  rsa_hash.data,
+                                  reinterpret_cast<unsigned char*>(
+                                      const_cast<char*>(unsigned_data.c_str())),
+                                  unsigned_data.size());
+  DCHECK(hashed == SECSuccess);
+  SECItem signature_item = {
+      siBuffer,
+      reinterpret_cast<unsigned char*>(const_cast<char*>(signed_data.c_str())),
+      static_cast<unsigned int>(signed_data.size())};
+  SECItem hash_item = {siBuffer, reinterpret_cast<unsigned char*>(&rsa_hash),
+                       sizeof(rsa_hash)};
+
+  verified = PK11_Verify(cert_public_key_, &signature_item, &hash_item, NULL);
+  if (verified != SECSuccess) {
+    LOG(ERROR) << "Signed blobs did not match.";
+    return false;
+  }
+
+  return true;
+}
+
+bool NetworkingPrivateCrypto::EncryptByteString(const std::string& public_key,
+                                                const std::string& data,
+                                                std::string* encrypted_output) {
+  crypto::EnsureNSSInit();
+
+  SECItem pub_key_der_item;
+  pub_key_der_item.data =
+      reinterpret_cast<unsigned char*>(const_cast<char*>(public_key.c_str()));
+  pub_key_der_item.len = public_key.size();
+
+  enc_public_key_ = SECKEY_ImportDERPublicKey(&pub_key_der_item, CKK_RSA);
+  if (!enc_public_key_) {
+    LOG(ERROR) << "Failed to parse public key.";
+    return false;
+  }
+
+  size_t encrypted_length = SECKEY_SignatureLen(enc_public_key_);
+  scoped_ptr<unsigned char[]> rsa_output(new unsigned char[encrypted_length]);
+
+  LOG(INFO) << "Encrypting data with public key.";
+  SECStatus encrypted = PK11_PubEncryptPKCS1(

[mef, 2013/08/28 18:05:59]

Is it ok to add this to third_party/nss/nss/exports_win.def?

[Ryan Sleevi, 2013/08/28 18:31:26]

Yes, this is part of nss.def. 

+      enc_public_key_,
+      rsa_output.get(),
+      // The API helpfully tells us that this operation will treat this buffer
+      // as read only, but fails to mark the parameter const.
+      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, "RSA 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());
+
+  LOG(ERROR) << "Private Key Len:" << private_key_data.size();
+
+  // TODO(mef): Figure out correct way to import DER private key.
+  scoped_ptr<crypto::RSAPrivateKey> private_key(
+      crypto::RSAPrivateKey::CreateFromPrivateKeyInfo(private_key_data));

[mef, 2013/08/28 18:05:59]

This returns a failure from ASN.1 parser, any suggestions on troubleshooting /
fixing it?
The NetworkingPrivateCrypto::DecryptByteString function is only used for
testing, so I can change private_key to use different format.

+  if (!private_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 output_length = 0;

[Ryan Sleevi, 2013/08/28 19:02:37]

Blergh, poor API spec if we're calling it 'unsigned' rather than 'unsigned int'.
Sad times.

[mef, 2013/08/28 21:28:58]

Done.

+
+  SECStatus decrypted = PK11_PrivDecryptPKCS1(
+      private_key->key(),
+      rsa_output.get(),
+      &output_length,
+      encrypted_length,
+
+      // The API helpfully tells us that this operation will treat this buffer
+      // as read only, but fails to mark the parameter const.
+      reinterpret_cast<unsigned char*>(
+          const_cast<char*>(encrypted_data.data())),
+      encrypted_data.length());
+
+  if (decrypted != SECSuccess) {
+    LOG(ERROR) << "Error during encryption.";
+    return false;
+  }
+
+  decrypted_output->assign(reinterpret_cast<char*>(rsa_output.get()),
+                           output_length);
+
+  return true;
+}