[codesign + GN + artifacts] embed entitlement files in engine artifacts #34112

XilaiZhang · 2022-06-16T18:28:45Z

embed entitlement files in engine artifacts. This is one of the many parts of the code signing project.

This commit makes changes to BUILD.gn files to include configuration files in engine artifacts.

context: During mac code signing, we would like to know which files in the zip artifact should be signed with entitlements, and this is done by comparing the file paths to an entitlement configuration file. To keep the entitlement configuration file up to date, we would like to host them inside flutter/engine.

design docs: go/code-signing-bot, go/code-signing-standalone

christopherfujino · 2022-06-17T22:13:28Z

build/zip.py

      else:
        zip_file.write(path, archive_name)
+  if args.entitlements or args.without_entitlements:
+    entitlement_txt = open('/tmp/entitlements.txt', 'w+')


is it safe to hardcode this temp path here? what about instead of writing to disk, and then copying to the zip, writing from memory directly to zip with https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.writestr

True, working on it

christopherfujino · 2022-06-17T22:15:37Z

tools/font-subset/BUILD.gn

    },
  ]

+  if (is_mac && target_cpu == "x64") {


is this not built on arm?

umm maybe it is built on arm as well but only code signed on x64? in the old code signing script, for font-subset I only find darwin-x64. https://github.com/christopherfujino/codesign.py/blob/a0c3e4c46b936061f7b350ca33109c27e61ad17e/codesign.py#L108-L112

to ask another way, why are we only running this block if target_cpu == "x64"?

I could be wrong but I think our old code signing script only signed darwin-x64/font-subset.zip and did not sign for other zips. just font-subset in https://github.com/christopherfujino/codesign.py/blob/a0c3e4c46b936061f7b350ca33109c27e61ad17e/codesign.py#L108-L112. It didn't sign something like darwin-arm/font-subset.zip, so I figured that we probably would just sign when( is_mac && target_cpu == "x64"), which would give us a darwin-x64/font-subset.zip

It looks like the answer is because we don't generate arm macos host artifacts yet. However, eventually we will, and if we only generate these files for the x64 case, then this code will be wrong once we start building for macOS arm.

umm I think Christopher mentioned earlier that it might be a good thing if the program fails when we produce new arm macos host artifacts. So that we can detect the new artifact, and adjust the path to entitlements as well, in case they change.

umm I think Christopher mentioned earlier that it might be a good thing if the program fails when we produce new arm macos host artifacts. So that we can detect the new artifact, and adjust the path to entitlements as well, in case they change.

which program to fail?

codesign standalone

Yeah if we impose no limit, and later arm64 is included in entitlements.txt, then the codesign program would assume its existence is legit, and treat it as an existing binary.

If arm64 is not included in entitlements.txt, during the traversal of filesystem, codesign standalone would detect that it exists in file system, but does not exist in the configuration file (entitlements.txt), and would therefore prompt the user to update path for arm64.

I am thinking the latter might be better?

godofredoc

My recommendation is to keep this PR as simple as we can. E.g. changes to zip.py, template and a single artifact. It is always easier and faster to get a review for a small PR.

Have we tested the change locally?

godofredoc

My recommendation is to keep this PR as simple as we can. E.g. changes to zip.py, template and a single artifact. It is always easier and faster to get a review for a small PR.

Have we tested the change locally?

godofredoc · 2022-06-18T01:13:15Z

build/archives/BUILD.gn

      prefix = "$full_platform_name-$flutter_runtime_mode/"
    }
    output = "$prefix/artifacts.zip"
+    encode_entitlements = false


Do we need this? or can we rely on the entitlements and without_entitlements list?

godofredoc · 2022-06-18T01:15:54Z

build/zip.py

        _zip_dir(path, zip_file, archive_name)
      else:
        zip_file.write(path, archive_name)
+  if args.entitlements or args.without_entitlements:


Should this be two separate if clauses? if one list exists and the other does not it will generate empty files.

godofredoc · 2022-06-18T01:17:19Z

build/zip_bundle.gni

 # The files variable is an array of scopes that specify a source file or
 # directory and a destination path in the archive to create.
 #
