RFR: 8300272: Improve readability of the test JarWithOneNonDisabledDigestAlg

Eirik Bjorsnos duke at openjdk.org
Tue Jan 17 19:59:36 UTC 2023


On Tue, 17 Jan 2023 15:08:36 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> This PR attempts to make JarWithOneNonDisabledDigestAlg a little easier to read. 
>> 
>> Some changes are made in the choice of algorithms and naming. The intent here is to reduce confusion and make the purpose of the test clearer:
>> 
>> - Updated the **enabled** digestAlgorithm in use from SHA1 to SHA256. The use of SHA1 here seems just a bit confusing, since it has been considered weak for a while 
>> - The two different signer aliases are now named SIGNER1, SIGNER2 instead of the somewhat confusing SHA1, SHA256
>> - Both signing keys are now generated with -sigalg SHA256withRSA since the sigalg of the keys does not seem to matter for this test
>> 
>> There are also some general code cleanups:
>> 
>> - Moved loading of the key store into the new method loadKeyStore
>> - Updated checkThatJarIsSigned to take a parameter Map<String, Integer> representing the expected signer counts for each path in the JAR. This provides a cleaner separation between expectiations and the enforcement of expectations.
>> - Introduced Path constants for various file names used throughout the test, reducing a number of redundant Path.of calls which seemed to clutter the code a bit
>> - Updated IO code to use new APIs, such as Files.newOutputStream, Files.newInputStream, InputStream.transferTo and OutputStream.nullOutputStream.
>> - Added/updated some comments where appropriate
>
> test/jdk/jdk/security/jarsigner/JarWithOneNonDisabledDigestAlg.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> 
> Should be `Copyright (c) 2022, 2023,`.

Thanks, fixed. Funny how the brain switches into mechanical pattern matching and then the matching isn't all that great :-)

> test/jdk/jdk/security/jarsigner/JarWithOneNonDisabledDigestAlg.java line 67:
> 
>> 65:     public static void main(String[] args) throws Exception {
>> 66:         SecurityUtils.removeFromDisabledAlgs("jdk.jar.disabledAlgorithms",
>> 67:                 List.of("SHA256"));
> 
> There is no need to remove SHA256. It is not disabled by default.

@wangweij 

I initially removed this code, then restored it because I thought the original author might have intended to future-proof the test. It also serves as a sort of documentation of the implicit assumtions the test makes about the permitted state of digest algorithms in the JVM.

I have now instead added a method which explicitly asserts that MD5 is disabled and SHA256 is permitted in the very beginning of the test. This way the assumtions are made clear and the test will fail clear and loudly should these assumtions fail in the future.

What do you think of this update?

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

PR: https://git.openjdk.org/jdk/pull/11997


More information about the security-dev mailing list