Call codesign after artifact substitution on macos by emillon · Pull Request #6137 · ocaml/dune

emillon · 2022-09-08T09:36:22Z

For future reference:

Technical Note TN2206 - macOS Code Signing In Depth
documentation of adhoc
open source implementation of codesign - in particular see this part about ad-hoc mode

Blaisorblade · 2022-09-08T10:17:55Z

Thanks for putting this together — this is encouraging!

I found a relevant failing testcase in main (using promotion), and tested it with this patch; I still get a SIGSEGV running hello.exe, and codesign says that one copy of hello.exe is invalid and one is signed — IIUC, the promoted copy is invalid:

$ codesign -vv _build/.sandbox/7100408e722289ef78bb707a62e7dc76/default/test/blackbox-tests/test-cases/promote/hello.exe
_build/.sandbox/7100408e722289ef78bb707a62e7dc76/default/test/blackbox-tests/test-cases/promote/hello.exe: invalid signature (code or signature have been modified)
In architecture: arm64
$ codesign -vv _build/.sandbox/7100408e722289ef78bb707a62e7dc76/default/test/blackbox-tests/test-cases/promote/_build/default/hello.exe
_build/.sandbox/7100408e722289ef78bb707a62e7dc76/default/test/blackbox-tests/test-cases/promote/_build/default/hello.exe: valid on disk
_build/.sandbox/7100408e722289ef78bb707a62e7dc76/default/test/blackbox-tests/test-cases/promote/_build/default/hello.exe: satisfies its Designated Requirement

Full logs here: https://gist.github.com/Blaisorblade/ca97b3c117798832a19fbcb7f7d51efb.

Two unfortunate notes:

Some other failures on Mac remain and some of those might be unrelated.
I'm traveling so I might not respond promptly.

emillon · 2022-09-08T13:11:26Z

It's weird, it looks like target promotion should already use the copy_file path through ~promote_source...
Note to self: OTOH, bin/install_uninstall.ml does not use this, and is doing non-atomic copies at the moment.

Et7f3 · 2022-09-08T14:37:40Z

Random note: codesign isn't reproductible. So the non signed binary should be preserved in the cache.

emillon · 2022-09-08T14:50:21Z

Random note: codesign isn't reproductible. So the non signed binary should be preserved in the cache.

I understand that the signature is going to be different every time, but the signature is stored in a xattr or something, right? I don't think we're considering that piece of data when computing the hash. So it doesn't matter if the signature is fresh or comes from the cache.

(and if this is an issue, it's an issue for "normal" binaries too, since a freshly linked binary is going to have a different signature from one pulled from the cache)

emillon · 2022-09-09T09:37:24Z

Hmm, I might be mistaken. According to some docs, the signature is part of the mach-o file. It's in a xattr just for non-mach-o binaries. So there might be something to do for the cache in general.

emillon · 2022-09-13T14:24:36Z

I now have access to a M1 machine. I fixed a couple issues and now the main testsuite is passing! There are still issues with the fsevents tests but they're disabled in CI so that's less of an issue.

Blaisorblade · 2022-09-13T21:49:08Z

There are still issues with the fsevents tests but they're disabled in CI so that's less of an issue.

Can you disabile them from dune test or document what to run on M1?

src/dune_rules/artifact_substitution.ml

emillon · 2022-09-14T08:03:03Z

Can you disabile them from dune test or document what to run on M1?

One hacky way of doing that is by running CI=true make test. I'd like to fix this issue too but I have not identified the cause yet.

Blaisorblade · 2022-09-14T08:32:36Z

Thanks! I confirm that this MR + unset DUNE_CACHE + CI=true make test gives me a successful test run!

emillon · 2022-09-14T09:24:49Z

Regarding reproducibility of code signing: my understanding is that ad-hoc signatures are not signatures in the cryptographic sense - they only record a hash of the data that would be signed. So the process of calling codesign -s - on a binary is reproducible.

For future reference:

Technical Note TN2206 - macOS Code Signing In Depth
documentation of adhoc
open source implementation of codesign - in particular see this part about ad-hoc mode

emillon · 2022-09-15T08:04:35Z

The fsevent thing is unrelated, I'll open a new issue (edit: it's #6151).

This refactors `copy_file` in `Install_uninstall` so that it uses `Artifact_substitution.copy_file`. This ensures that these copies are atomic at least for non-special files. It also allows hooks to trigger on install (ocaml#6137). Signed-off-by: Etienne Millon <me@emillon.org>

This refactors `copy_file` in `Install_uninstall` so that it uses `Artifact_substitution.copy_file`. This ensures that these copies are atomic at least for non-special files. It also allows hooks to trigger on install (#6137). Signed-off-by: Etienne Millon <me@emillon.org> Signed-off-by: Etienne Millon <me@emillon.org>

bobot

Thank you a lot for tackling this issue. Just a little needed change (if you agree that it is needed 😸 )

src/dune_rules/artifact_substitution.ml

CHANGES.md

src/dune_rules/artifact_substitution.ml

rgrinberg

LGTM

Closes ocaml#5650 For future reference: - [Technical Note TN2206 - macOS Code Signing In Depth](https://developer.apple.com/library/archive/technotes/tn2206/_index.html) - [documentation of `adhoc`](https://developer.apple.com/documentation/security/seccodesignatureflags/1397793-adhoc) - [open source implementation of codesign](https://github.com/indygreg/PyOxidizer/tree/main/apple-codesign) - in particular see [this part about ad-hoc mode](https://github.com/indygreg/PyOxidizer/blob/5fb59d1c269ad1097713bc358a5b62abf5814663/apple-codesign/src/signing_settings.rs#L300-L305) Signed-off-by: Etienne Millon <me@emillon.org>

correnson · 2022-10-05T08:55:18Z

✅ Tested on my mac M1: works perfectly!

@rgrinberg

…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.5.0~alpha1) CHANGES: - Sandbox running cinaps actions starting from cinaps 1.1 (ocaml/dune#6176, @rgrinberg) - Add a `runtime_deps` field in the `cinaps` stanza to specify runtime dependencies for running the cinaps preprocessing action (ocaml/dune#6175, @rgrinberg) - Shadow alias module `Foo__` when building a library `Foo` (ocaml/dune#6126, @rgrinberg) - Extend dune describe to include the root path of the workspace and the relative path to the build directory. (ocaml/dune#6136, @reubenrowe) - Allow dune describe workspace to accept directories as arguments. The provided directories restrict the worskpace description to those directories. (ocaml/dune#6107, fixes ocaml/dune#3893, @esope) - Add a terminal persistence mode that attempts to clear the terminal history. It is enabled by setting terminal persistence to `clear-on-rebuild-and-flush-history` (ocaml/dune#6065, @rgrinberg) - Disallow generating targets in sub direcories in inferred rules. The check to forbid this was accidentally done only for manually specified targets (ocaml/dune#6031, @rgrinberg) - Do not ignore rules marked `(promote (until-clean))` when `--ignore-promoted-rules` (or `-p`) is passed. (ocaml/dune#6010, fixes ocaml/dune#4401, @emillon) - Dune no longer considers .aux files as targets during Coq compilation. This means that .aux files are no longer cached. (ocaml/dune#6024, fixes ocaml/dune#6004, @Alizter) - Cinaps actions are now sandboxed by default (ocaml/dune#6062, @rgrinberg) - Allow rules producing directory targets to be not sandboxed (ocaml/dune#6056, @rgrinberg) - Introduce a `dirs` field in the `install` stanza to install entire directories (ocaml/dune#5097, fixes ocaml/dune#5059, @rgrinberg) - Menhir rules are now sandboxed by default (ocaml/dune#6076, @rgrinberg) - Allow rules producing directory targets to create symlinks (ocaml/dune#6077, fixes ocaml/dune#5945, @rgrinberg) - Inline tests are now sandboxed by default (ocaml/dune#6079, @rgrinberg) - Fix build-info version when used with flambda (ocaml/dune#6089, fixes ocaml/dune#6075, @jberdine) - Add an `(include <file>)` term to the `include_dirs` field for adding directories to the include paths sourced from a file. (ocaml/dune#6058, fixes ocaml/dune#3993, @gridbugs) - Support `(extra_objects ...)` field in `(executable ...)` and `(library ...)` stanzas (ocaml/dune#6084, fixes ocaml/dune#4129, @gridbugs) - Fix compilation of Dune under esy on Windows (ocaml/dune#6109, fixes ocaml/dune#6098, @nojb) - Improve error message when parsing several licenses in `(license)` (ocaml/dune#6114, fixes ocaml/dune#6103, @emillon) - odoc rules now about `ODOC_SYNTAX` and will rerun accordingly (ocaml/dune#6010, fixes ocaml/dune#1117, @emillon) - dune install: copy files in an atomic way (ocaml/dune#6150, @emillon) - Add `%{coq:...}` macro for accessing data about the configuration about Coq. For instance `%{coq:version}` (ocaml/dune#6049, @Alizter) - update vendored copy of cmdliner to 1.1.1. This improves the built-in documentation for command groups such as `dune ocaml`. (ocaml/dune#6038, @emillon, ocaml/dune#6169, @shonfeder) - The test suite for Coq now requires Coq >= 8.16 due to changes in the plugin loading mechanism upstream (which now uses findlib). - Starting with Coq build language 0.6, theories can be built without importing Coq's standard library by including `(stdlib no)`. (ocaml/dune#6165 ocaml/dune#6164, fixes ocaml/dune#6163, @ejgallego @Alizter @LasseBlaauwbroek) - on macOS, sign executables produced by artifact substitution (ocaml/dune#6137, fixes ocaml/dune#5650, @emillon) - Added an (aliases ...) field to the (rules ...) stanza which allows the specification of multiple aliases per rule (ocaml/dune#6194, @Alizter)

@rgrinberg

…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.5.0) CHANGES: - macOS: Handle unknown fsevents without crashing (ocaml/dune#6217, @rgrinberg) - Enable file watching on MacOS SDK < 10.13. (ocaml/dune#6218, @rgrinberg) - Sandbox running cinaps actions starting from cinaps 1.1 (ocaml/dune#6176, @rgrinberg) - Add a `runtime_deps` field in the `cinaps` stanza to specify runtime dependencies for running the cinaps preprocessing action (ocaml/dune#6175, @rgrinberg) - Shadow alias module `Foo__` when building a library `Foo` (ocaml/dune#6126, @rgrinberg) - Extend dune describe to include the root path of the workspace and the relative path to the build directory. (ocaml/dune#6136, @reubenrowe) - Allow dune describe workspace to accept directories as arguments. The provided directories restrict the worskpace description to those directories. (ocaml/dune#6107, fixes ocaml/dune#3893, @esope) - Add a terminal persistence mode that attempts to clear the terminal history. It is enabled by setting terminal persistence to `clear-on-rebuild-and-flush-history` (ocaml/dune#6065, @rgrinberg) - Disallow generating targets in sub direcories in inferred rules. The check to forbid this was accidentally done only for manually specified targets (ocaml/dune#6031, @rgrinberg) - Do not ignore rules marked `(promote (until-clean))` when `--ignore-promoted-rules` (or `-p`) is passed. (ocaml/dune#6010, fixes ocaml/dune#4401, @emillon) - Dune no longer considers .aux files as targets during Coq compilation. This means that .aux files are no longer cached. (ocaml/dune#6024, fixes ocaml/dune#6004, @Alizter) - Cinaps actions are now sandboxed by default (ocaml/dune#6062, @rgrinberg) - Allow rules producing directory targets to be not sandboxed (ocaml/dune#6056, @rgrinberg) - Introduce a `dirs` field in the `install` stanza to install entire directories (ocaml/dune#5097, fixes ocaml/dune#5059, @rgrinberg) - Menhir rules are now sandboxed by default (ocaml/dune#6076, @rgrinberg) - Allow rules producing directory targets to create symlinks (ocaml/dune#6077, fixes ocaml/dune#5945, @rgrinberg) - Inline tests are now sandboxed by default (ocaml/dune#6079, @rgrinberg) - Fix build-info version when used with flambda (ocaml/dune#6089, fixes ocaml/dune#6075, @jberdine) - Add an `(include <file>)` term to the `include_dirs` field for adding directories to the include paths sourced from a file. (ocaml/dune#6058, fixes ocaml/dune#3993, @gridbugs) - Support `(extra_objects ...)` field in `(executable ...)` and `(library ...)` stanzas (ocaml/dune#6084, fixes ocaml/dune#4129, @gridbugs) - Fix compilation of Dune under esy on Windows (ocaml/dune#6109, fixes ocaml/dune#6098, @nojb) - Improve error message when parsing several licenses in `(license)` (ocaml/dune#6114, fixes ocaml/dune#6103, @emillon) - odoc rules now about `ODOC_SYNTAX` and will rerun accordingly (ocaml/dune#6010, fixes ocaml/dune#1117, @emillon) - dune install: copy files in an atomic way (ocaml/dune#6150, @emillon) - Add `%{coq:...}` macro for accessing data about the configuration about Coq. For instance `%{coq:version}` (ocaml/dune#6049, @Alizter) - update vendored copy of cmdliner to 1.1.1. This improves the built-in documentation for command groups such as `dune ocaml`. (ocaml/dune#6038, @emillon, ocaml/dune#6169, @shonfeder) - The test suite for Coq now requires Coq >= 8.16 due to changes in the plugin loading mechanism upstream (which now uses `Findlib`). - Starting with Coq build language 0.6, theories can be built without importing Coq's standard library by including `(stdlib no)`. (ocaml/dune#6165 ocaml/dune#6164, fixes ocaml/dune#6163, @ejgallego @Alizter @LasseBlaauwbroek) - on macOS, sign executables produced by artifact substitution (ocaml/dune#6137, ocaml/dune#6231, fixes ocaml/dune#5650, fixes ocaml/dune#6226, @emillon) - Added an (aliases ...) field to the (rules ...) stanza which allows the specification of multiple aliases per rule (ocaml/dune#6194, @Alizter) - The `(coq.theory ...)` stanza will now ensure that for each declared `(plugin ...)`, the `META` file for it is built before calling `coqdep`. This enables the use of the new `Findlib`-based loading method in Coq 8.16; however as of Coq 8.16.0, Coq itself has some bugs preventing this to work yet. (ocaml/dune#6167 , workarounds ocaml/dune#5767, @ejgallego) - Allow include statement in install stanza (ocaml/dune#6139, fixes ocaml/dune#256, @gridbugs) - Handle CSI n K code in ANSI escape codes from commands. (ocaml/dune#6214, fixes ocaml/dune#5528, @emillon) - Add a new experimental feature `mode_specific_stubs` that allows the specification of different flags and sources for foreign stubs depending on the build mode (ocaml/dune#5649, @voodoos)

emillon mentioned this pull request Sep 8, 2022

For Apple M1, binary substitution breaks codesign #5650

Closed

emillon force-pushed the macos-codesign branch from e31304d to d948c85 Compare September 13, 2022 14:23

toots reviewed Sep 13, 2022

View reviewed changes

src/dune_rules/artifact_substitution.ml Show resolved Hide resolved

emillon mentioned this pull request Sep 15, 2022

Refactor: use copy_file in install/uninstall #6150

Merged

emillon force-pushed the macos-codesign branch from a356321 to 6f4366c Compare September 15, 2022 09:29

emillon added this to the 3.5.0 milestone Sep 22, 2022

emillon marked this pull request as ready for review September 22, 2022 14:21

emillon force-pushed the macos-codesign branch 2 times, most recently from d356044 to ee9874c Compare September 27, 2022 15:10

bobot requested changes Sep 29, 2022

View reviewed changes

src/dune_rules/artifact_substitution.ml Outdated Show resolved Hide resolved

emillon force-pushed the macos-codesign branch 3 times, most recently from afcf53e to af6f46e Compare September 30, 2022 12:38

emillon requested a review from rgrinberg October 4, 2022 10:05

rgrinberg reviewed Oct 4, 2022

View reviewed changes

CHANGES.md Show resolved Hide resolved

rgrinberg reviewed Oct 4, 2022

View reviewed changes

src/dune_rules/artifact_substitution.ml Outdated Show resolved Hide resolved

rgrinberg approved these changes Oct 4, 2022

View reviewed changes

bobot approved these changes Oct 5, 2022

View reviewed changes

emillon force-pushed the macos-codesign branch from af6f46e to 81a92ab Compare October 5, 2022 08:51

emillon force-pushed the macos-codesign branch from 81a92ab to 5f50aa1 Compare October 5, 2022 08:54

emillon merged commit 6f05173 into ocaml:main Oct 5, 2022

emillon deleted the macos-codesign branch October 5, 2022 08:55

emillon mentioned this pull request Oct 19, 2022

[new release] xdg, stdune, ordering, ocamlc-loc, fiber, dyn, dune, dune-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.5.0) ocaml/opam-repository#22317

Merged

Conversation

emillon commented Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Blaisorblade commented Sep 8, 2022

Uh oh!

emillon commented Sep 8, 2022

Uh oh!

Et7f3 commented Sep 8, 2022

Uh oh!

emillon commented Sep 8, 2022

Uh oh!

emillon commented Sep 9, 2022

Uh oh!

emillon commented Sep 13, 2022

Uh oh!

Blaisorblade commented Sep 13, 2022

Uh oh!

Uh oh!

emillon commented Sep 14, 2022

Uh oh!

Blaisorblade commented Sep 14, 2022

Uh oh!

emillon commented Sep 14, 2022

Uh oh!

emillon commented Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bobot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rgrinberg left a comment

Choose a reason for hiding this comment

Uh oh!

correnson commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

emillon commented Sep 8, 2022 •

edited

Loading

emillon commented Sep 15, 2022 •

edited

Loading

bobot left a comment •

edited

Loading

correnson commented Oct 5, 2022 •

edited

Loading