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

[Lindenmaier, Goetz]

Hi,

I would like to get a second opinion on how to deal
with the lost FIPS coding in this downport. 
Could someone from outside SAP have a look please?

Thanks,
  Goetz.


> -----Original Message-----
> From: Doerr, Martin <martin.doerr at sap.com>
> Sent: Tuesday, October 6, 2020 8:12 PM
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; jdk-updates-dev
> <jdk-updates-dev at openjdk.java.net>
> Subject: RE: [11u] RFR: 8171279: Support X25519 and X448 in TLS
> 
> 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