Mac package installer: Change PKG generation

Do not rely on `codesign` to sign pkg files; it's the wrong tool
for the job (see https://crbug.com/913074#c12).

In addition, the correct way to build an installer package for
the end-user is to make a component package, and then wrap it
into a product package. This now correctly builds product packages.

BUG=913074

Change-Id: Ifbdba65b4d1896fdf478f276c6134182dae8e0c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834843Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703878}

chrome/installer/mac/sign_chrome.py

@@ -70,7 +70,11 @@ def main():
     parser.add_argument(
         '--keychain', help='The keychain to load the identity from.')
     parser.add_argument(
-        '--identity', required=True, help='The identity to sign with.')
+        '--identity',
+        required=True,
+        help='The identity to sign everything but PKGs with.')
+    parser.add_argument(
+        '--installer-identity', help='The identity to sign PKGs with.')
     parser.add_argument(
         '--notary-user',
         help='The username used to authenticate to the Apple notary service.')
@@ -102,10 +106,15 @@ def main():
         'products and installer tools will be placed here.')
     parser.add_argument(
         '--disable-packaging',
-        dest='disable_packaging',
         action='store_true',
         help='Disable creating any packaging (.dmg/.pkg) specified by the '
         'configuration.')
+    parser.add_argument(
+        '--skip-brand',
+        dest='skip_brands',
+        action='append',
+        default=[],
+        help='Causes any distribution whose brand code matches to be skipped.')
 
     group = parser.add_mutually_exclusive_group(required=False)
     group.add_argument(
@@ -124,9 +133,10 @@ def main():
             parser.error('The --notary-user and --notary-password arguments '
                          'are required with --notarize.')
 
-    config = create_config((args.identity, args.keychain, args.notary_user,
-                            args.notary_password, args.notary_asc_provider),
-                           args.development)
+    config = create_config(
+        (args.identity, args.installer_identity, args.keychain,
+         args.notary_user, args.notary_password, args.notary_asc_provider),
+        args.development)
     paths = model.Paths(args.input, args.output, None)
 
     if not os.path.exists(paths.output):
@@ -136,7 +146,8 @@ def main():
         paths,
         config,
         disable_packaging=args.disable_packaging,
-        do_notarization=args.notarize)
+        do_notarization=args.notarize,
+        skip_brands=args.skip_brands)
 
 
 if __name__ == '__main__':

chrome/installer/mac/signing/commands.py

@@ -75,15 +75,20 @@ class PlistContext(object):
     """
     PlistContext is a context manager that reads a plist on entry, providing
     the contents as a dictionary. If |rewrite| is True, then the same dictionary
-    is re-serialized on exit.
+    is re-serialized on exit. If |create_new| is True, then the file is not read
+    but rather an empty dictionary is created.
     """
 
-    def __init__(self, plist_path, rewrite=False):
+    def __init__(self, plist_path, rewrite=False, create_new=False):
         self._path = plist_path
         self._rewrite = rewrite
+        self._create_new = create_new
 
     def __enter__(self):
-        self._plist = plistlib.readPlist(self._path)
+        if self._create_new:
+            self._plist = {}
+        else:
+            self._plist = plistlib.readPlist(self._path)
         return self._plist
 
     def __exit__(self, exc_type, exc_value, exc_tb):

chrome/installer/mac/signing/config.py

@@ -24,13 +24,14 @@ class CodeSignConfig(object):
     There is a class hierarchy of CodeSignConfig objects, with the
     build_props_config.BuildPropsConfig containing injected variables from the
     build process. Configs for Chromium and Google Chrome subclass that to
-    control signing options further. ANd then derived configurations are
+    control signing options further. And then derived configurations are
     created for internal signing artifacts and when using |model.Distribution|
     objects.
     """
 
     def __init__(self,
                  identity,
+                 installer_identity=None,
                  keychain=None,
                  notary_user=None,
                  notary_password=None,
@@ -40,9 +41,16 @@ class CodeSignConfig(object):
         constructor, which is found in the specified keychain.
 
         Args:
-            identity: The name of the code signing identity to use. This can
-                be any value that `codesign -s <identity>` accepts, like the
-                hex-encoded SHA1 hash of the certificate. Must not be None.
+            identity: The name of the code signing identity to use for non-PKG
+                files. This can be any value that `codesign -s <identity>`
+                accepts, like the hex-encoded SHA1 hash of the certificate. Must
+                not be None.
+            installer_identity: The name of the code signing identity to use for
+                PKG files. This will be passed as the parameter for the call to
+                `productbuild --sign <identity>`. Note that a hex-encoded SHA1
+                hash is not a valid option, as it is for |identity| above. The
+                common name of the cert will work. If there is any distribution
+                that is packaged in a PKG this must not be None.
             keychain: Optional path to the keychain file, in which the signing
                 |identity| will be searched for.
             notary_user: Optional string username that will be used to
@@ -56,6 +64,7 @@ class CodeSignConfig(object):
         """
         assert identity
         self._identity = identity
+        self._installer_identity = installer_identity
         self._keychain = keychain
         self._notary_user = notary_user
         self._notary_password = notary_password
@@ -64,10 +73,17 @@ class CodeSignConfig(object):
     @property
     def identity(self):
         """Returns the code signing identity that will be used to sign the
-        products.
+        products, everything but PKG files.
         """
         return self._identity
 
+    @property
+    def installer_identity(self):
+        """Returns the code signing identity that will be used to sign the
+        PKG file products.
+        """
+        return self._installer_identity
+
     @property
     def keychain(self):
         """Returns the filename of the keychain in which |identity| will be

chrome/installer/mac/signing/model.py

@@ -265,7 +265,8 @@ class Distribution(object):
                              self).packaging_basename
 
         return DistributionCodeSignConfig(
-            base_config.identity, base_config.keychain, base_config.notary_user,
+            base_config.identity, base_config.installer_identity,
+            base_config.keychain, base_config.notary_user,
             base_config.notary_password, base_config.notary_asc_provider)
 
 

chrome/installer/mac/signing/pipeline.py

@@ -118,62 +118,133 @@ def _staple_chrome(paths, dist_config):
         notarize.staple(os.path.join(paths.work, part_path))
 
 
-def _package_and_sign_item(packager, paths, dist_config):
-    """Creates, signs, and verifies a packaged item (PKG/DMG).
+def _productbuild_requirements_path(paths, dist_config):
+    """Creates a requirements file for use by `productbuild`. This specifies
+    that an x64 machine is required, and copies the OS requirement from the copy
+    of Chrome being packaged.
 
     Args:
-        packager: A function that turns the .app into a packaged item and
-            returns the path to the packaged item. The function will be called
-            with (model.Paths, model.Distribution, config.CodeSignConfig).
         paths: A |model.Paths| object.
         dist_config: The |config.CodeSignConfig| object.
 
     Returns:
-        The path to the signed packaged file.
+        The path to the requirements file.
     """
-    dist = dist_config.distribution
-
-    package_path = packager(paths, dist, dist_config)
-
-    # item_identifier is the PKG/DMG name but without the file extension. If a
-    # brand code is in use, use the actual brand code instead of the name
-    # fragment, to avoid leaking the association between brand codes and their
-    # meanings.
-    item_identifier = dist_config.packaging_basename
-    if dist.branding_code:
-        item_identifier = dist_config.packaging_basename.replace(
-            dist.packaging_name_fragment, dist.branding_code)
+    requirements_path = os.path.join(paths.work,
+                                     '{}.req'.format(dist_config.app_product))
 
-    product = model.CodeSignedProduct(
-        package_path, item_identifier, sign_with_identifier=True)
-    signing.sign_part(paths, dist_config, product)
-    signing.verify_part(paths, product)
+    app_plist_path = os.path.join(paths.work, dist_config.app_dir, 'Contents',
+                                  'Info.plist')
+    with commands.PlistContext(app_plist_path) as app_plist:
+        with commands.PlistContext(
+                requirements_path, rewrite=True,
+                create_new=True) as requirements:
+            requirements['os'] = [app_plist['LSMinimumSystemVersion']]
+            requirements['arch'] = ['x86_64']
 
