macOS Signing Scripts: Move the NotarizationAndStapleLevel into Config

Bug: 1442256
Change-Id: I0a174190e10dc57269791da947d24439e0d6bba6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4517690
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1142069}

chrome/installer/mac/sign_chrome.py

@@ -20,7 +20,8 @@ def create_config(config_args, development):
     specific certificate hashes.
 
     Args:
-        config_args: List of args to expand to the config class's constructor.
+        config_args: Dict of args to expand as kwargs to the config class's
+            constructor.
         development: Boolean indicating whether or not to modify the chosen
             config for development testing.
 
@@ -137,6 +138,7 @@ def main():
         choices=model.NotarizeAndStapleLevel.valid_strings(),
         const='staple',
         default='none',
+        type=model.NotarizeAndStapleLevel.from_string,
         help='Specifies the requested notarization actions to be taken. '
         '`none` causes no notarization tasks to be performed. '
         '`nowait` submits the signed application and packaging to Apple for '
@@ -154,12 +156,6 @@ def main():
 
     args = parser.parse_args()
 
-    notarization = model.NotarizeAndStapleLevel.from_string(args.notarize)
-    if notarization.should_notarize():
-        if not args.notary_user or not args.notary_password:
-            parser.error('The `--notary-user` and `--notary-password` '
-                         'arguments are required if notarizing.')
-
     config = create_config(
         model.pick(args, (
             'identity',
@@ -169,8 +165,14 @@ def main():
             'notary_asc_provider',
             'notary_team_id',
             'notarization_tool',
+            'notarize',
         )), args.development)
 
+    if config.notarize.should_notarize():
+        if not args.notary_user or not args.notary_password:
+            parser.error('The `--notary-user` and `--notary-password` '
+                         'arguments are required if notarizing.')
+
     if config.notarization_tool == model.NotarizationTool.NOTARYTOOL:
         # Let the config override notary_team_id, including a potentially
         # unspecified argument.
@@ -189,7 +191,6 @@ def main():
         paths,
         config,
         disable_packaging=args.disable_packaging,
-        notarization=notarization,
         skip_brands=args.skip_brands,
         channels=args.channels)
 

chrome/installer/mac/signing/config.py

@@ -4,7 +4,7 @@
 
 import os.path
 
-from .model import Distribution, NotarizationTool
+from .model import Distribution, NotarizeAndStapleLevel, NotarizationTool
 
 
 class ConfigError(Exception):
@@ -37,7 +37,8 @@ def __init__(self,
                  notary_asc_provider=None,
                  notary_team_id=None,
                  codesign_requirements_basic='',
-                 notarization_tool=None):
+                 notarization_tool=None,
+                 notarize=NotarizeAndStapleLevel.STAPLE):
         """Creates a CodeSignConfig that will sign the product using the static
         properties on the class, using the code signing identity passed to the
         constructor.
@@ -68,6 +69,7 @@ def __init__(self,
                 otherwise.
             notarization_tool: The tool to use to communicate with the Apple
                 notary service. If None, the config will choose a default.
+            notarize: The |model.NotarizeAndStapleLevel|.
         """
         assert identity is not None
         assert type(identity) is str
