macOS: Remove keychain reauthorizing logic.

The keychain was re-authorized to support Chrome's new signing
certificate 18+ months ago. At this point, users who haven't re-launch
Chrome in 18 months are unlikely to do so, or need the keychain. This
removes the re-authorization code.

Bug: 893729
Change-Id: I75b5a40407d196adfa5c31996024b9bb278221a1
Reviewed-on: https://chromium-review.googlesource.com/c/1271935Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Greg Kerr <kerrnel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599341}

chrome/BUILD.gn

@@ -640,10 +640,6 @@ if (is_win) {
       "//chrome/common:version_header",
     ]
 
-    if (is_chrome_branded) {
-      deps += [ ":chrome_helpers" ]
-    }
-
     if (enable_stripping) {
       # At link time, preserve the global symbols specified in the .exports
       # file. All other global symbols will be marked as private. The default
@@ -740,18 +736,6 @@ if (is_win) {
     ]
   }
 
-  if (is_chrome_branded) {
-    bundle_data("chrome_helpers") {
-      sources = [
-        "installer/mac/internal/keychain_reauthorizers/$chrome_mac_bundle_id",
-      ]
-
-      outputs = [
-        "{{bundle_contents_dir}}/Helpers/{{source_file_part}}",
-      ]
-    }
-  }
-
   bundle_data("chrome_versioned_bundle_data") {
     sources = [
       "$root_out_dir/$chrome_framework_name.framework",

chrome/app/framework.order

@@ -21,8 +21,5 @@ ___asan_default_options
 # Entry point from the app mode loader.
 _ChromeAppModeStart_v4
 
-# Entry point for the keychain reauthorization stub.
-_KeychainReauthorizeIfNeededAtUpdate
-
 # _ChromeMain must be listed last.  That's the whole point of this file.
 _ChromeMain

chrome/browser/BUILD.gn

@@ -630,8 +630,6 @@ jumbo_split_static_library("browser") {
     "mac/exception_processor.mm",
     "mac/install_from_dmg.h",
     "mac/install_from_dmg.mm",
-    "mac/keychain_reauthorize.h",
-    "mac/keychain_reauthorize.mm",
     "mac/keystone_glue.h",
     "mac/keystone_glue.mm",
     "mac/keystone_registration.h",

chrome/browser/chrome_browser_main_mac.mm

@@ -24,7 +24,6 @@
 #import "chrome/browser/chrome_browser_application_mac.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/mac/install_from_dmg.h"
-#include "chrome/browser/mac/keychain_reauthorize.h"
 #import "chrome/browser/mac/keystone_glue.h"
 #include "chrome/browser/mac/mac_startup_profiler.h"
 #include "chrome/browser/ui/cocoa/main_menu_builder.h"
@@ -132,17 +131,6 @@ void ChromeBrowserMainPartsMac::PreMainMessageLoopStart() {
   chrome::BuildMainMenu(NSApp, app_controller);
   [app_controller mainMenuCreated];
 
-  // Do Keychain reauthorization. This gets two chances to run. If the first
-  // try doesn't complete successfully (crashes or is interrupted for any
-  // reason), there will be a second chance. Once this step completes
-  // successfully, it should never have to run again.
-  NSString* const keychain_reauthorize_pref =
-      @"KeychainReauthorizeInAppSpring2017";
-  const int kKeychainReauthorizeMaxTries = 2;
-
-  chrome::KeychainReauthorizeIfNeeded(keychain_reauthorize_pref,
-                                      kKeychainReauthorizeMaxTries);
-
   // Initialize the OSCrypt.
   PrefService* local_state = g_browser_process->local_state();
   DCHECK(local_state);

chrome/browser/mac/keychain_reauthorize.h

-// Copyright 2017 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.
-
-#ifndef CHROME_BROWSER_MAC_KEYCHAIN_REAUTHORIZE_H_
-#define CHROME_BROWSER_MAC_KEYCHAIN_REAUTHORIZE_H_
-
-#ifdef __OBJC__
-@class NSString;
-#else
-class NSString;
-#endif
-
-namespace chrome {
-
-extern "C" {
-
-// Reauthorizes the keychain entry, but only if it's determined that it's
-// necessary. pref_key is looked up in the system's standard user defaults
-// (preferences) and it is associated with both the number of previous attempts,
-// and whether a previous attempt succeeded. Only if a previous attempt did not
-// succeed, and the number of previous tries is less than max_tries, is
-// reauthorization attempted. Before the attempt, the preference is
-// incremented, allowing a finite number of incomplete attempts at performing
-// the operation.
-
-// The system's standard user defaults for the application are used
-// (~/Library/Preferences/com.google.Chrome.plist,
-// com.google.Chrome.canary.plist, etc.) instead of Chrome preferences because
-// Keychain access is tied more closely to the bundle identifier and signed
-// product than it is to any specific profile (--user-data-dir).
-void KeychainReauthorizeIfNeeded(NSString* pref_key, int max_tries);
-
-// A wrapper to call KeychainReauthorizeIfNeeded under a local autorelease pool.
-// This is used by the keychain_reauthorization stub executable, which is run at
-// update time or at browser launch. The name cannot be changed to indicate that
-// it is not only run at update because the stub is already built and signed.
-__attribute__((visibility("default"))) void KeychainReauthorizeIfNeededAtUpdate(
-    NSString* pref_key,
-    int max_tries);
-
-}  // extern "C"
-
-}  // namespace chrome
-
-#endif  // CHROME_BROWSER_MAC_KEYCHAIN_REAUTHORIZE_H_

chrome/browser/mac/keychain_reauthorize.mm

-// Copyright 2017 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/mac/keychain_reauthorize.h"
-
-#import <Foundation/Foundation.h>
-#include <Security/Security.h>
-
-#include <crt_externs.h>
-#include <errno.h>
-#include <spawn.h>
-#include <string.h>
-
-#include <vector>
-
-#include "base/command_line.h"
-#include "base/files/file_path.h"
-#include "base/files/file_util.h"
-#include "base/logging.h"
-#include "base/mac/bundle_locations.h"
-#include "base/mac/foundation_util.h"
-#include "base/mac/mac_logging.h"
-#include "base/mac/scoped_cftyperef.h"
-#include "base/metrics/histogram_functions.h"
-#include "base/path_service.h"
-#include "base/posix/eintr_wrapper.h"
-#include "base/scoped_generic.h"
-#include "base/strings/sys_string_conversions.h"
-#include "components/os_crypt/keychain_password_mac.h"
-#include "crypto/apple_keychain.h"
-
-namespace chrome {
-
-namespace {
-
-struct VectorScramblerTraits {
-  static std::vector<uint8_t>* InvalidValue() { return nullptr; }
-
-  static void Free(std::vector<uint8_t>* buf) {
-    memset(buf->data(), 0x11, buf->size());
-    delete buf;
-  }
-};
-
-typedef base::ScopedGeneric<std::vector<uint8_t>*, VectorScramblerTraits>
-    ScopedVectorScrambler;
-
-// Reauthorizes the Safe Storage keychain item, which protects the randomly
-// generated password that encrypts the user's saved passwords. This reads out
-// the keychain item, deletes it, and re-adds it to the keychain. This works
-// because the keychain uses an app's designated requirement as the ACL for
-// reading an item. Chrome will be signed with a designated requirement that
-// accepts both the old and new certificates.
-bool KeychainReauthorize() {
-  base::ScopedCFTypeRef<SecKeychainItemRef> storage_item;
-  UInt32 pw_length = 0;
-  void* password_data = nullptr;
-
-  crypto::AppleKeychain keychain;
-  OSStatus error = keychain.FindGenericPassword(
-      strlen(KeychainPassword::service_name), KeychainPassword::service_name,
-      strlen(KeychainPassword::account_name), KeychainPassword::account_name,
-      &pw_length, &password_data, storage_item.InitializeInto());
-
-  base::ScopedCFTypeRef<SecKeychainItemRef> backup_item;
-  std::string backup_service_name =
-      std::string(KeychainPassword::service_name) + ".bak";
-  if (error != noErr) {
-    // If the main entry does not exist, nor does the backup, exit.
-    if (keychain.FindGenericPassword(
-            backup_service_name.size(), backup_service_name.data(),
-            strlen(KeychainPassword::account_name),
-            KeychainPassword::account_name, &pw_length, &password_data,
-            backup_item.InitializeInto()) != noErr) {
-      OSSTATUS_LOG(ERROR, error)
-          << "KeychainReauthorize failed. Cannot retrieve item.";
-      return false;
-    }
-  }
-
-  // At this point, a password was retrieved, either from the main or backup.
-  ScopedVectorScrambler password;
-  password.reset(new std::vector<uint8_t>(
-      static_cast<uint8_t*>(password_data),
-      static_cast<uint8_t*>(password_data) + pw_length));
-  memset(password_data, 0x11, pw_length);
-  keychain.ItemFreeContent(password_data);
-
-  if (backup_item.get() == nullptr) {
-    // If writing the backup fails, still attempt the re-auth.
-    keychain.AddGenericPassword(
-        backup_service_name.size(), backup_service_name.data(),
-        strlen(KeychainPassword::account_name), KeychainPassword::account_name,
-        password.get()->size(), password.get()->data(),
-        backup_item.InitializeInto());
-  }
-
-  if (storage_item) {
-    error = keychain.ItemDelete(storage_item);
-    if (error != noErr) {
-      OSSTATUS_LOG(WARNING, error)
-          << "KeychainReauthorize failed to delete item.";
-    }
-  }
-
-  error = keychain.AddGenericPassword(
-      strlen(KeychainPassword::service_name), KeychainPassword::service_name,
-      strlen(KeychainPassword::account_name), KeychainPassword::account_name,
-      password.get()->size(), password.get()->data(), nullptr);
-
-  if (error != noErr) {
-    OSSTATUS_LOG(ERROR, error) << "Failed to re-add storage password.";
-    return false;
-  }
-
-  if (backup_item.get() == nullptr) {
-    // Attempt to get the backup entry in case one exists. Ignore return value.
-    // This could happen if Chrome crashed after writing the backup entry and
-    // before deleting the main entry.
-    keychain.FindGenericPassword(
-        backup_service_name.size(), backup_service_name.data(),
-        strlen(KeychainPassword::account_name), KeychainPassword::account_name,
-        &pw_length, &password_data, backup_item.InitializeInto());
-  }
-
-  if (backup_item.get()) {
-    error = keychain.ItemDelete(backup_item);
-    if (error != noErr) {
-      OSSTATUS_LOG(WARNING, error) << "Deleting backup entry failed.";
-    }
-  }
-
-  return true;
-}
-
-// This performs the re-reauthorization from the stub executable.
-void KeychainReauthorizeFromStub(NSString* pref_key) {
-  NSUserDefaults* user_defaults = [NSUserDefaults standardUserDefaults];
-
-  NSString* success_pref_key = [pref_key stringByAppendingString:@"Success"];
-  BOOL success_value = [user_defaults boolForKey:success_pref_key];
-  if (success_value)
-    return;
-
-  bool success = KeychainReauthorize();
-  if (!success)
-    return;
-
-  [user_defaults setBool:YES forKey:success_pref_key];
-  [user_defaults synchronize];
-}
-
-}  // namespace
-
-void KeychainReauthorizeIfNeeded(NSString* pref_key, int max_tries) {
-#if defined(GOOGLE_CHROME_BUILD)
-  // Only launch the stub executable when running in the browser.
-  DCHECK(base::mac::AmIBundled());
-
-  NSUserDefaults* user_defaults = [NSUserDefaults standardUserDefaults];
-  int pref_value = [user_defaults integerForKey:pref_key];
-
-  if (pref_value >= max_tries)
-    return;
-
-  NSString* success_pref_key = [pref_key stringByAppendingString:@"Success"];
-  BOOL success_value = [user_defaults boolForKey:success_pref_key];
-  if (success_value)
-    return;
-
-  if (pref_value > 0) {
-    // Logs the number of previous tries that didn't complete.
-    base::UmaHistogramSparse("OSX.KeychainReauthorizeIfNeeded", pref_value);
-  }
-
-  ++pref_value;
-  [user_defaults setInteger:pref_value forKey:pref_key];
-  [user_defaults synchronize];
-
-  NSBundle* main_bundle = base::mac::OuterBundle();
-  std::string identifier =
-      base::SysNSStringToUTF8([main_bundle bundleIdentifier]);
-  std::string bundle_path = base::SysNSStringToUTF8([main_bundle bundlePath]);
-
-  base::FilePath reauth_binary = base::FilePath(bundle_path)
-                                     .Append("Contents")
-                                     .Append("Helpers")
-                                     .Append(identifier);
-
-  std::string framework_path =
-      base::SysNSStringToUTF8([base::mac::FrameworkBundle() executablePath]);
-
-  std::vector<std::string> argv = {reauth_binary.value(), framework_path};
-  std::vector<char*> argv_cstr;
-  argv_cstr.reserve(argv.size() + 1);
-  for (size_t i = 0; i < argv.size(); ++i) {
-    argv_cstr.push_back(const_cast<char*>(argv[i].c_str()));
-  }
-  argv_cstr.push_back(nullptr);
-
-  pid_t pid;
-  char** new_environ = *_NSGetEnviron();
-  errno = posix_spawn(&pid, reauth_binary.value().c_str(), nullptr, nullptr,
-                      &argv_cstr[0], new_environ);
-  if (errno != 0) {
-    PLOG(ERROR) << "posix_spawn";
-    return;
-  } else {
-    // If execution does not block on the helper stub, there is a
-    // race condition between it and Chrome creating a new keychain entry.
-    int status;
-    if (HANDLE_EINTR(waitpid(pid, &status, 0)) != pid || status != 0) {
-      return;
-    }
-  }
-
-  // Find out if the re-authorization succeeded by querying cfprefs.
-  bool stub_result = [user_defaults boolForKey:success_pref_key];
-  // Logs the try number (1, 2) that succeeded.
-  if (stub_result) {
-    base::UmaHistogramSparse("OSX.KeychainReauthorizeIfNeededSuccess",
-                             pref_value);
-  }
-#endif  // defined(GOOGLE_CHROME_BUILD)
-}
-
-void KeychainReauthorizeIfNeededAtUpdate(NSString* pref_key, int max_tries) {
-  @autoreleasepool {
-    KeychainReauthorizeFromStub(pref_key);
-  }
-}
-
-}  // namespace chrome

chrome/installer/mac/BUILD.gn

@@ -13,10 +13,6 @@ group("mac") {
     ":make_signers",
     "app:mac_installer_app",
   ]
-
-  if (is_chrome_branded) {
-    public_deps += [ ":keychain_reauthorizers" ]
-  }
 }
 
 _packaging_dir = "$root_out_dir/$chrome_product_full_name Packaging"
@@ -118,18 +114,3 @@ copy("copies") {
     "$_packaging_dir/{{source_file_part}}",
   ]
 }
-
-if (is_chrome_branded) {
-  copy("keychain_reauthorizers") {
-    visibility = [ ":mac" ]
-
-    sources = [
-      "internal/keychain_reauthorizers/com.google.Chrome",
-      "internal/keychain_reauthorizers/com.google.Chrome.canary",
-    ]
-
-    outputs = [
-      "$_packaging_dir/keychain_reauthorizers/{{source_file_part}}",
-    ]
-  }
-}

chrome/installer/mac/dmgdiffer.sh

@@ -243,18 +243,6 @@ make_patch_fs() {
     exit 13
   fi
 
-  local patch_keychain_reauthorize_dir="${patch_fs}/.keychain_reauthorize"
-  if ! mkdir "${patch_keychain_reauthorize_dir}"; then
-    err "could not mkdir patch_keychain_reauthorize_dir"
-    exit 13
-  fi
-
-  if ! cp -p "${SCRIPT_DIR}/keychain_reauthorizers/${old_app_bundleid}" \
-             "${patch_keychain_reauthorize_dir}/${old_app_bundleid}"; then
-    err "could not copy keychain_reauthorize"
-    exit 13
-  fi
-
   local patch_dotpatch_dir="${patch_fs}/.patch"
   if ! mkdir "${patch_dotpatch_dir}"; then
     err "could not mkdir patch_dotpatch_dir"

chrome/installer/mac/sign_app.sh.in

@@ -24,21 +24,24 @@ export -n SHELLOPTS
 ME="$(basename "${0}")"
 readonly ME
 
-if [[ ${#} -ne 5 && ${#} -ne 6 ]]; then
+if [[ ${#} -eq 4 ]]; then
+  app_path="${1}"
+  codesign_keychain="${2}"
+  codesign_id="${3}"
+  if [[ "${4}" == "--development" ]]; then
+    is_development=1
+  fi
+elif [[ ${#} -ne 5 ]]; then
   echo "usage: ${ME} app_path codesign_keychain codesign_id \
-provisioning_profile entitlements_plist [--development]" >& 2
+provisioning_profile entitlements_plist" >& 2
   exit 1
-fi
-
-app_path="${1}"
-codesign_keychain="${2}"
-codesign_id="${3}"
-provisioning_profile="${4}"
-entitlements_plist="${5}"
-is_development=
-
-if [[ ${#} == 6 && "${6}" == "--development" ]]; then
-  is_development=1
+else
+  app_path="${1}"
+  codesign_keychain="${2}"
+  codesign_id="${3}"
+  provisioning_profile="${4}"
+  entitlements_plist="${5}"
+  is_development=0
 fi
 
 script_dir="$(dirname "${0}")"
@@ -59,7 +62,9 @@ app_mode_loader_app="${framework}/Resources/app_mode_loader.app"
 app_mode_loader="${app_mode_loader_app}/Contents/MacOS/app_mode_loader"
 
 # Embed the supplied provisioning profile.
-cp "${provisioning_profile}" "${contents_dir}/embedded.provisionprofile"
+if [[ -z "${is_development}" ]]; then
+  cp "${provisioning_profile}" "${contents_dir}/embedded.provisionprofile"
+fi
 
 requirement="\
 designated => \
@@ -75,9 +80,9 @@ codesign_cmd=(
   "${browser_app}"
   --options "${enforcement_flags_app}"
   --resource-rules "${browser_app_rules}"
-  --entitlements "${entitlements_plist}"
 )
 if [[ -z "${is_development}" ]]; then
+  codesign_cmd+=(--entitlements="${entitlements_plist}")
   codesign_cmd+=( -r="${requirement}" )
 fi
 "${codesign_cmd[@]}"

chrome/installer/mac/variables.sh

@@ -19,6 +19,5 @@ enforcement_flags_installer_tools="${enforcement_flags_helpers},kill"
 # contains the hash of the certificate used to sign Chrome. When transitioning
 # signing certs, this may include the hash of both the old and new certificate.
 requirement_suffix="\
-and (certificate leaf = H\"85cee8254216185620ddc8851c7a9fc4dfe120ef\" or \
-certificate leaf = H\"c9a99324ca3fcb23dbcc36bd5fd4f9753305130a\") \
+and certificate leaf = H\"c9a99324ca3fcb23dbcc36bd5fd4f9753305130a\" \
 "