RFR: 8280494: (D)TLS signature schemes [v8]

[Sean Mullan]

On Tue, 1 Feb 2022 06:47:00 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> This update is to support signature schemes customization for individual (D)TLS connection.  Please review the CSR as well:
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8280495
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8280494
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Allow null input and return values

A few more comments on the API.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 706:

> 704:      * over the SSL/TLS/DTLS protocols.
> 705:      * <p>
> 706:      * Note that the standard list of signature scheme names may be found in

s/may be found/are defined/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 710:

> 708:      * "{@docRoot}/../specs/security/standard-names.html#signature-schemes">
> 709:      * Signature Schemes</a> section of the Java Security Standard Algorithm
> 710:      * Names Specification. Providers may support signature schemes not found

s/found/defined/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 711:

> 709:      * Signature Schemes</a> section of the Java Security Standard Algorithm
> 710:      * Names Specification. Providers may support signature schemes not found
> 711:      * in this list or might not use the recommended name for a certain

s/might/may/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 714:

> 712:      * signature scheme.
> 713:      * <p>
> 714:      * The returned array could be {@code null}, in which case the underlying

s/could be/may be/

Suggest rewording as "If the returned array is {@code null}, then the underlying ..."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 718:

> 716:      * SSL/TLS/DTLS connections.
> 717:      * <p>
> 718:      * The returned array could be empty (zero-length), in which case the

Suggest rewording as "If the returned array is empty (zero-length), then the ..."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 720:

> 718:      * The returned array could be empty (zero-length), in which case the
> 719:      * signature scheme negotiation mechanism is turned off for SSL/TLS/DTLS
> 720:      * protocols, and the connections may not be able estabilished if the

s/estabilished/to be established/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 723:

> 721:      * negotiation mechanism is required by a certain SSL/TLS/DTLS protocol.
> 722:      * <p>
> 723:      * The returned array could be neither {@code null} nor empty (zero-length),

Suggest rewording as "If the returned array is not {@code null} or empty (zero-length), then the signature schemes .."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 730:

> 728:      * For non-null returns, this method will return a new array each time it
> 729:      * is invoked. Providers should ignore unknown signature scheme names
> 730:      * while establishing the SSL/TLS/DTLS connections.

I think this condition should be part of the API specification, and not the implementation specification. This needs to be a required part of the API, otherwise applications cannot safely rely on other implementations of this class to return copies. I suggest moving it to the `@returns` clause.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 736:

> 734:      * schemes for each SSL/TLS/DTLS connection.  Applications may also use
> 735:      * System Property, {@systemProperty jdk.tls.client.SignatureSchemes}
> 736:      * and/or {@systemProperty jdk.tls.server.SignatureSchemes}, to customize

Suggest rewording as "the {@systemProperty jdk.tls.client.SignatureSchemes} and/or {@systemProperty jdk.tls.server.SignatureSchemes} system properties to customize ..."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 742:

> 740:      * objects, or {@code null} for pre-populated objects.
> 741:      *
> 742:      * @return {@code null} or an array of signature scheme {@code String}s.

Suggest rephrasing as "an array of signature scheme {@code Strings} or {@null} if none have been set."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 757:

> 755:      * can be used over the SSL/TLS/DTLS protocols.
> 756:      * <p>
> 757:      * Note that the standard list of signature scheme names may be found in

s/may be found/are defined/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 761:

> 759:      * "{@docRoot}/../specs/security/standard-names.html#signature-schemes">
> 760:      * Signature Schemes</a> section of the Java Security Standard Algorithm
> 761:      * Names Specification. Providers may support signature schemes not found

s/found/defined/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 762:

> 760:      * Signature Schemes</a> section of the Java Security Standard Algorithm
> 761:      * Names Specification. Providers may support signature schemes not found
> 762:      * in this list or might not use the recommended name for a certain

s/might/may/

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 766:

> 764:      *
> 765:      * @implSpec
> 766:      * This method will make a copy of the {@code signatureSchemes} array.

Should be part of API specification (see my similar comment on `getSignatureSchemes`.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 769:

> 767:      *
> 768:      * @param signatureSchemes {@code null} or an ordered array of signature
> 769:      *        scheme names, with the first entry being the most preferred.

Suggest rewording as: "an ordered array of signature scheme names with the first entry being the most preferred, or {@null}."

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 770:

> 768:      * @param signatureSchemes {@code null} or an ordered array of signature
> 769:      *        scheme names, with the first entry being the most preferred.
> 770:      * @throws IllegalArgumentException if any element in the non-empty

I don't think the word "non-empty" is necessary as an empty array has no elements.

src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 786:

> 784:                 if (scheme == null || scheme.isBlank()) {
> 785:                     throw new IllegalArgumentException(
> 786:                         "An element of signatureSchemes was null or blank");

s/was/is/

-------------

PR: https://git.openjdk.java.net/jdk/pull/7252