<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi,<br>
Please review the latest webrev at: <a
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejjiang/8179614/webrev.09/">http://cr.openjdk.java.net/~jjiang/8179614/webrev.09/</a><br>
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.<br>
<br>
Best regards,<br>
John Jiang<br>
</p>
<div class="moz-cite-prefix">On 13/06/2017 23:47, <a
class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:4dae591e-12c5-c5de-fb6b-793d1b8b3d96@oracle.com">Sean
and Max, <br>
Please review this updated webrev: <a
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejjiang/8179614/webrev.03/">http://cr.openjdk.java.net/~jjiang/8179614/webrev.03/</a>
<br>
<br>
The main changes are: <br>
1. It provides two new properties, tsaList and tsaListFile, for
specifying a list of TSA services. <br>
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. <br>
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. <br>
<br>
Best regards, <br>
John Jiang <br>
<br>
On 13/06/2017 06:51, <a class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a>
wrote: <br>
<blockquote type="cite">Hi Max, <br>
<br>
On 12/06/2017 17:29, Weijun Wang wrote: <br>
<blockquote type="cite">Great. Only 2 questions: <br>
<br>
459 // Return key sizes according to the specified key
algorithm. <br>
460 private static int[] keySizes(String digestAlgorithm,
String keyAlgorithm) { <br>
461 if (digestAlgorithm == DEFAULT) { <br>
462 return new int[] { 0 }; <br>
463 } <br>
464 <br>
465 if (keyAlgorithm == RSA || keyAlgorithm == DSA) {
<br>
466 return new int[] { 1024, 2048 }; <br>
467 } else if (keyAlgorithm == EC) { <br>
468 return new int[] { 384, 571 }; <br>
469 } <br>
470 <br>
471 return null; <br>
472 } <br>
<br>
Why is keysize dependent on digestalg? I mean, is it possible
to always return {1024,2048,0} and {384,571,0}? <br>
</blockquote>
Get it, thanks! <br>
<blockquote type="cite"> <br>
379 // If signing fails, the following verifying has to <br>
380 // be ignored. <br>
381 if (signingStatus == STATUS.ERROR) { <br>
382 continue; <br>
383 } <br>
<br>
Now that you've already checked sigalg support earlier in what
cases it could go wrong here? <br>
</blockquote>
Jar signing still could fail. For example, TSA service is
unavailable. <br>
<br>
Best regards, <br>
John Jiang <br>
<blockquote type="cite"> <br>
Thanks <br>
Max <br>
<br>
On 06/12/2017 03:20 PM, <a class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a>
wrote: <br>
<blockquote type="cite">Hi Max, <br>
Would you like to review the updated webrev: <a
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejjiang/8179614/webrev.02/">http://cr.openjdk.java.net/~jjiang/8179614/webrev.02/</a>
<br>
It can create certificate without -sigalg and -keysize, and
jar signing also can use this certificate. <br>
<br>
Best regards, <br>
John Jiang <br>
<br>
On 09/06/2017 22:04, Weijun Wang wrote: <br>
<blockquote type="cite"> <br>
On 06/09/2017 09:25 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a>
wrote: <br>
<blockquote type="cite">Hi Max, <br>
<br>
On 09/06/2017 20:05, Weijun Wang wrote: <br>
<blockquote type="cite">The test can be more friendly
with default values. <br>
<br>
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. <br>
<br>
And in jar signing when signing with one -sigalg you
can also choose cert generated with different or
default sigalgs. <br>
</blockquote>
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. <br>
</blockquote>
<br>
Well sometimes a test can do many things. If you only care
about jar verification, why bother creating certs with
different digest algorithms? <br>
<br>
On the other hand, if you do care about more, then in <br>
<br>
338 // If the digest algorithm is not specified, then
it <br>
339 // uses certificate with SHA256 digest and 1024
key <br>
340 // size. <br>
341 if (digestAlgorithm == DEFAULT) { <br>
342 certDigest = SHA256; <br>
343 certKeySize = 1024; <br>
344 } <br>
<br>
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. <br>
<br>
--Max <br>
<br>
<blockquote type="cite">
<blockquote type="cite"> <br>
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? <br>
</blockquote>
It looks SHA512withDSA is not supported yet. <br>
I was using JDK10 build 10. When the test tried to
create certificate with -keyalg DSA -sigalg
SHA512withDSA -keysize 1024, the below error raised: <br>
keytool error: java.security.NoSuchAlgorithmException:
unrecognized algorithm name: SHA512withDSA <br>
<br>
If used -keyalg DSA -sigalg SHA1withDSA -keysize 2048,
the error was: <br>
keytool error: java.security.InvalidKeyException: The
security strength of SHA-1 digest algorithm is not
sufficient for this key size <br>
<br>
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.
<br>
<br>
Best regards, <br>
John Jiang <br>
<blockquote type="cite"> <br>
Thanks <br>
Max <br>
<br>
<br>
On 06/09/2017 04:44 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a>
wrote: <br>
<blockquote type="cite">Hi Sean and Max, <br>
Thanks for your comments. <br>
Please review the updated webrev: <a
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejjiang/8179614/webrev.01/">http://cr.openjdk.java.net/~jjiang/8179614/webrev.01/</a>
<br>
<br>
The test has been modified significantly. The main
points are: <br>
1. Adds cases on EC. Now the test supports key
algorithms RSA, DSA and EC. <br>
2. Adds cases on SHA-512. Now the test supports
digest algorithms SHA-1, SHA-256 and SHA-512. <br>
3. Adds cases on key size. Exactly, [384, 571] for
EC, [1024, 2048] for RSA and DSA. <br>
4. Adds cases on default signature algorithm. Now
the test report can display the default algorithmat
column [Signature Algorithm]. <br>
5. Adds property -Djava.security.egd=<a
class="moz-txt-link-freetext"
href="file:/dev/./urandom">file:/dev/./urandom</a>
for keytool and jarsigner commands. <br>
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. <br>
7. Introduces a new property, named
javaSecurityFile, for allowing users to specify
alternative java security properties file. <br>
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). <br>
9. The test summary also be updated accordingly. <br>
<br>
Best regards, <br>
John Jiang <br>
<br>
<br>
On 07/06/2017 23:11, Sean Mullan wrote: <br>
<blockquote type="cite">On 6/6/17 9:14 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a>
wrote: <br>
<blockquote type="cite">Hi Sean, <br>
<br>
On 07/06/2017 04:27, Sean Mullan wrote: <br>
<blockquote type="cite">Hi John, <br>
<br>
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: <br>
<br>
- add tests for EC keys <br>
- add tests for SHA-512 variants of the
signature algorithms <br>
- add tests for larger key sizes (ex: 2048 for
DSA/RSA) <br>
- you can use the diamond operator <> in
various places <br>
- might be more compact if jdkList() used
Files.lines() to parse the file into a stream
then an array <br>
</blockquote>
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. <br>
I supposed that would make the backport easier.
Does it make sense? <br>
</blockquote>
<br>
Yes, that makes sense. <br>
<br>
--Sean <br>
<br>
<blockquote type="cite"> <br>
Best regards, <br>
John Jiang <br>
<blockquote type="cite">- 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. <br>
<br>
--Sean <br>
<br>
On 6/5/17 6:31 AM, <a
class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com">sha.jiang@oracle.com</a>
wrote: <br>
<blockquote type="cite">Hi, <br>
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. <br>
It also can be used to check if the default
timestamp digest algorithm on signing is
SHA-256. <br>
For more details, please look through the
test summary. <br>
<br>
Issue: <a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8179614">https://bugs.openjdk.java.net/browse/JDK-8179614</a>
<br>
Webrev: <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejjiang/8179614/webrev.00/">http://cr.openjdk.java.net/~jjiang/8179614/webrev.00/</a>
<br>
<br>
Best regards, <br>
John Jiang <br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
<br>
</blockquote>
<br>
<br>
</blockquote>
<br>
</body>
</html>