On Thu, 19 Nov 2020 17:48:34 GMT, Jamil Nimeh <jni...@openjdk.org> wrote:
>> Hello all,
>> This change brings in support for certificates with EdDSA keys (both Ed25519
>> and Ed448) allowing those signature algorithms to be used both on the
>> certificates themselves and used during the handshaking process for messages
>> like CertificateVerify, ServerKeyExchange and so forth.
>
> Jamil Nimeh has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains seven additional
> commits since the last revision:
>
> - Update test to account for JDK-8202343 fix
> - Merge
> - Merge
> - Applied code review comments to tests
> - Fix cut/paste error with ECDH-RSA key exchange
> - Merge
> - Initial EdDSA/TLS solution
src/java.base/share/classes/sun/security/ssl/CertificateRequest.java line 83:
> 81: ECDSA_SIGN ((byte)0x40, "ecdsa_sign",
> 82: List.of("EC", "EdDSA"),
> 83: JsseJce.isEcAvailable()),
Would you like to add RFC 8422 to the comment line as well?
src/java.base/share/classes/sun/security/ssl/JsseJce.java line 97:
> 95: */
> 96: static final String SIGNATURE_EDDSA = "EdDSA";
> 97:
Please update the copyright year.
Is it possible that "ed25519" or "ed448" is used as the signature algorithm,
especially in the X.509 certificate implementation?
src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 73:
> 71: ED448 (0x0808, "ed448", "Ed448",
> 72: "EdDSA",
> 73: ProtocolVersion.PROTOCOLS_12_13),
You may also want to check if EdDSA is available in the following block:
- 282 if ("EC".equals(keyAlgorithm)) {
+ 282 if ("EC".equals(keyAlgorithm) || "EdDSA"... ) {
283 mediator = JsseJce.isEcAvailable();
284 }
src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 48:
> 46: if (authentication != null) {
> 47: this.authentication = new ArrayList<>();
> 48: this.authentication.addAll(authentication);
I may use an immutable list, for example List.copyOf(authentication).
src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 134:
> 132: SSLAuthentication authType = li.next();
> 133: auHandshakes =
> authType.getRelatedHandshakers(handshakeContext);
> 134: }
I guess for-each loop could be a little bit more lightweight.
src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 163:
> 161: SSLAuthentication authType = li.next();
> 162: auProducers =
> authType.getHandshakeProducers(handshakeContext);
> 163: }
Use for-each?
-------------
PR: https://git.openjdk.java.net/jdk/pull/1197
