6

In one of my code reviews I stumbled across an interesting implementation of SecureString. Logically to hide the values in memory has merit, but my understanding of IConfiguration is that when injected and built via the ConfigurationBuilder a copy exists in memory already for usage. So the SecureString though is hiding the clear text values, the configuration access automatically negates the cipher text.

Is my notion is correct, really the value is insecure and should not even use SecureString because it is not secure to begin with- 

public class Sample
{
     private readonly SecureString secret;
     public Sample(IConfiguration configuration) => secret = new NetworkCredentials(String.Empty,
          configuration.GetSection("Api:Credentials")["secret"]).SecurePassword;
}
Greg
  • 11,302
  • 2
  • 48
  • 79
  • 3
    `SecureString` has not been considered secure for many many years; frankly, there is no meaningful attack vector that this will protect you from; see "Remarks" here (and the linked github post): https://learn.microsoft.com/en-us/dotnet/api/system.security.securestring?view=netframework-4.8#remarks – Marc Gravell Jan 28 '20 at 16:24
  • @MarcGravell That is what I thought, trying to figure out why the developer who wrote this actually did this. The values exists in clear text through the configuration, so why implement. – Greg Jan 28 '20 at 16:25
  • 2
    Remember, the configuration may not actually be a file on disk (environment variables), or it could be a file that is on the disk but not stored in your source control (in the same way as secrets are done). – Neil Jan 28 '20 at 16:38
  • 1
    If it's in cleartext in the config file anyway, it won't help in the least. If they have access to the server's memory they probably have access to the hard drive too, in which case they can just read the config file. – EJoshuaS - Stand with Ukraine Jan 28 '20 at 16:38
  • @EJoshuaS-ReinstateMonica configuration comes from multiple sources, not just a single file. Azure Key Vault and Azure Configuration are available as packages, while the disk itself could be encrypted. Or the settings file could be encrypted with a key stored in the Key Vault. That said, the Key Vault provider itself doesn't use SecureString – Panagiotis Kanavos Jan 28 '20 at 17:01

1 Answers1

1

Basically in the documentation it's mentioned:

Overall, SecureString is more secure than String because it limits the exposure of sensitive string data. However, those strings may still be exposed to any process or operation that has access to raw memory, such as a malicious process running on the host computer, a process dump, or a user-viewable swap file. Instead of using SecureString to protect passwords, the recommended alternative is to use an opaque handle to credentials that are stored outside of the process.

On some platforms it is not even implemented. It is even turned into a string in the .net framework at some point, so why is it there? Does it make sense to use it?

On you comment:

Is my notion is correct, really the value is insecure and should not even use SecureString because it is not secure to begin with-

It does make sense to use it. It doesn't make sense to consider the value 100% safe, it just adds an extra layer of security.

It does limit the exposure and this is something we should strive for while secure coding.

Some perfectly valid reasons to use it can be found in this excellent answer here: Is SecureString ever practical in a C# application?

One more scenario that I can think of: If you use some kind of remote key storage like azure key vault, it makes even more sense to use it.

Athanasios Kataras
  • 25,191
  • 4
  • 32
  • 61
  • 1
    But Microsoft states: "We don't recommend that you use the SecureString class for new development. For more information, see SecureString shouldn't be used on GitHub." – Greg Jan 28 '20 at 16:43
  • I get it. But since you are already base on config files, then why not add the extra layer? If you can port the solution to certificate based authentication by all means do so. – Athanasios Kataras Jan 28 '20 at 16:45
  • The only reason for not doing it, is that it might create a false sense of security. – Athanasios Kataras Jan 28 '20 at 16:51
  • Thank you for the feedback, appreciate the clarification. – Greg Jan 28 '20 at 16:56