Skip to content

Conversation

@heng-gezhizou
Copy link

Background

The current SDK requires configuring the complete RSA private key in the local
configuration file. However, in many production environments, RSA private keys
are managed by secure key management systems such as HSM or KMS, where private
key material must not be exported or exposed to the application layer.

This limitation makes it difficult to integrate the SDK in security- and
compliance-sensitive environments.

Solution

This PR introduces an extension mechanism that allows RSA cryptographic
operations to be delegated to external signing or key management systems.

A new ExternalRsaProvider interface is added to abstract RSA signing and
decryption capabilities, enabling the SDK to work with keys identified by
keyId instead of raw private key content.

Changes

  • Add ExternalRsaProvider interface for external RSA operations
    • RSA signing (PKCS#1 v1.5)
    • RSA-PSS signing
    • RSA decryption
  • Modify RSA signing flow to accept a keyId rather than a private key string
  • Delegate signing and decryption operations to the configured
    ExternalRsaProvider
  • Add a test case demonstrating how to integrate a custom
    ExternalRsaProvider

Use Cases

  • RSA private keys stored in HSM or cloud KMS
  • Environments with strict security or compliance requirements
  • Custom or enterprise-grade signing services

Compatibility

  • Backward compatible with existing private-key-based configuration
  • Existing functionality remains unchanged when ExternalRsaProvider is not configured

Notes

The test implementation uses a mock provider with local RSA signing for
demonstration purposes only. In production, the provider should delegate all
cryptographic operations to a secure external system.

@xigaoku
Copy link
Collaborator

xigaoku commented Dec 25, 2025

@heng-gezhizou Thank you very much for submitting this PR — we truly appreciate the time, effort, and thought you’ve put into it. The idea and direction behind this change are very valuable and helpful for the project.

We will review it carefully as soon as possible. During the review, we may choose to adopt the PR directly, or rework the implementation based on your proposed approach to better align with the overall architecture and long-term maintainability of the SDK.

Either way, your contribution is highly appreciated, and it has provided important insights for improving the project. Thank you again for your support!

@xigaoku xigaoku requested a review from absorprofess December 26, 2025 02:44
@xigaoku
Copy link
Collaborator

xigaoku commented Dec 29, 2025

@heng-gezhizou Thank you for the PR. After reviewing the code, there is one point we’d like to clarify with you.

In our API communication model, the same RSA key pair is used to sign request data and to decrypt response data. However, based on our understanding, asymmetric keys provided by cloud KMS services (such as AWS KMS and Azure Key Vault) can typically only be configured for a single purpose — either signing/verification or encryption/decryption — but not both at the same time.

Could you please clarify whether your implementation is intended to integrate with cloud-based KMS / HSM services, or with a self-managed key management system?

@heng-gezhizou
Copy link
Author

@xigaoku With a self-managed key management system

@xigaoku
Copy link
Collaborator

xigaoku commented Jan 7, 2026

@heng-gezhizou We’re very happy to receive your reply—it has addressed our questions clearly. To ensure the SDK remains extensible after the changes, we would like to have a deeper discussion with you about the PR adjustment plan. The overall approach is as follows:

  • Introduce a KeyProvider interface that unifies signing and decryption capabilities as a single extension point.

  • Provide a default built-in implementation, DefaultPrivateKeyProvider, which internally still uses the existing RsaUtil and preserves the original behavior.

  • Add a keyProvider configuration in SafeheronConfig to allow users to inject custom implementations. The existing rsaPrivateKey method is kept for backward compatibility (internally creating a DefaultPrivateKeyProvider). If both keyProvider and rsaPrivateKey are provided, the keyProvider configuration takes precedence over rsaPrivateKey.

  • RequestInterceptor and ResponseBodyConverter no longer depend directly on the private key or RsaUtil; all signing and decryption go through KeyProvider.

Existing SDK class diagram (simplified):

---
  config:
    class:
      hideEmptyMembersBox: true
---
classDiagram

class RequestInterceptor{
 -String rsaPrivateKey;
}

RequestInterceptor ..> RsaUtil

class ResponseBodyConverter{
 -String rsaPrivateKey;
}

ResponseBodyConverter ..> RsaUtil

class RsaUtil
Loading

PR adjustment class diagram (simplified):

---
  config:
    class:
      hideEmptyMembersBox: true
---
classDiagram
class KeyProvider {
	<<interface>>
	+sign(String content) String
	+signPSS(String content) String 
	+decrypt(String content, RSATypeEnum rsaType) byte[]
}

class RequestInterceptor {
	- KeyProvider provider
}

RequestInterceptor --> KeyProvider

class SafeheronConfig {
	-KeyProvider provider
	+getKeyProvider() KeyProvider
	+keyProvider(KeyProvider provider)
	+rsaPrivateKey(String privateKey)
}

SafeheronConfig o-- KeyProvider

class ResponseBodyConverter {
	- KeyProvider provider
}

ResponseBodyConverter --> KeyProvider

class DefaultPrivateKeyProvider {
  -String rsaPrivateKey;
	+sign(...) String
	+signPSS(...) String 
	+decrypt(...) byte[]
}

class SelfManagedKeyProvider:::extendClass {
	-String keyId
	+sign(...) String
	+signPSS(...) String 
	+decrypt(...) byte[]
}

KeyProvider <|.. DefaultPrivateKeyProvider
KeyProvider <|.. SelfManagedKeyProvider

class RsaUtil

DefaultPrivateKeyProvider ..> RsaUtil

classDef extendClass fill:#EEEEEE,stroke:,stroke-dasharray: 3 3
Loading

Built-in DefaultPrivateKeyProvider implementation example:

public final class DefaultPrivateKeyProvider implements KeyProvider {

    private final String privateKey;

    public DefaultPrivateKeyProvider(String privateKey) {
        this.privateKey = Objects.requireNonNull(privateKey, "privateKey must not be null");
    }

    @Override
    public String sign(String content) {
        // Use RsaUtil for signing
        return "signature";
    }

    @Override
    public String signPSS(String content) {
      	// Use RsaUtil for signing
        return "pss-signature";
    }

    @Override
    public byte[] decrypt(String content, RSATypeEnum rsaType) {
        // Use RsaUtil for decryption
        return new byte[0];
    }
}

Usage examples:

  1. Default usage (backward-compatible):
SafeheronConfig config = SafeheronConfig.builder()
  .baseUrl("...")
  .apiKey("...")
  .safeheronRsaPublicKey("...")
  // internally instantiates DefaultPrivateKeyProvider
  .rsaPrivateKey("...") 
  .build();
  
TransactionApiService transactionApi = ServiceCreator.create(TransactionApiService.class, config);
  1. For your scenario, simply implement KeyProvider (e.g., SelfManagedKeyProvider) and inject it when initializing SafeheronConfig; no other SDK logic needs to change.
SafeheronConfig config = SafeheronConfig.builder()
  .baseUrl("...")
  .apiKey("...")
  .safeheronRsaPublicKey("...")
  // If both `keyProvider` and `rsaPrivateKey` are provided, 
  // the `keyProvider` configuration takes precedence over `rsaPrivateKey`.
  .keyProvider(new SelfManagedKeyProvider(keyId))
  .build();
  
TransactionApiService transactionApi = ServiceCreator.create(TransactionApiService.class, config);

Benefits of this approach:

  • Provides a clear and stable extension point, making it easier to support self-managed key solutions.
  • Fully backward-compatible with existing users.
  • Decouples core SDK logic from specific key implementations, resulting in a cleaner overall structure.

Additional note:

The current proposal does not involve cloud-based KMS/HSM. References to KMS/HSM in the PR or code comments should be replaced with Self-Managed Key Management to avoid confusion.

Please review this proposed adjustment. We hope to further discuss this PR with you, and if you agree with our suggestions, you can modify the PR accordingly and resubmit.

- Add KeyProvider interface for custom signing/decryption.
- Support keyProvider injection in SafeheronConfig.
- Decouple SDK core from RSA private key strings.
- Update demo and terminology to "Self-Managed Key Management".
@heng-gezhizou
Copy link
Author

@xigaoku Thank you for the detailed explanation and the clear adjustment plan.

I’ve updated the PR according to the proposed design and have pushed the latest changes to the designated branch. The current implementation introduces the KeyProvider interface as the unified extension point for signing and decryption, adds the built-in DefaultPrivateKeyProvider to preserve existing behavior, and updates SafeheronConfig, RequestInterceptor, and ResponseBodyConverter to depend on KeyProvider instead of directly on the private key or RsaUtil.

Backward compatibility is maintained by keeping the rsaPrivateKey configuration, with keyProvider taking precedence when both are provided, as suggested. I’ve also adjusted the related comments and naming to use “Self-Managed Key Management” and removed references to KMS/HSM to avoid confusion.

Please take a look at the latest commit and let me know if there are any further adjustments or refinements you’d like to discuss. I’m happy to iterate based on your feedback.

@xigaoku
Copy link
Collaborator

xigaoku commented Jan 14, 2026

@heng-gezhizou Thank you. We will review it as soon as possible and get back to you.

@xigaoku xigaoku changed the title feat: add ExternalRsaProvider to support HSM/KMS-based RSA operations feat: support self-managed key management via KeyProvider Jan 15, 2026
@xigaoku
Copy link
Collaborator

xigaoku commented Jan 15, 2026

@heng-gezhizou We really appreciate the time you put into this PR. We’ve reviewed the changes and put together a few suggestions. We tried to cover everything in one pass, though there may still be some gaps. Please feel free to share your thoughts — we’re very happy to iterate and refine this together.

  1. The comment for SafeheronConfig.rsaPrivateKey might be adjusted to avoid any wording that could be interpreted as being related to a keyId. From our perspective, this field is intended purely for configuring the local private key, and keeping its responsibility explicit and consistent would help avoid confusion.
  2. The SafeheronConfig.getKeyProvider() method may currently return null. One possible improvement would be to throw a NullPointerException when neither keyProvider nor rsaPrivateKey is configured, so issues can fail fast rather than surfacing later when a request is executed.
  3. We’d suggest moving KeyProvider and DefaultPrivateKeyProvider into the com.safeheron.client.keys package, which may make the package structure a bit clearer and more intuitive.
  4. It could also be helpful to add a small demo under src/test/java/com/safeheron/demo/keys, with a dedicated test class showcasing the new KeyProvider functionality. This would make it easier for other developers to quickly discover and understand the feature.

If you agree with these suggestions, feel free to update the PR accordingly. Of course, we’re also open to discussing alternatives if you have different ideas — thanks again for the great contribution, and we’re looking forward to continuing the collaboration!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants