Use system crypto in pref-pane IsPinValid()

remoting/host/me2me_preference_pane.mm

 // Copyright (c) 2012 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.
 
 #import "remoting/host/me2me_preference_pane.h"
 
 #import <Cocoa/Cocoa.h>
+#include <CommonCrypto/CommonHMAC.h>
 #include <launch.h>
 #import <PreferencePanes/PreferencePanes.h>
 #import <SecurityInterface/SFAuthorizationView.h>
 
 #include "base/eintr_wrapper.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/mac/authorization_util.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/launchd.h"
 #include "base/mac/mac_logging.h"
 #include "base/mac/scoped_launch_data.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stringprintf.h"
 #include "base/sys_string_conversions.h"
 #include "remoting/host/host_config.h"
 #include "remoting/host/json_host_config.h"
-#include "remoting/protocol/me2me_host_authenticator_factory.h"
+#include "third_party/modp_b64/modp_b64.h"
 
 namespace {
 // The name of the Remoting Host service that is registered with launchd.
 #define kServiceName "org.chromium.chromoting"
 #define kConfigDir "/Library/PrivilegedHelperTools/"
 
 // This helper script is executed as root.  It is passed a command-line option
 // (--enable or --disable), which causes it to create or remove a file that
 // informs the host's launch script of whether the host is enabled or disabled.
 const char kHelperTool[] = kConfigDir kServiceName ".me2me.sh";
 
 bool GetTemporaryConfigFilePath(FilePath* path) {
   if (!file_util::GetTempDir(path))
     return false;
   *path = path->Append(kServiceName ".json");
   return true;
 }
 
 bool IsConfigValid(const remoting::JsonHostConfig* config) {
   std::string value;
   return (config->GetString(remoting::kHostIdConfigPath, &value) &&
           config->GetString(remoting::kHostSecretHashConfigPath, &value) &&
           config->GetString(remoting::kXmppLoginConfigPath, &value));
 }
 
 bool IsPinValid(const std::string& pin, const std::string& host_id,
                 const std::string& host_secret_hash) {
-  remoting::protocol::SharedSecretHash hash;

[Jamie, 2012/05/14 17:37:49]

Would it be possible to define a local class with the same interface as
SharedSecretHash? That would minimize the changes to the rest of the file and
greatly simplify the task of undoing this change once we have 64-bit support in
the core codebase.

[Lambros, 2012/05/14 19:58:34]

The SharedSecretHash/AuthenticationMethod interfaces don't seem very convenient
for embedding into here.

Everything is encapsulated into the IsPinValid() method here.  Once we have
64-bit support, all we need to do is replace this implementation with a simple
call to VerifyHostPinHash() in remoting/host/pin_hash.h.

-  if (!hash.Parse(host_secret_hash)) {
-    LOG(ERROR) << "Invalid host_secret_hash.";
+  size_t separator = host_secret_hash.find(':');
+  if (separator == std::string::npos)
+    return false;
+
+  std::string method = host_secret_hash.substr(0, separator);
+  if (method != "hmac") {
+    LOG(ERROR) << "Authentication method '" << method << "' not supported";
     return false;
   }
-  std::string result =
-      remoting::protocol::AuthenticationMethod::ApplyHashFunction(
-          hash.hash_function, host_id, pin);
-  return result == hash.value;
+
+  std::string hash_base64 = host_secret_hash.substr(separator + 1);
+
+  // Convert |hash_base64| to |hash|, based on code from base/base64.cc.
+  int hash_base64_size = static_cast<int>(hash_base64.size());
+  std::string hash;
+  hash.resize(modp_b64_decode_len(hash_base64_size));
+  int hash_size = modp_b64_decode(&(hash[0]), hash_base64.data(),
+                                  hash_base64_size);
+  if (hash_size < 0) {
+    LOG(ERROR) << "Failed to parse host_secret_hash";
+    return false;
+  }
+  hash.resize(hash_size);
+
+  std::string computed_hash;
+  computed_hash.resize(32);

[dcaiafa, 2012/05/14 15:04:43]

s/32/CC_SHA256_DIGEST_LENGTH/

[Lambros, 2012/05/14 19:58:34]

Thanks for that! Done.

+
+  CCHmac(kCCHmacAlgSHA256,
+         host_id.data(), host_id.size(),
+         pin.data(), pin.size(),
+         &(computed_hash[0]));
+
+  return computed_hash == hash;
 }
 
 }  // namespace
 
 
 @implementation Me2MePreferencePane
 
 - (void)mainViewDidLoad {
   [authorization_view_ setDelegate:self];
   [authorization_view_ setString:kAuthorizationRightExecute];
@@ skipping 321 matching lines @@
   int error = launch_data_get_errno(response.get());
   if (error) {
     LOG(ERROR) << "launchd returned error: " << error;
     [self showError];
     return NO;
   }
   return YES;
 }
 
 @end