On 7/26/2019 7:08 AM, Xuelei Fan wrote:
New webrev:
http://cr.openjdk.java.net/~xuelei/8226374/webrev.03/
* src/java.base/share/classes/sun/security/ssl/ECDHServerKeyExchange.java
114 if ((namedGroup == null) || (!namedGroup.isAvailable)) {
You don't do this check for null and isAvailable in other places, for
example ECDHClientKeyExchange.ECDHEClientKeyExchangeConsumer.consume()
- should you?
Good point! Currently, the restriction is only checked for the
supported group extension. I should add more check points in other
places where named groups are used, for example client key exchange and
certificate. Stay tune for the next webrev.
The ECDHClientKeyExchange.ECDHEClientKeyExchangeConsumer.consume()
should be fine as the namedGroup has been checked in the previous steps
(X509Authentication.X509PossessionGenerator.createServerPossession()).
However, I missed the check for certificate. The consumer of
certificate should check the named groups to make sure the supported
named group is used. It was not a problem in the past as the supported
named groups are used to indicate the EC curve or DH group is able to be
handled in both side.
It could be a problem now when we want to restrict named groups. The
named groups used in a certificate should be checked in key manager and
trust manager for TLS 1.2 and prior versions. Similar to the signature
schemes for TLS 1.3. As may required new APIs
(SSLParameters.getPeerSupportedNamedGroups()) for a generic solution.
Would you mind if I file a new RFE and make the improvement in JDK 14 later?
Thanks,
Xuelei
