8026976: ECParameters, Point does not match field size #2958

kurashige23 · 2024-10-21T07:15:55Z

Hi all,

This is a backport of JDK-8026976: ECParameters, Point does not match field size

Original patch apply cleanly to 11u.

Testing: jdk/sun/security/pkcs11 tests on RHEL9, GHA testing

Thanks.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
JDK-8026976 needs maintainer approval

Issue

JDK-8026976: ECParameters, Point does not match field size (Bug - P4 - Requested)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/2958/head:pull/2958
$ git checkout pull/2958

Update a local copy of the PR:
$ git checkout pull/2958
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/2958/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2958

View PR using the GUI difftool:
$ git pr show -t 2958

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/2958.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-10-21T07:16:13Z

👋 Welcome back kurashige23! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-10-21T07:16:51Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2024-10-21T07:17:10Z

This backport pull request has now been updated with issue from the original commit.

openjdk · 2024-10-21T07:17:52Z

⚠️ @kurashige23 This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

mlbridge · 2024-10-21T07:21:44Z

Webrevs

kurashige23 · 2024-10-21T07:24:44Z

/approval request Clean backport. The risk is low because the fix to the source is only for P11ECKeyFactory.java and it is just addition of switching by UseEcX963Encoding. pkcs11 tests on RHEL9 and GHA tests pass.

openjdk · 2024-10-21T07:25:04Z

@kurashige23
8026976: The approval request has been created successfully.

kurashige23 · 2024-11-05T00:37:02Z

Could anyone review this backport please?

kurashige23 · 2024-11-27T04:16:52Z

Could anyone review this backport please?

bridgekeeper · 2024-12-25T09:17:10Z

@kurashige23 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

kurashige23 · 2025-01-06T10:54:53Z

Comment to avoid automatic close.

bridgekeeper · 2025-02-03T14:07:22Z

@kurashige23 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

kurashige23 · 2025-02-05T23:41:37Z

@gnu-andrew

Sorry for the sudden mention, if possible, could you please review this PR?

jerboaa · 2025-02-20T16:39:36Z

@martinuy @franferrax Could you please take a look at this backport and see if that makes sense to bring to 11u at this stage of where 11u is currently? Thanks!

franferrax · 2025-02-24T14:59:07Z

@jerboaa: in my view this is a minor and clean backport. 11u already contains e6e820c, which is partial/incomplete without this change.

I agree that the risk is low considering this affects SunPKCS11, a security provider that is disabled by default.

The test removed from ProblemList.txt (sun/security/pkcs11/ec/TestKeyFactory.java) is now passing (I checked this in a local slowdebug build of this PR code). This test fails with the current version of NSS and without this PR change, meaning this bug may be being hit by users:

java.security.spec.InvalidKeySpecException: Could not parse key
	at jdk.crypto.cryptoki/sun.security.pkcs11.P11ECKeyFactory.implGetPublicKeySpec(P11ECKeyFactory.java:300)
	at jdk.crypto.cryptoki/sun.security.pkcs11.P11KeyFactory.engineGetKeySpec(P11KeyFactory.java:94)
	at java.base/java.security.KeyFactory.getKeySpec(KeyFactory.java:433)
	at TestKeyFactory.testPublic(TestKeyFactory.java:83)
	at TestKeyFactory.test(TestKeyFactory.java:117)
	at TestKeyFactory.main(TestKeyFactory.java:146)
	at PKCS11Test.premain(PKCS11Test.java:907)
	at PKCS11Test.testNSS(PKCS11Test.java:605)
	at PKCS11Test.main(PKCS11Test.java:254)
	at TestKeyFactory.main(TestKeyFactory.java:124)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.io.IOException: Point does not match field size
	at java.base/sun.security.util.ECUtil.decodePoint(ECUtil.java:48)
	at jdk.crypto.cryptoki/sun.security.pkcs11.P11ECKeyFactory.decodePoint(P11ECKeyFactory.java:89)
	at jdk.crypto.cryptoki/sun.security.pkcs11.P11ECKeyFactory.implGetPublicKeySpec(P11ECKeyFactory.java:297)
	... 15 more

franferrax · 2025-02-24T14:59:47Z

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11ECKeyFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2006, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.


@jerboaa: is it generally acceptable to downgrade the copyright year? Should we leave 2021 untouched? Should we update to 2025?

No. If the existing copyright date is newer we usually leave it alone.

So @kurashige23, could you please revert it back to 2021? According to openjdk/skara#1556, the Skara bot should still consider the backport clean.

I fixed it. Thank you for your review.

jerboaa · 2025-02-24T15:23:23Z

@jerboaa: in my view this is a minor and clean backport. 11u already contains e6e820c, which is partial/incomplete without this change.

