[8u] RFR 8035166: Remove dependency on EC classes from pkcs11 provider

[Alexander Scherbatiy]

On 12/3/20 8:30 PM, Andrew Hughes wrote:

> On 16:36 Thu 03 Dec     , Andrew Hughes wrote:
>> On 19:30 Thu 03 Dec     , Alexander Scherbatiy wrote:
>>> Hello,
>>>
>>> Could you review the backport of JDK-8035166 to 8u.
>>> This backport was requested during review [1] of
>>>     JDK-8233228 Disable weak named curves by default in TLS, CertPath, and
>>> Signed JAR
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8035166
>>> 11u patch: https://hg.openjdk.java.net/jdk-updates/jdk11u/rev/daa21271c03b
>>> 8u webrev: http://cr.openjdk.java.net/~alexsch/sercher/8035166/webrev.00
>>>
>>> The classes ECParameters, NamedCurve, and CurveDB needs to be moved from
>>> sun.security.ec package
>>> to sun.security.util for JDK-8233228 because sun.security.ec is placed in
>>> sunec.jar and these classes are not accessible
>>> from ConstraintsParameters, DisabledAlgorithmConstraints which are stored in
>>> rt.jar.
>>>
>>> 8035166 backport to 8u (compared to 11u):
>>> * Manual merge in ConstraintsParameters.java (XECKey, NamedParameterSpec are
>>> not available in 8u).
>>> * files java.security-<platform> are separate in each platform, applied
>>> identical changes in all
>>> * context differences in multiple files
>>>
> ^ This is JDK-8233228, isn't it? Those files aren't touched by JDK-8035166.
  Sorry.

Here's the summary of conflicts resolved (compared to 11u version)
* the patch doesn't apply to DOMKeyValue.java, as it doesn't access 
ECParameters class.
* SupportedEllipticCurvesExtension.java is not present in 8u
* context difference in ECKeyPairGenerator.java, SunPKCS11.java
* copyright note in CurveDB is already up to date

>
>>> The tests compact3, java_security, java_security_infra, needs_jdk, and
>>> needs_jre were run.
>>>
>>> In total they contain the following security and crypto tests:
>>>    sun/security/tools/jarsigner/*
>>>    com/sun/crypto/provider/*
>>>    com/sun/security/*
>>>    java/security/*
>>>    javax/crypto/*
>>>    javax/net/ssl/*
>>>    javax/security/*
>>>    javax/xml/crypto/*
>>>    sun/security/*
>>>    security/infra/java/security/*
>>>
>>> The are no new failures comparing to the build without the fix.
>>>
>>> [1]
>>> https://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-December/013164.html
>>>
>>> Thanks,
>>> Alexander.
>>>
>> It's still not been explained why these changes are required for JDK-8233228.
>> Can you please answer that question? There may be no need for this backport
>> if JDK-8233228 can be done another way.
>>
>> Thanks,
>> -- 
>> Andrew :)
>>
>> Senior Free Java Software Engineer
>> OpenJDK Package Owner
>> Red Hat, Inc. (http://www.redhat.com)
>>
>> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
>> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
> Sorry, missed your explaination on first glance at this.
>
> The patch looks fine. The omission of the DOMKeyValue changes is
> correct, as they were removed by 8046724: "XML Signature ECKeyValue
> elements cannot be marshalled or unmarshalled". The omission of the
> SSL change is correct, as it seems the TLSv1.3 import is already
> referring to a sun.security.util.CurveDB that doesn't yet exist! [0]
>
> Please flag the bug with jdk8u-fix-request. This is not a regression
> fix and so is not suitable for 8u282 during rampdown. Such labels
> should also not be applied before review is complete.

  Thank you for the review. The jdk8u-fix-request flag has been added to 
the JDK-8035166.

  Thanks,
  Alexander.

>
> [0] https://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/tip/src/share/classes/sun/security/ssl/SupportedGroupsExtension.java#l181
>
> Thanks,