Skip to content

Conversation

@alexeysemenyukoracle
Copy link
Member

@alexeysemenyukoracle alexeysemenyukoracle commented Oct 17, 2025

  • Rework the SigningPackageTwoStepTest test to cover the case of bundling an unsigned predefined app image into a signed .pkg installer; make it verify expected warnings in the jpackage output.

  • All jpackage tests will verify that without "--mac-sign" option, jpackage produces app images with a valid "adhoc" signature.

Additionally:

  • Fix jpackage to make it emit messages expected in the updated SigningPackageTwoStepTest test.
  • Add helper code for signing testing.
  • Automatically unlock jpackage test keychains in all signing tests.
  • Add a workaround for the /usr/bin/codesign—-verify command, which sometimes fails if executed without sudo.

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-8370126: Improve jpackage signing testing (Enhancement - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27875

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

Using diff file

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

Using Webrev

Link to Webrev Comment

…ains in signing tests; MacSignTest: better test coverage; MacSign: make MacSign.ResolvedKeychain more usable; JPackageCommand, MacHelper: add sign helpers and verify signature of jpackage output if it is unsigned; MacSignVerify: add functions to verify signed output of JPackageCommand instances; Rework SigningPackageTwoStepTest for better coverage. Add a workaround for `/usr/bin/codesign --verify` command that sometimes fails if executed without `sudo`.
@alexeysemenyukoracle alexeysemenyukoracle marked this pull request as draft October 17, 2025 18:45
@bridgekeeper
Copy link

bridgekeeper bot commented Oct 17, 2025

👋 Welcome back asemenyuk! 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
Copy link

openjdk bot commented Oct 17, 2025

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

8370126: Improve jpackage signing testing

Reviewed-by: almatvee

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

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
Copy link

openjdk bot commented Oct 17, 2025

@alexeysemenyukoracle 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.

@alexeysemenyukoracle alexeysemenyukoracle changed the title 8370126: Improve jpackage SigningPackageTwoStepTest test 8370126: Improve jpackage signing testing Oct 17, 2025
@alexeysemenyukoracle alexeysemenyukoracle marked this pull request as ready for review October 18, 2025 04:32
@alexeysemenyukoracle
Copy link
Member Author

@sashamatveev PTAL

@openjdk openjdk bot added the rfr Pull request is ready for review label Oct 18, 2025
@mlbridge
Copy link

mlbridge bot commented Oct 18, 2025

Webrevs

Copy link
Member

@sashamatveev sashamatveev 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 with some question.

// By some reason /usr/bin/codesign command fails for some installed bundles.
// It is known to fail for some AppContentTest test cases and all FileAssociationsTest test cases.
// Rerunning the command with "sudo" works, though.
return str.equals(String.format("%s: Permission denied", path));
Copy link
Member

Choose a reason for hiding this comment

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

Did you check permissions on generated application bundles in case of failure vs when it pass? Maybe we have issue with permissions in generated application images and by doing work around we hiding current bug or potential bugs.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what fails:

$ /usr/bin/codesign --verify --strict --verbose=2 /Applications/AppContentTest.app
/Applications/AppContentTest.app: Permission denied

This is what works:

$ sudo /usr/bin/codesign --verify --strict --verbose=2 /Applications/AppContentTest.app
/Applications/AppContentTest.app: valid on disk
/Applications/AppContentTest.app: satisfies its Designated Requirement

I can't spot anything suspicious, can you?

$ ls -alR /Applications/AppContentTest.app
total 0
drwxr-xr-x   3 root  wheel    96B Oct 20 20:40 ./
drwxrwxr-x  31 root  admin   992B Oct 20 20:40 ../
drwxr-xr-x   9 root  wheel   288B Oct 20 20:40 Contents/

/Applications/AppContentTest.app/Contents:
total 16
drwxr-xr-x  9 root  wheel   288B Oct 20 20:40 ./
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ../
-rw-r--r--  1 root  wheel   1.3K Oct 20 20:40 Info.plist
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 MacOS/
-rw-r--r--  1 root  wheel     8B Oct 20 20:40 PkgInfo
drwxr-xr-x  5 root  wheel   160B Oct 20 20:40 Resources/
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 _CodeSignature/
drwxr-xr-x  5 root  wheel   160B Oct 20 20:40 app/
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 runtime/

/Applications/AppContentTest.app/Contents/MacOS:
total 440
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ./
drwxr-xr-x  9 root  wheel   288B Oct 20 20:40 ../
-rwxr-xr-x  1 root  wheel   217K Oct 20 20:40 AppContentTest*

/Applications/AppContentTest.app/Contents/Resources:
total 760
drwxr-xr-x  5 root  wheel   160B Oct 20 20:40 ./
drwxr-xr-x  9 root  wheel   288B Oct 20 20:40 ../
-rw-r--r--  1 root  wheel   370K Oct 20 20:40 AppContentTest.icns
-rwx------  1 root  wheel   3.5K Oct 20 20:40 PrintEnv.java*
-rwx------  1 root  wheel   1.4K Oct 20 20:40 dukeplug.png*

/Applications/AppContentTest.app/Contents/_CodeSignature:
total 16
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ./
drwxr-xr-x  9 root  wheel   288B Oct 20 20:40 ../
-rw-r--r--  1 root  wheel   4.7K Oct 20 20:40 CodeResources

/Applications/AppContentTest.app/Contents/app:
total 32
drwxr-xr-x  5 root  wheel   160B Oct 20 20:40 ./
drwxr-xr-x  9 root  wheel   288B Oct 20 20:40 ../
-rw-r--r--  1 root  wheel    14B Oct 20 20:40 .package
-rw-r--r--  1 root  wheel   121B Oct 20 20:40 AppContentTest.cfg
-rw-r--r--  1 root  wheel   4.4K Oct 20 20:40 hello.jar

/Applications/AppContentTest.app/Contents/runtime:
total 0
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ./
drwxr-xr-x  9 root  wheel   288B Oct 20 20:40 ../
drwxr-xr-x  6 root  wheel   192B Oct 20 20:40 Contents/

/Applications/AppContentTest.app/Contents/runtime/Contents:
total 8
drwxr-xr-x  6 root  wheel   192B Oct 20 20:40 ./
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ../
drwxr-xr-x  4 root  wheel   128B Oct 20 20:40 Home/
-rw-r--r--  1 root  wheel   835B Oct 20 20:40 Info.plist
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 MacOS/
drwxr-xr-x  7 root  wheel   224B Oct 20 20:40 _CodeSignature/

/Applications/AppContentTest.app/Contents/runtime/Contents/Home:
total 0
drwxr-xr-x  4 root  wheel   128B Oct 20 20:40 ./
drwxr-xr-x  6 root  wheel   192B Oct 20 20:40 ../
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 bin/
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 lib/

/Applications/AppContentTest.app/Contents/runtime/Contents/Home/bin:
total 8
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ./
drwxr-xr-x  4 root  wheel   128B Oct 20 20:40 ../
-rw-r--r--  1 root  wheel   4.0K Oct 20 20:40 bulk

/Applications/AppContentTest.app/Contents/runtime/Contents/Home/lib:
total 0
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ./
drwxr-xr-x  4 root  wheel   128B Oct 20 20:40 ../
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 jli/

/Applications/AppContentTest.app/Contents/runtime/Contents/Home/lib/jli:
total 8
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ./
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ../
-rw-r--r--@ 1 root  wheel   4.0K Oct 20 20:40 libjli.dylib

/Applications/AppContentTest.app/Contents/runtime/Contents/MacOS:
total 8
drwxr-xr-x  3 root  wheel    96B Oct 20 20:40 ./
drwxr-xr-x  6 root  wheel   192B Oct 20 20:40 ../
-rw-r--r--  1 root  wheel   4.0K Oct 20 20:40 libjli.dylib

/Applications/AppContentTest.app/Contents/runtime/Contents/_CodeSignature:
total 32
drwxr-xr-x  7 root  wheel   224B Oct 20 20:40 ./
drwxr-xr-x  6 root  wheel   192B Oct 20 20:40 ../
-rw-r--r--  1 root  wheel   134B Oct 20 20:40 CodeDirectory
-rw-r--r--  1 root  wheel    12B Oct 20 20:40 CodeRequirements
-rw-r--r--  1 root  wheel   182B Oct 20 20:40 CodeRequirements-1
-rw-r--r--  1 root  wheel   2.6K Oct 20 20:40 CodeResources
-rw-r--r--  1 root  wheel     0B Oct 20 20:40 CodeSignature

Copy link
Member Author

Choose a reason for hiding this comment

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

It always fails for me. This is an unsigned bundle, and we never checked the signature of unsigned bundles before this fix in the tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like this is antivirus blocking access to the main launcher:

$ /usr/bin/codesign --verify --strict --verbose=2 /Applications/AppContentTest.app/Contents/MacOS/AppContentTest
/Applications/AppContentTest.app/Contents/MacOS/AppContentTest: Permission denied
$ /usr/bin/codesign --verify --strict --verbose=2 /Applications/AppContentTest.app/Contents/runtime/Contents/MacOS/libjli.dylib
/Applications/AppContentTest.app/Contents/runtime/Contents/MacOS/libjli.dylib: valid on disk
/Applications/AppContentTest.app/Contents/runtime/Contents/MacOS/libjli.dylib: satisfies its Designated Requirement

I tried to run the main launcher:

$ /Applications/AppContentTest.app/Contents/MacOS/AppContentTest
Killed: 9

Next attempt - it is gone. Got deleted by Crowdstrike 100%

$ /Applications/AppContentTest.app/Contents/MacOS/AppContentTest
-bash: /Applications/AppContentTest.app/Contents/MacOS/AppContentTest: No such file or directory

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for explanation. I think it can be related to recent issue on macOS 15.5 and up when application bundles installed with PKG cannot be overwritten by application bundles from DMG with drag and drop command. I can see by provided log that you refer to applications installed with PKG since they under root user.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could be. But it fails only for some bundles and pretty consistently. Anyway, I don't think this is caused by some jpackage bug.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Oct 21, 2025
@alexeysemenyukoracle
Copy link
Member Author

/integrate

@openjdk
Copy link

openjdk bot commented Oct 21, 2025

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

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Oct 21, 2025
@openjdk openjdk bot closed this Oct 21, 2025
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 21, 2025
@openjdk
Copy link

openjdk bot commented Oct 21, 2025

@alexeysemenyukoracle Pushed as commit 2aa0efd.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core-libs [email protected] integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

2 participants