This adds support for encrypted ONC import

chrome/browser/chromeos/cros/onc_network_parser.cc

 // Copyright (c) 2011 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/chromeos/cros/onc_network_parser.h"
 
 #include <keyhi.h>
 #include <pk11pub.h>
 
 #include "base/base64.h"
 #include "base/json/json_value_serializer.h"
 #include "base/json/json_writer.h"  // for debug output only.
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/cros/native_network_constants.h"
 #include "chrome/browser/chromeos/cros/native_network_parser.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/cros/onc_constants.h"
 #include "chrome/common/net/x509_certificate_model.h"
+#include "crypto/scoped_nss_types.h"
+#include "crypto/symmetric_key.h"
+#include "crypto/encryptor.h"
 #include "grit/generated_resources.h"
 #include "net/base/cert_database.h"
 #include "net/base/crypto_module.h"
 #include "net/base/net_errors.h"
 #include "net/base/x509_certificate.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace chromeos {
 
@@ skipping 157 matching lines @@
   value.GetAsBoolean(&bool_value);
   return bool_value;
 }
 
 std::string ConvertValueToString(const base::Value& value) {
   std::string value_json;
   base::JSONWriter::Write(&value, false, &value_json);
   return value_json;
 }
 
+bool VerifyHMAC(const std::string& hmac,
+                const std::string& plaintext,
+                const crypto::SymmetricKey* key) {

[Ryan Sleevi, 2011/12/24 07:34:08]

See crypto::HMAC::Verify()

Same implementation, but re-uses the existing code used elsewhere.

+  SECItem empty_parameters;
+  empty_parameters.type = siBuffer;
+  empty_parameters.data = 0;
+  empty_parameters.len = 0;
+
+  SECStatus status = SECSuccess;
+  crypto::ScopedPK11Context context(PK11_CreateContextBySymKey(
+      CKM_SHA_1_HMAC,
+      CKA_SIGN,
+      key->key(),
+      &empty_parameters));
+  if (!context.get()) {
+    LOG(WARNING) << "PK11_CreateContextBySymKey failed";
+    return false;
+  }
+
+  status = PK11_DigestBegin(context.get());
+  if (status != SECSuccess) {
+    LOG(WARNING) << "PK11_DigestBegin failed";
+    return false;
+  }
+
+  status = PK11_DigestOp(
+      context.get(),
+      reinterpret_cast<const unsigned char*>(plaintext.c_str()),
+      plaintext.size());
+
+  if (status != SECSuccess) {
+    LOG(WARNING) << "PK11_DigestOp failed";
+    return false;
+  }
+
+  // SHA1 HMACs are 160 bits.
+  char digest[20];
+  unsigned int length = 0;
+  status = PK11_DigestFinal(context.get(),
+                            reinterpret_cast<unsigned char*>(&digest[0]),
+                            &length,
+                            20);
+  if (status != SECSuccess) {
+    LOG(WARNING) << "PK11_DigestFinal failed";
+    return false;
+  }
+
+  if (std::string(digest, length) == hmac)

[Ryan Sleevi, 2011/12/24 07:34:08]

See above comment. This comparison is not a constant time comparison, therefore
it leaks timing information as a side-channel.

+    return true;
+
+  return false;

[Charlie Lee, 2011/12/22 20:55:17]

should add a log warning here for why this would fail.

+}
+
 }  // namespace
 
 // -------------------- OncNetworkParser --------------------
 
 OncNetworkParser::OncNetworkParser(const std::string& onc_blob,
+                                   const std::string& passphrase,
                                    NetworkUIData::ONCSource onc_source)
     : NetworkParser(get_onc_mapper()),
       onc_source_(onc_source),
       network_configs_(NULL),
       certificates_(NULL) {
   VLOG(2) << __func__ << ": OncNetworkParser called on " << onc_blob;
   JSONStringValueSerializer deserializer(onc_blob);
   deserializer.set_allow_trailing_comma(true);
   scoped_ptr<base::Value> root(deserializer.Deserialize(NULL, &parse_error_));
 
   if (!root.get() || root->GetType() != base::Value::TYPE_DICTIONARY) {
     LOG(WARNING) << "OncNetworkParser received bad ONC file: " << parse_error_;
   } else {
     root_dict_.reset(static_cast<DictionaryValue*>(root.release()));
+
+    // Check and see if this is an encrypted ONC file.  If so, decrypt it.
+    std::string ciphertext_test;
+    if (root_dict_->GetString("Ciphertext", &ciphertext_test))
+      root_dict_.reset(Decrypt(passphrase, root_dict_.get()));
+
     // At least one of NetworkConfigurations or Certificates is required.
     bool has_network_configurations =
       root_dict_->GetList("NetworkConfigurations", &network_configs_);
     bool has_certificates =
       root_dict_->GetList("Certificates", &certificates_);
     VLOG(2) << "ONC file has " << GetNetworkConfigsSize() << " networks and "
             << GetCertificatesSize() << " certificates";
     LOG_IF(WARNING, (!has_network_configurations && !has_certificates))
       << "ONC file has no NetworkConfigurations or Certificates.";
   }
 }
 
 OncNetworkParser::OncNetworkParser()
     : NetworkParser(get_onc_mapper()),
       network_configs_(NULL),
       certificates_(NULL) {
 }
 
 OncNetworkParser::~OncNetworkParser() {
 }
 
 // static
 const EnumMapper<PropertyIndex>* OncNetworkParser::property_mapper() {
   return get_onc_mapper();
 }
 
