On upgrade to .NET 7, Data Protection throws "Payload was invalid" error when unprotecting values from .NET 6

[analogrelay]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

We use Data Protection to encrypt sensitive values in long-term storage. While testing .NET 7.0, we found that our app was unable to decrypt payloads encrypted by .NET 6.0, even when the same keys were available and the same purpose was used. This means that upgrading to .NET 7 causes users to be logged out of our site (since the auth cookie cannot be validated) and our app is no longer able to decrypt data which it had previously encrypted and stored in durable storage (database, etc.).

I was able to isolate this into a simple repro console app: https://github.com/anurse/dataprotection-repro

The app runs on both net6.0 and net7.0 and can protect/unprotect values. Running .\ReproApp protect "Some string" on net6.0 produces a Base64 protected value. Passing that same payload into .\ReproApp unprotect on net6.0 successfully unprotects the string. However, passing it in to the same .\ReproApp unprotect command in the same app when running on net7.0 fails with the following error:

Unhandled exception. System.Security.Cryptography.CryptographicException: The payload was invalid. For more information go to http://aka.ms/dataprotectionwarning
   at Microsoft.AspNetCore.DataProtection.Managed.ManagedAuthenticatedEncryptor.Decrypt(ArraySegment`1 protectedPayload, ArraySegment`1 additionalAuthenticatedData)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.UnprotectCore(Byte[] protectedData, Boolean allowOperationsOnRevokedKeys, UnprotectStatus& status)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.Unprotect(Byte[] protectedData)
   at Microsoft.AspNetCore.DataProtection.DataProtectionCommonExtensions.Unprotect(IDataProtector protector, String protectedData)
   at Program.Main(String[] args) in /Users/anurse/code/anurse/dataprotection-repro/ReproApp/Program.cs:line 35
   at Program.<Main>(String[] args)

Further debugging traced this down to this line:

aspnetcore/src/DataProtection/DataProtection/src/Managed/ManagedAuthenticatedEncryptor.cs

Lines 235 to 240 in 8f0d440

	// Step 4: Validate the MAC provided as part of the payload.
	if (!CryptoUtil.TimeConstantBuffersAreEqual(correctHash, 0, correctHash.Length, protectedPayload.Array!, macOffset, eofOffset - macOffset))
	{
	throw Error.CryptCommon_PayloadInvalid(); // integrity check failure
	}

The MAC check appears to be failing. I wasn't using a Debug build, so I wasn't able to check if the MAC was actually different, or if this is a bug in the equality comparison.

Expected Behavior

When running the run-repro script in the provided repository, the output should be something like this:

... build output ...

Protecting: 'This is a test message'
*** Protecting with .NET 6.0 ... ***
Protected string: <trimmed>
Unprotecting with .NET 6.0 ...
This is a test message
Unprotecting with .NET 7.0 ...
This is a test message
*** Protecting with .NET 7.0 ... ***
Protected string: <trimmed>
Unprotecting with .NET 6.0 ...
This is a test message
Unprotecting with .NET 7.0 ...
This is a test message

The actual behavior is that an exception is thrown when unprotecting with a different major version than the data was originally protected with:

... build output ...

Protecting: 'This is a test message'
*** Protecting with .NET 6.0 ... ***
Protected string: <trimmed>
Unprotecting with .NET 6.0 ...
This is a test message
Unprotecting with .NET 7.0 ...
Unhandled exception. System.Security.Cryptography.CryptographicException: The payload was invalid. For more information go to http://aka.ms/dataprotectionwarning
   at Microsoft.AspNetCore.DataProtection.Managed.ManagedAuthenticatedEncryptor.Decrypt(ArraySegment`1 protectedPayload, ArraySegment`1 additionalAuthenticatedData)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.UnprotectCore(Byte[] protectedData, Boolean allowOperationsOnRevokedKeys, UnprotectStatus& status)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.Unprotect(Byte[] protectedData)
   at Microsoft.AspNetCore.DataProtection.DataProtectionCommonExtensions.Unprotect(IDataProtector protector, String protectedData)
   at Program.Main(String[] args) in /Users/anurse/code/anurse/dataprotection-repro/ReproApp/Program.cs:line 35
   at Program.<Main>(String[] args)
