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

[API Proposal]: Expose ProtectedMemory in ProtectedData extension #63589

Closed
AJH16 opened this issue Jan 10, 2022 · 8 comments
Closed

[API Proposal]: Expose ProtectedMemory in ProtectedData extension #63589

AJH16 opened this issue Jan 10, 2022 · 8 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security untriaged New issue has not been triaged by the area owner

Comments

@AJH16
Copy link

AJH16 commented Jan 10, 2022

Background and motivation

The protected memory extension for DPAPI offers support for SAME PROCESS encryption that is not available on ProtectData. Previously this had been brought up, but at the time there was no implementations within dotnet for the interop. Support for SecureString included the necessary interop for calling ProtectedMemory from Crypt32 so I don't see why it wouldn't make sense to expose since ProtectedData is already being exposed in the System.Security.Cryptography.ProtectedData extension.

API Proposal

namespace System.Security.Cryptography.ProtectedData
{
    public class ProtectedMemory
    {
    }
}

API Usage

Usage would match current ProtectedMemory

Alternative Designs

No response

Risks

No response

@AJH16 AJH16 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 10, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Jan 10, 2022
@ghost
Copy link

ghost commented Jan 10, 2022

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

The protected memory extension for DPAPI offers support for SAME PROCESS encryption that is not available on ProtectData. Previously this had been brought up, but at the time there was no implementations within dotnet for the interop. Support for SecureString included the necessary interop for calling ProtectedMemory from Crypt32 so I don't see why it wouldn't make sense to expose since ProtectedData is already being exposed in the System.Security.Cryptography.ProtectedData extension.

API Proposal

namespace System.Security.Cryptography.ProtectedData
{
    public class ProtectedMemory
    {
    }
}

API Usage

Usage would match current ProtectedMemory

Alternative Designs

No response

Risks

No response

Author: AJH16
Assignees: -
Labels:

api-suggestion, area-System.Security, untriaged

Milestone: -

@AJH16 AJH16 closed this as completed Jan 10, 2022
@AJH16
Copy link
Author

AJH16 commented Jan 10, 2022

Never mind. I found the reasoning a few links down on another issue. I don't really agree with it as a best effort is better than nothing and it's mostly there already.

@AJH16
Copy link
Author

AJH16 commented Jan 10, 2022

Actually, I'll reopen this to see if the thinking has changed. I noticed upon further reading that it appears that the reasoning pre-dated the inclusion of SecureString, so perhaps the reasoning has changed now. If not, feel free to re-close, but I would argue that there is still value in being able to access the SameProcess encryption for making a best effort even if it isn't 100%.

@AJH16 AJH16 reopened this Jan 10, 2022
@teo-tsirpanis
Copy link
Contributor

What prevents you from importing and using CryptProtectMemory yourself? It's a pretty simple API (no need to open handles to providers or anything like that) and since it's Windows-only exposing it is already questionable.

I am also afraid that people will misuse this API to obtain a false sense of security. SecureString already does that which is the reason it is being obsoleted. And it's not easy (if possible) to correctly use them (both SecureString and your proposal); data will still be in plaintext in memory at some point in time.

@bartonjs
Copy link
Member

I appreciate the request from the "I want this, so others probably do, too, and maybe the thinking has changed" perspective, but, unfortunately, there's still no interest to include this in the product.

  • It's a Windows-only API, and we don't like adding more single-OS API unless there's a good reason.
  • We don't think CryptProtectMemory provides a lot of value.
    • It's even (sort of) remarked as having limited value: "Using CryptProtectMemory and CryptUnprotectMemory for password encryption is not secure because the data exists as plaintext in memory before it is encrypted and at any time the caller decrypts it for use."
  • It's not that hard to P/Invoke, so we're not saving anyone a whole lot of time.

@AJH16
Copy link
Author

AJH16 commented Jan 10, 2022 via email

@AJH16
Copy link
Author

AJH16 commented Jan 10, 2022 via email

@bartonjs
Copy link
Member

... Some things like SecureString ... are implemented and actually under the hood ProtectMemory is exposed and wrapped internally.

SecureString is on the path to deprecation (which is harder than we originally thought). The stance of .NET Security is that it provides effectively no value and therefore no one should ever use it... but it got included in .NET Core / .NET5+ because of the .NET Standard 2.0 effort. It'll keep doing what it does right up until we replace its implementation with throw new PlatformNotSupportedException() (assuming we ever get the OK to do that).

@ghost ghost locked as resolved and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

3 participants