[jdk8u-dev] RFR: 8269039: Disable SHA-1 Signed JARs

[Martin Balao]

On Thu, 3 Nov 2022 15:34:01 GMT, Alexey Bakhtin <abakhtin at openjdk.org> wrote:

> I'd like to backport this enhancement for parity with Oracle and the security roadmap [1]
> 
> The patch is based on the OpenJDK11 backport [2]
> 
> The changes are the following:
> 
> 1. java.security.* files are changed on the base of java.security
> 2. jdk/src/share/classes/sun/security/tools/jarsigner/Main.java : signJar method is merged manually because of differences in the JDK-8172404 backport. 
> 3. jdk/test/java/security/Security/signedfirst/DynStatic.java : modules dependency removed, changed path to utility classes
> 4. jdk/test/sun/security/tools/jarsigner/Test4431684.java : changed library path. List.of() is not available in JDK8, so it is replaced with Arrays.asList()
> 5. jdk/test/lib/security/SecurityUtils.java is updated to make removeFromDisabledAlgs method public. It is required by newly added test Test4431684.java
> 6. jdk/test/sun/security/tools/jarsigner/DefaultOptions.java is skipped, it was introduced in JDK9 by JDK-8049834 as default_options.sh but never backported to JDK8
> 7. JDK8 has jdk/test/sun/security/tools/jarsigner/diffend.sh test instead of jdk/test/sun/security/tools/jarsigner/DiffEnd.java. diffend.sh was not renamed to DiffEnd.java because of JDK-8180573 is not backported to JDK8. JDK-8180573 is a big refactoring and out of scope for this issue. diffend.sh updated accordingly - SHA1 replaced to SHA-256
> 8. JDK8 has jdk/test/sun/security/tools/jarsigner/ec.sh test instead of jdk/test/sun/security/tools/jarsigner/EC.java. ec.sh was not renamed to EC.java because of JDK-8180573 is not backported to JDK8. JDK-8180573 is a big refactoring and out of scope for this issue. ec.sh has all required changes by JDK-8172404
> 9. JDK8 has jdk/test/sun/security/tools/jarsigner/nameclash.sh test instead of jdk/test/sun/security/tools/jarsigner/NameClash.java. nameclash.sh was not renamed to NameClash.java because of JDK-8180573 is not backported to JDK8. JDK-8180573 is a big refactoring and out of scope for this issue. nameclash.sh has all required changes by JDK-8172404
> 10. JDK8 has jdk/test/sun/security/tools/jarsigner/oldsig.sh test instead of  jdk/test/sun/security/tools/jarsigner/OldSig.java. oldsig.sh was not renamed to OldSig.java because of JDK-8180573 is not backported to JDK8. JDK-8180573 is a big refactoring and out of scope for this issue. The changes in the oldsig.sh are not required because of JDK-8217375 is not backported to JDK8.
> 11. jdk/test/sun/security/tools/jarsigner/OldSig.props is not backported as it is not used in the  jdk/test/sun/security/tools/jarsigner/oldsig.sh
> 
> All java/security/Security sun/security/tools regression tests passed
> 
> [1] - https://www.java.com/en/jre-jdk-cryptoroadmap.html
> [2] - https://github.com/openjdk/jdk11u-dev/commit/5a0824ba813ceda47847c9162c8a10bb0b8898e8

Hi @alexeybakhtin ,

Thanks for proposing this backport.

In addition to the inlined comments, I have this additional observation:

 * `jdk/test/sun/security/tools/jarsigner/oldsig.sh` is using SHA1, as well as JDK-11's OldSig.java. The security properties file that is now passed has the following comment: "Re-enable SHA-1 since OldSig.java test uses it". You mentioned 8217375 as the reason why changes are not required for the backport, but I'm having difficulties to find the connection and wonder if we should re-enable SHA1 for `oldsig.sh`. Can you please elaborate a bit on this?

Thanks,
Martin.-

jdk/test/java/security/Security/signedfirst/DynStatic.java line 36:

> 34: import java.util.List;
> 35: 
> 36: import jdk.test.lib.process.ProcessTools;

The `ProcessTools` class in JDK-8 has always been `jdk.testlibrary.ProcessTools`. To use it, you would only need `@library /lib/testlibrary` in the test's JTREG header. Unfortunately, one backport -which I presume to be JFR- introduced the same class, from a newer JDK release, at a different location. So we now have duplicated code for this test library and it is causing confusion: I can now see a couple of TLS test using this library when they shouldn't. Until we get rid of this technical debt, I suggest that all JDK-8 tests keep using `jdk.testlibrary.ProcessTools`.

jdk/test/sun/security/tools/jarsigner/TimestampCheck.java line 461:

> 459:                         .shouldMatch("Timestamp.*digest.*SHA-1.*(disabled)");
> 460: 
> 461:                 // Disabled algorithms

Isn't this comment removed?

-------------

PR: https://git.openjdk.org/jdk8u-dev/pull/154