Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for HSM #8349

Open
1 of 2 tasks
jzheaux opened this issue Apr 7, 2020 · 17 comments
Open
1 of 2 tasks

Add support for HSM #8349

jzheaux opened this issue Apr 7, 2020 · 17 comments
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Apr 7, 2020

Many applications will not deal with keys at all but will instead send data to a service like Vault to be encrypted, decrypted, signed, and verified.

Currently, an application needs to implement their own Saml2AuthenticationRequestFactory and AuthenticationProvider to achieve this. It would be nice if applications could implement something more targeted to cryptographic operations.

@jzheaux jzheaux added type: enhancement A general enhancement in: saml2 An issue in SAML2 modules labels Apr 7, 2020
@jzheaux jzheaux added this to the General Backlog milestone Apr 7, 2020
@ryan13mt
Copy link

ryan13mt commented Apr 8, 2020

+1

This would be very useful to have supported due to security requirements on where the private keys are stored.

@Koshux
Copy link
Contributor

Koshux commented Apr 8, 2020

+1
Would appreciate this due to a particular use case!

@jnunqui
Copy link

jnunqui commented Apr 8, 2020

+1
would be very useful

@cvenkatreddy
Copy link

would be very useful enhancement

@ghost
Copy link

ghost commented Apr 8, 2020

+1
This would be useful for us

@nafidhaque
Copy link

nafidhaque commented Apr 8, 2020

+1
Would be useful to a particular use case

@briannmic
Copy link

briannmic commented Apr 8, 2020

I need this for my project

@duncanportelli
Copy link

duncanportelli commented Apr 14, 2020

@jzheaux,

Thanks for opening this issue. I am currently working on an IDP-initiated solution and can confirm that a custom AuthenticationProvider must be implemented from scratch in order to achieve this.

The problem is that the OpenSamlAuthenticationProvider is a final class and cannot be extended. If at least, this was not final, this would have been so much easier as only the decrypt(Saml2AuthenticationToken token, EncryptedAssertion assertion) would need to be overridden in order to provide a custom Decrypter

@ryan13mt
Copy link

ryan13mt commented Jul 1, 2020

Hi @jzheaux by any chance was there any progress on this or at least a plan to include it in an upcoming release?

@jzheaux
Copy link
Contributor Author

jzheaux commented Jul 1, 2020

Hi, @ryan13mt, there is no plan or progress on this issue.

One thing that might help, though, is to begin with a sample.

@duncanportelli, is your implementation something that you can share via GitHub? This could at least get the conversation started about what is the appropriate thing to expose to simplify this. While introducing an inheritance-based solution is a possibility, I wonder if there is a sensible composition-based one.

@duncanportelli
Copy link

Hi @jzheaux,

Sure. I uploaded a sample spring boot application in which you can see the number of overridden classes in order to create a custom decrypter to include the decryption logic (in this case via the Azure Key Vault).

@ryan13mt
Copy link

ryan13mt commented Aug 4, 2020

Hi @jzheaux and @duncanportelli,

Our use case (having private key on an HSM) would only require to handle the decryption part since that is the only time we need the actual private key to be used. @duncanportelli i'm not sure about your use case, but it seems like ours since in your sample application you mentioned a custom decrypter as well. Since the other keys are all 'public' they can technically be stored locally on the machine.

@jzheaux the company i work at is ready to invest resources in contributing to this issue and develop the code needed to handle a private key stored on an HSM. Would you be willing to accept a pull request of this feature (decryption by HSM)? Or do you require that the contribution includes a full HSM integration that includes encryption, signing and verification as well? Maybe we can create a separate issue that handles the decryption specifically.

Thanks and looking forward to contribute to this open-source community

@duncanportelli
Copy link

@ryan13mt, thanks for your message. We are currently working on an IDP initiated SSO solution so signing and encryption is not required in our case. With regards to verification, we are doing it using a public key which doesn't need to be stored in an HSM. That being said, only decryption is required in our scenario as well.

@ryan13mt
Copy link

ryan13mt commented Aug 27, 2020

Hey @jzheaux

i just saw that the getDecrypter method has been changed into a DecrypterConverter. If we add a setter for this converter, could we technically just pass it our own custom decrypter to fix this issue?

@jzheaux jzheaux modified the milestones: General Backlog, 5.5.0-M1 Sep 17, 2020
@jzheaux
Copy link
Contributor Author

jzheaux commented Sep 17, 2020

@ryan13mt, yes, that was the idea I was playing with while preparing the 5.4 release.

Yes, a pull request would be welcome. While I understand that your use case is for decryption only, would you be able to provide a PR that handles signature validation as well? I believe it will be the same conceptually, returning a SignatureTrustEngine instead.

@ryan13mt
Copy link

@jzheaux wouldn't the signature validation be done using the public certificate provided by the asserting party in their metadata?

Or have i misunderstood your use case?

@jzheaux
Copy link
Contributor Author

jzheaux commented Sep 24, 2020

No, @ryan13mt, I just got my wires crossed. The ticket here calls for support in OpenSamlAuthenticationProvider to decrypt assertions as well as OpenSamlAuthenticationRequestFactory to sign requests, and I was thinking it would be nice to address both of these in the same PR.

However, thinking more about this, it would be better to wait on OpenSamlAuthenticationRequestFactory until a contributor has a concrete use case, so I've updated the ticket's description to break things down into individual tasks.

I've got some additional thoughts about decryption. Now that these tasks are separated, I've added those thoughts to #9044.

@jgrandja jgrandja modified the milestones: 5.5.0-M1, 5.5.0-M2 Nov 3, 2020
@jzheaux jzheaux removed this from the 5.5.0-M2 milestone Feb 11, 2021
@jzheaux jzheaux added this to the 6.4.x milestone Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

9 participants