On Wed, 7 Apr 2021 23:17:53 GMT, Hai-May Chao <hc...@openjdk.org> wrote:
>> Please review the changes that adds the -signer option to keytool >> -genkeypair command. As key agreement algorithms do not have a signing >> algorithm, the specified signer's private key will be used to sign and >> generate a key agreement certificate. >> CSR review is at: https://bugs.openjdk.java.net/browse/JDK-8264325 > > Hai-May Chao has updated the pull request incrementally with one additional > commit since the last revision: > > update with review comments No comment on src side. src/java.base/share/classes/sun/security/tools/keytool/CertAndKeyGen.java line 88: > 86: */ > 87: public CertAndKeyGen (String keyType, String sigAlg, String > providerName) > 88: throws NoSuchAlgorithmException, NoSuchProviderException Indent the line 8 spaces further. test/jdk/sun/security/tools/keytool/GenKeyPairSigner.java line 96: > 94: > 95: Certificate[] certChain = kstore.getCertificateChain("e1"); > 96: if (certChain.length != 2) { Try using `Asserts` class in `/test/lib` to make code simpler. Also, why not throw an exception but call `System.exit(1)`? We usually do not call this method in a test because the test framework must take great care so that itself does not get terminated. test/jdk/sun/security/tools/keytool/GenKeyPairSigner.java line 299: > 297: System.exit(1); > 298: } > 299: Since you are here, you can check if the new entry is indeed protected by the new key password. ------------- PR: https://git.openjdk.java.net/jdk/pull/3281