*** Protecting with .NET 7.0 ... ***
Protected string: <trimmed>
Unprotecting with .NET 6.0 ...
Unhandled exception. System.Security.Cryptography.CryptographicException: The payload was invalid. For more information go to http://aka.ms/dataprotectionwarning
   at Microsoft.AspNetCore.DataProtection.Managed.ManagedAuthenticatedEncryptor.Decrypt(ArraySegment`1 protectedPayload, ArraySegment`1 additionalAuthenticatedData)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.UnprotectCore(Byte[] protectedData, Boolean allowOperationsOnRevokedKeys, UnprotectStatus& status)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.Unprotect(Byte[] protectedData)
   at Microsoft.AspNetCore.DataProtection.DataProtectionCommonExtensions.Unprotect(IDataProtector protector, String protectedData)
   at Program.Main(String[] args) in /Users/anurse/code/anurse/dataprotection-repro/ReproApp/Program.cs:line 35
   at Program.<Main>(String[] args)
Unprotecting with .NET 7.0 ...
This is a test message

Steps To Reproduce

1. Clone the repro repo: http://github.com/anurse/dataprotection-repro
2. Ensure you have .NET 6 and .NET 7 installed
3. Mac/Linux: Run ./run-repro script
4. Windows:
   1. Run dotnet run --project .\ReproApp --framework net6.0 -- protect "a test string"
   2. Copy the output string
   3. Run dotnet run --project .\ReproApp --framework net6.0 -- unprotect "<paste>"
   4. Run dotnet run --project .\ReproApp --framework net7.0 -- unprotect "<paste>"

Exceptions (if any)

Unhandled exception. System.Security.Cryptography.CryptographicException: The payload was invalid. For more information go to http://aka.ms/dataprotectionwarning
   at Microsoft.AspNetCore.DataProtection.Managed.ManagedAuthenticatedEncryptor.Decrypt(ArraySegment`1 protectedPayload, ArraySegment`1 additionalAuthenticatedData)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.UnprotectCore(Byte[] protectedData, Boolean allowOperationsOnRevokedKeys, UnprotectStatus& status)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.Unprotect(Byte[] protectedData)
   at Microsoft.AspNetCore.DataProtection.DataProtectionCommonExtensions.Unprotect(IDataProtector protector, String protectedData)
   at Program.Main(String[] args) in /Users/anurse/code/anurse/dataprotection-repro/ReproApp/Program.cs:line 35
   at Program.<Main>(String[] args)

.NET Version

7.0.100-preview.7.22377.5

Anything else?

Since multiple .NET runtimes are involved, here's my dotnet --info output:

.NET SDK:
 Version:   7.0.100-preview.7.22377.5
 Commit:    ba310d9309

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  12.0
 OS Platform: Darwin
 RID:         osx.12-arm64
 Base Path:   /usr/local/share/dotnet/sdk/7.0.100-preview.7.22377.5/

Host:
  Version:      7.0.0-preview.7.22375.6
  Architecture: arm64
  Commit:       eecb028078

.NET SDKs installed:
  6.0.301 [/usr/local/share/dotnet/sdk]
  6.0.400 [/usr/local/share/dotnet/sdk]
  7.0.100-preview.7.22377.5 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.6 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.8 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.0-preview.7.22376.6 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.6 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.8 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0-preview.7.22375.6 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

[blowdart]

@adityamandaleeka this should get triaged asap. If upgrading breaks data protection with an existing keyring that's bad.

[HaoK]

@wtgodbe this might be related to #40964 again, maybe there's a different code path that also needs normalization

[wtgodbe]

@anurse thanks for the report! I'm investigating this now

[wtgodbe]

It turns out this repros on any version-to-version upgrade (3.1 to 5.0, 5.0 to 6.0, 6.0 to 7.0) - if you protect with one version of .Net, you can't unprotect with another. Maybe it's expected? CC @GrabYourPitchforks - it could be because part of the protected string is ContentRoot, which by default is the output path of the app, which includes the TFM (e.g. "C:\\code\\dataprotection\\dataprotection-repro-7\\ReproApp\\bin\\Debug\\net7.0\\")

[GrabYourPitchforks]

Answered this in an email thread, but the app discriminator should be stable across app upgrades. If it includes the TFM, that would explain the behavior we're seeing.

If you trace the history and unit tests for the DataProtection component, you'll find that it traditionally has used stable strings like the IIS virtual directory as the app discriminator. I don't know why the application directory is being passed in here.

[wtgodbe]

It looks like HostingApplicationDiscriminator has been using ContentRoot since at least 2.1:

aspnetcore/src/DataProtection/DataProtection/src/Internal/HostingApplicationDiscriminator.cs

Line 23 in fd6923c

public string Discriminator => _hosting?.ContentRootPath;