-    return package_path
+    return requirements_path
 
 
-def _package_pkg(paths, dist, config):
-    """Packages a Chrome application bundle into a PKG.
+def _package_and_sign_pkg(paths, dist_config):
+    """Packages, signs, and verifies a PKG for a signed build product.
 
     Args:
         paths: A |model.Paths| object.
-        dist: The |model.Distribution| for which the product was customized.
-        config: The |config.CodeSignConfig| object.
+        dist_config: The |config.CodeSignConfig| object.
 
     Returns:
-        A path to the produced PKG file.
+        The path to the signed PKG file.
     """
-    pkg_path = os.path.join(paths.output,
-                            '{}.pkg'.format(config.packaging_basename))
-    app_path = os.path.join(paths.work, config.app_dir)
+    assert dist_config.installer_identity
 
+    # There are two .pkg files to be built:
+    #   1. The inner component package (which is the one that can contain things
+    #      like postinstall scripts). This is built with `pkgbuild`.
+    #   2. The outer product archive (which is the installable thing that has
+    #      pre-install requirements). This is built with `productbuild`.
+
+    # The component package.
+
+    component_pkg_path = os.path.join(paths.work,
+                                      '{}.pkg'.format(dist_config.app_product))
+    app_path = os.path.join(paths.work, dist_config.app_dir)
+
+    commands.run_command([
+        'pkgbuild', '--identifier', dist_config.base_bundle_id, '--version',
+        dist_config.version, '--component', app_path, '--install-location',
+        '/Applications', component_pkg_path
+    ])
+
+    # The product archive.
+
+    # There are two steps here. The first is to create the "distribution file"
+    # which describes the product archive. `productbuild` has a mode to generate
+    # such a file, with the optional input of a "requirements file" that
+    # describes the desired installation requirements of the product archive.
+    # Use this mode to generate a distribution file. Note that if, in the
+    # future, it's desired that the product archive have UI customization, then
+    # this distribution file will need to be hand-crafted. With any luck, this
+    # auto-generated distribution file continue to suffice.
+
+    requirements_path = _productbuild_requirements_path(paths, dist_config)
+
+    distribution_path = os.path.join(paths.work,
+                                     '{}.dist'.format(dist_config.app_product))
     commands.run_command([
-        'pkgbuild', '--identifier', config.base_bundle_id, '--version',
-        config.version, '--component', app_path, '--install-location',
-        '/Applications', pkg_path
+        'productbuild', '--synthesize', '--product', requirements_path,
+        '--package', component_pkg_path, distribution_path
     ])
 
-    return pkg_path
+    # The second step is to actually create the product archive using
+    # `productbuild`.
+
+    product_pkg_path = os.path.join(
+        paths.output, '{}.pkg'.format(dist_config.packaging_basename))
+
+    command = [
+        'productbuild', '--distribution', distribution_path, '--package-path',
+        paths.work, '--sign', dist_config.installer_identity
+    ]
+    if dist_config.notary_user:
+        # Assume if the config has notary authentication information that the
+        # products will be notarized, which requires a secure timestamp.
+        command.append('--timestamp')
+    if dist_config.keychain:
+        command.extend(['--keychain', dist_config.keychain])
+    command.append(product_pkg_path)
+    commands.run_command(command)
+
+    return product_pkg_path
+
+
+def _package_and_sign_dmg(paths, dist_config):
+    """Packages, signs, and verifies a DMG for a signed build product.
+
+    Args:
+        paths: A |model.Paths| object.
+        dist_config: The |config.CodeSignConfig| object.
+
+    Returns:
+        The path to the signed DMG file.
+    """
+    dist = dist_config.distribution
+
+    dmg_path = _package_dmg(paths, dist, dist_config)
+
+    # dmg_identifier is like dmg_name but without the file extension. If a brand
+    # code is in use, use the actual brand code instead of the name fragment, to
+    # avoid leaking the association between brand codes and their meanings.
+    dmg_identifier = dist_config.packaging_basename
+    if dist.branding_code:
+        dmg_identifier = dist_config.packaging_basename.replace(
+            dist.packaging_name_fragment, dist.branding_code)
+
+    product = model.CodeSignedProduct(
+        dmg_path, dmg_identifier, sign_with_identifier=True)
+    signing.sign_part(paths, dist_config, product)
+    signing.verify_part(paths, product)
+
+    return dmg_path
 
 
 def _package_dmg(paths, dist, config):
@@ -290,7 +361,11 @@ def _intermediate_work_dir_name(dist_config):
     return dist_config.packaging_basename
 
 
-def sign_all(orig_paths, config, disable_packaging=False, do_notarization=True):
+def sign_all(orig_paths,
+             config,
+             disable_packaging=False,
+             do_notarization=True,
+             skip_brands=[]):
     """For each distribution in |config|, performs customization, signing, and
     DMG packaging and places the resulting signed DMG in |orig_paths.output|.
     The |paths.input| must contain the products to customize and sign.
@@ -306,6 +381,8 @@ def sign_all(orig_paths, config, disable_packaging=False, do_notarization=True):
             be stapled. If |package_dmg| is also True, the stapled application
             will be packaged in the DMG and then the DMG itself will be
             notarized and stapled.
+        skip_brands: A list of brand code strings. If a distribution has a brand
+            code in this list, that distribution will be skipped.
     """
     with commands.WorkDirectory(orig_paths) as notary_paths:
         # First, sign all the distributions and optionally submit the
@@ -313,6 +390,9 @@ def sign_all(orig_paths, config, disable_packaging=False, do_notarization=True):
         uuids_to_config = {}
         signed_frameworks = {}
         for dist in config.distributions:
+            if dist.branding_code in skip_brands:
+                continue
+
             with commands.WorkDirectory(orig_paths) as paths:
                 dist_config = dist.to_config(config)
                 do_packaging = (dist.package_as_dmg or
@@ -357,22 +437,23 @@ def sign_all(orig_paths, config, disable_packaging=False, do_notarization=True):
         if not disable_packaging:
             uuids_to_package_path = {}
             for dist in config.distributions:
+                if dist.branding_code in skip_brands:
+                    continue
+
                 dist_config = dist.to_config(config)
                 paths = orig_paths.replace_work(
                     os.path.join(notary_paths.work,
                                  _intermediate_work_dir_name(dist_config)))
 
                 if dist.package_as_dmg:
-                    dmg_path = _package_and_sign_item(_package_dmg, paths,
-                                                      dist_config)
+                    dmg_path = _package_and_sign_dmg(paths, dist_config)
 
                     if do_notarization:
                         uuid = notarize.submit(dmg_path, dist_config)
                         uuids_to_package_path[uuid] = dmg_path
 
                 if dist.package_as_pkg:
-                    pkg_path = _package_and_sign_item(_package_pkg, paths,
-                                                      dist_config)
+                    pkg_path = _package_and_sign_pkg(paths, dist_config)
 
                     if do_notarization:
                         uuid = notarize.submit(pkg_path, dist_config)

chrome/installer/mac/signing/signing_test.py

@@ -120,7 +120,7 @@ class TestSignPart(unittest.TestCase):
         ])
 
     def test_sign_part_no_keychain(self, run_command):
-        config = test_config.TestConfig('[IDENTITY]', None)
+        config = test_config.TestConfig(identity='[IDENTITY]', keychain=None)
         part = model.CodeSignedProduct('Test.app', 'test.signing.app')
         signing.sign_part(self.paths, config, part)
         run_command.assert_called_once_with([

chrome/installer/mac/signing/test_config.py

@@ -9,12 +9,14 @@ class TestConfig(config.CodeSignConfig):
 
     def __init__(self,
                  identity='[IDENTITY]',
+                 installer_identity='[INSTALLER-IDENTITY]',
                  keychain='[KEYCHAIN]',
                  notary_user='[NOTARY-USER]',
                  notary_password='[NOTARY-PASSWORD]',
                  notary_asc_provider=None):
-        super(TestConfig, self).__init__(identity, keychain, notary_user,
-                                         notary_password, notary_asc_provider)
+        super(TestConfig,
+              self).__init__(identity, installer_identity, keychain,
+                             notary_user, notary_password, notary_asc_provider)
 
     @property
     def app_product(self):