On Fri, 20 Nov 2020 18:37:36 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> 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/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 }
JsseJce.isEcAvailable doesn't check for EdDSA availability so I'm not sure we
want that second clause. I don't think the EdDSA code is implemented in the
same module as the other EC code is so I don't know if we'd want to extend
isEcAvailable to include EdDSA. I'll need to go look to see where EdDSA is
located relative to the EC code.
> 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).
That's probably a good idea. It doesn't look like we're making changes to the
List outside of the constructor.
> 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.
Yeah, I thought about that. I just like having all the loop termination
clauses in one place rather than testing in the loop body and breaking. I can
change it to for-each though, no strong feelings one way or the other.
> 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?
Will change to for-each.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1197