Which is used by the ServiceCollectionExtensions for DataProtection (What Andrew is using in their app):

aspnetcore/src/DataProtection/DataProtection/src/DataProtectionServiceCollectionExtensions.cs

Line 79 in fd6923c

services.TryAddSingleton<IApplicationDiscriminator, HostingApplicationDiscriminator>();

Whereas the the Builder extension method has traditionally used the app name:

aspnetcore/src/DataProtection/DataProtection/src/DataProtectionBuilderExtensions.cs

Line 47 in fd6923c

options.ApplicationDiscriminator = applicationName;

[Tratcher]

The TFM is only in the ContentRoot for an un-published app (e.g. dotnet run). A published app shouldn't discriminate by the TFM. I think that would explain your sample, but there might be a second underlying issue.

ContentRoot is used as the discriminator so you can run multiple instances of a site and keep them separate. We shouldn't change that.

[wtgodbe]

I just confirmed this works (5.0 -> 6.0 & 6.0 -> 7.0) if the app has been dotnet published first, but not if you're just running dotnet run - so I don't think we'll be breaking customers in production. @anurse were you testing this w/ just dotnet run? In that scenario ContentRoot will have the TFM string in it, which explains the behavior.

[analogrelay]

I was running the repro app with dotnet run, and I was testing our app via launching it in Rider. I expect that runs from the build output, which includes the TFM in it.

The fact that the ContentRoot is the app discriminator indicates a bigger problem in using Data Protection for "durable" storage of protected values, since we are currently in the process of changing how we deploy our production app and will likely change the ContentRoot as a result.

It feels surprising that the path on which the app was deployed is used as part of the encryption since that's definitely not guaranteed to be stable over the lifetime of an app. Consider the case where the app is moved to a different deployment model (for example, from App Service to Docker) and suddenly all the previously-encrypted values are lost.

To solve the immediate problem, we can specify an explicit ApplicationDiscriminator (likely just using the old ContentRoot-based value explicitly for now). It just feels like a surprising behavior to me that moving my app to a different directory would break it's access to all encrypted payloads. Perhaps a model more like User Secrets would be more stable, where the project file specifies a "DataProtectionAppDiscriminator" property that's generated once when the project is created and should remain stable unless explicitly changed.

[brockallen]

It feels surprising that the path on which the app was deployed is used as part of the encryption since that's definitely not guaranteed to be stable over the lifetime of an app. Consider the case where the app is moved to a different deployment model (for example, from App Service to Docker) and suddenly all the previously-encrypted values are lost.

This is one of the biggest support problems we have for customers. The ultimate solution is that they need to explicitly configure things, because the ambient values to make it F5 experience work well don't work all that well in practice. I get the rational for it all, so not a criticism -- it's just that data protection is an afterthought, and the fact that it's not more in the forefront of developers' mind is unfortunate. Education here is important.

[analogrelay]

It looks like HostingApplicationDiscriminator has been using ContentRoot since at least 2.1:

It's also notable that the 7.0 version is doing normalization of the trailing slash that wasn't being done before. The 6.0 version is:

aspnetcore/src/DataProtection/DataProtection/src/Internal/HostingApplicationDiscriminator.cs

Line 23 in 6a01dd1

public string? Discriminator => _hosting?.ContentRootPath;

Whereas the version in main is:

aspnetcore/src/DataProtection/DataProtection/src/Internal/HostingApplicationDiscriminator.cs

Lines 30 to 43 in e40c0fd

	public string? Discriminator
	{
	get
	{
	var contentRoot = _hosting?.ContentRootPath?.Trim();
	if (string.IsNullOrEmpty(contentRoot) ||
	contentRoot.EndsWith(DirectorySeparator, StringComparison.OrdinalIgnoreCase) ||
	contentRoot.EndsWith(AltDirectorySeparator, StringComparison.OrdinalIgnoreCase))
	{
	return contentRoot;
	}
	return contentRoot + DirectorySeparator;
	}
	}

On my macOS machine, when launching from Rider, the ContentRoot is:

/Users/anurse/code/aseriousbiz/abbot/src/product/Abbot.Web

This is the same in both .NET 6.0 and .NET 7.0, but because of the normalization, the .NET 7.0 version ends up putting a trailing slash on the end:

/Users/anurse/code/aseriousbiz/abbot/src/product/Abbot.Web/

I don't think dotnet publish has an impact on the trailing slashes in ContentRootPath, so it seems from that like there is still a risk of breaking things in production.