Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

feature: allow macOS code signing by including payload in str table #1164

Merged
merged 2 commits into from
May 10, 2021

Conversation

jesec
Copy link
Contributor

@jesec jesec commented May 5, 2021

Inspired by dotnet/runtime#46558 .

Test: manual, Apple M1 (with mandatory code signing requirement)

 Θ pkg -t node14-mac-arm64 . 
> pkg@5.1.0
> Warning Cannot resolve 'config'
  /Users/jc/Sources/pkg/lib-es5/index.js
  Dynamic require may fail at run time, because the requested file
  is unknown at compilation time and not included into executable.
  Use a string literal as an argument for 'require', or leave it
  as is and specify the resolved file name in 'scripts' option.
> Warning Cannot find module 'internal/util' from '/Users/jc/Sources/pkg/prelude'  in /Users/jc/Sources/pkg/prelude/bootstrap.js
> Warning Cannot find module 'internal/module' from '/Users/jc/Sources/pkg/prelude'  in /Users/jc/Sources/pkg/prelude/bootstrap.js
> Warning Cannot find module 'internal/modules/cjs/helpers' from '/Users/jc/Sources/pkg/prelude'  in /Users/jc/Sources/pkg/prelude/bootstrap.js

 [~/S/pkg] [G pr/macos-sign 00a3e2e ?] [Ad-hoc sign the fabricator binary temporarily to generate bytecode on macOS]
 Θ lipo -info pkg
Non-fat file: pkg is architecture: arm64

 [~/S/pkg] [G pr/macos-sign 00a3e2e ?] [Ad-hoc sign the fabricator binary temporarily to generate bytecode on macOS]
 Θ ./pkg
> pkg@5.1.0
...

 [~/S/pkg] [G pr/macos-sign 00a3e2e ?] [Ad-hoc sign the fabricator binary temporarily to generate bytecode on macOS]
 Θ codesign --verify ./pkg && echo $?
0

Bug: #66, #128, #1023

Copy link
Contributor

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from a code side, BTW tests are broken.

@jesec
Copy link
Contributor Author

jesec commented May 5, 2021

Looks good from a code side, BTW tests are broken.

I think the tests are flaky, and they only fail with Node 16. I will run them again.

@jesec
Copy link
Contributor Author

jesec commented May 5, 2021

I rerun the tests on the HEAD (ea7d72b), and they fail. Clearly our tests don't work with Node 16.1.

@robertsLando
Copy link
Contributor

Could you take a look at them?

@jesec
Copy link
Contributor Author

jesec commented May 7, 2021

Could you take a look at them?

Apparently nodejs/node@9c06103 is the commit that breaks the tests.

jesec added 2 commits May 7, 2021 23:33
… macOS

The fetched/built base binaries MUST NOT have an existing signature if we
want to sign the final executable. However, we do need to run the base binary
to generate bytecode on macOS, and the binary has to be signed due to the new
mandatory signing requirement.

This change ad-hoc signs the base binary to allow pkg to generate bytecode on
macOS.
@jesec jesec force-pushed the pr/macos-sign branch from 8021cb0 to 1305a4e Compare May 7, 2021 15:33
@jesec jesec requested a review from robertsLando May 7, 2021 15:34
Copy link
Contributor

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Good job :)

@jesec jesec merged commit 0b55f9a into master May 10, 2021
@jesec jesec deleted the pr/macos-sign branch May 10, 2021 07:56
jesec added a commit to vercel/pkg-fetch that referenced this pull request May 10, 2021
Newer versions of Apple Clang automatically ad-hoc sign the compiled
executable, due to the new mandatory code signing requirement [1].

However, for final executable to be signable, base binary MUST NOT
have an existing signature.

This change strips the ad-hoc signature from compiled macOS base binary.

[1]: https://developer.apple.com/documentation/macos-release-notes/macos-big-sur-11_0_1-universal-apps-release-notes

Refs: vercel/pkg#1164
Bug: vercel/pkg#1023
jesec added a commit to vercel/pkg-fetch that referenced this pull request May 10, 2021
Newer versions of Apple Clang automatically ad-hoc sign the compiled
executable, due to the new mandatory code signing requirement [1].

However, for final executable to be signable, base binary MUST NOT
have an existing signature.

This change strips the ad-hoc signature from compiled macOS base binary.

[1]: https://developer.apple.com/documentation/macos-release-notes/macos-big-sur-11_0_1-universal-apps-release-notes

Refs: vercel/pkg#1164
Bug: vercel/pkg#1023
bruce-one added a commit to bruce-one/nexe that referenced this pull request Apr 12, 2023
Using the technique from pkg vercel/pkg#1164.
bruce-one added a commit to bruce-one/nexe that referenced this pull request Apr 12, 2023
Using the technique from pkg vercel/pkg#1164.
bruce-one added a commit to bruce-one/nexe that referenced this pull request Apr 13, 2023
Using the technique from pkg vercel/pkg#1164.
bruce-one added a commit to bruce-one/nexe that referenced this pull request Apr 18, 2023
Using the technique from pkg vercel/pkg#1164.
@tmm1
Copy link

tmm1 commented Jun 20, 2023

@jesec any idea how this could be extended to support fat binaries? #1597

bruce-one added a commit to bruce-one/nexe that referenced this pull request Aug 27, 2023
Using the technique from pkg vercel/pkg#1164.
bruce-one added a commit to bruce-one/nexe that referenced this pull request Nov 18, 2023
Using the technique from pkg vercel/pkg#1164.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants