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.ji...@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.ji...@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.ji...@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.ji...@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.ji...@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.ji...@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.ji...@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.ji...@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.ji...@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
