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

Introduce a read-only mode for data protection keyring consumers #52915

Closed
1 task done
amcasey opened this issue Dec 19, 2023 · 1 comment · Fixed by #53539
Closed
1 task done

Introduce a read-only mode for data protection keyring consumers #52915

amcasey opened this issue Dec 19, 2023 · 1 comment · Fixed by #53539
Assignees
Labels
area-dataprotection Includes: DataProtection

Comments

@amcasey
Copy link
Member

amcasey commented Dec 19, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

When multiple app instances consume the same keyring, they all try to rotate it, leading to races. We should have a mode where they only read the keyring and some other component updates it uncontestedly.

Describe the solution you'd like

It would be nice if this could be configured on the app author's behalf, the way single-instance data protection is now (i.e. the default configuration if you enable anti-forgery, auth tokens, etc). One easy way to accomplish that would be to make it settable via IConfiguration, so that an orchestrator could simply set an environment variable when launching the process/container.

The complexity of exposing all of data protection via IConfiguration is probably not justified (yet). Instead, I think we want two things:

  1. A configuration setting that gives a path to a read-only keyring
  2. The ability to specify in code that keyring access should be read-only

Additional context

This basically amounts to disabling automatic key-generation. As a nicety, we could also stub out write access to the keyring to make it log/throw if it happens.

@amcasey amcasey added the area-dataprotection Includes: DataProtection label Dec 19, 2023
@amcasey amcasey self-assigned this Dec 19, 2023
amcasey added a commit to amcasey/aspnetcore that referenced this issue Jan 22, 2024
When multiple app instances consume the same keyring, they all try to rotate it, leading to races.  This change introduces an IConfiguration property (usually set as an env var) that puts data protection in a read-only mode.  The expectation is that writing will be done by a separate (i.e. non-app-instance) component.

Part of dotnet#52915
@amcasey
Copy link
Member Author

amcasey commented Jan 26, 2024

Note: we don't actually need a new API - we already have a property for disabling automatic key generation. The programmatic approach is demonstrated in #53539 (though a real implementation would likely not defer to an explicit configuration).

amcasey added a commit that referenced this issue Jan 26, 2024
)

* Introduce a read-only mode for data protection keyring consumers

When multiple app instances consume the same keyring, they all try to rotate it, leading to races.  This change introduces an IConfiguration property (usually set as an env var) that puts data protection in a read-only mode.  The expectation is that writing will be done by a separate (i.e. non-app-instance) component.

Fixes #52915
github-actions bot pushed a commit that referenced this issue Feb 29, 2024
When multiple app instances consume the same keyring, they all try to rotate it, leading to races.  This change introduces an IConfiguration property (usually set as an env var) that puts data protection in a read-only mode.  The expectation is that writing will be done by a separate (i.e. non-app-instance) component.

Part of #52915
wtgodbe pushed a commit that referenced this issue Mar 6, 2024
…consumers (#54266)

* Introduce a read-only mode for data protection keyring consumers

When multiple app instances consume the same keyring, they all try to rotate it, leading to races.  This change introduces an IConfiguration property (usually set as an env var) that puts data protection in a read-only mode.  The expectation is that writing will be done by a separate (i.e. non-app-instance) component.

Part of #52915

* Add simple tests

* Handle an empty path

* Add logging

* Increase positive-case log level

Co-authored-by: Chris Ross <Tratcher@Outlook.com>

* Fix style warning

* Add an AddDataProtection test

* Add a negative AddDataProtection test

* Use a directory that exists in ConfigureReadOnly_ExplicitRepository

---------

Co-authored-by: Andrew Casey <andrew.casey@microsoft.com>
Co-authored-by: Andrew Casey <amcasey@users.noreply.github.com>
Co-authored-by: Chris Ross <Tratcher@Outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dataprotection Includes: DataProtection
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant