[11u] RFR: 8171279: Support X25519 and X448 in TLS

[Doerr, Martin]

Hi Götz,

thanks for backporting this change. I think it's good to put it into 11.0.10, now, so we have enough testing time.
Please find comments and questions inline.


> file src/java.base/share/classes/sun/security/ssl/CipherSuite.java
> Non-applying chunk fixes typo in a comment that is not in 11.
> I copied the comment to 11.
Good.


> file src/java.base/share/classes/sun/security/ssl/DHKeyExchange.java
> Deleting class DHEKAKeyDerivation failed because the code in 11 uses
> JsseJce.getKeyAgreement()
> where the patch uses KeyAgreement.getInstance().
Right.


> file
> src/java.base/share/classes/sun/security/ssl/ECDHClientKeyExchange.java
> 11 has different imports.
> The remaining conflicts arise from different usages of KeyFactory, ECUtil and
> JsseJce.
Good.


> file src/java.base/share/classes/sun/security/ssl/ECDHKeyExchange.java
> The conflicts arise from different usages of KeyAgreement and JsseJce.
Ok.


> file
> src/java.base/share/classes/sun/security/ssl/ECDHServerKeyExchange.java
> Different imports.
> The remaining conflicts arise from different usages of KeyFactory, ECUtil and
> JsseJce.
Ok.


> file src/java.base/share/classes/sun/security/ssl/SSLExtension.java
> Copyright
> 
> file src/java.base/share/classes/sun/security/ssl/SSLSocketInputRecord.java
> Copyright
We don't edit Copyright headers for backports. Ok.


> file src/java.base/share/classes/sun/security/ssl/SignatureScheme.java
> imports
Ok.


> file
> src/java.base/share/classes/sun/security/ssl/SupportedGroupsExtension.jav
> a
> Different imports.
Ok.

> Enums are moved to a file of their own: NamedGroup.java
> Removing the enums does not apply because in 13 FIPS support was
> removed.
So you just deleted them manually. Fine.

> Also, a name string was added.
You mean "String name" in the code you removed?

> The supported group types differ, too.
That's expected. Did that cause manual integration?

> In 11, the enums have a row of additional functionality that
> I now removed, too.  E.g., additional constructors, isSupported(),
> isECAvailable.
That's in the code which is gone. That's fine.
 
> Some further differences are again usages of ECUtil, JsseJce etc.
> The original patch has more NamedGroups: "// Secondary NIST curves"
Ok.

> file test/jdk/javax/net/ssl/templates/SSLSocketTemplate.java
> configureClientSocket() already in 11. We downported this with
> a previous change.
Ok. I can see this code is already there.

> The NamedGroups enum is moved to a file of it's own.
> Before this change, the named Groups knew about FIPS in 11. The new ones
> don't.
> https://bugs.openjdk.java.net/browse/JDK-8217835 "Remove the
> experimental SunJSSE FIPS compliant mode"
> removed FIPS support in 13 before this change was implemented.
>
> As I understand 8217835, the experimental FIPS feature cannot be used in
> 11, it just remained in the code. So I skipped adapting NamedGroups to the
> old behaviour, I can not test it anyways.
> What do you think, do we need to add this code to NamedGroupd.java
> again?
Sounds like this is ok according to the CSR https://bugs.openjdk.java.net/browse/JDK-8217907
Since this file is new with this backport, I prefer keeping it like the original one for the backport.
We could open a new bug in case anybody wants it back, but I guess that's not the case.

There's still some Fips code there:
(!requireFips || namedGroup.isFips)*/     ) {  // GL fix isFips
With a comment!
What are you planning to do with it?


> There are two follow ups,
> 8224650: Add tests to support X25519 and X448 in TLS
> 8243549: sun/security/ssl/CipherSuite/NamedGroupsWithCipherSuite.java
> failed with Unsupported signature algorithm: DSA
> 
> I have downported these too, and will send RFR once this is stable.
> Existing and new test are all passing.
> 
> https://bugs.openjdk.java.net/browse/JDK-8171279
> http://hg.openjdk.java.net/jdk/jdk/rev/946f7f2d321c
Ok. That's an important point.
There may be more to come, but that's all we currently have AFAICS.


Best regards,
Martin