pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard
verified publisher icon apache

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

Open izumo27 opened this issue 2 years ago • 3 comments

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.

  • https://github.com/apache/pulsar/blob/200c925add239115365d16d63559b46399f32eca/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java#L238
  • https://github.com/bcgit/bc-java/blob/r1rv75/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L120
  • https://github.com/bcgit/bc-java/blob/r1rv75/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L153-L216
    • https://github.com/bcgit/bc-java/blob/r1rv75/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L95
    • https://github.com/bcgit/bc-java/blob/r1rv75/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L65
  • https://github.com/bcgit/bc-java/blob/r1rv75/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L191

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.

  • https://github.com/bcgit/bc-java/blob/r1rv75/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L567-L578
    • https://github.com/bcgit/bc-java/blob/r1rv75/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L99
    • https://github.com/bcgit/bc-java/blob/main/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L69
  • https://github.com/bcgit/bc-java/blob/r1rv75/pkix/src/main/java/org/bouncycastle/openssl/PEMParser.java#L572

Verifying this change

  • [x] 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
  • [x] doc-not-needed
  • [ ] doc-complete

Matching PR in forked repository

PR in forked repository:

  • https://github.com/izumo27/pulsar/pull/2
  • https://github.com/izumo27/pulsar/pull/3 (test failure before this modification)

izumo27 avatar Dec 31 '23 13:12 izumo27

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. https://github.com/apache/pulsar/blob/529e1ab80998143cfe793c018429e9d2bbee5473/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java#L2780-L2783

Actually, in ECDSA's test, there is only one producer. https://github.com/apache/pulsar/blob/529e1ab80998143cfe793c018429e9d2bbee5473/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java#L2701-L2703

izumo27 avatar Jan 02 '24 15:01 izumo27

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

izumo27 avatar Jan 02 '24 15:01 izumo27

PTAL, thanks. @Technoboy-

izumo27 avatar Feb 02 '24 13:02 izumo27