-
Notifications
You must be signed in to change notification settings - Fork 30.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: macOS package notarization #31459
Conversation
805b6ce
to
4531cf5
Compare
Thanks for taking this on @rvagg! Let me know if you have any issues with my patch! |
The only thing to be aware of is that going forward this won't work as Apple has only temporarily loosened the notarization requirements. (That warning will soon become an error). One possible approach would be to codesign everything in node_modules with the Foundation codesign cert. You can add |
How sure are you of this? It doesn't raise any issues on the install side. Have they stated that all "issues" are going to be fatal in the future? That will mean that we're taking responsibility, reputationally, for term-size. I suppose we're doing that anyway for everything we package, so maybe there's no difference? It just doesn't feel right to be signing other people's binaries! |
See https://developer.apple.com/news/?id=12232019a. Essentially after February 3 2020, the original requirements will come back into place (Those warnings about Unsigned code will turn to errors)
I'm kind of torn on this, part of me thinks that we should be taking responsibility, reputationally for everything that we ship but I'd much rather work with the developers to put it right at their end. The main issue is that it requires a codesign cert which cost money and therefore can be challenging for small open-source projects. If we took on the role of codesigning all the binaries we would also have to assume a set of entitlements. One possible solution might be to check each binary and only sign what is not already signed (term-size in this case) |
3d5b92f
to
e3c3711
Compare
OK, so I've pushed a commit that does a codesign for I asked sindresorhus about bundling a signed version of the executable but I also think it's reasonable to say no to that request. We just have to be comfortable with codesigning code we aren't really responsible for. |
@rvagg I would leave it as the same entitlements for now. We can always narrow it down later, if you've got a clean report without any warnings then you should be good to go. Just be prepared for more packages to possibly appear later on that also aren't codesigned. |
@rvagg I know sindrehous by reputation, I don't suspect him even a bit of being untrustworthy.... but I didn't find the code to the term-size executable, so we are notarizing an executable that has unknown capabilities. Of course, even if the code was reviewable, that doesn't guarantee the executable came from that code. Maybe that's a good thing about notarization, if its notarized as only being able to run and query term widths (whatever capability that is), but then the exe tries to do anything else, alaram bells will go off? For that purpose, its probably worth the extra effort to reduce its capabilities. I was looking for the code because if its a feature that is important enough npm is going to depend on it, then perhaps it should be a uv/node.js API. I can see why he provides the two executables rather than a C++ addon, its a clever workaround for the nastiness of addons, but I wish there was another way. |
I will look into getting it notarized. I'm also happy to have you build the binary, notarize it, and include the binary you built into I'm also willing to explore other options. The module will work without those binaries, so there is an option for you to just run a script to remove them.
I think it's definitely something that should be supported by Node.js built-in.
If only Node.js had built-in FFI support, we could solve most of these problems in user-land without binaries or messy native addons... |
@nodejs/build would there be any objections to submitting a PR with a codesigned version of this binary to @sindresorhus' term-size repo using the "Node.js Foundation" codesigning certificate? btw @sindresorhus if you wanted to do this yourself you only need to codesign, not notarize, but you'd need to compile with a hardened runtime profile too. |
No objection from me. I thought it might be possible to do what term-size does in node, but it didn't work. It should not be so hard :-(, maybe worth opening an issue about. But that's a bit tangential to getting what we have notarized now.
|
Removed the "ignore warnings" feature, added a note in codesign.sh and pointed back here. This is ready for review I think. It's still pending new build machines that we can switch to and start producing these, plus some agreement from @nodejs/build that we're committing to switching to Xcode 11 and the latest macOS for all of our release builds. I've removed this env var from the pkg build script in iojs+release in ci-release: I believe that macOS release machines are pending in nodejs/build#1695, we have some new boxes being set up at nearForm. We're currently having a bit of trouble getting responses from MacStadium about whether we can get access to newer hardware (and software). Tracking that in nodejs/build#1732 |
If we're codesigning the |
Yes, that's what I'm suggesting. |
Yes, that's a TODO, but it needs some bandwidth to write a signing script, figuring out the right hardening configuration, submit that back to macos-term-size and then a binary to term-size. I'm deferring that right now because I don't have that bandwidth, maybe later in the week. If this is a requirement to merging this PR, know that it'll delay getting notarized builds of Node out because we have to go through a chain of releases of term-size -> boxen -> npm, then an npm PR back to nodejs/node, then a backport of that npm to all active release lines. Which is fine, but expect Catalina users to squawk louder as the Apple turn on strict-mode on February 3rd and they can no longer install .pkgs. Getting release machines sorted out may be more of a bother than we anticipate too. |
btw there is a shortcut to this dependency problem of waiting for npm to land with a signed term-size, we just merge one in to our tree inside npm, just got to get it compiled and signed though. |
https://github.com/nodejs-private/secrets/pull/75 has hopefully enough stuff to get a new release machine set up, @sam-github @AshCripps let me know how you go with the nearForm machines. From memory, doing things from cmdline is a pain in the backside because it requires proper keyboard interactive passwords for some key things. I don't know what the current state of the ansible scripts is, hopefully that has all that's needed. 🤞 |
PR-URL: nodejs#31459 Refs: nodejs#29216 Refs: sindresorhus/macos-terminal-size#3 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
Includes hardened-runtime patch from gdams from nodejs#29216 (comment) PR-URL: nodejs#31459 Refs: nodejs#29216 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
Ref: #2199 Ref: #2202 Ref: nodejs/node#31459
Ref: #2199 Ref: #2202 Ref: nodejs/node#31459
Ref: #2199 Ref: #2202 Ref: nodejs/node#31459
Ref: #2199 Ref: #2202 Ref: nodejs/node#31459
PR-URL: nodejs#31459 Refs: nodejs#29216 Refs: sindresorhus/macos-terminal-size#3 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
Includes hardened-runtime patch from gdams from nodejs#29216 (comment) PR-URL: nodejs#31459 Refs: nodejs#29216 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
PR-URL: nodejs#31459 Refs: nodejs#29216 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
Backport-PR-URL: #32527 PR-URL: #31459 Refs: #29216 Refs: sindresorhus/macos-terminal-size#3 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
Includes hardened-runtime patch from gdams from #29216 (comment) Backport-PR-URL: #32527 PR-URL: #31459 Refs: #29216 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
Backport-PR-URL: #32528 PR-URL: #31459 Refs: #29216 Refs: sindresorhus/macos-terminal-size#3 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
Includes hardened-runtime patch from gdams from #29216 (comment) Backport-PR-URL: #32528 PR-URL: #31459 Refs: #29216 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ash Cripps <ashley.cripps@ibm.com> Signed-off-by: Rod Vagg <rod@vagg.org>
Notable changes: - buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc) [#19691](#19691) - build: macOS package notarization (Rod Vagg) [#31459](#31459) - deps: - update npm to 6.14.3 (Myles Borins) [#32368](#32368) - upgrade openssl sources to 1.1.1e (Hassaan Pasha) [#32328](#32328) - upgrade to libuv 1.34.2 (cjihrig) [#31477](#31477) - n-api: - add napi\_get\_all\_property\_names (himself65) [#30006](#30006) - add APIs for per-instance state management (Gabriel Schulhof) [#28682](#28682) - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen) [#26128](#26128) - tls: - expose keylog event on TLSSocket (Alba Mendez) [#27654](#27654) - support TLS min/max protocol defaults in CLI (Sam Roberts) [#27946](#27946) - url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig) [#26226](#26226) PR-URL: #31984
Notable changes: - buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc) [#19691](#19691) - build: macOS package notarization (Rod Vagg) [#31459](#31459) - deps: - update npm to 6.14.3 (Myles Borins) [#32368](#32368) - upgrade openssl sources to 1.1.1e (Hassaan Pasha) [#32328](#32328) - upgrade to libuv 1.34.2 (cjihrig) [#31477](#31477) - n-api: - add napi\_get\_all\_property\_names (himself65) [#30006](#30006) - add APIs for per-instance state management (Gabriel Schulhof) [#28682](#28682) - define release 6 [#32058](#32058) - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen) [#26128](#26128) - tls: - expose keylog event on TLSSocket (Alba Mendez) [#27654](#27654) - support TLS min/max protocol defaults in CLI (Sam Roberts) [#27946](#27946) - url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig) [#26226](#26226) PR-URL: #31984
macOS package notarization and a change in builder configuration The macOS binaries for this release, and future 10.x releases, are now being compiled on macOS 10.15 (Catalina) with Xcode 11 to support package notarization, a requirement for installing .pkg files on macOS 10.15 and later. Previous builds of Node.js 10.x were compiled on macOS 10.7 (Lion). As binaries are still being compiled to support a minimum of macOS 10.7 (Lion) we do not anticipate this having a negative impact on Node.js 10.x users with older versions of macOS. Notable changes: - buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc) [#19691](#19691) - build: macOS package notarization (Rod Vagg) [#31459](#31459) - deps: - update npm to 6.14.3 (Myles Borins) [#32368](#32368) - upgrade openssl sources to 1.1.1e (Hassaan Pasha) [#32328](#32328) - upgrade to libuv 1.34.2 (cjihrig) [#31477](#31477) - n-api: - add napi\_get\_all\_property\_names (himself65) [#30006](#30006) - add APIs for per-instance state management (Gabriel Schulhof) [#28682](#28682) - define release 6 [#32058](#32058) - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen) [#26128](#26128) - tls: - expose keylog event on TLSSocket (Alba Mendez) [#27654](#27654) - support TLS min/max protocol defaults in CLI (Sam Roberts) [#27946](#27946) - url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig) [#26226](#26226) PR-URL: #31984
macOS package notarization and a change in builder configuration The macOS binaries for this release, and future 10.x releases, are now being compiled on macOS 10.15 (Catalina) with Xcode 11 to support package notarization, a requirement for installing .pkg files on macOS 10.15 and later. Previous builds of Node.js 10.x were compiled on macOS 10.10 (Yosemite) with a minimum deployment target of macOS 10.7 (Lion). As binaries are still being compiled to support a minimum of macOS 10.7 (Lion) we do not anticipate this having a negative impact on Node.js 10.x users with older versions of macOS. Notable changes: - buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc) [#19691](#19691) - build: macOS package notarization (Rod Vagg) [#31459](#31459) - deps: - update npm to 6.14.3 (Myles Borins) [#32368](#32368) - upgrade openssl sources to 1.1.1e (Hassaan Pasha) [#32328](#32328) - upgrade to libuv 1.34.2 (cjihrig) [#31477](#31477) - n-api: - add napi\_get\_all\_property\_names (himself65) [#30006](#30006) - add APIs for per-instance state management (Gabriel Schulhof) [#28682](#28682) - define release 6 [#32058](#32058) - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen) [#26128](#26128) - tls: - expose keylog event on TLSSocket (Alba Mendez) [#27654](#27654) - support TLS min/max protocol defaults in CLI (Sam Roberts) [#27946](#27946) - url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig) [#26226](#26226) PR-URL: #31984
macOS package notarization and a change in builder configuration The macOS binaries for this release, and future 10.x releases, are now being compiled on macOS 10.15 (Catalina) with Xcode 11 to support package notarization, a requirement for installing .pkg files on macOS 10.15 and later. Previous builds of Node.js 10.x were compiled on macOS 10.10 (Yosemite) with a minimum deployment target of macOS 10.7 (Lion). As binaries are still being compiled to support a minimum of macOS 10.7 (Lion) we do not anticipate this having a negative impact on Node.js 10.x users with older versions of macOS. Notable changes: - buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc) [#19691](#19691) - build: macOS package notarization (Rod Vagg) [#31459](#31459) - deps: - update npm to 6.14.3 (Myles Borins) [#32368](#32368) - upgrade openssl sources to 1.1.1e (Hassaan Pasha) [#32328](#32328) - upgrade to libuv 1.34.2 (cjihrig) [#31477](#31477) - n-api: - add napi\_get\_all\_property\_names (himself65) [#30006](#30006) - add APIs for per-instance state management (Gabriel Schulhof) [#28682](#28682) - define release 6 [#32058](#32058) - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen) [#26128](#26128) - tls: - expose keylog event on TLSSocket (Alba Mendez) [#27654](#27654) - support TLS min/max protocol defaults in CLI (Sam Roberts) [#27946](#27946) - url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig) [#26226](#26226) PR-URL: #31984
macOS package notarization and a change in builder configuration The macOS binaries for this release, and future 10.x releases, are now being compiled on macOS 10.15 (Catalina) with Xcode 11 to support package notarization, a requirement for installing .pkg files on macOS 10.15 and later. Previous builds of Node.js 10.x were compiled on macOS 10.10 (Yosemite) with a minimum deployment target of macOS 10.7 (Lion). As binaries are still being compiled to support a minimum of macOS 10.7 (Lion) we do not anticipate this having a negative impact on Node.js 10.x users with older versions of macOS. Notable changes: - buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc) [#19691](#19691) - build: macOS package notarization (Rod Vagg) [#31459](#31459) - deps: - update npm to 6.14.3 (Myles Borins) [#32368](#32368) - upgrade openssl sources to 1.1.1e (Hassaan Pasha) [#32328](#32328) - upgrade to libuv 1.34.2 (cjihrig) [#31477](#31477) - n-api: - add napi\_get\_all\_property\_names (himself65) [#30006](#30006) - add APIs for per-instance state management (Gabriel Schulhof) [#28682](#28682) - define release 6 [#32058](#32058) - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen) [#26128](#26128) - tls: - expose keylog event on TLSSocket (Alba Mendez) [#27654](#27654) - support TLS min/max protocol defaults in CLI (Sam Roberts) [#27946](#27946) - url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig) [#26226](#26226) PR-URL: #31984
macOS package notarization and a change in builder configuration The macOS binaries for this release, and future 10.x releases, are now being compiled on macOS 10.15 (Catalina) with Xcode 11 to support package notarization, a requirement for installing .pkg files on macOS 10.15 and later. Previous builds of Node.js 10.x were compiled on macOS 10.10 (Yosemite) with a minimum deployment target of macOS 10.7 (Lion). As binaries are still being compiled to support a minimum of macOS 10.7 (Lion) we do not anticipate this having a negative impact on Node.js 10.x users with older versions of macOS. Notable changes: - buffer: add {read|write}Big\[U\]Int64{BE|LE} methods (garygsc) [#19691](#19691) - build: macOS package notarization (Rod Vagg) [#31459](#31459) - deps: - update npm to 6.14.3 (Myles Borins) [#32368](#32368) - upgrade openssl sources to 1.1.1e (Hassaan Pasha) [#32328](#32328) - upgrade to libuv 1.34.2 (cjihrig) [#31477](#31477) - n-api: - add napi\_get\_all\_property\_names (himself65) [#30006](#30006) - add APIs for per-instance state management (Gabriel Schulhof) [#28682](#28682) - define release 6 [#32058](#32058) - turn NAPI\_CALL\_INTO\_MODULE into a function (Anna Henningsen) [#26128](#26128) - tls: - expose keylog event on TLSSocket (Alba Mendez) [#27654](#27654) - support TLS min/max protocol defaults in CLI (Sam Roberts) [#27946](#27946) - url: handle quasi-WHATWG URLs in urlToOptions() (cjihrig) [#26226](#26226) PR-URL: #31984
Hey folks in this issue, @sindresorhus has a new release of We previously contributed a signed binary to make everything happy, we may need to do so again. Heads-up: sindresorhus/macos-terminal-size#5 I'll let someone else make the call wrt necessity and urgency but I can help get the signing done if that's needed, and the help is needed. |
Works locally, and I can cleanly install .pkg's created by this on Catalina, but there are some things we need to sort out before it can be deployed:
NOTARIZATION_ID
in the ci-release job to (1) signal that notarization is required after .pkg creation and (2) point to the correct Apple ID.An(giving up on keychain, see jenkins & docs: macOS 10.15 build#2199).AC_PASSWORD
entry in the Keychain on the release machine(s) for the Apple ID identified byNOTARIZATION_ID
(xcrun altool --store-password-in-keychain-item "AC_PASSWORD" -u "<appleid>" -p '<password>'
(I'm documenting this in nodejs/secrets and I think there's another place we have release machine docs in nodejs/build).A patch togon
, the tool I'm using here to handle the messy bits of the notarization process. Unfortunately we have an unsigned executable inside our packages,/usr/local/lib/node_modules/npm/node_modules/term-size/vendor/macos/term-size
, used by a dependency of a dependency of npm's. Apple reports this as an "issue" but still notarizes the package.gon
treats all issues as fatal because this can mean the package isn't installable (see Download notarization log, parse issues, error if any issues mitchellh/gon#6), but in our case, my testing suggests that this isn't a problem at all, I think because it all ends up being command-line applications anyway. I have a proposal @ IgnorePathIssues config option to treat some issues as non-fatal mitchellh/gon#14 (which is used here,"ignorable_path_issues"
) but we'll see how that goes.Includes @gdams' hardened-runtime patch from #29216 (comment)
@nodejs/build @nodejs/platform-macos
Ref: #29216