microsoft-identity-web icon indicating copy to clipboard operation
microsoft-identity-web copied to clipboard

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

Open jlautman opened this issue 4 years ago • 14 comments

Documentation related to component

Token Decryption

Please check all that apply

  • [ ] typo
  • [ ] documentation doesn't exist
  • [ x] documentation needs clarification
  • [ ] error(s) in the example
  • [ ] needs an example

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?

jlautman avatar Jul 15 '21 22:07 jlautman

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)

jmprieur avatar Jul 16 '21 09:07 jmprieur

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 .

jlautman avatar Jul 17 '21 18:07 jlautman

@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).

jmprieur avatar Jul 19 '21 07:07 jmprieur

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 .

jlautman avatar Jul 21 '21 23:07 jlautman

@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)

jmprieur avatar Jul 22 '21 09:07 jmprieur

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 .

jlautman avatar Jul 22 '21 16:07 jlautman

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

jmprieur avatar Jul 23 '21 11:07 jmprieur

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.

jlautman avatar Jul 26 '21 16:07 jlautman

@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?

kingces95 avatar Feb 11 '22 20:02 kingces95

Not that I'm aware of.

jlautman avatar Feb 11 '22 21:02 jlautman

@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 avatar Apr 19 '23 00:04 xinyi-joffre

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.

xinyi-joffre avatar Sep 29 '23 02:09 xinyi-joffre

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.

alphabt avatar Apr 19 '24 07:04 alphabt