Skip to content
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

8051959: Add thread and timestamp options to java.security.debug system property #2998

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vieiro
Copy link
Contributor

@vieiro vieiro commented Feb 17, 2025

Almost clean backport of JDK-8051959 that adds options to java.security.debug to enhance traces with thread, log record and timestamp information, improving traceability and easying troubleshooting, on par with "The java.security.debug System Property" in JDK17 and above, and with 11.0.26-oracle. Low risk.

Backport is not completely clean because, among other things, JDK-8292177 was applied differently in 11 (also HexFormat is not in 11).

One of the tests cases had to be changed, since JDK11 does not keep track of Security#initialSystemProperties and thus searching for properties: Initial in the test stderr makes no sense. We're searching for properties: java.security instead (this is indicated in the github PR).

Tested on Linux with tier1...

==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/hotspot/jtreg:tier1                     1497  1497     0     0   
   jtreg:test/jdk:tier1                               1899  1899     0     0   
   jtreg:test/langtools:tier1                         3941  3941     0     0   
   jtreg:test/nashorn:tier1                              0     0     0     0   
   jtreg:test/jaxp:tier1                                 0     0     0     0   
==============================
TEST SUCCESS

... and security tests ...

==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk/sun/security                         658   658     0     0   
==============================
TEST SUCCESS

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
  • Change requires a CSR request matching fixVersion 11.0.27 to be approved (needs to be created)
  • JDK-8051959 needs maintainer approval

Issue

  • JDK-8051959: Add thread and timestamp options to java.security.debug system property (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/2998/head:pull/2998
$ git checkout pull/2998

Update a local copy of the PR:
$ git checkout pull/2998
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/2998/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2998

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/2998.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Feb 17, 2025

👋 Welcome back vieiro! 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 Feb 17, 2025

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk bot changed the title Backport f277b3999f8cd30d5874ce58f290a06538d0189f 8051959: Add thread and timestamp options to java.security.debug system property Feb 17, 2025
@openjdk
Copy link

openjdk bot commented Feb 17, 2025

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added the backport label Feb 17, 2025
@openjdk
Copy link

openjdk bot commented Feb 17, 2025

At least one of the issues associated with this backport has a resolved CSR for a different version. As this means that this backport may also need a CSR, the csr label is being added to this pull request to signal this potential requirement. The command /csr unneeded can be used to remove the label in case a CSR is not needed.

@openjdk openjdk bot added csr Pull request needs approved CSR before integration rfr Pull request is ready for review labels Feb 17, 2025
return Stream.of(
// no extra info present
Arguments.of("properties",
"properties: java.security",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This reads properties: Initial in JDK17, since JDK17 keeps track of initial properties and prints something like this on stderr (note the "Initial security..." message):

$ [JDK17]/bin/java -Djava.security.debug=properties
properties: java.security
properties: java.security.disableSystemPropertiesFile=false
properties: security.useSystemPropertiesFile=false
properties: System security property support disabled by user.
properties: WARNING: FIPS mode support can not be enabled without system security properties being enabled.
properties: Initial security property: jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < 1024, SHA1 denyAfter 2019-01-01
properties: Initial security property: fips.provider.3=SunEC
properties: Initial security property: fips.provider.4=SunJSSE
properties: Initial security property: fips.provider.1=SunPKCS11 ${java.home}/conf/security/nss.fips.cfg

Whereas, for 11:

$ [JDK11]/bin/java -Djava.security.debug=properties
properties: java.security
Usage: java [options] <mainclass> [args...]
           (to execute a class)

@mlbridge
Copy link

mlbridge bot commented Feb 17, 2025

Webrevs

@vieiro
Copy link
Contributor Author

vieiro commented Feb 18, 2025

At least one of the issues associated with this backport has a resolved CSR ...

Let's wait for a review before creating the CSR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport csr Pull request needs approved CSR before integration rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

1 participant