dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.2k stars 9.94k forks source link

Allow option for DataProtection keys to not be cached #43500

Open maganuk opened 2 years ago

maganuk commented 2 years ago

Is there an existing issue for this?

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

I have a multi-tenant application where I would like for each tenant to have their own data protection keys which are saved in a DB. I find that there is no way to do this (without writing my own DataProtectionProvider) since the keys are always cached. Even if I were to write my own XML Repository, I am unable to use it on a per-request basis as the keys are already valid for a period of 24 hours.

Describe the solution you'd like

Provide an option to not cache keys so that it may be possible for developers to use their own cache and resolve keys on a per request basis.

adityamandaleeka commented 2 years ago

Removing milestone temporarily so it comes up in triage.

amcasey commented 7 months ago

@maganuk Sorry for the delay. Can you please elaborate a bit more on the problem you're encountering? Why would caching prevent you from using multi-tenancy?

maganuk commented 7 months ago

@amcasey thanks for picking this up.

So assume that new tenants are being registered dynamically at runtime. I'd like each tenant to have its own data protection key.

Currently, the way the Keyring provider is setup, the keys cannot be resolved at runtime. They are always picked up from the internal cache. Even if I try to implement my own IKeyManager, the keys will always be cached.

In my case I want to use my own caching layer and be able to resolve keys at runtime rather than depending on the framework to handle the caching. Further, I have a scoped Tenant Service which I need to access to resolve the current tenant to fetch the data protection key. I've therefore had to write my own DataProtectionProvider to do this. Would be better if the framework allows more flexibility for doing this.

amcasey commented 7 months ago

Unfortunately, the term "key" is quite overloaded in this space, so I'd like to make sure we're on the same page. The way Data Protection expects to enable multi-tenancy is through purpose strings, the values of which can be specified dynamically. Separate derived keys (sometimes called Data Encryption Keys (DEKs)) will be created for each purpose.

Is this the sort of key you're referring to when you say you "like each tenant to have its own data protection key"?

maganuk commented 7 months ago

Actually no, I mean a separate master key for each tenant. The reason I say that is because as part of our product offering (it's an IAM service), we would like to offer our customers the ability to store these keys at their end or take the keys with them when they migrate away from us. We are using the derived keys too, but they are scoped to different purposes within a tenant.

amcasey commented 7 months ago

I think it's great that you're giving your customers that level of control, but that scenario's a bit outside the design goals of the data protection system. In particular, it's only intended for ephemeral (i.e. expendable) data, which there would normally be no reason to export. If you did want to offer some sort of export functionality, I would think you'd want to decrypt the data on-demand and then re-encrypt it with a new key specifically for/from that customer.

amcasey commented 7 months ago

I remain a little unclear on which keys you want to have more control over. Is it the ones that encrypt the keyring or the ones that encrypt the data?

maganuk commented 7 months ago

Thanks for your consideration. The key we're after are the ones that encrypt the data.

Even though the comparison is a bit distant, but have a look at the Field level encryption key in MongoDB Atlas. They let you create as many keys as you want and each key can belong to a tenant. The idea there is that if you want to forget a customer, all you do is delete the respective key.

With the above approach that you have suggested we still cant have our customers own their keys. Our product is an authentication-as-a-service platform, where customers have users who may have long lived cookies encrypted by the Data Protection provider. Now if our customers want to migrate away from us, they would want to take these keys with them so that they don't loose these cookies, else their users would have lost state and would be required to re-authenticate.

amcasey commented 7 months ago

Wow, changing authentication platforms without disrupting user sessions is a pretty ambitious goal.

So, I think the "cache" you're referring to is the fact that the keyring is (only) polled (e.g. from disk) every 24 hours. Is that right? If so, I'm not sure I understand how going straight to disk (or whatever repository) every time would help, since it's actually derived keys that are used to encrypt data. Without having stepped through the code, I would only expect the keyring keys to be accessed when a new purpose string is introduced. Is that what you're doing - generating a lot of dynamic purpose strings and trying to use a (potentially) different keyring each time? And the reason for doing that is to be able to offer your customer the backing keyring.xml file(s)?

maganuk commented 7 months ago

Ambitious indeed. Initially we plan to use it only to move customers from Public to Private cloud (without disruption).

In the KeyRingBasedDataProtector, it is this particular line:

var currentKeyRing = _keyRingProvider.GetCurrentKeyRing();

On every request, after resolving the tenant, we would like to fetch the key ring for the tenant (resolving a scoped instance of the keyring provider store).

In the custom provider that we have written, we are injecting a dataprotection store in the KeyRingBasedDataProtector and calling:

var defaultKey = _dataProtectionStore.GetDefaultKey();
var defaultEncryptorInstance = defaultKey.CreateEncryptor();
amcasey commented 7 months ago

53455 is another request to be able to partition master keys (i.e. the ones in the keyring) by tenant.

amcasey commented 7 months ago

@maganuk Thanks for taking the time to lay out your requirements! That's not a scenario we can prioritize right now, but the conversation above will make it much easier to collect additional support for the request and revisit it in the future.

ghost commented 7 months ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.