Per discussion, implement the Omaha Client Update Protocol (CUP) in src/crypto.

crypto/cup.cc

Index: crypto/cup.cc
===================================================================
--- crypto/cup.cc	(revision 0)
+++ crypto/cup.cc	(revision 0)
@@ -0,0 +1,271 @@
+// Copyright (c) 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 "crypto/cup.h"
+
+#include "base/base64.h"
+#include "base/sha1.h"
+#include "base/string_util.h"
+#include "base/stringprintf.h"
+#include "crypto/hmac.h"
+#include "crypto/random.h"
+
+// NOTE: LoadPublicKey() and EncryptSharedKey() are implemented individually
+// for each possible crypto engine; this file only holds common functionality.
+// See cup_nss.cc or cup_openssl.cc for implementations of those methods.

[Ryan Sleevi, 2013/05/30 02:28:08]

Generally unnecessary; this is understood when dealing with Chromium code,
particularly in crypto/

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done.

+
+namespace crypto {
+
+ClientUpdateProtocol::ClientUpdateProtocol()
+    : v_(0) {
+}
+
+ClientUpdateProtocol::~ClientUpdateProtocol() {
+}
+
+bool ClientUpdateProtocol::Init(int key_version, const char* public_key) {
+  if (v_ > 0) {

[Ryan Sleevi, 2013/05/30 02:28:08]

style nit: This is not strictly required by the style guide, other than "follow
local style", but both "crypto/" and "net/" omit braces for one-line
conditionals.

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done.

+    return false;  // Already initialized.
+  }
+
+  DCHECK(key_version > 0 && public_key);

[Ryan Sleevi, 2013/05/30 02:28:08]

DCHECK_GT(key_version, 0);
DCHECK(public_key);

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done.

+  if (key_version <= 0 || !public_key) {
+    return false;  // At least one mandatory parameter is not valid.
+  }
+
+  size_t key_size = LoadPublicKey(public_key);
+  if (key_size < HashDigestSize()) {
+    return false;  // Public key couldn't be loaded, or is too small to be used.
+  }
+
+  InitializeEntropy(key_size);
+  if (!BuildSharedKey()) {
+    return false;  // Failed to generate w.
+  }
+
+  v_ = key_version;
+  return true;
+}
+
+bool ClientUpdateProtocol::SignRequest(const base::StringPiece& url_host,
+                                       const base::StringPiece& url_path,
+                                       const base::StringPiece& url_query,
+                                       const base::StringPiece& request_body,
+                                       std::string* url_query_out,
+                                       std::string* cp_out) {
+  if (vw_.empty()) {
+    return false;  // We haven't been initialized.
+  }
+
+  // Compute the URL for the "outer" CUP request -- append a query string (or
+  // append to an existing query string, if there is one) containing vw.
+  std::string cup_url_query(url_query.data());
+  if (url_query.find('?') != std::string::npos) {
+    cup_url_query.append("&w=");
+  } else {
+    cup_url_query.append("?w=");
+  }
+  cup_url_query.append(vw_);
+
+  const std::string packed_url = base::StringPrintf("//%s%s%s",
+                                                    url_host.data(),
+                                                    url_path.data(),
+                                                    cup_url_query.data());

[Ryan Sleevi, 2013/05/30 02:28:08]

I'm generally not a fan of constructing and manipulating URLs like this. This is
why we have googleurl/, to avoid manipulating URLs as strings - particularly
when dealing with something like "url_query" (which crypto/ can't/shouldn't
depend on)

It feels like the URL construction might be better separated, if at all
possible.

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Agreed.  I've pushed this out to the caller; they can do it easily enough with
GURL.

+
+  // Compute hw = HASH(HASH(v|w)|HASH(request_url)|HASH(body)).  We'll need to
+  // keep the value of hw for later to validate the server's response.
+  std::vector<uint8> internal_hashes;
+  std::vector<uint8> h;
+
+  h = Hash(vw_);
+  internal_hashes.insert(internal_hashes.end(), h.begin(), h.end());
+  h = Hash(packed_url);
+  internal_hashes.insert(internal_hashes.end(), h.begin(), h.end());
+  h = Hash(request_body);
+  internal_hashes.insert(internal_hashes.end(), h.begin(), h.end());
+  DCHECK(internal_hashes.size() == 3 * HashDigestSize());

[Ryan Sleevi, 2013/05/30 02:28:08]

DCHECK_EQ

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done.

+
+  hw_ = Hash(internal_hashes);
+
+  // Sign the challenge hash (hw) using the shared key (sk) to produce the
+  // client proof (cp).
+  std::vector<uint8> cp = SymSign(sk_, 3, &hw_, NULL, NULL);
+  if (cp.empty()) {
+    hw_.clear();
+    return false;  // HMAC failed for some reason.
+  }
+
+  if (url_query_out) {
+    *url_query_out = cup_url_query;
+  }
+  if (cp_out) {
+    *cp_out = UrlSafeB64Encode(cp);
+  }
+
+  return true;
+}
+
+bool ClientUpdateProtocol::ValidateResponse(
+    const base::StringPiece& response_body,
+    const base::StringPiece& c_in,
+    const base::StringPiece& sp_in) {
+  if (hw_.empty()) {
+    return false;  // There hasn't been a call to SignRequest() yet.
+  }
+
+  // If our request got to the server in one piece, it's going to use its
+  // private key to decrypt v|w, yielding the contents of r.  It can use
+  // that to recreate sk, then compute hw and SymSign(3|hw) on its end,
+  // and ensure that the cp we sent matches up.  It will then use sk to
+  // sign its response.
+

[Ryan Sleevi, 2013/05/30 02:28:08]

comment nit: Rewrite the comment without using "our" or "we". Avoid using
pronouns in comments.

// If the request was received by the server, the server will use its
// private key to decrypt v|w, yielding the contents of r. The server
// can then recreate sk, compute hw, and SymSign(3|hw) to ensure that
// the cp sent matches. It will then use sk to sign its response.

Probably need quotes (or more likely, |hw_|, etc) around variables (or more
aptly, their expanded names - see previous comment)

style nit: delete blank line

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done.

+  std::vector<uint8> hm = Hash(response_body);
+  std::vector<uint8> hc = Hash(c_in);
+  std::vector<uint8> expected_hmac = SymSign(sk_, 1, &hw_, &hm, &hc);
+  std::string expected_sp = UrlSafeB64Encode(expected_hmac);
+
+  return sp_in == expected_sp;

[Ryan Sleevi, 2013/05/30 02:28:08]

SECURITY: This raises red flags as an unsafe comparison (especially when the
field is called "expected_hmac").

See crypto/secure_util.h for securely (eg: in a timing-independent manner)
comparing two buffers.

I've not done cryptanalysis on the protocol to know what impact would be had
from such leaking, but it's enough to say "easier to be safe"...

eg: This is why we have HMAC::Verify() to begin with :)

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Replaced with HMAC::Verify().

+}
+
+void ClientUpdateProtocol::InitializeEntropy(size_t public_key_length) {
+  DCHECK(public_key_length >= HashDigestSize());

[Ryan Sleevi, 2013/05/30 02:28:08]

DCHECK_GE

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done.

+
+  if (public_key_length >= HashDigestSize()) {
+    std::vector<uint8> entropy(public_key_length - HashDigestSize());
+    crypto::RandBytes(&entropy[0], entropy.size());
+    r_ = RsaPad(public_key_length, entropy);
+  }
+}
+
+bool ClientUpdateProtocol::BuildSharedKey() {
+  // Derive a new new shared key sk' as the hash of r_.  This will be used for
+  // the HMAC step.
+  sk_ = Hash(r_);
+
+  // Encrypt r with pk[v] to generate challenge w.  (We no longer need r after
+  // this step.)
+  if (!EncryptSharedKey()) {
+    return false;
+  }
+  r_.clear();
+
+  // Generate the versioned challenge (v|w) by Base64-encoding w.  Since this
+  // string is typically appended to the request URL, it needs a URL-safe
+  // variant of Base64.
+  vw_ = base::StringPrintf("%d:%s", v_, UrlSafeB64Encode(w_).c_str());
+
+  return true;
+}
+
+std::string ClientUpdateProtocol::UrlSafeB64Encode(
+    const std::vector<uint8>& data) {
+  // This function was originally based off Omaha's base/security/b64.c, which
+  // is a URL-safe variant of Base64 with no padding.  Rather than copy-paste an
+  // implementation here, we just call the standard Base64 encoder, and fix up.
+
+  std::string result;
+  const base::StringPiece input(reinterpret_cast<const char*>(&data[0]),
+                                data.size());
+  if (!base::Base64Encode(input, &result)) {
+    return std::string();
+  }
+
+  // Do an tr|+/|-_| on the output, and strip any '=' padding.
+  for (std::string::iterator it = result.begin(); it != result.end(); ++it) {
+    switch (*it) {
+      case '+':
+        *it = '-';
+        continue;
+      case '/':
+        *it = '_';
+        continue;
+      default:
+        continue;
+    }
+  }
+  TrimString(result, "=", &result);
+
+  return result;
+}
+
+// This class needs to implement the same hashing functions as the Google Update
+// server; for now, this is SHA-1, but we may change this to SHA-256 in the
+// near future.  Hence, the wrappers.
+
+size_t ClientUpdateProtocol::HashDigestSize() {
+  return base::kSHA1Length;
+}
+
+std::vector<uint8> ClientUpdateProtocol::Hash(const std::vector<uint8>& data) {
+  std::vector<uint8> result(HashDigestSize());
+  base::SHA1HashBytes(data.empty() ? NULL : &data[0],
+                      data.size(),
+                      &result[0]);
+  return result;
+}
+
+std::vector<uint8> ClientUpdateProtocol::Hash(const base::StringPiece& sdata) {
+  std::vector<uint8> result(HashDigestSize());
+  base::SHA1HashBytes(sdata.empty() ?
+                          NULL :
+                          reinterpret_cast<const unsigned char*>(sdata.data()),
+                      sdata.length(),
+                      &result[0]);
+  return result;
+}
+
+std::vector<uint8> ClientUpdateProtocol::SymSign(const std::vector<uint8>& key,
+                                                 uint8 id,
+                                                 const std::vector<uint8>* h1,
+                                                 const std::vector<uint8>* h2,
+                                                 const std::vector<uint8>* h3) {
+  crypto::HMAC hmac(crypto::HMAC::SHA1);
+  if (!hmac.Init(&key[0], key.size())) {
+    return std::vector<uint8>();
+  }
+
+  std::vector<uint8> data;
+  data.push_back(id);
+  const std::vector<uint8>* args[] = { h1, h2, h3 };
+  for (size_t i = 0; i != arraysize(args); ++i) {
+    if (args[i]) {
+      DCHECK(args[i]->size() == HashDigestSize());

[Ryan Sleevi, 2013/05/30 02:28:08]

DCHECK_EQ

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done.

+      data.insert(data.end(), args[i]->begin(), args[i]->end());
+    }
+  }
+
+  std::vector<uint8> result(hmac.DigestLength());
+  if (!hmac.Sign(base::StringPiece(reinterpret_cast<const char*>(&data[0]),
+                                   data.size()),
+                 &result[0],
+                 result.size())) {
+    return std::vector<uint8>();
+  }
+
+  return result;
+}
+
+std::vector<uint8> ClientUpdateProtocol::RsaPad(
+    size_t rsa_key_size,
+    const std::vector<uint8>& entropy) {
+  DCHECK(rsa_key_size >= HashDigestSize());

[Ryan Sleevi, 2013/05/30 02:28:08]

DCHECK_GE

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done.

+
+  // The result gets padded with zeros if the result size is greater than
+  // the size of the buffer provided by the caller.
+  std::vector<uint8> result(entropy);
+  result.resize(rsa_key_size - HashDigestSize());
+
+  // For use with RSA, the input needs to be smaller than the RSA modulus,
+  // which has always the msb set.
+  result[0] &= 127;  // Reset msb
+  result[0] |= 64;   // Set second highest bit.
+
+  std::vector<uint8> digest = Hash(result);
+  result.insert(result.end(), digest.begin(), digest.end());
+  DCHECK(result.size() == rsa_key_size);

[Ryan Sleevi, 2013/05/30 02:28:08]

DCHECK_GE

[Ryan Myers (chromium), 2013/05/30 21:01:10]

Done (used DCHECK_EQ).

+  return result;
+}
+
+}  // namespace crypto
+