+base::DictionaryValue* OncNetworkParser::Decrypt(
+    const std::string& passphrase,
+    base::DictionaryValue* root) {
+  std::string salt;
+  int iterations;
+  const int key_size_in_bits = 256;
+  std::string initial_vector_str;
+  std::string ciphertext;
+  std::string cipher;
+  std::string hmac_method;
+
+  if (!root->GetString("Salt", &salt) ||
+      !root->GetInteger("Iterations", &iterations) ||
+      !root->GetString("Cipher", &cipher) ||
+      !root->GetString("HMAC", &hmac_method) ||
+      !root->GetString("IV", &initial_vector_str) ||
+      !root->GetString("Ciphertext", &ciphertext)) {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_MALFORMED);
+    return NULL;
+  }
+
+  if (hmac_method != "SHA1" || cipher != "AES256") {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_UNSUPPORTED_ENCRYPTION);
+    return NULL;
+  }
+
+  if (!base::Base64Decode(salt, &salt)) {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_UNABLE_TO_DECODE);
+    return NULL;
+  }
+
+  scoped_ptr<crypto::SymmetricKey> key(
+      crypto::SymmetricKey::DeriveKeyFromPassword(crypto::SymmetricKey::AES,
+                                                  passphrase,
+                                                  salt,
+                                                  iterations,
+                                                  key_size_in_bits));
+
+  crypto::Encryptor decryptor;
+  std::string initial_vector;
+  std::string plaintext;
+  if (!base::Base64Decode(initial_vector_str, &initial_vector)) {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_UNABLE_TO_DECODE);
+    return NULL;
+  }
+  if (!decryptor.Init(key.get(), crypto::Encryptor::CBC, initial_vector))  {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_UNABLE_TO_DECRYPT);
+    return NULL;
+  }
+  if (!base::Base64Decode(ciphertext, &ciphertext)) {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_UNABLE_TO_DECODE);
+    return NULL;
+  }
+  if (!decryptor.Decrypt(ciphertext, &plaintext)) {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_UNABLE_TO_DECRYPT);
+    return NULL;
+  }
+
+  // Split out the HMAC.

[Ryan Sleevi, 2011/12/24 07:34:08]

As a drive-by, if it's not too late, wouldn't this make more sense from a
cryptographic security principal to have the HMAC cover the encrypted
ciphertext, rather than be encryptd along with the plaintext?

A good understanding of why this is can be had at
http://blog.thoughtcrime.org/the-cryptographic-doom-principle

If this is a new format being developed at Google (which, as best I can tell, it
is - at least the design docs are all restricted to .google.com), then perhaps
there's the opportunity to avoid some of the common pitfalls here.

+  std::string::size_type split = plaintext.find_last_of('|');
+  std::string hmac = plaintext.substr(split + 1, plaintext.size() - split - 1);
+  plaintext = plaintext.substr(0, split);
+  if (!base::Base64Decode(hmac, &hmac)) {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_UNABLE_TO_DECODE);
+    return NULL;
+  }
+
+  // Verify the HMAC.  If it fails, then we have failed to decrypt (even though
+  // we decrypted *something*, it's been tampered with).
+  if (!VerifyHMAC(hmac, plaintext, key.get())) {
+    parse_error_ = l10n_util::GetStringUTF8(
+        IDS_NETWORK_CONFIG_ERROR_ENCRYPTED_ONC_HMAC_CHECK_FAILED);

[Ryan Sleevi, 2011/12/24 07:34:08]

The fact that HMAC errors are distinguished from encryption errors can be used
to potentially mount a Vaudenay attack. At the least, there is some timing
information being leaked here about which failed.

+    return NULL;
+  }
+
+  // Now we've decrypted it, let's deserialize the decrypted data.
+  JSONStringValueSerializer deserializer(plaintext);
+  deserializer.set_allow_trailing_comma(true);
+  scoped_ptr<base::Value> new_root(deserializer.Deserialize(NULL,
+                                                            &parse_error_));
+  if (!new_root.get() || new_root->GetType() != base::Value::TYPE_DICTIONARY) {
+    if (parse_error_.empty())

[Charlie Lee, 2011/12/22 20:55:17]

when would this not be empty?

[Greg Spencer (Chromium), 2012/01/03 21:54:22]

When the Deserialize call above places an error into it.

The idea is that if the deserializer doesn't add an error, but we still don't
get a valid root for some reason, then we  need to supply an error.

+      parse_error_ = l10n_util::GetStringUTF8(
+          IDS_NETWORK_CONFIG_ERROR_NETWORK_PROP_DICT_MALFORMED);
+    return NULL;
+  }
+  return static_cast<base::DictionaryValue*>(new_root.release());
+}
+
 int OncNetworkParser::GetNetworkConfigsSize() const {
   return network_configs_ ? network_configs_->GetSize() : 0;
 }
 
 const base::DictionaryValue* OncNetworkParser::GetNetworkConfig(int n) {
   CHECK(network_configs_);
   CHECK(static_cast<size_t>(n) < network_configs_->GetSize());
   CHECK_GE(n, 0);
   base::DictionaryValue* info = NULL;
   if (!network_configs_->GetDictionary(n, &info)) {
@@ skipping 1017 matching lines @@
     // on the value of AuthenticationType.
     { "L2TP-IPsec", PROVIDER_TYPE_L2TP_IPSEC_PSK },
     { "OpenVPN", PROVIDER_TYPE_OPEN_VPN },
   };
   CR_DEFINE_STATIC_LOCAL(EnumMapper<ProviderType>, parser,
       (table, arraysize(table), PROVIDER_TYPE_MAX));
   return parser.Get(type);
 }
 
 }  // namespace chromeos