8332110: [macos] jpackage tries to sign added files without the --mac-sign option #19377

sashamatveev · 2024-05-24T01:08:03Z

This issue is reproducible with and without --mac-sign. jpackage will "ad-hoc" sign application bundle when --mac-sign is not specified by using pseudo-identity "-". This is why jpackage tries to sign added files and this is expected behavior by jpackage. "codesign" fails since added content made application bundle structure invalid. There is nothing we can do on jpackage side to sign such invalid bundles. As proposed solution we will output possible reason for "codesign" failure if it fails and --app-content was specified and possible solution. Proposed message: "One of the possible reason for "codesign" failure is additional content provided via "--app-content", which made application bundle structure invalid. Make sure to provide additional content in a way it will not break application bundle structure, otherwise add additional content as post-processing step."

Example:
Lets assume we have "ReadMe" folder with "ReadMe.txt" file in it.

jpackage --type app-image -n Test --app-content ReadMe/ReadMe.txt ...
"codesign" will fail with "In subcomponent: Test.app/Contents/ReadMe.txt". This is expected and "ReadMe.txt" placed in "Test.app/Contents" which is also expected.
jpackage --type app-image -n Test --app-content ReadMe ...
Works and "ReadMe.txt" will be placed under "Test.app/Contents/ReadMe".

Sample output before fix:

Error: "codesign" failed with following output:
Test.app: replacing existing signature
Test.app: code object is not signed at all
In subcomponent: Test.app/Contents/ReadMe.txt

Sample output after fix:

"codesign" failed and additional application content was supplied via the "--app-content" parameter. Probably the additional content broke the integrity of the application bundle and caused the failure. Ensure content supplied via the "--app-content" parameter does not break the integrity of the application bundle, or add it in the post-processing step.
Error: "codesign" failed with following output:
Test.app: replacing existing signature
Test.app: code object is not signed at all
In subcomponent: Test.app/Contents/ReadMe.txt

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-8332110: [macos] jpackage tries to sign added files without the --mac-sign option (Bug - P3)

Reviewers

Alexey Semenyuk (@alexeysemenyukoracle - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19377

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

Using diff file

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

Webrev

Link to Webrev Comment

…tion

bridgekeeper · 2024-05-24T01:08:44Z

👋 Welcome back almatvee! 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 · 2024-05-24T01:09:40Z

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

8332110: [macos] jpackage tries to sign added files without the --mac-sign option

Reviewed-by: asemenyuk

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

e304a8a: 8333307: Don't suppress jpackage logging in tests when it is detecting packaging tools in the system
3634a91: 8332751: Broken link in VirtualMachine.html
ffb0867: 8331485: Odd Results when Parsing Scientific Notation with Large Exponent
79f4998: 8321314: Reinstate disabling the compiler's default active annotation processing
ec88c6a: 8332917: failure_handler should execute gdb "info threads" command on linux
b3e29db: 8333108: Update vmTestbase/nsk/share/DebugeeProcess.java to don't use finalization
11e926c: 8332777: Update JCStress test suite
44c1845: 8330852: All callers of JvmtiEnvBase::get_threadOop_and_JavaThread should pass current thread explicitly
922e312: 8328611: Thread safety issue in com.sun.tools.jdi.ReferenceTypeImpl::classObject
1d889e5: 8332487: Regression in Crypto-AESGCMBench.encrypt (and others) after JDK-8328181
... and 114 more: https://git.openjdk.org/jdk/compare/3d4185a9ce482cc655a4c67f39cb2682b02ae4fe...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.

openjdk · 2024-05-24T01:10:10Z

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

core-libs

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 · 2024-05-24T01:13:24Z

Webrevs

01: Full - Incremental (7c1973ad)
00: Full (0ad02cbb)

mlbridge · 2024-05-25T01:12:55Z

Mailing list message from Michael Hall on core-libs-dev:

On May 24, 2024, at 3:08?AM, Michael Hall <mik3hall at gmail.com> wrote:

On May 23, 2024, at 8:13?PM, Alexander Matveev <almatvee at openjdk.org <mailto:almatvee at openjdk.org>> wrote:

otherwise add additional content as post-processing step.

Doesn?t this still leave you with an application that isn?t validly signed? And probably won?t run because of that.

2) jpackage --type app-image -n Test --app-content ReadMe ...

For your example. This almost seems like an Apple bug if you can add a directory to the Contents directory but not a file?

Sorry I made my prior off-list.

Would it also generally be a good idea to include a final codesign verify to fail the build if something is wrong with the signature?

Something like?

echo '*******************'
echo 'verifying signature'
echo '*******************'
codesign -v --verbose=4 outputdir/HalfPipe.app

Expected output?

*******************
verifying signature
*******************
outputdir/HalfPipe.app: valid on disk
outputdir/HalfPipe.app: satisfies its Designated Requirement

I think I have suggested this before but don?t remember if I did an enhancement request. Maybe you do that and I?m just not aware of it if it doesn?t appear in the jpackage output.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20240524/ada5e0a4/attachment-0001.htm>

mlbridge · 2024-05-25T01:12:56Z

Mailing list message from Alexander Matveev on core-libs-dev:

Hi Michael,

Doesn?t this still leave you with an application that isn?t validly signed? And probably won?t run because of that.

Yes, it will leave you with an application that isn?t signed. I was able to run such application on same machine as it was generated by jpackage.

For your example. This almost seems like an Apple bug if you can add a directory to the Contents directory but not a file?

Not sure if it is an Apple bug.

Would it also generally be a good idea to include a final codesign verify to fail the build if something is wrong with the signature?

Yes, you already suggested it. See https://bugs.openjdk.org/browse/JDK-8318063 and it was closed as won?t fix because such verification is redundant.

Thanks,
Alexander

From: Michael Hall <mik3hall at gmail.com>
Date: Friday, May 24, 2024 at 1:47?AM
To: Alexander Matveev <almatvee at openjdk.org>
Cc: core-libs-dev <core-libs-dev at openjdk.org>
Subject: Re: RFR: 8332110: [macos] jpackage tries to sign added files without the --mac-sign option

On May 24, 2024, at 3:08?AM, Michael Hall <mik3hall at gmail.com> wrote:

On May 23, 2024, at 8:13?PM, Alexander Matveev <almatvee at openjdk.org<mailto:almatvee at openjdk.org>> wrote:

otherwise add additional content as post-processing step.

Doesn?t this still leave you with an application that isn?t validly signed? And probably won?t run because of that.

2) jpackage --type app-image -n Test --app-content ReadMe ...

For your example. This almost seems like an Apple bug if you can add a directory to the Contents directory but not a file?

Sorry I made my prior off-list.

Would it also generally be a good idea to include a final codesign verify to fail the build if something is wrong with the signature?

Something like?

echo '*******************'
echo 'verifying signature'
echo '*******************'
codesign -v --verbose=4 outputdir/HalfPipe.app

Expected output?

*******************
verifying signature
*******************
outputdir/HalfPipe.app: valid on disk
outputdir/HalfPipe.app: satisfies its Designated Requirement

I think I have suggested this before but don?t remember if I did an enhancement request. Maybe you do that and I?m just not aware of it if it doesn?t appear in the jpackage output.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20240524/a8387c0b/attachment-0001.htm>

sashamatveev · 2024-05-28T21:35:11Z

@alexeysemenyukoracle please review

alexeysemenyukoracle · 2024-05-30T14:40:09Z

How about this wording for the message:

"codesign" failed and additional application content was supplied via the "--app-content" parameter. Probably the additional content broke the integrity of the application bundle and caused the failure. Ensure content supplied via the "--app-content" parameter does not break the integrity of the application bundle, or add it in the post-processing step.

…tion [v2]

sashamatveev · 2024-05-30T22:54:26Z

8332110: jpackage tries to sign added files without the --mac-sign option [v2]

Updated error message as suggested.

alexeysemenyukoracle · 2024-05-30T14:21:51Z

test/jdk/tools/jpackage/macosx/SigningOptionsTest.java

+            {"Hello",
+                    new String[]{"--app-content", TEST_DUKE},
+                    null,
+                    "\"codesign\" failure is additional content provided via \"--app-content\""},


Why is this not a One of the possible reason for "{0}" failure is additional content provided via "--app-content"?

It was part of old message. Latest test version has latest message: ""codesign" failed and additional application content was supplied via the "--app-content" parameter."

sashamatveev · 2024-05-31T20:29:11Z

/integrate

openjdk · 2024-05-31T20:30:34Z

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

8aeada1: 8331159: VM build without C2 fails after JDK-8180450
e99f6a6: 8333236: Test java/foreign/TestAccessModes.java is timing out after passing
e650bdf: 8332507: compilation result depends on compilation order
e4fbb15: 8320215: HeapDumper can use DumpWriter buffer during merge
681137c: 8333131: Source launcher should work with service loader SPI
914423e: 8332899: RISC-V: add comment and make the code more readable (if possible) in MacroAssembler::movptr
5abc029: 8331877: JFR: Remove JIInliner framework
d9e7b7e: 8210471: GZIPInputStream constructor could leak an un-end()ed Inflater
1e04ee6: 8331579: Reference to primitive type fails without error or warning
32ee252: 8333169: javac NullPointerException record.type
... and 137 more: https://git.openjdk.org/jdk/compare/3d4185a9ce482cc655a4c67f39cb2682b02ae4fe...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-05-31T20:30:39Z