@@ -79,6 +81,7 @@ def __init__(self,
         self._codesign_requirements_basic = codesign_requirements_basic
         self._notary_team_id = notary_team_id
         self._notarization_tool = notarization_tool
+        self._notarize = notarize
 
     @staticmethod
     def is_chrome_branded():
@@ -153,6 +156,13 @@ def notarization_tool_path(self):
         """
         return None
 
+    @property
+    def notarize(self):
+        """Returns the |model.NotarizeAndStapleLevel| that controls how, if
+        at all, notarization and stapling of CodeSignedProducts should occur.
+        """
+        return self._notarize
+
     @property
     def app_product(self):
         """Returns the product name that is used for the outer .app bundle.

chrome/installer/mac/signing/model.py

@@ -222,7 +222,10 @@ def valid_strings(cls):
 
     @classmethod
     def from_string(cls, str):
-        return cls[str.upper().replace('-', '_')]
+        try:
+            return cls[str.upper().replace('-', '_')]
+        except KeyError:
+            raise ValueError(f'Invalid NotarizeAndStapleLevel: {str}')
 
 
 class NotarizationTool(enum.Enum):

chrome/installer/mac/signing/pipeline.py

@@ -645,7 +645,6 @@ def include_channel(dist):
 def sign_all(orig_paths,
              config,
              disable_packaging=False,
-             notarization=model.NotarizeAndStapleLevel.STAPLE,
              skip_brands=[],
              channels=[]):
     """For each distribution in |config|, performs customization, signing, and
@@ -659,9 +658,6 @@ def sign_all(orig_paths,
             unpackaged signed app bundle will be copied to |paths.output|. If
             False, the packaging specified in the distribution will be
             performed.
-        notarization: The level of notarization to be performed. If
-            |disable_packaging| is False, the packages (dmg/pkg) will undergo
-            the same notarization.
         skip_brands: A list of brand code strings. If a distribution has a brand
             code in this list, or if a distribution has a brand code and
             |skip_brands| contains *, that distribution will be skipped.
@@ -687,7 +683,7 @@ def sign_all(orig_paths,
 
                 # If not packaging and not notarizing, then simply drop the
                 # signed bundle in the output directory when done signing.
-                if not do_packaging and not notarization.should_notarize():
+                if not do_packaging and not config.notarize.should_notarize():
                     dest_dir = paths.output
                 else:
                     dest_dir = notary_paths.work
@@ -708,7 +704,7 @@ def sign_all(orig_paths,
 
                 # If the build products are to be notarized, ZIP the app bundle
                 # and submit it for notarization.
-                if notarization.should_notarize():
+                if config.notarize.should_notarize():
                     zip_file = os.path.join(
                         notary_paths.work,
                         dist_config.packaging_basename + '.zip')
@@ -722,10 +718,10 @@ def sign_all(orig_paths,
 
         # If needed, wait for app notarization results to come back, and staple
         # if required.
-        if notarization.should_wait():
+        if config.notarize.should_wait():
             for result in notarize.wait_for_results(uuids_to_config.keys(),
                                                     config):
-                if notarization.should_staple():
+                if config.notarize.should_staple():
                     dist_config = uuids_to_config[result]
                     dest_dir = os.path.join(
                         notary_paths.work,
@@ -754,23 +750,23 @@ def sign_all(orig_paths,
                 if dist.package_as_dmg:
                     dmg_path = _package_and_sign_dmg(paths, dist_config)
 
-                    if notarization.should_notarize():
+                    if config.notarize.should_notarize():
                         uuid = notarize.submit(dmg_path, dist_config)
                         uuids_to_package_path[uuid] = dmg_path
 
                 if dist.package_as_pkg:
                     pkg_path = _package_and_sign_pkg(paths, dist_config)
 
-                    if notarization.should_notarize():
+                    if config.notarize.should_notarize():
                         uuid = notarize.submit(pkg_path, dist_config)
                         uuids_to_package_path[uuid] = pkg_path
 
             # If needed, wait for package notarization results to come back, and
             # staple if required.
-            if notarization.should_wait():
+            if config.notarize.should_wait():
                 for result in notarize.wait_for_results(
                         uuids_to_package_path.keys(), config):
-                    if notarization.should_staple():
+                    if config.notarize.should_staple():
                         package_path = uuids_to_package_path[result]
                         notarize.staple(package_path)
 

chrome/installer/mac/signing/pipeline_test.py

@@ -1319,11 +1319,9 @@ def test_sign_notarize_no_wait(self, **kwargs):
         kwargs[
             '_package_and_sign_dmg'].return_value = '/$O/AppProduct-99.0.9999.99.dmg'
 
-        config = test_config.TestConfig()
-        pipeline.sign_all(
-            self.paths,
-            config,
-            notarization=model.NotarizeAndStapleLevel.NOWAIT)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.NOWAIT)
+        pipeline.sign_all(self.paths, config)
 
         self.assertEqual(1, kwargs['_package_installer_tools'].call_count)
 
@@ -1366,11 +1364,9 @@ def test_sign_notarize_wait_no_staple(self, **kwargs):
         kwargs[
             '_package_and_sign_dmg'].return_value = '/$O/AppProduct-99.0.9999.99.dmg'
 
-        config = test_config.TestConfig()
-        pipeline.sign_all(
-            self.paths,
-            config,
-            notarization=model.NotarizeAndStapleLevel.WAIT_NOSTAPLE)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.WAIT_NOSTAPLE)
+        pipeline.sign_all(self.paths, config)
 
         self.assertEqual(1, kwargs['_package_installer_tools'].call_count)
 
@@ -1406,9 +1402,9 @@ def test_sign_no_notarization(self, **kwargs):
         for attr in kwargs:
             manager.attach_mock(kwargs[attr], attr)
 
-        config = test_config.TestConfig()
-        pipeline.sign_all(
-            self.paths, config, notarization=model.NotarizeAndStapleLevel.NONE)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.NONE)
+        pipeline.sign_all(self.paths, config)
 
         self.assertEqual(1, kwargs['_package_installer_tools'].call_count)
 
@@ -1431,12 +1427,9 @@ def test_sign_no_packaging_no_notarization(self, **kwargs):
         for attr in kwargs:
             manager.attach_mock(kwargs[attr], attr)
 
-        config = test_config.TestConfig()
-        pipeline.sign_all(
-            self.paths,
-            config,
-            disable_packaging=True,
-            notarization=model.NotarizeAndStapleLevel.NONE)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.NONE)
+        pipeline.sign_all(self.paths, config, disable_packaging=True)
 
         manager.assert_has_calls([
             # First customize the distribution and sign it.
@@ -1480,9 +1473,8 @@ def distributions(self):
                         package_as_pkg=True),
                 ]
 
-        config = Config()
-        pipeline.sign_all(
-            self.paths, config, notarization=model.NotarizeAndStapleLevel.NONE)
+        config = Config(notarize=model.NotarizeAndStapleLevel.NONE)
+        pipeline.sign_all(self.paths, config)
 
         self.assertEqual(1, kwargs['_package_installer_tools'].call_count)
         self.assertEqual(3, kwargs['_customize_and_sign_chrome'].call_count)
@@ -1534,11 +1526,10 @@ def distributions(self):
                     model.Distribution(),
                 ]
 
-        config = Config()
+        config = Config(notarize=model.NotarizeAndStapleLevel.NONE)
         pipeline.sign_all(
             self.paths,
             config,
-            notarization=model.NotarizeAndStapleLevel.NONE,
             skip_brands=skip_brands,
             channels=include_channels)
 

chrome/installer/mac/signing/signing.py

@@ -73,9 +73,9 @@ def sign_part(paths, config, part):
     path = os.path.join(paths.work, part.path)
     if _linker_signed_arm64_needs_force(path):
         command.append('--force')
-    if config.notary_user:
-        # Assume if the config has notary authentication information that the
-        # products will be notarized, which requires a secure timestamp.
+    if config.notarize.should_notarize():
+        # If the products will be notarized, the signature requires a secure
+        # timestamp.
         command.append('--timestamp')
     if part.sign_with_identifier:
         command.extend(['--identifier', part.identifier])

chrome/installer/mac/signing/signing_test.py

@@ -112,7 +112,6 @@ def test_sign_part_with_requirements(self, run_command,
             '/$W/Test.app'
         ])
 
-
     def test_sign_part_needs_force(self, run_command,
                                    linker_signed_arm64_needs_force):
         linker_signed_arm64_needs_force.return_value = True
@@ -138,7 +137,8 @@ def test_sign_part_with_requirements_needs_force(
 
     def test_sign_part_no_notary(self, run_command,
                                  linker_signed_arm64_needs_force):
-        config = test_config.TestConfig(notary_user=None, notary_password=None)
+        config = test_config.TestConfig(
+            notarize=model.NotarizeAndStapleLevel.NONE)
         part = model.CodeSignedProduct('Test.app', 'test.signing.app')
         signing.sign_part(self.paths, config, part)
         run_command.assert_called_once_with(

chrome/updater/mac/signing/pipeline.py

@@ -113,7 +113,6 @@ def _package_zip(paths, config):
 def sign_all(orig_paths,
              config,
              disable_packaging=False,
-             notarization=model.NotarizeAndStapleLevel.STAPLE,
              skip_brands=[],
              channels=[]):
     """Code signs, packages, and signs the package, placing the result into
@@ -124,9 +123,6 @@ def sign_all(orig_paths,
         orig_paths: A |model.Paths| object.
         config: The |config.CodeSignConfig| object.
         disable_packaging: Ignored.
-        notarization: The level of notarization to be performed. If
-            |disable_packaging| is False, the dmg will undergo the same
-            notarization.
         skip_brands: Ignored.
         channels: Ignored.
     """
@@ -138,7 +134,7 @@ def sign_all(orig_paths,
                                     config.packaging_basename)
             _sign_app(paths, config, dest_dir)
 
-            if notarization.should_notarize():
+            if config.notarize.should_notarize():
                 zip_file = os.path.join(notary_paths.work,
                                         config.packaging_basename + '.zip')
                 commands.run_command([
@@ -149,10 +145,10 @@ def sign_all(orig_paths,
                 uuid = notarize.submit(zip_file, config)
 
         # Wait for the app notarization result to come back and staple.
-        if notarization.should_wait():
+        if config.notarize.should_wait():
             for _ in notarize.wait_for_results([uuid], config):
                 pass  # We are only waiting for a single notarization.
-            if notarization.should_staple():
+            if config.notarize.should_staple():
                 notarize.staple_bundled_parts(
                     # Only staple to the outermost app.
                     parts.get_parts(config)[-1:],
@@ -169,9 +165,9 @@ def sign_all(orig_paths,
         dmg_path = _package_and_sign_dmg(package_paths, config)
 
         # Notarize the package, then staple.
-        if notarization.should_wait():
+        if config.notarize.should_wait():
             for _ in notarize.wait_for_results(
                 [notarize.submit(dmg_path, config)], config):
                 pass  # We are only waiting for a single notarization.
-            if notarization.should_staple():
+            if config.notarize.should_staple():
                 notarize.staple(dmg_path)