RFR JDK-8179614: Test for jarsigner on verifying jars that are signed and timestamped by other JDK releases

sha.jiang at oracle.com sha.jiang at oracle.com
Wed Aug 16 02:32:23 UTC 2017


Hi Sean,
Thanks for your review!
This patch has been pushed. And the README contains the copyright.

Best regards,
John Jiang


On 16/08/2017 01:34, Sean Mullan wrote:
> Hi John,
>
> You should add a copyright to the README. Otherwise, this update looks 
> good.
>
> --Sean
>
> On 8/14/17 11:52 PM, sha.jiang at oracle.com wrote:
>> Hi,
>> The webrev [1] is updated on the following points:
>> 1. It allows TSA URL to append a set of supported digest algorithms. 
>> If a TSA URL doesn't append the digests parameter, it means that the 
>> TSA supports SHA-1, SHA-256 and SHA-512.
>> 2. EC cases are excluded for JDK 6.
>> 3. Certificates are generated by the signer JDKs themselves 
>> respectively.
>> 4. jarsigner uses option "-debug".
>> 5. Test mode "strict" is removed.
>>
>> [1] http://cr.openjdk.java.net/~jjiang/8179614/webrev.11/
>>
>> Best regards,
>> John Jiang
>>
>>
>> On 14/07/2017 15:11, sha.jiang at oracle.com wrote:
>>>
>>> Hi,
>>> Please review the latest webrev at: 
>>> http://cr.openjdk.java.net/~jjiang/8179614/webrev.09/
>>> This test has been updated significantly. It removes useless case 
>>> combinations, and generates reports in HTML. For more details, 
>>> please look through the test summary.
>>>
>>> Best regards,
>>> John Jiang
>>>
>>> On 13/06/2017 23:47, sha.jiang at oracle.com wrote:
>>>> Sean and Max,
>>>> Please review this updated webrev: 
>>>> http://cr.openjdk.java.net/~jjiang/8179614/webrev.03/
>>>>
>>>> The main changes are:
>>>> 1. It provides two new properties, tsaList and tsaListFile, for 
>>>> specifying a list of TSA services.
>>>> And a new report column [TSA] is introduced. This column just 
>>>> display the TSA indices and all of TSA services are displayed at 
>>>> the top of the report.
>>>> 2. If property strict is true, the cases on failed signing are not 
>>>> ignored. They still be listed in the test report, and the status of 
>>>> verifying are NONE.
>>>>
>>>> Best regards,
>>>> John Jiang
>>>>
>>>> On 13/06/2017 06:51, sha.jiang at oracle.com wrote:
>>>>> Hi Max,
>>>>>
>>>>> On 12/06/2017 17:29, Weijun Wang wrote:
>>>>>> Great. Only 2 questions:
>>>>>>
>>>>>>  459     // Return key sizes according to the specified key 
>>>>>> algorithm.
>>>>>>  460     private static int[] keySizes(String digestAlgorithm, 
>>>>>> String keyAlgorithm) {
>>>>>>  461         if (digestAlgorithm == DEFAULT) {
>>>>>>  462             return new int[] { 0 };
>>>>>>  463         }
>>>>>>  464
>>>>>>  465         if (keyAlgorithm == RSA || keyAlgorithm == DSA) {
>>>>>>  466             return new int[] { 1024, 2048 };
>>>>>>  467         } else if (keyAlgorithm == EC) {
>>>>>>  468             return new int[] { 384, 571 };
>>>>>>  469         }
>>>>>>  470
>>>>>>  471         return null;
>>>>>>  472     }
>>>>>>
>>>>>> Why is keysize dependent on digestalg? I mean, is it possible to 
>>>>>> always return {1024,2048,0} and {384,571,0}?
>>>>> Get it, thanks!
>>>>>>
>>>>>>  379     // If signing fails, the following verifying has to
>>>>>>  380     // be ignored.
>>>>>>  381     if (signingStatus == STATUS.ERROR) {
>>>>>>  382         continue;
>>>>>>  383     }
>>>>>>
>>>>>> Now that you've already checked sigalg support earlier in what 
>>>>>> cases it could go wrong here?
>>>>> Jar signing still could fail. For example, TSA service is 
>>>>> unavailable.
>>>>>
>>>>> Best regards,
>>>>> John Jiang
>>>>>>
>>>>>> Thanks
>>>>>> Max
>>>>>>
>>>>>> On 06/12/2017 03:20 PM, sha.jiang at oracle.com wrote:
>>>>>>> Hi Max,
>>>>>>> Would you like to review the updated webrev: 
>>>>>>> http://cr.openjdk.java.net/~jjiang/8179614/webrev.02/
>>>>>>> It can create certificate without -sigalg and -keysize, and jar 
>>>>>>> signing also can use this certificate.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> John Jiang
>>>>>>>
>>>>>>> On 09/06/2017 22:04, Weijun Wang wrote:
>>>>>>>>
>>>>>>>> On 06/09/2017 09:25 PM, sha.jiang at oracle.com wrote:
>>>>>>>>> Hi Max,
>>>>>>>>>
>>>>>>>>> On 09/06/2017 20:05, Weijun Wang wrote:
>>>>>>>>>> The test can be more friendly with default values.
>>>>>>>>>>
>>>>>>>>>> For example, in createCertificates(), you can generate certs 
>>>>>>>>>> that use default sigalg and keysize (i.e. without specifying 
>>>>>>>>>> -siglag and -keysize), and give them aliases with "default" 
>>>>>>>>>> or "null" inside.
>>>>>>>>>>
>>>>>>>>>> And in jar signing when signing with one -sigalg you can also 
>>>>>>>>>> choose cert generated with different or default sigalgs.
>>>>>>>>> I supposed this test just focus on signed jar verifying, but 
>>>>>>>>> not certificate creating and jar signing. So, I'm not sure 
>>>>>>>>> such cases are necessary.
>>>>>>>>
>>>>>>>> Well sometimes a test can do many things. If you only care 
>>>>>>>> about jar verification, why bother creating certs with 
>>>>>>>> different digest algorithms?
>>>>>>>>
>>>>>>>> On the other hand, if you do care about more, then in
>>>>>>>>
>>>>>>>>  338     // If the digest algorithm is not specified, then it
>>>>>>>>  339     // uses certificate with SHA256 digest and 1024 key
>>>>>>>>  340     // size.
>>>>>>>>  341     if (digestAlgorithm == DEFAULT) {
>>>>>>>>  342         certDigest = SHA256;
>>>>>>>>  343         certKeySize = 1024;
>>>>>>>>  344     }
>>>>>>>>
>>>>>>>> it seems a little awkward to hardcode the algorithm and 
>>>>>>>> keysize. If signing is using a default algorithm, it seems 
>>>>>>>> natural to use the cert that was generated with a default 
>>>>>>>> algorithm. In fact, this test case is quite useful that it 
>>>>>>>> ensures our different tools are using the same (or at least 
>>>>>>>> interoperable) default algorithms.
>>>>>>>>
>>>>>>>> --Max
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> BTW, I remember certain pairs of -keysize and -sigalg do not 
>>>>>>>>>> work together. For example, 1024 bit of DSA key cannot be 
>>>>>>>>>> used with SHA512withDSA signature algorithm. Have you noticed 
>>>>>>>>>> it?
>>>>>>>>> It looks SHA512withDSA is not supported yet.
>>>>>>>>> I was using JDK10 build 10. When the test tried to create 
>>>>>>>>> certificate with -keyalg DSA -sigalg SHA512withDSA -keysize 
>>>>>>>>> 1024, the below error raised:
>>>>>>>>> keytool error: java.security.NoSuchAlgorithmException: 
>>>>>>>>> unrecognized algorithm name: SHA512withDSA
>>>>>>>>>
>>>>>>>>> If used -keyalg DSA -sigalg SHA1withDSA -keysize 2048, the 
>>>>>>>>> error was:
>>>>>>>>> keytool error: java.security.InvalidKeyException: The security 
>>>>>>>>> strength of SHA-1 digest algorithm is not sufficient for this 
>>>>>>>>> key size
>>>>>>>>>
>>>>>>>>> Again, this test focus on signed jar verifying. If some 
>>>>>>>>> problems are raised on certificate creating or jar signing, 
>>>>>>>>> the associated verifying cases will be ignored.
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> John Jiang
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Max
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 06/09/2017 04:44 PM, sha.jiang at oracle.com wrote:
>>>>>>>>>>> Hi Sean and Max,
>>>>>>>>>>> Thanks for your comments.
>>>>>>>>>>> Please review the updated webrev: 
>>>>>>>>>>> http://cr.openjdk.java.net/~jjiang/8179614/webrev.01/
>>>>>>>>>>>
>>>>>>>>>>> The test has been modified significantly. The main points are:
>>>>>>>>>>> 1. Adds cases on EC. Now the test supports key algorithms 
>>>>>>>>>>> RSA, DSA and EC.
>>>>>>>>>>> 2. Adds cases on SHA-512. Now the test supports digest 
>>>>>>>>>>> algorithms SHA-1, SHA-256 and SHA-512.
>>>>>>>>>>> 3. Adds cases on key size. Exactly, [384, 571] for EC, 
>>>>>>>>>>> [1024, 2048] for RSA and DSA.
>>>>>>>>>>> 4. Adds cases on default signature algorithm. Now the test 
>>>>>>>>>>> report can display the default algorithmat column [Signature 
>>>>>>>>>>> Algorithm].
>>>>>>>>>>> 5. Adds property -Djava.security.egd=file:/dev/./urandom for 
>>>>>>>>>>> keytool and jarsigner commands.
>>>>>>>>>>> 6. Create a separated application, JdkUtils.java, to 
>>>>>>>>>>> determine the JDK build version (java.runtime.version) and 
>>>>>>>>>>> check if a signature algorithm is supported by a JDK.
>>>>>>>>>>> 7. Introduces a new property, named javaSecurityFile, for 
>>>>>>>>>>> allowing users to specify alternative java security 
>>>>>>>>>>> properties file.
>>>>>>>>>>> 8. Renames report column [Cert Type] to [Certificate]. This 
>>>>>>>>>>> column displays the certificate identifiers, which is a 
>>>>>>>>>>> combination of key algorithm, digest algorithm, key size and 
>>>>>>>>>>> expired mark (if any).
>>>>>>>>>>> 9. The test summary also be updated accordingly.
>>>>>>>>>>>
>>>>>>>>>>> Best regards,
>>>>>>>>>>> John Jiang
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 07/06/2017 23:11, Sean Mullan wrote:
>>>>>>>>>>>> On 6/6/17 9:14 PM, sha.jiang at oracle.com wrote:
>>>>>>>>>>>>> Hi Sean,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 07/06/2017 04:27, Sean Mullan wrote:
>>>>>>>>>>>>>> Hi John,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This looks like a very useful test. I have not gone 
>>>>>>>>>>>>>> through all of the code, but here are a few comments for 
>>>>>>>>>>>>>> now until I have more time:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> - add tests for EC keys
>>>>>>>>>>>>>> - add tests for SHA-512 variants of the signature algorithms
>>>>>>>>>>>>>> - add tests for larger key sizes (ex: 2048 for DSA/RSA)
>>>>>>>>>>>>>> - you can use the diamond operator <> in various places
>>>>>>>>>>>>>> - might be more compact if jdkList() used Files.lines() 
>>>>>>>>>>>>>> to parse the file into a stream then an array
>>>>>>>>>>>>> I did consider about the above two points. Because the 
>>>>>>>>>>>>> test will be backported to JDK 6, so I only used the 
>>>>>>>>>>>>> features those supported by JDK 6.
>>>>>>>>>>>>> I supposed that would make the backport easier. Does it 
>>>>>>>>>>>>> make sense?
>>>>>>>>>>>>
>>>>>>>>>>>> Yes, that makes sense.
>>>>>>>>>>>>
>>>>>>>>>>>> --Sean
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>> John Jiang
>>>>>>>>>>>>>> - did you consider using the jarsigner API 
>>>>>>>>>>>>>> (jdk.security.jarsigner) instead of the command-line? I 
>>>>>>>>>>>>>> think this would be better (if possible) and it would 
>>>>>>>>>>>>>> give us some more tests of that API.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --Sean
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 6/5/17 6:31 AM, sha.jiang at oracle.com wrote:
>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>> Please review this manual test for checking if a jar, 
>>>>>>>>>>>>>>> which is signed and timestamped by a JDK build, could be 
>>>>>>>>>>>>>>> verified by other JDK builds.
>>>>>>>>>>>>>>> It also can be used to check if the default timestamp 
>>>>>>>>>>>>>>> digest algorithm on signing is SHA-256.
>>>>>>>>>>>>>>> For more details, please look through the test summary.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8179614
>>>>>>>>>>>>>>> Webrev: 
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jjiang/8179614/webrev.00/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>> John Jiang
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>




More information about the security-dev mailing list