Closed GrabYourPitchforks closed 4 years ago
@jozefizso You thumbed down this idea, so I'm assuming you use securestring right now. Is there any chance you could expand on your objection here?
Yes, for example the PasswordBox
control returns password as SecureString
. This prevents the password from being written out to log files and it’s required to work with some native Win32 authentication API.
The same way you can remove MD5 hash algorithm from .NET because people are using it to insecurely store passwords.
You've got about the only good example of using it right, powershell passwords being the other. MD5 at least as more legitimate uses (and is baked into the HTTP standard. Heck we can't remove 3DES because it's part of EMV).
If it was moved out of corefx and into a separate, unsuipported package, where eventually a lot of bad decisions will go and which would have a horrific name like Microsoft.Unsupported.UnsafeThings (this is why no-one lets me name stuff), where you could opt into using it, and would have to put an extra flag in your solution to maybe make people think some more, would that be helpful?
FYI I'm not suggesting deleting the type, only obsoleting it.
Jozef, who is the adversary in your scenarios threat model? How does the use of SecureString in this particular scenario thwart the adversary?
Edit: I ask not to be argumentative, but because there might exist an opportunity to create a better type for your scenario. We might be able to craft something more narrow in scope than what SecureString was trying to accomplish, and perhaps that replacement would be better suited for your application.
In addition to logging, PowerShell also relies on this type to determine when to prompt without echo. Would it make sense to have a SensitiveString
type that would also zero out memory on dispose? At least this would set some expectation, and maybe allow migration over time.
SecureString
has it's obvious usages in the WinForms, WPF and PowerShell API.
Without providing any replacement nor information how to do it right and what will you do to make existing workflows like asking for passwords in console apps, password boxes, etc. and preventing passwords from leaking in places like loggin it does make sense to delete a class from .NET Framwork.
Having such a needed class in unsupported library? Ehm, no, just no.
Part of the point of the issue is to gather feedback on what you use it for, however "obvious" uses doesn't help us very much. We want to know
We don't want to let our own biases make assumptions, we want you to be explicit please.
On crash dumps: https://devblogs.microsoft.com/oldnewthing/20161104-00/?p=94645
Related issue: GC support for zeroing the managed heap on compaction (https://github.com/dotnet/coreclr/issues/18386). The premise of that work item is to limit exposure of sensitive data in memory, regardless of whether it's in a string, array, or some other type.
@jozefizso I admit I am a little confused when you mention Win32. Windows has no idea about securestrings, they're purely a .NET concept. Which APIs are you thinking of here?
@jozefizso That's not a Win32 method, that's the marshaller method for how to pass a SecureString to a C function that takes wchar*
. (Decrypt it, pass the pointer to where it's decrypted) Whatever Win32 functions are being called could work just as well with System.String
, or char[]
.
I'm not a professional developer yet, so I don't know if my opinion counts, but it seems weird to me that this would be obsoleted. Aren't there still benefits in using this? If you're careful you can make sure that the string has a short, predetermined, lifetime. But maybe I'm missing something: 'obsoleted' means that it is no longer needed because there is a better alternative, right? Can you tell me what this alternative would be? (For cases where we request a password from the user.) It seems to me that using a normal string would not be better, so I guess this is not what you mean.
Aren't there still benefits in using this?
SecureString, on Windows, is useful if:
wchar*
(aka C# char*
) in the SecureString(char*, int)
constructor.char*
from the Marshaller output).Effectively, if you've ever had the value in a System.String
instance then you've lost all the "value" SecureString offers.
'obsoleted' means that it is no longer needed because there is a better alternative, right? Can you tell me what this alternative would be?
It depends on your threat model / risk concern:
System.String
in a struct with no ToString
defined (or public override string ToString() => "** REDACTED **";
System.String
, and upvote https://github.com/dotnet/coreclr/issues/18386.System.String
.It seems to me that using a normal string would not be better, so I guess this is not what you mean.
It is. Just use System.String
. Statistically speaking, you already had the value in a System.String
(via a web request, or File.ReadAllText, or whatever), or you're at some point going to write a method like SecureStringToString
, and now you've just warmed up your CPU for no real benefit.
If you're careful you can make sure that the string has a short, predetermined, lifetime.
So long as you never copied the data into byte[]
, char[]
, or System.String
, sure. Most people aren't careful.
but it seems weird to me that this would be obsoleted
We actually docs-deprecated it more than 2 years ago: https://github.com/dotnet/platform-compat/blob/master/docs/DE0001.md, with a big warning on MSDN/docs.microsoft.com (https://docs.microsoft.com/en-us/dotnet/api/system.security.securestring?view=netframework-4.8)
Now we're just moving it to a compile warning so more people notice.
@bartonjs Thanks for the info! It's unfortunate that people are not careful in their use of SecureString
, of course in the cases you mentioned there is no difference except for the word Secure. I still have a question: is the point of this warning that people move to System.String
, which is less secure or as secure at best (if you weren't careful with SecureString
), or is the point that people avoid storing passwords in their memory altogether?
From the code sample on MSDN it was obvious, that the password is secured in a general scenario - PasswordBox up to the LogonUser.
As a developer, SecureString provides a promise of storing password securely and I don't have to deal with Secure Kernel stuff.
I don't know which one is worse - that you failed to document the SecureString class does not provide any security (it appears it behaves exactly contrary to documented behavior) or that you are removing a useful class.
Can it be misused? Of course it can. With such premise we can delete the whole .NET Framework and not just this single class.
PS: if you are concerned by misusing the SecureString class, what do you think people will do without it? They will create their own crypto so applications will be even more insecure.
It depends on your threat model / risk concern:
- If you believe someone is actively inspecting your memory (live or hiberfil.sys): This threat applies equally to any secret in memory in a non-encrypted form. If the answer is "don't use passwords but use certificate based logon instead" the private key associated with the cert is in memory - is it removed quickly enough to be safe ? If a user has a password manager which pastes passwords into a browser, those are in memory (encrypted via the clipboard) for an unknowable period of time. Authenticators implemented as apps have their secret in memory....
The only way to avoid this is for the secret to be stored and used outside the machines memory (e.g. by a smart card , TPM or co-processor), and can be mitigated by not keeping the secret in memory for any longer than needed.
The "DE0001: SecureString shouldn't be used" also starts with a misconception, I think, its purpose - is not to avoid having secrets stored in the process memory as plain text. The primary use - and I am talking from the purpose of someone engaged in automation, rather than end user apps - is to persist secrets securely. This is a job it does reasonably well... If the concern is the user's typed password, anyone able to scan memory can probably log keystrokes.
Now we're just moving it to a compile warning so more people notice. And that just teaches people to ignore compile warnings; unless the warning is to dispose of the secure string object safely as early as possible it is of a very little value
PS: if you are concerned by misusing the SecureString class, what do you think people will do without it? They will create their own crypto so applications will be even more insecure.
Indeed. Not only will that mean homebrew applications which store things less securely - "Just XOR everything with 42, it'll be fine", but the problem of the secret being in memory in the clear is not solved.
SecureString is not perfect. SecureString can be misused. SecureString can be misunderstood. Deprecating SecureString is pretty much giving up on providing a solution for providing some degree of protection of secrets.
No, SecureString's value is not limited to Win32 APIs. We are working on Azure integration service and we deal with passwords, credentials and secrets all the time (we handle over 300 services, which have a great range of authentication methods). We don't want those in the clear in memory. We do provide support for non-password based credentials when possible, which is not always the case. There are many, many APIs -local OS and remote services- which still rely on username password credentials. Some 3rd party clients have learn to use SecureString to protect the passwords they pass to their own server service (with on-the-wire encryption, so that the secrets goes from in-memory encrypted to on-the-wire encrypted). We further would like to see SecureString implemented in more client for security sensitive content such as HTTPS, KeyVault, ... so that we can do as third party have learnt to do (keeping secrets encrypted at rest, on the wire, in-memory at all time while never decrypting full buffer at once). We also use SecureString as a way to flag all sensitive fields, it makes it much easier to spot them for stripping for logs, UX, and even have different user access level controls (e.g. an ops engineer can see the service's health but can't see secure payload, or JIT access approval is required).
Removing SecureString and telling developers to go implement their own encrypted approach is baffling. Improve it instead. Provide better pattern. Implement code analysis detector for unsafe use of SecureString. Don't give up on security. Ever.
Removing SecureString and telling developers to go implement their own encrypted approach is baffling. Improve it instead. Provide better pattern. Implement code analysis detector for unsafe use of SecureString. Don't give up on security. Ever.
Nobody is telling developers to implement their own approach to encryption, and nobody is giving up on security. In fact, the proposed replacement (see https://github.com/dotnet/coreclr/issues/18386) clears out memory more aggressively and prevents secrets leakage better than SecureString
ever did. It's a way to move the entire platform forward while leaving behind types like SecureString
which were never able to live up to their promise.
Very interesting discussion around SecureString. We've got some services that connect to Azure SQL DB from a WinForms app (No ASP.NET). Need to encrypt the ConnectionString's password (this password is not typed, by an user, it's hardcoded in the ConnectionString itself). I'm not able to use Azure's AKS as I'd still need to hardcode at least one secret. Would you recommend SecureString or any other approach to get this done securely? Thanks!
@sterenas Your scenario involves encryption of data at rest, such as connection strings being read from a config file? SecureString
never helped with those scenarios; it only ever worked on ephemeral data.
If you've got persisted data you need to encrypt on the local box, using X.509 certificates (for server farms) or DPAPI (for consumer machines) is a typical solution. The data is encrypted at rest. Your application decrypts the payload locally and then can operate on the plaintext as it sees fit.
Your scenario involves encryption of data at rest, such as connection strings being read from a config file?
SecureString
never helped with those scenarios; it only ever worked on ephemeral data.
This may be technically true - in that when creating (for example) a PowerShell credential object it uses a secure string, but when we export that credential (e.g. get-credential | export-clixml cred.xml
) the data is a long series of digits printed to a text file (not technically a secure string). That gets turned back into a secure string, and back into a credential. In that sense SecureString
is pivotal to a process of securing keys and passwords at rest.
With a certificate the problem is merely moved. Whatever has the private key can authenticate. So now instead of having to keep a password or API key secure, the need is to keep the Private Key both secure secure at rest, AND available transparently.
So now instead of having to keep a password or API key secure, the need is to keep the Private Key both secure secure at rest, AND available transparently.
Certificate private keys can be marked non-exportable. As long as the cert is in the personal or machine store, the calling process never has access to the raw private key material. That's why using certs is so popular especially in headless environments like production servers. For consumer environments, DPAPI offers a similar capability.
Updated proposal at https://github.com/dotnet/designs/pull/147.
The
SecureString
type does not actually fulfill the promises it makes to developers. MSDN already disrecommends its use in new code, and there's other documentation listing some technical reasons why it can't fulfill its promises.We've already seen customers who use this type in server applications in an effort to fulfill auditing / compliance requirements, then those customers get bitten because they fail their audits. I detailed this a little more on Twitter, including suggesting mechanisms that customers could use to meet compliance requirements.
This legacy type is a different scenario than legacy types like
ArrayList
(the non-generic collection type). Types likeArrayList
aren't recommended for new code because there are better alternatives available, but there's no real harm in keeping those older types around for compatibility reasons. Contrast this againstSecureString
, where the existence of the type is causing active harm to customers who use it and subsequently fail audits.Since the type purports to make security promises to developers, and since it cannot fulfill those promises, it should be obsoleted in the next version of .NET Core.
/cc @blowdart @bartonjs