Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataProtection and scaling /8 #33104

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

Conversation

Rick-Anderson
Copy link
Contributor

@Rick-Anderson Rick-Anderson commented Jul 16, 2024

Fixes #32530
Replaces #33086

@amcasey maybe I don't understand the topic, In your draft " However, when an app scales horizontally by adding more instances, it becomes necessary to explicitly configure Data Protection to establish a shared storage location for Data Protection keys. "

But not in all horizontal scaling (right). It's important to not assume much knowledge for newer users.

I think we can lower the friction if I use your draft to create the article.


Internal previews

📄 File 🔗 Preview link
aspnetcore/host-and-deploy/web-farm.md aspnetcore/host-and-deploy/web-farm
aspnetcore/security/data-protection/configuration/default-settings.md aspnetcore/security/data-protection/configuration/default-settings
aspnetcore/security/data-protection/configuration/scaling.md aspnetcore/security/data-protection/configuration/scaling

@Rick-Anderson Rick-Anderson requested a review from amcasey July 16, 2024 23:53
@amcasey
Copy link
Member

amcasey commented Jul 16, 2024

" However, when an app scales horizontally by adding more instances, it becomes necessary to explicitly configure Data Protection to establish a shared storage location for Data Protection keys. "

I think my statement was correct, but I agree that it could mislead new users and should be updated. It is true that someone has to decide and configure where that shared storage location will be, but that someone may not be the app author (e.g. in ACA or App Services on Windows).

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for driving this, @Rick-Anderson! I appreciate it.

@amcasey amcasey requested a review from claudiaregio July 17, 2024 00:11
author: acasey
description: Learn how to configure Data Protection in ASP.NET Core for multi-instance apps.
ms.author: acasey
ms.custom: mvc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rick-Anderson Do you happen to know what this means/does? I just copied it from the overview.

