RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]

[Jamil Nimeh]

On Fri, 20 Nov 2020 18:37:36 GMT, Xue-Lei Andrew Fan <xuelei at 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