8270199: Most SA tests are skipped on macosx-aarch64 because all executables are signed #6906

plummercj · 2021-12-21T06:17:25Z

For any SA test that attaches to an OSX process (this would be all SA tests except for those that test core file support), there is a check to make sure that the target jvm process is not a signed binary. If it is, SkippedException is thrown, and the test passes without doing anything. This is all we can do since being signed implied being notarized, and debuggers cannot attach to a notarized binary.

I noticed that on macosx-aarch64, all our SA tests that attach to a process were being skipped because the binary was signed, even for debug builds. It turns out that for macosx-aarch64, the linker always ads what is known as ad-hoc signing. You can find some info on ad-hoc signing here:

https://eclecticlight.co/2020/08/22/apple-silicon-macs-will-require-signed-code/

The tests use the codesign tool to determine if the binary is signed. Normally the check just relies on getting an error code of 1 when not signed, but since all binaries are now signed on macosx-aarch64, we need to modify the check to ignore ad-hoc signing.

Using "codesign --display" on an an ad-hoc signed binary shows some output that is of interest:

bash-3.2$ codesign --display --verbose=4 a.out
CodeDirectory v=20400 size=254 flags=0x20002(adhoc,linker-signed) hashes=5+0 location=embedded

Looking for flags=0x20002(adhoc,linker-signed) will tell us if the binary is adhoc signed, which means we can assume that the SA tests can still be run against it.

Looking for flags=0x10000(runtime) will tell us that the binary is hardened. These are binaries that we cannot use with SA.

If the binary is not signed at all, which should only happen on macosx-x64, codesign will print the message "code object is not signed at all". We can also run SA against these binaries.

Due to fixing the issue, we now have to problem list a few more core file tests on macosx-aarch64 due to 8269982. This issue was not noticed in the past because the tests were not being run. This was due to most of our macosx-aarch64 machines not being capable of producing a core file. Without this fix the test assumes it couldn't produce the core file because the binary was signed. With this fix it, for binaries that are not hardened the test will fail if it does not produce a core file.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8270199: Most SA tests are skipped on macosx-aarch64 because all executables are signed

Reviewers

