DuendeSoftware / Support

Support for Duende Software products
20 stars 0 forks source link

Automatic key management not cleaning up retired keys? #1284

Closed QvAchthoven closed 1 month ago

QvAchthoven commented 1 month ago

Which version of Duende IdentityServer are you using? Identity Server 7.0.4

Which version of .NET are you using? .NET 8.0.200

Describe the bug (more a question really) We are having some trouble with rapid signing key cycling on our production environment which I assume is due to a lack of Data Protection configuration. We will be releasing an update with basic Data Protection configuration which should resolve the rapid key cycling. This update will include a migration from IdentityServer 5.x to version 7.0.4 (which might change KeyManagement behaviour).

We have not changed any of the default Key management settings. However I noticed the table contains a lot of keys which are older than the default 14 (PropagationTime) + 90 (RotationInterval) + 14 (RetentionDuration) days.

I would expect keys which are older than (14 + 90 + 14) days to be removed from the keys table as 'DeleteRetiredKeys' defaults to true. Our table currently holds multiple records a day, dating all the way back to May 2022.

Am I missing something or is my assumption correct? Did anything mayor change between 5.x and 7.04 concering the Key managament? (maybe the previous IdentityServer version's key management didn't remove old keys yet?)

Rapid key cycling (most likely due to missing Data Protection configuration and load balancing/ IIS recyling the IdentityServer instance) recent keys

RolandGuijt commented 1 month ago

@QvAchthoven I think your assumption is correct. The issue is that when keys can't be unprotected they are not deleted. That is something we're going to consider changing but it will require some internal discussions to see if we can change this behavior without major consequences. I've created an issue in the IdentityServer repository for this. Please track the issue using that from now on. I'm closing this one.

I haven't been able to determine why this behavior would be any different in version 5. The code that does the retiring looks the same in essence.