8293550: Optionally add get-task-allow entitlement to macos binaries #10275

erikj79 · 2022-09-14T20:25:03Z

When signing Macos binaries, it's possible to add various entitlements. We already do this for things that Java and the JDK needs when actually signing the binaries.

There is a special entitlement "com.apple.security.get-task-allow" which is needed to be able to debug an application and to get core dumps. Xcode will automatically set this on debug builds, but not on release builds. We never include this as it's not allowed when notarizing applications.

I was recently made aware of the possibility of adding entitlements without actually signing a binary, using the codesign tool. This makes it possible for us to add the get-task-allow entitlement to builds that are never intended to be notarized. We can also be consistent with adding the standard set of entitlements to all builds, regardless of if proper signing is going to be performed.

Not adding any entitlements to non signed builds is currently not a problem on x64, however, on aarch64, the Xcode linker will unconditionally always perform an "adhoc" signing without any entitlements. This is blocking at least core file generation from those binaries, and probably other kinds of debug operations as well.

In this change, I propose that we by default always add entitlements to all builds, and as long as we aren't explicitly signing with a real signing identity with hardened runtime enabled, we also add the get-task-allow entitlement. The codesign behavior is controlled with the new configure parameter --with-macosx-codesign=[hardened|debug|auto].

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8293550: Optionally add get-task-allow entitlement to macos binaries

Reviewers

Mikael Vidstedt (@vidmik - Reviewer) ⚠️ Review applies to 75e1e5b2
Chris Plummer (@plummercj - Reviewer)
Magnus Ihse Bursie (@magicus - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10275/head:pull/10275
$ git checkout pull/10275

Update a local copy of the PR:
$ git checkout pull/10275
$ git pull https://git.openjdk.org/jdk pull/10275/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10275

View PR using the GUI difftool:
$ git pr show -t 10275

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10275.diff

bridgekeeper · 2022-09-14T20:26:54Z

👋 Welcome back erikj! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-09-14T20:29:29Z

@erikj79 The following label will be automatically applied to this pull request:

build

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-09-14T20:32:39Z

Webrevs

make/autoconf/jdk-options.m4

openjdk · 2022-09-14T21:11:22Z

@erikj79 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8293550: Optionally add get-task-allow entitlement to macos binaries

Reviewed-by: mikael, cjplummer, ihse

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 74 new commits pushed to the master branch:

5feca68: 8293840: RISC-V: Remove cbuf parameter from far_call/far_jump/trampoline_call
39cd163: 8293578: Duplicate ldc generated by javac
7765942: 8290367: Update default value and extend the scope of com.sun.jndi.ldap.object.trustSerialData system property
11e7d53: 8293819: sun/util/logging/PlatformLoggerTest.java failed with "RuntimeException: Retrieved backing PlatformLogger level null is not the expected CONFIG"
141d5f5: 8293767: AWT test TestSinhalaChar.java has old SCCS markings
3beca2d: 8291600: [vectorapi] vector cast op check is not always needed for vector mask cast
9a40b76: 8293842: IPv6-only systems throws UnsupportedOperationException for several socket/TCP options
bb9aa4e: 8293813: ProblemList com/sun/jdi/JdbLastErrorTest.java on windows-x64 in Xcomp mode
4cec141: 8291509: Minor cleanup could be done in sun.security
6beeb84: 8293875: ProblemList sun/management/jmxremote/bootstrap/RmiBootstrapTest.java#id1 on linux-x64
... and 64 more: https://git.openjdk.org/jdk/compare/14eb5ad0dc987ffe3621f4eeeebeb6b5a2cd691b...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

make/data/macosxsigning/default-debug.plist

plummercj

I verified that the changes do fix the SA issue with not getting core files produced on macosx-aarch64. I didn't review the makefile logic since I don't understand the makefiles well enough to do so, but what you have outlined in the documentation makes sense, sans the few comments I had.

doc/building.html

make/autoconf/jdk-options.m4

magicus · 2022-09-15T16:50:29Z

doc/building.md

+specified identity is valid. If hardened isn't possible, then `debug` signing is
+chosen if it works. If nothing works, the codesign step is skipped. Note that on
+`aarch64`, the Xcode linker will always perform a default `adhoc` signing
+without any entitlements, causing attaching and core dumps not to work.


I'm getting conflicting messages, both from this text and the PR description, with what is reported in the comments to this PR (and the reason for this PR at all, I assume).

Here and elsewhere it sounds like Xcode will always make adhoc signing without any entitlements on aarch64. If that is done unconditionally, then this fix cannot possible work on aarch64?

I think what you mean is that if no signing is provided, on x64, Xlink just skips signing, but on aarch64, it will create an adhoc signing without any entitlements. But if singing is provided, on both x64 and aarch64 that will be used instead.

If my understanding is correct, then I think this message needs updating. In fact, since we now add signing with entitlements, maybe we don't need to talk at all about what happens if we happen to build without them, and can just remove the last sentence?

Technically, on aarch64, we overwrite the default linker signing using codesign (using the -f (force) flag). I just wanted to include something that warned of the consequences of not letting the build perform the "debug" mode codesign option.

I tried to clarify further.

magicus

Mostly looks good, but I've left a few remarks that I think needs addressing.

doc/building.html

magicus

This looks good. Thanks for clarifying the documentation!

erikj79 · 2022-09-16T12:30:49Z

Thanks for reviewing!

/integrate

openjdk · 2022-09-16T12:33:44Z

Going to push as commit f42caef.
Since your change was applied there have been 74 commits pushed to the master branch:

5feca68: 8293840: RISC-V: Remove cbuf parameter from far_call/far_jump/trampoline_call
39cd163: 8293578: Duplicate ldc generated by javac
7765942: 8290367: Update default value and extend the scope of com.sun.jndi.ldap.object.trustSerialData system property
11e7d53: 8293819: sun/util/logging/PlatformLoggerTest.java failed with "RuntimeException: Retrieved backing PlatformLogger level null is not the expected CONFIG"
141d5f5: 8293767: AWT test TestSinhalaChar.java has old SCCS markings
3beca2d: 8291600: [vectorapi] vector cast op check is not always needed for vector mask cast
9a40b76: 8293842: IPv6-only systems throws UnsupportedOperationException for several socket/TCP options
bb9aa4e: 8293813: ProblemList com/sun/jdi/JdbLastErrorTest.java on windows-x64 in Xcomp mode
4cec141: 8291509: Minor cleanup could be done in sun.security
6beeb84: 8293875: ProblemList sun/management/jmxremote/bootstrap/RmiBootstrapTest.java#id1 on linux-x64
... and 64 more: https://git.openjdk.org/jdk/compare/14eb5ad0dc987ffe3621f4eeeebeb6b5a2cd691b...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-09-16T12:34:00Z

@erikj79 Pushed as commit f42caef.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JDK-8293550

75e1e5b

openjdk bot changed the title ~~JDK-8293550~~ 8293550: Optionally add get-task-allow entitlement to macos binaries Sep 14, 2022

openjdk bot added the rfr Pull request is ready for review label Sep 14, 2022

openjdk bot added the build build-dev@openjdk.org label Sep 14, 2022

vidmik approved these changes Sep 14, 2022

View reviewed changes

make/autoconf/jdk-options.m4 Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Sep 14, 2022

Comment fix

538d899

plummercj reviewed Sep 15, 2022

View reviewed changes

make/data/macosxsigning/default-debug.plist Show resolved Hide resolved

plummercj reviewed Sep 15, 2022

View reviewed changes

doc/building.html Outdated Show resolved Hide resolved

doc/building.html Outdated Show resolved Hide resolved

Reworked doc text

8e068a3

magicus reviewed Sep 15, 2022

View reviewed changes

make/autoconf/jdk-options.m4 Outdated Show resolved Hide resolved

magicus reviewed Sep 15, 2022

View reviewed changes

make/autoconf/jdk-options.m4 Show resolved Hide resolved

magicus reviewed Sep 15, 2022

View reviewed changes

magicus approved these changes Sep 15, 2022

View reviewed changes

erikj79 added 2 commits September 15, 2022 10:03

Move codesign-testfile to CONFIGURESUPPORT_OUTPUTDIR

b0b4c7b

Updated doc again

b14a6b1

plummercj reviewed Sep 15, 2022

View reviewed changes

doc/building.html Outdated Show resolved Hide resolved

Updated doc

56262db

plummercj approved these changes Sep 15, 2022

View reviewed changes

magicus approved these changes Sep 16, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Sep 16, 2022

openjdk bot closed this Sep 16, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8293550: Optionally add get-task-allow entitlement to macos binaries #10275

8293550: Optionally add get-task-allow entitlement to macos binaries #10275

erikj79 commented Sep 14, 2022 •

edited by openjdk bot

bridgekeeper bot commented Sep 14, 2022

openjdk bot commented Sep 14, 2022

mlbridge bot commented Sep 14, 2022 •

edited

openjdk bot commented Sep 14, 2022 •

edited

plummercj left a comment

magicus Sep 15, 2022

erikj79 Sep 15, 2022

erikj79 Sep 15, 2022

magicus left a comment

magicus left a comment

erikj79 commented Sep 16, 2022

openjdk bot commented Sep 16, 2022

openjdk bot commented Sep 16, 2022

8293550: Optionally add get-task-allow entitlement to macos binaries #10275

8293550: Optionally add get-task-allow entitlement to macos binaries #10275

Conversation

erikj79 commented Sep 14, 2022 • edited by openjdk bot

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Sep 14, 2022

openjdk bot commented Sep 14, 2022

mlbridge bot commented Sep 14, 2022 • edited

Webrevs

openjdk bot commented Sep 14, 2022 • edited

plummercj left a comment

Choose a reason for hiding this comment

magicus Sep 15, 2022

Choose a reason for hiding this comment

erikj79 Sep 15, 2022

Choose a reason for hiding this comment

erikj79 Sep 15, 2022

Choose a reason for hiding this comment

magicus left a comment

Choose a reason for hiding this comment

magicus left a comment

Choose a reason for hiding this comment

erikj79 commented Sep 16, 2022

openjdk bot commented Sep 16, 2022

openjdk bot commented Sep 16, 2022

erikj79 commented Sep 14, 2022 •

edited by openjdk bot

mlbridge bot commented Sep 14, 2022 •

edited

openjdk bot commented Sep 14, 2022 •

edited