I agree that the risk is low considering this affects SunPKCS11, a security provider that is disabled by default.

The test removed from ProblemList.txt (sun/security/pkcs11/ec/TestKeyFactory.java) is now passing (I checked this in a local slowdebug build of this PR code). This test fails with the current version of NSS and without this PR change, meaning this bug may be being hit by users:

java.security.spec.InvalidKeySpecException: Could not parse key
	at jdk.crypto.cryptoki/sun.security.pkcs11.P11ECKeyFactory.implGetPublicKeySpec(P11ECKeyFactory.java:300)
	at jdk.crypto.cryptoki/sun.security.pkcs11.P11KeyFactory.engineGetKeySpec(P11KeyFactory.java:94)
	at java.base/java.security.KeyFactory.getKeySpec(KeyFactory.java:433)
	at TestKeyFactory.testPublic(TestKeyFactory.java:83)
	at TestKeyFactory.test(TestKeyFactory.java:117)
	at TestKeyFactory.main(TestKeyFactory.java:146)
	at PKCS11Test.premain(PKCS11Test.java:907)
	at PKCS11Test.testNSS(PKCS11Test.java:605)
	at PKCS11Test.main(PKCS11Test.java:254)
	at TestKeyFactory.main(TestKeyFactory.java:124)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.io.IOException: Point does not match field size
	at java.base/sun.security.util.ECUtil.decodePoint(ECUtil.java:48)
	at jdk.crypto.cryptoki/sun.security.pkcs11.P11ECKeyFactory.decodePoint(P11ECKeyFactory.java:89)
	at jdk.crypto.cryptoki/sun.security.pkcs11.P11ECKeyFactory.implGetPublicKeySpec(P11ECKeyFactory.java:297)
	... 15 more

OK, thanks. That's helpful.

Backport 55f5542

bb5ed9c

openjdk bot changed the title ~~Backport 55f5542ca2104df91e14693534cc7b3c36e81953~~ 8026976: ECParameters, Point does not match field size Oct 21, 2024

openjdk bot added backport clean labels Oct 21, 2024

openjdk bot added the rfr Pull request is ready for review label Oct 21, 2024

openjdk bot added the approval label Oct 21, 2024

franferrax reviewed Feb 24, 2025

View reviewed changes

kurashige23 and others added 2 commits February 26, 2025 08:26

Merge branch 'openjdk:master' into JDK-8026976

131868b

Fix copyright year

f019f2d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8026976: ECParameters, Point does not match field size #2958

8026976: ECParameters, Point does not match field size #2958

kurashige23 commented Oct 21, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Oct 21, 2024

openjdk bot commented Oct 21, 2024

openjdk bot commented Oct 21, 2024

openjdk bot commented Oct 21, 2024

mlbridge bot commented Oct 21, 2024 •

edited

Loading

kurashige23 commented Oct 21, 2024

openjdk bot commented Oct 21, 2024

kurashige23 commented Nov 5, 2024

kurashige23 commented Nov 27, 2024

bridgekeeper bot commented Dec 25, 2024

kurashige23 commented Jan 6, 2025

bridgekeeper bot commented Feb 3, 2025

kurashige23 commented Feb 5, 2025

jerboaa commented Feb 20, 2025

franferrax commented Feb 24, 2025

franferrax Feb 24, 2025

jerboaa Feb 24, 2025

franferrax Feb 24, 2025

kurashige23 Feb 25, 2025

jerboaa commented Feb 24, 2025

8026976: ECParameters, Point does not match field size #2958

Are you sure you want to change the base?

8026976: ECParameters, Point does not match field size #2958

Conversation

kurashige23 commented Oct 21, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewing

bridgekeeper bot commented Oct 21, 2024

openjdk bot commented Oct 21, 2024

openjdk bot commented Oct 21, 2024

openjdk bot commented Oct 21, 2024

mlbridge bot commented Oct 21, 2024 • edited Loading

Webrevs

kurashige23 commented Oct 21, 2024

openjdk bot commented Oct 21, 2024

kurashige23 commented Nov 5, 2024

kurashige23 commented Nov 27, 2024

bridgekeeper bot commented Dec 25, 2024

kurashige23 commented Jan 6, 2025

bridgekeeper bot commented Feb 3, 2025

kurashige23 commented Feb 5, 2025

jerboaa commented Feb 20, 2025

franferrax commented Feb 24, 2025

franferrax Feb 24, 2025

Choose a reason for hiding this comment

jerboaa Feb 24, 2025

Choose a reason for hiding this comment

franferrax Feb 24, 2025

Choose a reason for hiding this comment

kurashige23 Feb 25, 2025

Choose a reason for hiding this comment

jerboaa commented Feb 24, 2025

kurashige23 commented Oct 21, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 21, 2024 •

edited

Loading