* Newly created Azure Container Apps built using ASP.NET Core Kestrel. For more information see [Autoscaling considerations
](/azure/container-apps/dotnet-overview#autoscaling-considerations).

The following distributed environments do ***NOT*** provide automatic key storage in a shared location:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I might characterize these as scenarios where keys will not be shared across instances (even if they have a shared location).

* Separate [deployment slots](/azure/app-service/deploy-staging-slots), such as Staging and Production.
* Azure Container Apps built using ASP.NET Core Kestrel 7.0 or earlier. For more information see [Autoscaling considerations
](/azure/container-apps/dotnet-overview#autoscaling-considerations).
* Asp.net core apps hosted on multiple non-Azure VMs that don't use server affinity.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server affinity doesn't get you shared storage, it makes shared storage unnecessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, how are we going to keep this up-to-date if/when non-Azure clouds add support for this (which they can straightforwardly do, if they're interested)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, how are we going to keep this up-to-date if/when non-Azure clouds add support for this (which they can straightforwardly do, if they're interested)?

See #33108

The following distributed environments provide automatic key storage in a shared location:

* [Azure apps](/aspnet/core/security/data-protection/configuration/default-settings). For more information see <xref:security/data-protection/configuration/default-settings#key-management>.
* Newly created Azure Container Apps built using ASP.NET Core. For more information see [Autoscaling considerations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've asked ACA for a date we can use in place of "newly created".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you get a date?

The following distributed environments provide automatic key storage in a shared location:

* [Azure apps](/aspnet/core/security/data-protection/configuration/default-settings). For more information see <xref:security/data-protection/configuration/default-settings#key-management>.
* Newly created Azure Container Apps built using ASP.NET Core. For more information see [Autoscaling considerations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claudiaregio Can you remember how we scoped this? I've suddenly remembered that this might only affect Aspire apps for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claudiaregio Can you remember how we scoped this? I've suddenly remembered that this might only affect Aspire apps for now?

@claudiaregio ?

* Newly created Azure Container Apps built using ASP.NET Core. For more information see [Autoscaling considerations
](/azure/container-apps/dotnet-overview#autoscaling-considerations).

The following scenarios do ***NOT*** provide automatic key storage in a shared location:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list still feels off to me. What if we framed it as a list of caveats and dropped the mention of non-Azure and ARR?

If ARR is important, we could say that it's an alternative (that applies everywhere, not just non-Azure) though, personally, I think we covered that adequately in the intro.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amcasey Can you use the suggestion feature to rough draft your approach? I can wordsmith your suggestion

@Rick-Anderson Rick-Anderson changed the title DataProtection and scaling DataProtection and scaling /8 Jul 23, 2024

## Managing Data Protection keys outside the app

An app with multiple instances may occasionally see an error like `System.Security.Cryptography.CryptographicException: The key {A6EF5BC2-FDCC-4C0C-A3A5-CDA9A1733D70} was not found in the key ring.`. This can happen when instances get out of sync and data protected on one instance (e.g. an anti-forgery token) is unprotected on another instance (e.g. because a form was served from the former and posted to the latter) that doesn't yet know about that key. When this happens, an app user may have to resubmit a form or re-authenticate (if it was an authentication token that couldn't be unprotected).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wordsmith away.


For example, with Azure blob storage as the key repository, the key manager could be a simple console app run on a schedule.

```csharp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure there's a more conventional way to include code samples. Let me know if you want a zip of the project.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved them.


Note that app instances will throw exceptions if they need to perform any `Protect` or `Unprotect` operations before the key manager has run for the first time, so it is preferable to execute it before creating app instances.

:::moniker-end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out a nice way to phrase "ACA will do this for you automatically, if you let it".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review my rewrite and use the suggestion feature if needed.

aspnetcore/host-and-deploy/web-farm.md Show resolved Hide resolved
Comment on lines 27 to 29
The following distributed environments do ***NOT*** provide automatic key storage in a shared location:

* Separate [deployment slots](/azure/app-service/deploy-staging-slots), such as Staging and Production.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amcasey @adityamandaleeka, for customers on the .NET 8+, the list where it doesn't work is just deployment slots. Is that the only place it doesn't work with .NET 8+

The following distributed environments provide automatic key storage in a shared location:

* [Azure apps](/aspnet/core/security/data-protection/configuration/default-settings). For more information see <xref:security/data-protection/configuration/default-settings#key-management>.
* Newly created Azure Container Apps built using ASP.NET Core Kestrel. For more information see [Autoscaling considerations
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amcasey What's newly created? Can we be more precise?


For example, with Azure blob storage as the key repository, the key manager could be a simple console app run on a schedule.

```csharp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved them.


Note that app instances will throw exceptions if they need to perform any `Protect` or `Unprotect` operations before the key manager has run for the first time, so it is preferable to execute it before creating app instances.

:::moniker-end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review my rewrite and use the suggestion feature if needed.


var hostBuilder = new HostApplicationBuilder();

// hostBuilder.Configuration.AddJsonFile("appsettings.json", optional: false, reloadOnChange: false);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amcasey not needed so commented out. Will remove line or comment characters.

{
"AzureURIs": {
"BlobStorage": "https://<storage-account>.blob.core.windows.net/<container>/keys.xml",
"KeyVault": "https://<key-vault-name>.vault.azure.net/keys/<key-name>/"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"KeyVault": "https://<key-vault-name>.vault.azure.net/keys/<key-name>/"
"KeyVault": "https://<key-vault-name>.vault.azure.net/"

Pls review my suggested change from your version.

Comment on lines +6 to +12
"Logging": {
"LogLevel": {
"Default": "Information",
"Microsoft.AspNetCore": "Warning"
}
},
"AllowedHosts": "*"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this JSON but that's what the templates produce and in the article I yellow highlight the AKV URIs

@Rick-Anderson Rick-Anderson requested review from amcasey and claudiaregio and removed request for claudiaregio December 6, 2024 01:52
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.

Update Data Protection docs to include default behavior when deploying ASP.NET Core apps to ACA
3 participants