Skip to content

[fix][client] Support for PKCS#8 private key#21828

Closed
izumo27 wants to merge 2 commits intoapache:masterfrom
izumo27:support_pkcs8
Closed

[fix][client] Support for PKCS#8 private key#21828
izumo27 wants to merge 2 commits intoapache:masterfrom
izumo27:support_pkcs8

Conversation

@izumo27
Copy link
Contributor

@izumo27 izumo27 commented Dec 31, 2023

Motivation

When attempting to load an RSA private key in PKCS#8 format for message encryption, the key loading fails.

Modifications

When the RSA private key is in PKCS#1 format (begin with BEGIN RSA PRIVATE KEY), the type of pemObj is PEMKeyPair.

However, When the RSA private key is in PKCS#8 format (begin with BEGIN PRIVATE KEY), BouncyCastle utilizes PrivateKeyParser, resulting in the type of pemObj being PrivateKeyInfo.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Add a PKCS#8 private key for testing

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 31, 2023
Copy link
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep the original test with x.pemkey.

@izumo27 izumo27 force-pushed the support_pkcs8 branch 3 times, most recently from 0a4ae88 to 5051aeb Compare January 2, 2024 14:08
@izumo27
Copy link
Contributor Author

izumo27 commented Jan 2, 2024

We should keep the original test with x.pemkey.

In testRSAEncryption, producer and producer2 use the same key, so it looks like producer2 is not needed.

Producer<byte[]> producer = pulsarClient.newProducer().topic("persistent://my-property/my-ns/myrsa-topic1")
.addEncryptionKey("client-rsa.pem").cryptoKeyReader(new EncKeyReader()).create();
Producer<byte[]> producer2 = pulsarClient.newProducer().topic("persistent://my-property/my-ns/myrsa-topic1")
.addEncryptionKey("client-rsa.pem").cryptoKeyReader(new EncKeyReader()).create();

Actually, in ECDSA's test, there is only one producer.

Producer<byte[]> cryptoProducer = pulsarClient.newProducer()
.topic(topicName).addEncryptionKey("client-ecdsa.pem")
.cryptoKeyReader(new EncKeyReader()).create();

@izumo27
Copy link
Contributor Author

izumo27 commented Jan 2, 2024

I missed many tests using private-key.client-rsa.pem.
I modified them.

@Technoboy- Technoboy- added this to the 3.3.0 milestone Jan 3, 2024
@izumo27
Copy link
Contributor Author

izumo27 commented Feb 2, 2024

PTAL, thanks. @Technoboy-

@izumo27 izumo27 closed this Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants