RFR: 8280494: (D)TLS signature schemes

[Bernd]

On Thu, 27 Jan 2022 22:06:21 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

I like the per-parameters configuration and we really should get it for more Systeme properties in the future. But I wonder if the additional allocations (some while doing handshakes?) can be avoided by doing more parsing/analysing in the parameter object already. It could even do the analysis of the system property at that time. Also the toggle semantic seems to be a bit different then you would expect, a extra flag to keep track of customized settings would probably fix that and avoid the array compares.

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

> 90:     private int maximumPacketSize = 0;
> 91:     private String[] applicationProtocols = new String[0];
> 92:     private String[] signatureSchemes = new String[0];

There are some places in here which could use a local or global static EMPTY = new String[0]` value. That would be safe to share and reduces allocations (they surely escape).

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

> 743:      * This method will make a copy of the {@code signatureSchemes} array.
> 744:      *
> 745:      * @param signatureSchemes an ordered array of signature scheme names,

Should it mention that unknown schemes are ignored and not trigger a exception?

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

> 761: 
> 762:         String[] tempSchemes = signatureSchemes.clone();
> 763:         for (String scheme : tempSchemes) {

In addition to this loop we could also parse/decompose the strings. This would do the work only once (if the parameter is reused)

src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 66:

> 64:     // The configured signature schemes for "signature_algorithms" and
> 65:     // "signature_algorithms_cert" extensions
> 66:     String[]                   signatureSchemes;

I would keep the typed list (get it from the parameters or system property cache).

src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 262:

> 260:         }   // otherwise, use the default values
> 261: 
> 262:         String[] ss = params.getSignatureSchemes();

If we don’t use the cloning getter here (and use the pre-parsed list) it would be more efficient.

src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 410:

> 408: 
> 409:         // Reset the signature schemes, if it was configured with SSLParameters.
> 410:         if (Arrays.equals(signatureSchemes,

The duals here does not mean it was configure with parameters, it only means it is currently the same configuration as the parameters, but it still could be a different source?

src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 439:

> 437:                     (config.signatureSchemes == null ||
> 438:                         config.signatureSchemes.length == 0 ||
> 439:                         Arrays.asList(config.signatureSchemes)

Is that a allocation per handshake*availablesize?

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

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