Skip to content

Updated AppLeaseManager hash algorithm to be FIPS compliant#640

Merged
amdeel merged 2 commits intomainfrom
hashFix
Nov 17, 2021
Merged

Updated AppLeaseManager hash algorithm to be FIPS compliant#640
amdeel merged 2 commits intomainfrom
hashFix

Conversation

@amdeel
Copy link
Contributor

@amdeel amdeel commented Nov 12, 2021

AppLeaseManager is currently using MD5 to create a hash of the appName used for appLeaseId, which is not FIPS compliant. This PR replaces that hash algorithm with an existing algorithm we use to group instanceIds to partitions.

resolves #637

@amdeel amdeel requested a review from cgillum November 12, 2021 19:20
@cgillum
Copy link
Member

cgillum commented Nov 12, 2021

The code change itself looks good (I'm glad we're able to reuse existing code). However, what will happen when existing apps upgrade to this new version of the storage provider? If the app lease ID changes after an upgrade, will that cause any issues? For example, if a customer has a primary and backup region already established, and if they upgrade one of those regions initially, will that cause both primary and backup apps to think they own the lease since they're (at least temporarily) using different app lease IDs?

@amdeel
Copy link
Contributor Author

amdeel commented Nov 12, 2021

@cgillum I'll have to be really clear about this in the release notes.

When this change goes into effect and the app is restarted, neither the primary nor the backup will be able to immediately acquire the lease while the old leaseId still is considered the app owner. So, if the customer does nothing, there will be a delay while the old lease times out and another app can acquire the lease.

I added the ForceChangeAppLeaseAsync API a while back which will allow customers to decide which app the primary is and allow for an immediate switch. This means that when the customer updates their DurableTask.AzureStorage version, they will have to use this API on their desired primary app. Customers who use the AppLease feature might be doing this already.

However, it is a bit different for customers who might not intentionally be using this feature. Because UseAppLease defaults to true currently customers could see a delay after updating if they don't use the ForceChangeAppLease API.

In the release notes, I will be clear about using that API to prevent the delay or suggesting setting UseAppLease to false temporarily.

@amdeel amdeel merged commit 5a90ece into main Nov 17, 2021
@amdeel amdeel deleted the hashFix branch November 17, 2021 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Latest DT.AzureStorage versions are not FIPS compliant

2 participants