RFR: 8262186: Call X509KeyManager.chooseClientAlias once for all key types [v2]

[Xue-Lei Andrew Fan]

On Mon, 30 Aug 2021 14:56:29 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/ssl/CertificateMessage.java line 1047:
>> 
>>> 1045: 
>>> 1046:             Collection<String> checkedKeyTypes = new HashSet<>();
>>> 1047:             LinkedHashSet<String> allAuths = new LinkedHashSet<>();
>> 
>> I was wondering if it is necessary to use a linked hash set here.  Maybe, we can reuse the checkedKeyTypes for all checked key types, no matter it is supported or unsupported.  If a type is checked, ignore it.  Otherwise, add this type as checked and then move forward.  Then, we could use the checkedKeyTypes array list for supported key types, and avoid duplicated validation of duplicated types.  And the allAuths could be a array list for available authentications.
>> 
>> 
>> if (ss.keyAlgorithm in checkedKeyTypes) {
>>    // log: "unsupported or duplicated authentication scheme"
>>    continue;
>> }
>> 
>> checkedKeyTypes.add(ss.keyAlgorithm);
>>  ...
>> availableAuthns.add(ss.keyAlgorithm);
>
> Good suggestion.
> 
> One more thing: `checkedKeyTypes` only looks at `ss.keyAlgorithm`. I know the other checks (`SignatureScheme.getPreferableAlgorithm` and `X509Authentication.valueOf`) also only look at `ss.keyAlgorithm`, but are we going to check for more (Ex: group name) later? In the meantime, I would suggest changing the parameter type of these methods from `SignatureScheme` to `String` so we know only `keyAlgorithm` is checked.

Did you mean to change hc.peerRequestedCertSignSchemes to String?  This field would be used for algorithm constraints and more parameters are required there.

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

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