@sashamatveev Pushed as commit 9fd0e73.

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

nlisker · 2024-05-31T20:43:45Z

For your example. This almost seems like an Apple bug if you can add a directory to the Contents directory but not a file?

Not sure if it is an Apple bug.

Can this be reported to Apple somehow?

sashamatveev · 2024-05-31T21:10:00Z

For your example. This almost seems like an Apple bug if you can add a directory to the Contents directory but not a file?

Not sure if it is an Apple bug.

Can this be reported to Apple somehow?

I do not think that it is an Apple bug. If you look at an Apple documentation [1] about "Placing content in a bundle" you will see highlighted important message that you should not put content in the wrong location.

[1] https://developer.apple.com/documentation/bundleresources/placing_content_in_a_bundle?language=objc

nlisker · 2024-05-31T21:36:56Z

I see, but it doesn't say where to put license files, which are usually in the root. Do you know where these belong?

sashamatveev · 2024-05-31T21:51:27Z

I see, but it doesn't say where to put license files, which are usually in the root. Do you know where these belong?

No idea.

mlbridge · 2024-06-01T05:24:55Z

Mailing list message from Michael Hall on core-libs-dev:

Yes you can file an Apple bug report but I think these days it requires a developer account to get to the bug reporter.

The indicated documentation doesn?t mention anything about your own files/directories. So I think a bug report might be appropriate. One in but the other out doesn?t seem right.

I used to have my own directory in an application but that was pre-jpackage. And prior to Apple?s incremental security crack-downs with Gatekeeper. I think I had code with special class loading requirements where I wanted to load it outside class path.

My app no longer includes that. I have a resources jar where I dump all my miscellaneous and retrieve them from there.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20240531/7b1e385f/attachment.htm>

8332110: jpackage tries to sign added files without the --mac-sign op…

0ad02cb

…tion

openjdk bot added the rfr Pull request is ready for review label May 24, 2024

openjdk bot added the core-libs core-libs-dev@openjdk.org label May 24, 2024

8332110: jpackage tries to sign added files without the --mac-sign op…

7c1973a

…tion [v2]

alexeysemenyukoracle approved these changes May 31, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label May 31, 2024

openjdk bot added the integrated Pull request has been integrated label May 31, 2024

openjdk bot closed this May 31, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8332110: [macos] jpackage tries to sign added files without the --mac-sign option #19377

8332110: [macos] jpackage tries to sign added files without the --mac-sign option #19377

sashamatveev commented May 24, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented May 24, 2024

openjdk bot commented May 24, 2024 •

edited

Loading

openjdk bot commented May 24, 2024

mlbridge bot commented May 24, 2024 •

edited

Loading

mlbridge bot commented May 25, 2024

mlbridge bot commented May 25, 2024

sashamatveev commented May 28, 2024

alexeysemenyukoracle commented May 30, 2024

sashamatveev commented May 30, 2024

alexeysemenyukoracle May 30, 2024

sashamatveev May 31, 2024

sashamatveev commented May 31, 2024

openjdk bot commented May 31, 2024

openjdk bot commented May 31, 2024

nlisker commented May 31, 2024

sashamatveev commented May 31, 2024

nlisker commented May 31, 2024

sashamatveev commented May 31, 2024

mlbridge bot commented Jun 1, 2024

8332110: [macos] jpackage tries to sign added files without the --mac-sign option #19377

8332110: [macos] jpackage tries to sign added files without the --mac-sign option #19377

Conversation

sashamatveev commented May 24, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented May 24, 2024

openjdk bot commented May 24, 2024 • edited Loading

openjdk bot commented May 24, 2024

mlbridge bot commented May 24, 2024 • edited Loading

Webrevs

mlbridge bot commented May 25, 2024

mlbridge bot commented May 25, 2024

sashamatveev commented May 28, 2024

alexeysemenyukoracle commented May 30, 2024

sashamatveev commented May 30, 2024

alexeysemenyukoracle May 30, 2024

Choose a reason for hiding this comment

sashamatveev May 31, 2024

Choose a reason for hiding this comment

sashamatveev commented May 31, 2024

openjdk bot commented May 31, 2024

openjdk bot commented May 31, 2024

nlisker commented May 31, 2024

sashamatveev commented May 31, 2024

nlisker commented May 31, 2024

sashamatveev commented May 31, 2024

mlbridge bot commented Jun 1, 2024

sashamatveev commented May 24, 2024 •

edited by openjdk bot

Loading

openjdk bot commented May 24, 2024 •

edited

Loading

mlbridge bot commented May 24, 2024 •

edited

Loading