AzureAD / microsoft-identity-web

Helps creating protected web apps and web APIs with Microsoft identity platform and Azure AD B2C
MIT License
684 stars 217 forks source link

[Feature] Clarification of behavior of Token Decryption Certs during cert rotation #1321

Open jlautman opened 3 years ago

jlautman commented 3 years ago

Documentation related to component

Token Decryption

Please check all that apply

Description of the issue

If we use the TokenDecryptionCertificates declaration and let Microsoft.Identity.Web manage our decryption certs, what does the library do when we need to rotate the decryption certificate? Is it able to detect a new version of the certificate (on keyvault or local) and try both of them?

During decryption certificate rotation there's a period of time when the app needs to be able to decrypt tokens using either the old or new certificate using the same distinguished name (although SAN would be better given upcoming changes where issuers can omit Subject Name).

For the keyvault option, how is the identity to use to access keyvault specified? Is it hard-coded to use Azure managed service identity?

jmprieur commented 3 years ago

Thanks for the heads-up. We'll update the doc.

jlautman commented 3 years ago

Could I add a feature request for auto-loading new certificate versions? This would be very helpful for the zero touch certificate rotations Azure is aiming for. Otherwise new certificates wouldn't be picked up until the next time the vm starts. Or is that out of scope for Microsoft.Identity.Web?

On Fri, Jul 16, 2021, 5:34 AM Jean-Marc Prieur @.***> wrote:

Thanks for the heads-up. We'll update the doc.

  • You can provide several TokenDecryptionCertificates in the configuration. They are both passed to middleware (Identity.Model) which will try one and then the other.
  • We don't currently try to re-load the decrypt certs (we do for the certificate credentials)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AzureAD/microsoft-identity-web/issues/1321#issuecomment-881313381, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADC2UPNG2CDOXMO4LMRJI33TX74MFANCNFSM5AOOXKIA .

jmprieur commented 3 years ago

@jlautman : we'll try.

Decrypt certificate rotation has a bigger issue which is that certs need to be rotated in the portal, whereas client certificates can be rotated more easily by using subject name issuer by using .SendX5C(true).

jlautman commented 3 years ago

This feature request is as a result of exactly that difficulty.

Our decryption cert rotation strategy requires that we have both the old and new decryption certificates loaded into the system, so that when we upload the new certificate the service can decrypt without interruption. Currently we have to restart all instances of our service to get to this state. This feature request would make this scenario much easier.

On a related note, it would be very helpful for new adopters of the library if you could update the default version of Identity.Model past 6.8.0 so that multiple decryption certificates can be used once loaded. Earlier versions of that library only use the first decryption certificate despite being provided more than one. We were able to fix that behavior by explicitly including a newer version of the dependency but it took a lot of time to figure that out.

On Mon, Jul 19, 2021, 3:10 AM Jean-Marc Prieur @.***> wrote:

@jlautman https://github.com/jlautman : we'll try.

Decrypt certificate rotation has a bigger issue which is that certs need to be rotated in the portal, whereas client certificates can be rotated more easily by using subject name issuer by using .SendX5C(true).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AzureAD/microsoft-identity-web/issues/1321#issuecomment-882300161, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADC2UPJIEO56TRNEEMXTP7TTYPFW5ANCNFSM5AOOXKIA .

jmprieur commented 3 years ago

@jlautman. Agreed for the cert rotation. I've turned this issue into an enhancement.

However, for your question about Identitty.Model, I'm confused because Microsoft.Identity.Web (currently 1.14.1) has been referencing Identity.Model 6.11.1 since the beginning of June. Are you on the latest version?

Identity.Model just released 6.12.0 yesterday (cc: @jennyf19)

jlautman commented 3 years ago

I am using Microsoft.Identity.Web 1.14.1 with .Net Core 3.1. On further investigation it appears that Microsoft.AspNetCore.Authentication.* version 3.1.17 (the highest version available for .Net Core) depends on IdentityModel >=5.5.0. Since Microsoft.Identity.Web for .Net Core 3.1 does not have an explicit version dependency on IdentityModel, it loads 5.5.0 as requested by that dependency by default. The ask would be to update the dependency on IdentityModel in either Microsoft.Identity.Web or Microsoft.AspNetCore.Authentication to at least 6.8.0 so that this functionality works without the customer needing to figure all of this out.

Microsoft.Identity.Web 1.14.1 for net5.0 depends on IdentityModel 6.11.1, so that upgrade would be making.Net Core 3.1 match .Net Framework.

On Thu, Jul 22, 2021 at 2:27 AM Jean-Marc Prieur @.***> wrote:

@jlautman https://github.com/jlautman. Agreed for the cert rotation. I've turned this issue into an enhancement.

However, for your question about Identitty.Model, I'm confused because Microsoft.Identity.Web (currently 1.14.1) has been referencing Identity.Model 6.11.1 since the beginning of June. Are you on the latest version?

Identity.Model just released 6.12.0 yesterday (cc: @jennyf19 https://github.com/jennyf19)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AzureAD/microsoft-identity-web/issues/1321#issuecomment-884775197, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADC2UPNQAN4H36ZODJHZLTDTY7P73ANCNFSM5AOOXKIA .

jmprieur commented 3 years ago

Thanks for the heads-up, @jlautman @jennyf19 fixed it: https://github.com/AzureAD/microsoft-identity-web/pull/1334

jlautman commented 3 years ago

Would it be helpful if I created a separate issue to track the enhancement request for enabling decryption cert loading to check for updates periodically? I don't want to lose that feature ask since this issue split into a dependency update request and a new feature request. I don't want someone to accidentally close the issue since we resolved the former.

kingces95 commented 2 years ago

@jlautman, hello. Has there been any progress on supporting the key scenario rotation you described above? Maybe tracked somewhere else? Have you found an alternative way to dynamically load new decryption certs?

jlautman commented 2 years ago

Not that I'm aware of.

xinyi-joffre commented 1 year ago

@jmprieur, for rotation of token decryption certs, can we also add a way to specify key vault secret version, so that we can load the current and previous of the same certificate? Something like snippet below.

Currently, we have to gradually rollout First Party App changes for token encryption key (usually over a week), so it helps to have both certs loaded. With today's implementation, we have to store each key vault certificate under a different certificate name, it seems like. But if we supported keyvault certificate version, then it becomes easier to just rotate the certificate in place in key vault (without recreating a new certificate), and then reference current and previous version in our configuration.

    "TokenDecryptionCertificates": [
      {
        "SourceType": "KeyVault",
        "KeyVaultUrl": "https://kv-aqa-test.vault.azure.net",
        "KeyVaultCertificateName": "AzureQuantumTokenDecryption",
        "KeyVaultCertificateVersion": "<current cert version>"
      },
      {
        "SourceType": "KeyVault",
        "KeyVaultUrl": "https://kv-aqa-test.vault.azure.net",
        "KeyVaultCertificateName": "AzureQuantumTokenDecryption",
        "KeyVaultCertificateVersion": "<prev cert version>"
      }
    ]
xinyi-joffre commented 1 year ago

Is there any update on this feature? It would be great to get support for KeyVaultCertificateVersion to support automatic rotations via key vault. Currently, the only workaround is for us to manually copy over a certificate to a different key vault certificate name, so that we can reference both the previous and current certificate during gradual rollouts.

alphabt commented 7 months ago

Hi @jmprieur, I'm also interested in the feature to dynamically load rotated decryption certs. If you have any update on this it'd be great if you can share with us.

xinyi-joffre commented 2 months ago

Hello, checking in on this to see if there's any updates!