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