Closed MandeepShahi closed 1 year ago
@jmprieur I'm currently using this mode of data protection for encrypting MSAL tokens to be cached. However, since the keys are generated on different machines, while maintaining the key ring, we're running into Concurrency limitations for which there are a couple of threads currently open with DataProtection team. My query is that, why not remove this additional hog of Key sets and directly use Certificates to protect/unprotect MsalBytes? We're anyway protecting the keys with certificates as a way of protecting the keys!!Is there a limitation of using Cert encryption that I'm missing?
When using a distributed cache and you encrypt it, you want to share the encryption keys between the instance of your app (which might run on different machines).
All instances of the distributed system can have shared certificate(s)!!
The problem with using certs directly is that cert rotation is a hard problem. When you rotate the cert the cached data becomes invalid and is difficult / expensive to handle. To make matters worse, in some systems the rotation happens gradually - i.e. you can have N pods using cert_1 and N pods using cert_2
The only other way is to configure AAD to emit encrypted tokens, which is available only to 1p users as far as I know. @jmprieur - can you confirm ?
@bgavrilMS I understand the Cert rotation part. But this can be handled as it has been handled when encrypting DataProtection keys with Certificates (Unprotect for a while with the older certificate while protecting with the new one).
Talking about the solution - We can extend the partial class MsalDistributedTokenCacheAdapter to add encryption before the WriteCacheBytesAsync method and add decryption after ReadCacheBytesAsync method. But I understand the pain associated with cert rotation.
It still feels like a worth while feature to add!! Thoughts?
I am following up offline, since I believe we have a solution for 1p which relies on AAD encrypting the tokens. Will post here any fidings for 3p as well, as getting rid of certs in Identity is a top priority. Certs are hard.
I think it should be relatively simple to create a custom implementation of IDataProtector
and to register this custom implementation. See https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.dataprotection.idataprotector?view=aspnetcore-6.0
This way you can define whatever encryption you want (or any string transformation really). And you don't have to create a cache from scratch.
Here's how it's used internally. https://github.com/AzureAD/microsoft-identity-web/blob/master/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs#L19
Thoughts?
CC @jennyf19 and @jmprieur
Hi @bgavrilMS this makes sense what you're suggesting - Custom implementation of IDataProtector! I thought about it from the angle of overriding the implementations of Writing to persisted resource and Reading from the resource and including implementations of Protect and Unprotect there. Will give your suggestion a try, Thanks for your suggestion!!
Is your feature request related to a problem? Please describe. I'm working on an app service which uses Distributed token caching feature. This feature has ways of protecting tokens with DataProtection service which uses key generation and I'm running into concurrency issue using this. Also, there's no way of directly protecting the tokens with certificates that are shared across the distributed system machines. Is there a reason we don't use direct certificate encryption of tokens?
Describe the solution you'd like Introduce certificate based protection of tokens to be cached.
Describe alternatives you've considered I was using DataProtection service but ran into some concurrency issue as they use generated keys and encrypt payloads using those keys and store keys to persisted locations for usability in distributed systems. The concurrency issues seems troubling and there'll be a separate issue thread for the same with DataProtection team. Meanwhile, not sure why we don't use Certificate encryption!!