[jdk11u-dev] RFR: 8242151: Improve OID mapping and reuse among JDK security providers for aliases registration [v3]

Tue Jun 13 13:45:05 UTC 2023

On Tue, 13 Jun 2023 09:21:13 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>> This change required some work to get it backported. 
>> I touched the following files to resolve or get the code/test working:
>> 
>> src/java.base/share/classes/com/sun/crypto/provider/SunJCE.java
>> 
>> These entries are not in 11:
>>         List<String> macSHA512_224Aliases = createAliasesWithOid(macOidBase + "12");
>>         List<String> macSHA512_256Aliases = createAliasesWithOid(macOidBase + "13");
>> 
>>         String nistHashAlgsOidBase = "2.16.840.1.101.3.4.2.";
>>         List<String> macSHA3_224Aliases =
>>             createAliasesWithOid(nistHashAlgsOidBase + "13");
>>         List<String> macSHA3_256Aliases =
>>             createAliasesWithOid(nistHashAlgsOidBase + "14");
>>         List<String> macSHA3_384Aliases =
>>             createAliasesWithOid(nistHashAlgsOidBase + "15");
>>         List<String> macSHA3_512Aliases =
>>             createAliasesWithOid(nistHashAlgsOidBase + "16");
>> 
>> Thus I could not remove them.
>> Also, the Hmac keys differ. Some are not in 11.
>> 
>> src/java.base/share/classes/java/security/cert/CertificateRevokedException.java
>> Copyright.
>> 
>> src/java.base/share/classes/sun/security/pkcs/PKCS7.java
>> src/java.base/share/classes/sun/security/pkcs/PKCS9Attribute.java
>> Already applied in "8268801: Improve PKCS attribute handling"
>> 
>> src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java
>> Resolve imports due to context.
>> 
>> src/java.base/share/classes/sun/security/provider/KeyProtector.java
>> Copyright
>> 
>> src/java.base/share/classes/sun/security/ssl/SunJSSE.java
>> Resolved due to context.
>> 
>> src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java
>> Copyright.
>> 
>> src/java.base/share/classes/sun/security/tools/keytool/Main.java
>> Basically applied by hand.
>> 
>> src/java.base/share/classes/sun/security/util/ConstraintsParameters.java
>> This file was turned into an interface by "8249906: Enhance opening JARs" which
>> moved the implementation to CertPathConstraintsParameters.java and DisabledAlgorithmConstraints.
>> 8249906 was applied later than this change, but is already in 11.
>> 
>> The method getNamedCurveFromKey(Key key) was moved to DisabledAlgorithmConstraints, I
>> applied the hunk there similar as in 17.
>> 
>> src/java.base/share/classes/sun/security/util/CurveDB.java
>> I had to resolve because "8226307: Curve names should be case-insensitive" is not in 11.
>> 
>> src/java.base/share/classes/sun/security/util/KnownOIDs.java,
>> Already in 11, a small difference in line 428 which seem fine to ...
>
> Goetz Lindenmaier has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits:
> 
>  - Merge master after pushing 8239264
>  - Merge branch 'goetz_backport_8239264' into goetz_backport_8242151
>  - Add back newInternal()
>  - Fix build and tests
>  - Resolve
>  - Backport 080b3b83ebffe5149fbc9ac48e921fb51e9c3c63
>  - remove of1()
>  - Fix whitespace
>  - Make it compile
>  - Resolve patch
>  - ... and 1 more: https://git.openjdk.org/jdk11u-dev/compare/5067ea24...ef076a2a

Hi Martin,

thanks for going through this huge change!

> In sun/security/util/ConstraintsParameters.java:
>     If there weren't any changes to the file, we should probably keep the current copyright date in the header.

We usually always backport the Copyrights and resolve to the later year if necessary.
This file should have been edited by this change, but the edit was made in
8249906 which forgot to adapt the Copyright. So I would like adapt this now here.

> In sun/security/util/CurveDB.java:
>     I have the impression that we don't need to call "trim" anymore because the curve name and aliases come from KnownOIDs and do not have any spaces at the beginning or end. This was necessary before because we were splitting strings such as "secp192r1 [NIST P-192, X9.62 prime192v1]" to get the name and aliases separately. You can check how this same information now comes from "secp192r1("1.2.840.10045.3.1.1", "secp192r1", "NIST P-192", "X9.62 prime192v1")" in KnownOIDs.

Makes sense. Fixed.

> In sun/security/util/ObjectIdentifier.java:
>     Is it possible to remove "public ObjectIdentifier(int[] values)" and "public static ObjectIdentifier newInternal(int[] values)" now? This could be part of 8239264 backport perhaps. This will make help to check that there are no current uses and enforce that it's not used in the future.

I removed it before. Build and tests pass without it.
But Matthias asked me to add it again to not break compatibility.

> In sun/security/x509/AlgorithmId.java:
>     There are a couple of OIDs which were not part of jdk11u: ed25519_oid and ed448_oid.

Fixed. Thanks for this catch!

>
> In sun/security/x509/OIDName.java:
>     Should we bump the copyright date?

Yes, fixed.

> In sun/security/jgss/GSSNameImpl.java:
>     Should we bump the copyright date?

Yes, fixed.

> In sun/security/jgss/wrapper/GSSNameElement.java:
>     Should we bump the copyright date?

Yes, fixed.

> In sun/security/jgss/wrapper/NativeGSSContext.java:
>     Should we bump the copyright date?

Yes, fixed.

> In sun/security/ec/SunEC.java:
>     I think that we can remove the import of java.util.regex.Pattern because it's unused.

Thanks, fixed.

> In sun/security/pkcs12/ParamsPreferences.java:
>     I'd add that 8242151 is related to this test.

Yes, fixed.

> In sun/security/tools/keytool/KeyToolTest.java:
>     Copyright date bump?
>     Shouldn't we indicate that it's related to 8242151?

Yes, fixed both.

> Shouldn't we indicate that KeytoolOpensslInteropTest.java is related to 8242151?

Yes! Fixed.

Best regards,
  Goetz.

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

PR Comment: https://git.openjdk.org/jdk11u-dev/pull/1908#issuecomment-1589340666