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

New ReadOnlySpan constructor for SecureString #44873

Closed
xtqqczze opened this issue Nov 18, 2020 · 2 comments
Closed

New ReadOnlySpan constructor for SecureString #44873

xtqqczze opened this issue Nov 18, 2020 · 2 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner

Comments

@xtqqczze
Copy link
Contributor

Background and Motivation

In PowerShell project we sometimes create a SecureString from a string, e.g. in SecureStringCommands, by using the following pattern:

SecureString ss = new SecureString();
foreach (char t in password)
    ss.AppendChar(t);

Unfortunately, it appears that every call to AppendChar results in an allocation, which I suppose we could avoid with the following unsafe code:

SecureString ss;
fixed (char* pChars = &password.GetPinnableReference())
    ss = new SecureString(pChars, password.Length);

However the constructor SecureString(char* value, int length) creates a ReadOnlySpan to initialize from, so why not expose a constructor that takes a ReadOnlySpan?

Proposed API

namespace System.Security
{
+    public SecureString(ReadOnlySpan<char> value) {
+        Initialize(value);
+    }

Usage Examples

Alternative Designs

Risks

@xtqqczze xtqqczze added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 18, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Nov 18, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@stephentoub
Copy link
Member

stephentoub commented Nov 18, 2020

Thank you for the suggestion, but we aren't investing in SecureString and we don't want to increase its usage. In fact, we're moving in the opposite direction: dotnet/designs#147. The example you highlight is a good example of why: it's populating a SecureString from a string, which obviates any possible benefit of SecureString in the first place.
cc: @GrabYourPitchforks

@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 2020
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 untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

3 participants