+# If this zip file is not intended to be codesigned as a Mac artifact,


This is probably unnecessary if we can rely on the entitlements lists to decide whether to generate files or not.

godofredoc · 2022-06-18T01:18:52Z

build/zip_bundle.gni

+# set variable encode_entitlements to false.
+#
+# If encode_entitlements is set to true, this api will embed Mac
+# codesign entitlements information in the zip file at


Maybe more explicitly stating here that it will embed a entitlement.txt file with the list of artifacts to get signed with entitlements ...

godofredoc · 2022-06-18T01:19:48Z

build/zip_bundle.gni

+# 'entitlements' and 'without_entitlements'.
+# The entitlements array specifies the list of filepaths that should be
+# codesigned with entitlements.
+# The without_entitlements array specifies the list of filepaths that


This seems a bit contradictory. Is this still signing but signing without entitlements?

godofredoc · 2022-06-18T01:22:50Z

build/zip_bundle.gni

      ]
      sources += [ input.source ]
    }
+    if (invoker.encode_entitlements) {


Can you please add docs for the entitlements parameters. E.g. are they expecting the names with directories or just file names? e.g. for a file a/b/c.txt does it need to include path info as a/b/c.txt or just c.txt?

zanderso · 2022-06-20T18:18:35Z

build/zip.py

        zip_file.write(path, archive_name)
+  if args.entitlements or args.without_entitlements:
+    entitlements_txt = '\n'.join(args.entitlements)
+    zip_file.writestr('entitlements.txt', entitlements_txt)


This script should only be a simple wrapper around zip. We should not be loading it up with any other features.

For files that need to be written from GN, we can use GN's write_file function. See: https://gn.googlesource.com/gn/+/main/docs/reference.md#func_write_file

XilaiZhang · 2022-06-21T00:21:18Z

Thanks everyone for the fantastic feedbacks! I pushed up #34180 to incorporate all of them

chinmaygarde · 2022-06-23T20:24:16Z

From Triage: This PR is obsoleted by #34180. Closing. Please re-open if I am mistaken.

flutter-dashboard bot added platform-android platform-macos labels Jun 16, 2022

XilaiZhang changed the title ~~add entitlements to zips~~ [codesign + GN + artifacts] embed entitlement files in engine artifacts Jun 16, 2022

XilaiZhang requested review from christopherfujino and godofredoc June 16, 2022 18:35

zanderso self-requested a review June 17, 2022 15:29

XilaiZhang added 4 commits June 17, 2022 11:36

entitlements argument for zip build

9d24885

Merge remote-tracking branch 'upstream/main' into alt-history

238a5ff

format files

f145c5b

Merge remote-tracking branch 'upstream/main' into alt-history

54124cb

XilaiZhang force-pushed the xilaizhang-entitlement-1 branch from 52a043f to 54124cb Compare June 17, 2022 20:55

XilaiZhang added 2 commits June 17, 2022 14:06

add missing variable

ba3c43c

fix clang tidy quotes

ea16992

christopherfujino reviewed Jun 17, 2022

View reviewed changes

write from memory to zip

c627b93

godofredoc reviewed Jun 18, 2022

View reviewed changes

zanderso suggested changes Jun 20, 2022

View reviewed changes

XilaiZhang mentioned this pull request Jun 20, 2022

[GN] wrapper template to embed entitlement information in Mac engine artifacts #34180

Closed

XilaiZhang mentioned this pull request Jun 22, 2022

[gn] use generated_files to encode entitlement information #34216

Merged

chinmaygarde closed this Jun 23, 2022

[codesign + GN + artifacts] embed entitlement files in engine artifacts #34112

[codesign + GN + artifacts] embed entitlement files in engine artifacts #34112

Uh oh!

Conversation

XilaiZhang commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

godofredoc left a comment

Choose a reason for hiding this comment

Uh oh!

godofredoc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XilaiZhang commented Jun 21, 2022

Uh oh!

chinmaygarde commented Jun 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

XilaiZhang commented Jun 16, 2022 •

edited

Loading