On Tue, 17 Jan 2023 15:08:36 GMT, Weijun Wang <wei...@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
