dvsekhvalnov / jose-jwt

Ultimate Javascript Object Signing and Encryption (JOSE), JSON Web Token (JWT) and Json Web Keys (JWK) Implementation for .NET and .NET Core
MIT License
921 stars 183 forks source link

Support Asynchronous Signing in IJwsAlgorithm interface #218

Open AliKhalili opened 1 year ago

AliKhalili commented 1 year ago

Description

Currently, the IJwsAlgorithm interface does not support asynchronous signing, but the AWS KMS Client has implemented the Sign method in an async manner. As a result, when implementing a custom signing algorithm that uses the KMS Client as the underlying method for actual signing, the current solution involves blocking the calling thread using the GetResult() method. This approach may lead to resource starvation and deadlock if the workload is high.

To enable non-blocking signing using AWS KMS Client, we need to add support for asynchronous signing in the IJwsAlgorithm interface.

any thoughts or comments, please?

dvsekhvalnov commented 1 year ago

Hi @AliKhalili ,

well, most of crypto primitives that library is using are inherently synchronous and calling thread will be blocked somewhere anyway. Even if IJwsAlgorithm provides async method library most likely will just await it right?

What's your thoughts if you using AWS KMS in your project, how you envision ideal interface to library?

AliKhalili commented 1 year ago

Hi @dvsekhvalnov ,

Thank you for your response. I appreciate your openness to discussing this matter further.

While it is true that the majority of scenarios may be synchronous, it is becoming increasingly common for developers to expect asynchronous interfaces, especially in scenarios where cloud services(Cloud key management, Cloud HSM) are involved.

On the other hand, as you mentioned, the majority of the consumers are not expected to use the asynchronous methods, so it does not make sense to update the interface to make all methods asynchronous(in some cases it would be nearly impossible to change a legacy code base to be async and works without any problem).

Therefore, I would recommend providing both synchronous and asynchronous versions of each method. This allows consumers to choose which version they want to use based on their needs. However, this approach can lead to code duplication and may make the interface more difficult to maintain.

dvsekhvalnov commented 1 year ago

Would you mind draft one (async IJwsAlgorithm for instance) here for further discussion?

AliKhalili commented 1 year ago

Thank you for your response. That would be great to do that. However, I would like to conduct further investigation on the matter before proceeding with any implementation or discussion of specific solutions.

I've also noticed a similar issue with dotnet BCL when attempting to generate CSR(CertificateRequest) and sign(CreateSigningRequest) with a custom X509SignatureGenerator. it should be noted that this class lacks an asynchronous version of the sign method, which creates the same problem.

I will take some time to review the issue more thoroughly and explore potential solutions. I will be happy to continue our discussion

dvsekhvalnov commented 1 year ago

Sure @AliKhalili , feel free to come back with your ideas, always welcome.

I'll try to play with async idea too.

nfogg commented 2 months ago

Hello, Has there been any progress or further thought on this issue?

I want to use Azure Key Vault to wrap/unwrap CEKs in a custom implementation of IKeyManagement. However, like the OP's problem, the Key Vault client is designed to be called asynchronously, yet IKeyManagement only provides synchronous functions. Our application will be under high load, and blocking threads is unsuitable for our use case.

dvsekhvalnov commented 2 months ago

Hi @nfogg ,

unfortunately there were never any progress on this one. Before considering the feature, would be interesting to see sketches how async SDK can look like client wise. E.g. how you expect to use library async?

nfogg commented 2 months ago

Hello @dvsekhvalnov, I would imagine that from a user’s point of view the SDK would remain as-is, but with the addition of async versions of the API methods. But this is quite a significant change to the internals of the SDK so I appreciate it is not small matter.

I also discovered it is unnecessary for the wrapping/unwarpping task in my use-case. To solve the problem, my app does the Key Vault work before calling the SDK and then provides the prepared data as the encryption key parameter, which a custom implementation of IkeyManagement knows how to deal with.