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

External storage for OpenID certificates #13205

Open
Piedone opened this issue Feb 5, 2023 · 3 comments
Open

External storage for OpenID certificates #13205

Piedone opened this issue Feb 5, 2023 · 3 comments

Comments

@Piedone
Copy link
Member

Piedone commented Feb 5, 2023

Is your feature request related to a problem? Please describe.

Having the web server stateless, i.e. it not storing anything that you want to keep and can't be redeployed (like content, media), is useful for having a clear deployment story and a requirement for horizontal scaling. The OpenID module's OpenIdServerService stores certificates on the local file system, however. This causes the below exception if you wipe the storage (like when you deploy a new version of the app:

An error occurred while trying to extract a X.509 certificate.	

The key {ebf444b5-e9ea-43b6-9a1a-bdb4858a800e} was not found in the key ring. For more information go to http://aka.ms/dataprotectionwarning	

System.Security.Cryptography.CryptographicException:
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.UnprotectCore (Microsoft.AspNetCore.DataProtection, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.Unprotect (Microsoft.AspNetCore.DataProtection, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60)
   at Microsoft.AspNetCore.DataProtection.DataProtectionCommonExtensions.Unprotect (Microsoft.AspNetCore.DataProtection.Abstractions, Version=6.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60)
   at OrchardCore.OpenId.Services.OpenIdServerService+<<GetCertificatesAsync>g__GetPasswordAsync|20_0>d.MoveNext (OrchardCore.OpenId, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw (System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e)
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification (System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e)
   at OrchardCore.OpenId.Services.OpenIdServerService+<<GetCertificatesAsync>g__GetCertificateAsync|20_1>d.MoveNext (OrchardCore.OpenId, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw (System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e)
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess (System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e)
   at OrchardCore.OpenId.Services.OpenIdServerService+<GetCertificatesAsync>d__20.MoveNext (OrchardCore.OpenId, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null)

This is logged here.

Despite this BTW, OpenID still appears to be working.

Related: #7137

Describe the solution you'd like

Similar to Azure Data Protection we could have an implementation to store the certificates in a Blob Storage account. Basically, we could have something like IOpenIdServerCertificateStorage, with a default implementation that accesses local files like OpenIdServerService does today, and another feature that provides a Blob Storage-based implementation. (And later others can be added too.)

Describe alternatives you've considered

You need to fully override OpenIdServerService to implement this currently. Also see the comment by @kevinchalet here.

@sebastienros
Copy link
Member

Kevin:

Also, using raw RSA keys instead of RSA keys embedded in X.509 certificates (for which we tend to see fewer issues in hostile environments) in conjunction with #7891 might be a better approach than making the storage of the current feature replaceable.

Should we trust him?

@sebastienros sebastienros added this to the backlog milestone Feb 16, 2023
@kevinchalet
Copy link
Member

Should we trust him?

A tes risques et périls :trollface:

@Piedone
Copy link
Member Author

Piedone commented Jun 8, 2023

Hmm, perhaps IOpenIdServerService.PruneManagedCertificatesAsync() can be used (or something else) to reset the certificate store after deployment, to get rid of the exception, at least.

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

Successfully merging a pull request may close this issue.

4 participants