Re-enable app shim code signature checking with fixes.

Two changes:
- Remove the IS_OFFICIAL_BUILD check (don't require official builds to
  be signed for app shims to run at all).
- Construct a custom designated requirement to verify the app shim,
  which checks its identifier and that its certificate matches the
  browser's.

For now, keep failures log-only.

This reverts commit add2f7a1.

Bug: 913362, 923612
Change-Id: I1c536e2172519dbf8e5b36a2dd9b5f1b26fb8302
Reviewed-on: https://chromium-review.googlesource.com/c/1427385Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Auto-Submit: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625394}

base/mac/foundation_util.h

@@ -53,9 +53,11 @@ typedef unsigned int NSSearchPathDomainMask;
 #endif
 
 #if defined(OS_IOS)
+typedef struct CF_BRIDGED_TYPE(id) __SecCertificate* SecCertificateRef;
 typedef struct CF_BRIDGED_TYPE(id) __SecKey* SecKeyRef;
 typedef struct CF_BRIDGED_TYPE(id) __SecPolicy* SecPolicyRef;
 #else
+typedef struct OpaqueSecCertificateRef* SecCertificateRef;
 typedef struct OpaqueSecKeyRef* SecKeyRef;
 typedef struct OpaqueSecPolicyRef* SecPolicyRef;
 #endif
@@ -146,6 +148,7 @@ TYPE_NAME_FOR_CF_TYPE_DECL(CGColor);
 TYPE_NAME_FOR_CF_TYPE_DECL(CTFont);
 TYPE_NAME_FOR_CF_TYPE_DECL(CTRun);
 
+TYPE_NAME_FOR_CF_TYPE_DECL(SecCertificate);
 TYPE_NAME_FOR_CF_TYPE_DECL(SecKey);
 TYPE_NAME_FOR_CF_TYPE_DECL(SecPolicy);
 
@@ -308,6 +311,7 @@ CF_CAST_DECL(CTFont);
 CF_CAST_DECL(CTFontDescriptor);
 CF_CAST_DECL(CTRun);
 
+CF_CAST_DECL(SecCertificate);
 CF_CAST_DECL(SecKey);
 CF_CAST_DECL(SecPolicy);
 

base/mac/foundation_util.mm

@@ -215,6 +215,7 @@ TYPE_NAME_FOR_CF_TYPE_DEFN(CTFont);
 TYPE_NAME_FOR_CF_TYPE_DEFN(CTRun);
 
 #if !defined(OS_IOS)
+TYPE_NAME_FOR_CF_TYPE_DEFN(SecCertificate);
 TYPE_NAME_FOR_CF_TYPE_DEFN(SecKey);
 TYPE_NAME_FOR_CF_TYPE_DEFN(SecPolicy);
 #endif
@@ -414,6 +415,7 @@ CFCastStrict<CTFontRef>(const CFTypeRef& cf_val) {
 
 #if !defined(OS_IOS)
 CF_CAST_DEFN(SecACL);
+CF_CAST_DEFN(SecCertificate);
 CF_CAST_DEFN(SecKey);
 CF_CAST_DEFN(SecPolicy);
 CF_CAST_DEFN(SecTrustedApplication);

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc

@@ -17,6 +17,7 @@
 #include "base/mac/foundation_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/macros.h"
+#include "base/sha1.h"
 #include "chrome/browser/apps/app_shim/app_shim_host_bootstrap_mac.h"
 #include "chrome/browser/apps/app_shim/app_shim_host_mac.h"
 #include "chrome/browser/apps/app_shim/app_shim_host_manager_mac.h"
@@ -107,23 +108,25 @@ bool FocusHostedAppWindows(const std::set<Browser*>& browsers) {
   return true;
 }
 
+// Create a SHA1 hex digest of a certificate, for use specifically in building
+// a code signing requirement string in IsAcceptablyCodeSigned(), below.
+std::string CertificateSHA1Digest(SecCertificateRef certificate) {
+  base::ScopedCFTypeRef<CFDataRef> certificate_data(
+      SecCertificateCopyData(certificate));
+  char hash[base::kSHA1Length];
+  base::SHA1HashBytes(CFDataGetBytePtr(certificate_data),
+                      CFDataGetLength(certificate_data),
+                      reinterpret_cast<unsigned char*>(hash));
+  return base::HexEncode(hash, base::kSHA1Length);
+}
+
 // Returns whether |pid|'s code signature is trusted:
-// - True if |pid| is validly signed and satisfies the designated requirement
-//   embedded in the calling process's signature.
-// - True if the caller is not signed *and* not an official Chrome build.
-// - False otherwise (e.g. the shim doesn't satisfy the browser's designated
-//   requirement, or the browser is an official Chrome build but unsigned).
+// - True if the caller is unsigned (there's nothing to verify).
+// - True if |pid| satisfies the caller's designated requirement.
+// - False otherwise (|pid| does not satisfy caller's designated requirement).
 bool IsAcceptablyCodeSigned(pid_t pid) {
-  // Only require signatures for official Chrome builds.
-#if !defined(OFFICIAL_BUILD) || !defined(GOOGLE_CHROME_BUILD)
-  return true;
-#endif
-  // TODO(https://crbug.com/624228): Re-enable signature checking when shims
-  // can start.
-  return true;
-
   base::ScopedCFTypeRef<SecCodeRef> own_code;
-  base::ScopedCFTypeRef<SecRequirementRef> own_designated_requirement;
+  base::ScopedCFTypeRef<CFDictionaryRef> own_signing_info;
 
   // Fetch the calling process's designated requirement. The shim can only be
   // validated if the caller has one (i.e. if the caller is code signed).
@@ -133,9 +136,37 @@ bool IsAcceptablyCodeSigned(pid_t pid) {
   // revisited after https://crbug.com/496298 is resolved.
   if (SecCodeCopySelf(kSecCSDefaultFlags, own_code.InitializeInto()) !=
           errSecSuccess ||
-      SecCodeCopyDesignatedRequirement(
-          own_code, kSecCSDefaultFlags,
-          own_designated_requirement.InitializeInto()) != errSecSuccess) {
+      SecCodeCopySigningInformation(own_code.get(), kSecCSSigningInformation,
+                                    own_signing_info.InitializeInto()) !=
+          errSecSuccess) {
+    LOG(ERROR) << "Failed to get own code signing information.";
+    return false;
+  }
+
+  auto* own_certificates = base::mac::GetValueFromDictionary<CFArrayRef>(
+      own_signing_info, kSecCodeInfoCertificates);
+  if (!own_certificates || CFArrayGetCount(own_certificates) < 1) {
+    return true;
+  }
+
+  auto* own_certificate = base::mac::CFCast<SecCertificateRef>(
+      CFArrayGetValueAtIndex(own_certificates, 0));
+  auto own_certificate_hash = CertificateSHA1Digest(own_certificate);
+
+  base::ScopedCFTypeRef<CFStringRef> shim_requirement_string(
+      CFStringCreateWithFormat(
+          kCFAllocatorDefault, nullptr,
+          CFSTR(
+              "identifier \"app_mode_loader\" and certificate leaf = H\"%s\""),
+          own_certificate_hash.c_str()));
+
+  base::ScopedCFTypeRef<SecRequirementRef> shim_requirement;
+  if (SecRequirementCreateWithString(
+          shim_requirement_string, kSecCSDefaultFlags,
+          shim_requirement.InitializeInto()) != errSecSuccess) {
+    LOG(ERROR)
+        << "Failed to create a SecRequirementRef from the requirement string \""
+        << shim_requirement_string << "\"";
     return false;
   }
 
@@ -152,11 +183,12 @@ bool IsAcceptablyCodeSigned(pid_t pid) {
   if (SecCodeCopyGuestWithAttributes(nullptr, guest_attributes,
                                      kSecCSDefaultFlags,
                                      guest_code.InitializeInto())) {
+    LOG(ERROR) << "Failed to create a SecCodeRef from the app shim's pid.";
     return false;
   }
 
   return SecCodeCheckValidity(guest_code, kSecCSDefaultFlags,
-                              own_designated_requirement) == errSecSuccess;
+                              shim_requirement) == errSecSuccess;
 }
 
 // Attempts to launch a packaged app, prompting the user to enable it if
@@ -641,7 +673,10 @@ void ExtensionAppShimHandler::OnExtensionEnabled(
   // If the connecting shim process doesn't have an acceptable code signature,
   // reject the connection and recreate the shim.
   if (!IsAcceptablyCodeSigned(bootstrap->GetAppShimPid())) {
-    LOG(WARNING) << "Attaching app shim process is not signed, regenerating.";
+    // TODO(https://crbug.com/923612): Should only be here for a day or two.
+    LOG(ERROR) << "The attaching app shim's code signature is invalid. This "
+                  "will fail in future builds of Chrome.";
+#if 0
     if (bootstrap->GetLaunchType() == APP_SHIM_LAUNCH_NORMAL) {
       constexpr bool recreate_shims = true;
       delegate_->LaunchShim(profile, extension, recreate_shims,
@@ -649,6 +684,7 @@ void ExtensionAppShimHandler::OnExtensionEnabled(
     }
     bootstrap->OnFailedToConnectToHost(APP_SHIM_LAUNCH_FAILED_VALIDATION);
     return;
+#endif
   }
 
   AppShimHost* host = delegate_->AllowShimToConnect(profile, extension)