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

Question: best practice of accepting password for X509Certificate2 constructors in .NET5 #51415

Closed
heng-liu opened this issue Apr 16, 2021 · 4 comments
Labels
area-System.Security question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@heng-liu
Copy link

heng-liu commented Apr 16, 2021

Description

From the doc , there are several constructors have a parameter of a password used to access the certificate.
e.g.

X509Certificate2(String, String)
X509Certificate2(String, SecureString)
X509Certificate2(String, ReadOnlySpan<Char>, X509KeyStorageFlags)

As we found the SecureString should not be used at https://github.com/dotnet/platform-compat/blob/master/docs/DE0001.md.
And we also found new API proposals in .NET 6 at https://github.com/dotnet/designs/pull/147/files

If we are going to implement a command with .NET 5, accepting the password(if there is any) from customer, then sign with the certificate, is it a problem if just using String to store the password?
If it's not acceptable, is there any standard we should follow, since SecureString is also not recommended?
And is there any applicable way to get a password and sign with the certificate safely?
Especially in .NET 5 on Windows and non-Windows platforms.
Thanks!

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Apr 16, 2021
@ghost
Copy link

ghost commented Apr 16, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

From the doc , there are several constructors have a parameter of a password used to access the certificate.
e.g.

X509Certificate2(String, String)
X509Certificate2(String, SecureString)
X509Certificate2(String, ReadOnlySpan<Char>, X509KeyStorageFlags)

As we found the SecureString should not be used at https://github.com/dotnet/platform-compat/blob/master/docs/DE0001.md.
And we also found new API proposals in .NET 6 at https://github.com/dotnet/designs/blob/4765bb3d4f6df0a0a2fed09f0e09c1a56d209415/accepted/2020/better-obsoletion/securestring-obsoletion.md#new-proposed-securestring-replacement-apis

If we are going to implement a command with .NET 5, accepting the password(if there is any) from customer, then sign with the certificate, is it a problem if just using String to store the password?
If it's not acceptable, is there any standard we should follow since SecureString is also not recommended to be used?
And is there any applicable way to get a password and sign with the certificate safely?
Especially in .NET 5 on Windows and non-Windows platforms.
Thanks!

Author: heng-liu
Assignees: -
Labels:

area-System.Security, untriaged

Milestone: -

@huoyaoyuan
Copy link
Member

Related: #48697 (comment)

If you really care about security, you could manage some memory yourself, and pass it through span.

@bartonjs
Copy link
Member

is it a problem if just using String to store the password?

This is a very polarizing question, and can depend on app specifics, but the general answer is "no, it's not a problem". The risk of using System.String is that the password value can remain in-memory long after it is no longer needed. If memory inspection and memory dumps aren't a strong threat for your scenarios, then you're good to go.

If it's not acceptable, is there any standard we should follow, since SecureString is also not recommended?

If memory inspection and/or memory dumps of your process containing a password (beyond when it's needed) is a crisis, then you can do something like

char[] arr = GC.AllocateArray<char>(512, pinned: true);
int offset = 0;

while (true)
{
    int ch = Console.Read();

    if (ch == -1 || ch == (int)'\n')
    {
        break;
    }

    if (ch == (int)'\r')
    {
        ch = Console.Peek();

        if (ch == (int)'\n')
        {
            Console.Read();
        }

        break;
    }

    if (offset == arr.Length)
    {
        char[] arr2 = GC.AllocateArray<char>(arr.Length * 2, pinned: true);
        arr.AsSpan().CopyTo(arr2);
        arr.Clear();
        arr = arr2;
    }

    arr[offset] = (char)ch;
    offset++;
}

X509Certificate2 cert = new X509Certificate2(bytes, arr.AsSpan(0, offset), flags);
arr.Clear();
...

But that's a lot of work.

@heng-liu
Copy link
Author

Thanks for your help @bartonjs !

@bartonjs bartonjs added this to the Future milestone Apr 19, 2021
@bartonjs bartonjs added question Answer questions and provide assistance, not an issue with source code or documentation. and removed untriaged New issue has not been triaged by the area owner labels Apr 19, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

3 participants