David Holmes (@dholmes-ora - Reviewer)
Kevin Walls (@kevinjwalls - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/6906/head:pull/6906
$ git checkout pull/6906

Update a local copy of the PR:
$ git checkout pull/6906
$ git pull https://git.openjdk.java.net/jdk pull/6906/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 6906

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/6906.diff

bridgekeeper · 2021-12-21T06:18:31Z

👋 Welcome back cjplummer! 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 · 2021-12-21T06:21:04Z

@plummercj The following labels will be automatically applied to this pull request:

core-libs
hotspot-runtime
serviceability

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

plummercj · 2021-12-21T17:33:35Z

/label remove core-libs
/label remove hotspot-runtime

openjdk · 2021-12-21T17:34:31Z

@plummercj
The core-libs label was successfully removed.

openjdk · 2021-12-21T17:35:44Z

@plummercj
The hotspot-runtime label was successfully removed.

Merge

bridgekeeper · 2022-01-18T22:03:06Z

@plummercj This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

mlbridge · 2022-01-18T23:03:50Z

Webrevs

01: Full - Incremental (3b979921)
00: Full (69f804bb)

dholmes-ora

Hi Chris,

Seems fine. My only query/concern here is how reliable/stable this codesign verbose output is?

Thanks,
David

test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java

openjdk · 2022-01-19T06:51:50Z

@plummercj 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:

8270199: Most SA tests are skipped on macosx-aarch64 because all executables are signed

Reviewed-by: dholmes, kevinw

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 141 new commits pushed to the master branch:

f34f8d4: 8277983: Remove unused fields from sun.net.www.protocol.jar.JarURLConnection
a07e19d: 8278410: Improve argument processing around UseHeavyMonitors
ed0df2f: 8268033: compiler/intrinsics/bmi/verifycode/BzhiTestI2L.java fails with "fatal error: Not compilable at tier 3: CodeBuffer overflow"
c180070: 8280373: Update Xalan serializer / SystemIDResolver to align with JDK-8270492
a24f44d: 8280526: x86_32 Math.sqrt performance regression with -XX:UseSSE={0,1}
e72eefd: 8280531: Remove unused DeferredCloseInputStream
2c64a7f: 8280374: G1: Remove unnecessary prev bitmap mark
a183bfb: 8280377: MethodHandleProxies does not correctly invoke default methods with varags
2eab86b: 8213905: reflection not working for type annotations applied to exception types in the inner class constructor
295c047: 8279242: Reflection newInstance() error message when constructor has no access modifiers could use improvement
... and 131 more: https://git.openjdk.java.net/jdk/compare/46fd683820bb7149c0605a0ba03f59e76de69c16...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.

plummercj · 2022-01-19T18:15:43Z

Hi Chris,

Seems fine. My only query/concern here is how reliable/stable this codesign verbose output is?

Thanks, David

Hard to say, but this seems like the best solution at the moment. We can always adapt/adjust if needed.

dholmes-ora

Okay. If the verbose output changes then we won't match and so will assume not-hardened and so proceed with the test. If then test then fails because the runtime actually was hardened then we will see in the log (hopefully) that the output was not as expected.

Thanks,
David

plummercj · 2022-01-26T05:26:07Z

I could use one more review for this PR. Thanks.

kevinjwalls

Looks good. No problem with dealing with future codesign output changes as they happen, or handling more flags in future, above the cases recognised here.

plummercj · 2022-01-26T18:03:42Z

Kevin and David, thanks for the reviews!

plummercj · 2022-01-26T18:03:55Z

/integrate

openjdk · 2022-01-26T18:06:16Z

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

d2a50a6: 8279636: Update JCov version to 3.0.12
b5de2cc: 8280546: Remove hard-coded 127.0.0.1 loopback address
0c42e43: 8280550: SplittableRandom#nextDouble(double,double) can return result >= bound
a5a11f1: Merge
ef08e2c: 8280592: Small javadoc tweaks to foreign API
b8365aa: 8268978: Document the javadoc software stack
4b2370e: 8279294: NonblockingQueue::try_pop may improperly indicate queue is empty
e1d8f55: 8280402: Add new convenience forms to HtmlTree
f34f8d4: 8277983: Remove unused fields from sun.net.www.protocol.jar.JarURLConnection
a07e19d: 8278410: Improve argument processing around UseHeavyMonitors
... and 139 more: https://git.openjdk.java.net/jdk/compare/46fd683820bb7149c0605a0ba03f59e76de69c16...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-01-26T18:06:34Z

@plummercj Pushed as commit 16e0ad0.

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

Fix OSX test issues with determining if binary is hardened or not.

080ed7f

openjdk bot added serviceability serviceability-dev@openjdk.org hotspot-runtime hotspot-runtime-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Dec 21, 2021

openjdk bot removed the core-libs core-libs-dev@openjdk.org label Dec 21, 2021

openjdk bot removed the hotspot-runtime hotspot-runtime-dev@openjdk.org label Dec 21, 2021

Merge branch 'master' into 8270199-osx_hardened

69f804b

Merge

plummercj marked this pull request as ready for review January 18, 2022 22:58

openjdk bot added the rfr Pull request is ready for review label Jan 18, 2022

dholmes-ora approved these changes Jan 19, 2022

View reviewed changes

test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Jan 19, 2022

Update copyrights

3b97992

dholmes-ora approved these changes Jan 19, 2022

View reviewed changes

kevinjwalls approved these changes Jan 26, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jan 26, 2022

openjdk bot closed this Jan 26, 2022

openjdk bot removed the ready Pull request is ready to be integrated label Jan 26, 2022

openjdk bot removed the rfr Pull request is ready for review label Jan 26, 2022

plummercj deleted the 8270199-osx_hardened branch February 1, 2022 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8270199: Most SA tests are skipped on macosx-aarch64 because all executables are signed #6906

8270199: Most SA tests are skipped on macosx-aarch64 because all executables are signed #6906

plummercj commented Dec 21, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Dec 21, 2021

openjdk bot commented Dec 21, 2021

plummercj commented Dec 21, 2021

openjdk bot commented Dec 21, 2021

openjdk bot commented Dec 21, 2021

bridgekeeper bot commented Jan 18, 2022

mlbridge bot commented Jan 18, 2022 •

edited

Loading

dholmes-ora left a comment

openjdk bot commented Jan 19, 2022 •

edited

Loading

plummercj commented Jan 19, 2022

dholmes-ora left a comment

plummercj commented Jan 26, 2022

kevinjwalls left a comment

plummercj commented Jan 26, 2022

plummercj commented Jan 26, 2022

openjdk bot commented Jan 26, 2022

openjdk bot commented Jan 26, 2022

8270199: Most SA tests are skipped on macosx-aarch64 because all executables are signed #6906

8270199: Most SA tests are skipped on macosx-aarch64 because all executables are signed #6906

Conversation

plummercj commented Dec 21, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Dec 21, 2021

openjdk bot commented Dec 21, 2021

plummercj commented Dec 21, 2021

openjdk bot commented Dec 21, 2021

openjdk bot commented Dec 21, 2021

bridgekeeper bot commented Jan 18, 2022

mlbridge bot commented Jan 18, 2022 • edited Loading

Webrevs

dholmes-ora left a comment

Choose a reason for hiding this comment

openjdk bot commented Jan 19, 2022 • edited Loading

plummercj commented Jan 19, 2022

dholmes-ora left a comment

Choose a reason for hiding this comment

plummercj commented Jan 26, 2022

kevinjwalls left a comment

Choose a reason for hiding this comment

plummercj commented Jan 26, 2022

plummercj commented Jan 26, 2022

openjdk bot commented Jan 26, 2022

openjdk bot commented Jan 26, 2022

plummercj commented Dec 21, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Jan 18, 2022 •

edited

Loading

openjdk bot commented Jan 19, 2022 •

edited

Loading