macOS Signing Scripts: Code sign options should be properties of parts, not config.

The options used to code sign parts are integral to their function, and
so they do not belong as options of the config. Instead, they should be
associated with the CodeSignedProduct part that is being signed.

This also removes the `-o library` option to the Helper application,
which was accidentally added during the transition to the new scripts.
The Helper is not yet ready for library validation, since it currently
loads Flash which is signed by a different Team ID.

Bug: 964673, 958163
Change-Id: Iefc8dfb56decc94f6618d4d27fd4c0daae9a059b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1618274Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661229}

chrome/installer/mac/signing/config.py.in

@@ -96,25 +96,6 @@ class CodeSignConfig(object):
         # build signing.
         return set(('libwidevinecdm.dylib',))
 
-    @property
-    def codesign_options_outer_app(self):
-        """Returns the codesign -o flags for the outer app bundle."""
-        return 'restrict'
-
-    @property
-    def codesign_options_helpers(self):
-        """Returns the codesign -o flags for helper executables within the
-        bundle.
-        """
-        return self.codesign_options_outer_app + ',library'
-
-    @property
-    def codesign_options_installer_tools(self):
-        """Returns the codesign -o flags for the installer tools, which
-        are not shipped to end-users.
-        """
-        return self.codesign_options_helpers + ',kill'
-
     @property
     def codesign_requirements_basic(self):
         """Returns the codesign --requirements string that is combined with

chrome/installer/mac/signing/model.py

@@ -30,8 +30,8 @@ class CodeSignedProduct(object):
             identifier: The unique identifier set when code signing. This is
                 only explicitly passed with the `--identifier` flag if
                 |sign_with_identifier| is True.
-            options: Comma-separated string of option names passed to the
-                `codesign -o` flag.
+            options: Options flags to pass to `codesign --options`, from
+                |CodeSignOptions|.
             requirements: String for additional `--requirements` to pass to the
                 `codesign` command. These are joined with a space to the
                 |config.CodeSignConfig.codesign_requirements_basic| string. See
@@ -53,6 +53,8 @@ class CodeSignedProduct(object):
         """
         self.path = path
         self.identifier = identifier
+        if not CodeSignOptions.valid(options):
+            raise ValueError('Invalid CodeSignOptions: {}'.format(options))
         self.options = options
         self.requirements = requirements
         self.identifier_requirement = identifier_requirement
@@ -88,23 +90,23 @@ class CodeSignedProduct(object):
                 'options={0.options}, path={0.path})'.format(self)
 
 
-class VerifyOptions(object):
-    """Enum for the options that can be specified when validating the results of
-    code signing.
+def make_enum(class_name, options):
+    """Makes a new class type for an enum.
 
-    These options are passed to `codesign --verify` after the
-    |CodeSignedProduct| has been signed.
-    """
-    DEEP = ('--deep',)
-    NO_STRICT = ('--no-strict',)
-    IGNORE_RESOURCES = ('--ignore-resources',)
+    Args:
+        class_name: Name of the new type to make.
+        options: A dictionary of enum options to use. The keys will become
+            attributes on the class, and the values will be wrapped in a tuple
+            so that the options can be joined together.
 
-    def __init__(self):
-        raise TypeError('VerifyOptions cannot be constructed')
+    Returns:
+        A new class for the enum.
+    """
+    attrs = {}
 
     @classmethod
-    def valid(cls, options):
-        """Tests if the specified |options| are valid.
+    def valid(cls, opts_to_check):
+        """Tests if the specified |opts_to_check| are valid.
 
         Args:
             options: Iterable of option strings.
@@ -112,10 +114,41 @@ class VerifyOptions(object):
         Returns:
             True if all the options are valid, False if otherwise.
         """
-        if options is None:
+        if opts_to_check is None:
             return True
-        all_options = cls.DEEP + cls.NO_STRICT + cls.IGNORE_RESOURCES
-        return all([option in all_options for option in options])
+        valid_values = options.values()
+        return all([option in valid_values for option in opts_to_check])
+
+    attrs['valid'] = valid
+
+    for name, value in options.items():
+        assert type(name) is str
+        assert type(value) is str
+        attrs[name] = (value,)
+
+    return type(class_name, (object,), attrs)
+
+
+"""Enum for the options that can be specified when validating the results of
+code signing.
+
+These options are passed to `codesign --verify` after the
+|CodeSignedProduct| has been signed.
+"""
+VerifyOptions = make_enum(
+    'signing.model.VerifyOptions', {
+        'DEEP': '--deep',
+        'NO_STRICT': '--no-strict',
+        'IGNORE_RESOURCES': '--ignore-resources',
+    })
+
+CodeSignOptions = make_enum(
+    'signing.model.CodeSignOptions', {
+        'RESTRICT': 'restrict',
+        'LIBRARY_VALIDATION': 'library',
+        'HARDENED_RUNTIME': 'runtime',
+        'KILL': 'kill',
+    })
 
 
 class Distribution(object):

chrome/installer/mac/signing/signing.py

@@ -10,7 +10,7 @@ import copy
 import os.path
 
 from . import commands
-from .model import CodeSignedProduct, VerifyOptions
+from .model import CodeSignOptions, CodeSignedProduct, VerifyOptions
 
 _PROVISIONPROFILE_EXT = '.provisionprofile'
 _PROVISIONPROFILE_DEST = 'embedded.provisionprofile'
@@ -39,7 +39,7 @@ def get_parts(config):
             CodeSignedProduct(
                 '{.app_product}.app'.format(config),
                 config.base_bundle_id,
-                options=config.codesign_options_outer_app,
+                options=CodeSignOptions.RESTRICT,
                 requirements=config.codesign_requirements_outer_app,
                 identifier_requirement=False,
                 resource_rules=None if config.use_new_mac_bundle_structure else
@@ -58,14 +58,16 @@ def get_parts(config):
                 .format(config),
                 '{}.framework.AlertNotificationService'.format(
                     config.base_bundle_id),
-                options=config.codesign_options_helpers,
+                options=CodeSignOptions.RESTRICT +
+                CodeSignOptions.LIBRARY_VALIDATION,
                 verify_options=VerifyOptions.DEEP),
         'crashpad':
             CodeSignedProduct(
                 '{.framework_dir}/Helpers/chrome_crashpad_handler'.format(
                     config),
                 'chrome_crashpad_handler',
-                options=config.codesign_options_helpers,
+                options=CodeSignOptions.RESTRICT +
+                CodeSignOptions.LIBRARY_VALIDATION,
                 verify_options=VerifyOptions.DEEP),
         'helper-app':
             CodeSignedProduct(
@@ -74,13 +76,14 @@ def get_parts(config):
                  '{0.app_product}.app/Contents/Versions/{0.version}/{0.product} Helper.app'
                 ).format(config),
                 '{}.helper'.format(uncustomized_bundle_id),
-                options=config.codesign_options_helpers,
+                options=CodeSignOptions.RESTRICT,
                 verify_options=VerifyOptions.DEEP),
         'app-mode-app':
             CodeSignedProduct(
                 '{.framework_dir}/Helpers/app_mode_loader'.format(config),
                 'app_mode_loader',
-                options=config.codesign_options_helpers,
+                options=CodeSignOptions.RESTRICT +
+                CodeSignOptions.LIBRARY_VALIDATION,
                 verify_options=VerifyOptions.IGNORE_RESOURCES),
     }
 
@@ -122,12 +125,11 @@ def get_installer_tools(config):
         'xzdec',
     )
     for binary in binaries:
-        options = config.codesign_options_installer_tools if not binary.endswith(
-            '.dylib') else None
+        options = CodeSignOptions.RESTRICT + CodeSignOptions.LIBRARY_VALIDATION + CodeSignOptions.KILL
         tools[binary] = CodeSignedProduct(
             '{.packaging_dir}/{binary}'.format(config, binary=binary),
             binary.replace('.dylib', ''),
-            options=options,
+            options=options if not binary.endswith('dylib') else None,
             verify_options=VerifyOptions.DEEP)
 
     return tools
@@ -151,7 +153,7 @@ def sign_part(paths, config, part):
     if config.keychain:
         command.extend(['--keychain', config.keychain])
     if part.options:
-        command.extend(['--options', part.options])
+        command.extend(['--options', ','.join(part.options)])
     if part.resource_rules:
         command.extend([
             '--resource-rules',

chrome/installer/mac/signing/signing_test.py

@@ -58,6 +58,27 @@ class TestGetParts(unittest.TestCase):
                          all_parts['helper-app'].identifier)
 
 
+    def test_part_options(self):
+        parts = signing.get_parts(test_config.TestConfig())
+        self.assertEqual(
+            set(model.CodeSignOptions.RESTRICT), set(parts['app'].options))
+        self.assertEqual(
+            set(model.CodeSignOptions.RESTRICT),
+            set(parts['helper-app'].options))
+        self.assertEqual(
+            set(model.CodeSignOptions.RESTRICT +
+                model.CodeSignOptions.LIBRARY_VALIDATION),
+            set(parts['crashpad'].options))
+        self.assertEqual(
+            set(model.CodeSignOptions.RESTRICT +
+                model.CodeSignOptions.LIBRARY_VALIDATION),
+            set(parts['notification-xpc'].options))
+        self.assertEqual(
+            set(model.CodeSignOptions.RESTRICT +
+                model.CodeSignOptions.LIBRARY_VALIDATION),
+            set(parts['app-mode-app'].options))
+
+
 @mock.patch('signing.commands.run_command')
 class TestSignPart(unittest.TestCase):
 
@@ -117,7 +138,10 @@ class TestSignPart(unittest.TestCase):
 
     def test_sign_part_with_options(self, run_command):
         part = model.CodeSignedProduct(
-            'Test.app', 'test.signing.app', options='restrict,library')
+            'Test.app',
+            'test.signing.app',
+            options=model.CodeSignOptions.RESTRICT +
+            model.CodeSignOptions.LIBRARY_VALIDATION)
         signing.sign_part(self.paths, self.config, part)
         run_command.assert_called_once_with([
             'codesign', '--sign', '[IDENTITY]', '--requirements',