8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1

[valeriepeng]

Can someone help review this?

This change enhances RSA KeyFactory impl of SunRsaSign and SunPKCS11 providers to accept RSA keys in PKCS#1 format and encoding and translate them to provider-specific RSA keys. Updated the relevant tests with a sample PKCS#1 encoded key pair.

Thanks,
Valerie

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1

Reviewers

• Weijun Wang (@wangweij - Reviewer) ⚠️ Review applies to b48afdd

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1787/head:pull/1787
$ git checkout pull/1787

[bridgekeeper]

👋 Welcome back valeriep! 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]

@valeriepeng The following label will be automatically applied to this pull request:

• security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 03: Full - Incremental (6f498af)
• 02: Full - Incremental (b48afdd)
• 01: Full - Incremental (a4cddf4)
• 00: Full (eed143e)

[bridgekeeper]

@valeriepeng 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!

[valeriepeng]

Ping~ Anyone has time to looks at this? I will sync with the master since it's been a while...

The changes are straight forward, just add support for PKCS#1 encoded RSA keys. There is already existing code parsing the PKCS#1 encoding underneath the PKCS#8 layer. This is more like code-refactoring. Comments?

[wangweij]

Will you clean up the getEncoded() output or shall I?

[valeriepeng]

Maybe it's better that you do it this time? Just so that the backport won't miss it.

[valeriepeng]

Or if you integrated before me, I will manually merge the changes and clean up the getEncoded() also.

[valeriepeng]

/integrate

[openjdk]

@valeriepeng This PR has not yet been marked as ready for integration.

[mlbridge]

Mailing list message from Michael StJohns on security-dev:

Sorry - I'm coming to this a bit late.

Any chance of adding the logic for generatePublic() from a PKCS8 RSA
private key??? RFC3477 has the PKCS1 RSAPrivateKey ASN1 which includes
the modulus and publicExponent - so it should be a pretty straight
forward add to generate a public key.

PKCS11 2.40 started requiring that the publicExponent be stored with the
private key to allow for the public key to be regenerated from a private
key object.?? Going forward,? it might be a good idea to modify the
RSAPrivate(Crt)KeyImpl class to store the publicExponent if provided.

Mike

On 1/14/2021 4:06 PM, Valerie Peng wrote:

[openjdk]

@valeriepeng This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1

Reviewed-by: weijun

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 54 new commits pushed to the master branch:

• bf28f92: 8259713: Fix comments about ResetNoHandleMark in deoptimization
• 4f881ba: 8258652: Assert in JvmtiThreadState::cur_stack_depth() can noticeably slow down debugging single stepping
• d18d26e: 8259350: Add some internal debugging APIs to the debug agent
• a6b2162: 8259278: Optimize Vector API slice and unslice operations
• da6bcf9: 8255019: Shenandoah: Split STW and concurrent mark into separate classes
• aba3431: 8258956: Memory Leak in StringCoding on ThreadLocal resultCached StringCoding.Result
• 8554fe6: 8253866: Security Libs Terminology Refresh
• c2a3c7e: 8259727: Remove redundant "target" arguments to methods in Links
• 1620664: 8259723: Move Table class to formats.html package
• 38a1201: 8258912: Remove JVM options CountJNICalls and CountJVMCalls
• ... and 44 more: https://git.openjdk.java.net/jdk/compare/c6d798c25a799dffb86769b9e1c99c3442adc121...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[valeriepeng]

Mailing list message from Michael StJohns on security-dev:

Sorry - I'm coming to this a bit late.

Any chance of adding the logic for generatePublic() from a PKCS8 RSA
private key??? RFC3477 has the PKCS1 RSAPrivateKey ASN1 which includes
the modulus and publicExponent - so it should be a pretty straight
forward add to generate a public key.

PKCS11 2.40 started requiring that the publicExponent be stored with the
private key to allow for the public key to be regenerated from a private
key object.?? Going forward,? it might be a good idea to modify the
RSAPrivate(Crt)KeyImpl class to store the publicExponent if provided.

Mike

You are correct that for RSA private CRT keys the necessary values are there for figuring out its corresponding public keys.

This change is about adding support for PKCS#1 encoded RSA keys and be able to translate them into PKCS#8 encoded keys and/or extract various key specs out of them. If you already have PKCS#8 RSAPrivateCrtKey obj from SunRsaSign provider, you can call its getPublicExponent() method and use that to create a RSAPublicKeySpec and generate RSA public key with it. If you are using 3rd party impl which does not return the public exponent value somehow, then you can translate it using the RSA key factory impl from SunRsaSign provider and then repeat the fore-mentioned step. Will this address your need? If not, could you elaborate the usage that you have in mind? Not sure if you are suggesting a new KeyFactory.generatePublic() method which take a PrivateKey or else.

[valeriepeng]

Mike,
We can continue your feedback with a separate RFE since this RFE is just about adding support for PKCS#1 encoding.
I need to wrap this up before my upcoming trip this Wed, hope that's ok with you.

Thanks! Valerie

[valeriepeng]

/integrate

[openjdk]

@valeriepeng Since your change was applied there have been 86 commits pushed to the master branch:

• 5dc5d94: 8256110: Create implementation for NSAccessibilityStepper protocol
• 5f2e280: 8259865: (fs) test/jdk/java/nio/file/attribute/UserDefinedFileAttributeView/Basic.java failing on macOS 10.13
• da4cf05: 8258755: jpackage: Invalid 32-bit exe when building app-image
• c3bdbf9: 8259238: Clean up Log.java and remove usage of non-final static variables.
• 6d6a23e: 8259062: Remove MacAppStoreBundler
• afd3f78: 8030048: (fs) Support UserDefinedFileAttributeView/extended attributes on OS X / HFS+
• bbb93ca: 8256126: Create implementation for NSAccessibilityImage protocol peer
• 90c73d0: 8259569: gtest os.dll_address_to_function_and_library_name_vm fails
• 536082d: Merge
• e85892b: 8258396: SIGILL in jdk.jfr.internal.PlatformRecorder.rotateDisk()
• ... and 76 more: https://git.openjdk.java.net/jdk/compare/c6d798c25a799dffb86769b9e1c99c3442adc121...master

Your commit was automatically rebased without conflicts.

Pushed as commit 68cf65d.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[mlbridge]

Mailing list message from Michael StJohns on security-dev:

On 1/17/2021 9:29 PM, Valerie Peng wrote:

No worries - I got busy with other things for a few days.

To answer your other question, I've had a few cases where the public key
for a private key has been misplaced and where we needed it back.?
Ideally,? it should be possible to take a PrivateKey object (in whatever
form including just PrivateKey) and run it through the KeyFactory to
extract a PublicKey.? I've used the technique you suggest above before
(and the related multiply the private scalar against G for EC keys) to
retrieve the data needed to build a public key, but each of these is a
bit ad hoc. I'd really prefer to have a standard pattern for all key types.

About 4 years or so ago (e.g. when 2.40 was released), the PKCS11 group
started requiring that the private keys include the attributes necessary
to retrieve the public key - for RSA it was the public exponent, and for
EC it was the public point (which could be stored or regenerated upon
demand).? It may be time to think about doing something similar here and
going forward for any given asymmetric key type.

That's a more general RFE than just updating the current implementing
classes, but as far as I can tell, doesn't change any of the APIs, but
may change the factory class implementation guidance.

An interesting addition would be to have the Impl classes implement both
the appropriate public key and private key interfaces.