SecureString obsoletions and shrouded buffer proposal

[GrabYourPitchforks]

This document addresses obsoleting certain SecureString APIs for .NET 6. It is a type we have wanted to wind down for a while, and we have some outstanding work items that can address the reasons devs used SecureString in the first place.

This document also proposes a new API ShroudedBuffer<T> that is a more flexible and usable form of SecureString. There is a link to the work (already done) of rewriting SecureString in terms of this API as an implementation detail.

Related: dotnet/runtime#30612

/cc @terrajobst @blowdart @bartonjs @jeffhandley

[GrabYourPitchforks]

And yes, the proposed API name is horrible. I leave it to you smart folks to figure out a better name. :)

[terrajobst]

Video

• We think we need to make decision whether or not the framework is interested in providing building blocks like ShroudedBuffer<T> in order for a customer to build hygienic solutions around handling privileged information, which also includes enables customers to build a PCI compliant solution. This requires and end-to-end how privileged is read into a process and how it leaves the process (file I/O, networking API, ASP.NET buffer pools, etc). This also includes GC behavior around copying memory, crash dumps, and logging.
• We should think through what it means to obsoleted SecureString for APIs that can only be used be used with it, such as many PowerShell Cmdlets and some of our networking/crypto APIs.

[GrabYourPitchforks]

Also from the discussion: also consider whether the solution would be to obsolete the API + provide guidance about how to accomplish the scenarios. It's not always appropriate (or possible!) to provide a replacement one-size-fits-all API.

[GrabYourPitchforks]

One alternative is that we don't obsolete SecureString itself (since PS relies on it so heavily), but we do obsolete .NET APIs that interface with SecureString. That includes Process.Start, NetworkCredential.SecurePassword, etc. Then SecureString becomes an exchange type solely for PS's own use.

Ideally PS would be able to introduce their own PSSecureString type as part of their own SDK, and they'd be able to convert back and forth between that type and SecureString as needed. I don't quite know what this would mean for existing .ps1 or .ps1d files, though.

[iSazonov]

Ideally PS would be able to introduce their own PSSecureString type as part of their own SDK,

I wonder if it is not "ideally" for .Net why do this ideally for PowerShell?
Main PowerShell policy is to follow .Net. All power of PowerShell is in a power of .Net. If .Net consider to introduce new approach instead of SecureString PowerShell should be the first to benefit from it.

If SecureString will be reimplemented on SensitiveData PowerSHell gets this automatically. Other applications too. It would be good step in .Net 6.0 milestone.

Next step obsolete SecureString? Obviously this is a big problem for the community without first creating a clear and modern alternative.

Alternative for what scenarios if we consider console applications including PowerShell?

1. Using clear text passwords
   1.1. Reading user input.
   1.2. Keeping internally
   1.3. Storing in an vault store
   1.4. Passing to an OS API
   1.5. Passing to remote system/service
2. Using modern authentication tokens (for above scenarios)
   Can we use two factor authentication in console applications? Biometric? Windows Hello? Certificates/SmartCard tokens?

So main question is - can .Net introduce new API to address these scenarios in modern way? If yes, obsoletion of SecureString is not a problem - .Net (and PowerShell) community will enthusiastically switch to the new API.

[terrajobst]

Suppressing just the small handful of instances where these types are created or consumed should be enough for those developers to signal that they're comfortable using the type.

Assuming we use a single diagnostic ID for all secure string related obsoletions they wouldn't have to sprinkle anything -- they could just globally turn this particular obsoletion off.

[terrajobst]

You don't call this out explicitly, but it looks like all these APIs have string equivalents (Process.Start doesn't, but one can construct ProcessStartupInfo and set PasswordClearText). I assume the (implicit) goal is that we'll ensure that the scenario (passing a credential) can still be achieved without using SecureString, correct?

[terrajobst]

I'm hardly an expert in this domain but I've built enough web sites and console apps for cloud services now that I see emerging patterns: tokens and OAuth. Both rely on secrets being passed to APIs and both tend to be strings coming from configuration or environment variables, both of which tend to be strings. NetworkCredential could be used (because both userName and password are nullable) but that seems like a clutch to me. Also, it would suffer the same problem as SecureString in that the origin is already a string (set of strings). The concern here isn't so much security/clear text data in memory, it's more the exchange type problem. If the origin is a string, people are super likely to use that as the exchange type and everything else becomes annoying. See Uri. We end up in a world where a ton of APIs are either only taking string, only taking Uri, and some both. This forces users to frequently convert back and forth which is annoying. And that was even an area where we had tooling early on to push people towards Uri (FxCop).

So without covering entry points and having good analyzers/fixers, promoting a type as the exchange mechanism (whether or not the type already exists) will be hard. Also, because string is so pervasively used today, we'll have to make it easy to convert back and forth between whatever exchange type we'll want and string. But this also means that the other benefits, such as avoiding the data in crash dumps, will be harder to achieve. I still believe it's doable but we should think of a strategy to get there and pair it with analyzers and code fixers to help folks to get better over time, by, for example suggesting to not convert a <the exchange type> to a string if the receiver also accepts <the exchange type>.

[GrabYourPitchforks]

TBH I expect SensitiveData<byte> to be used more frequently than SensitiveData<char>. This matches aspnet's existing Secret class today, whose purpose is to hold cryptographic key material. There is sample code for converting a SensitiveData<T> back into a T[] or a string (see line 94 of this doc). Is that code sufficient, or do you expect there will need to be first-class APIs for this?

[terrajobst]

I think we should expand the list of secrets a bit more and also list how much we can/should protect them. For example, what about tokens? OAuth client IDs/secrets? Private keys for signing? Certs? Passwords are just one class of secrets.

Practically, I'm not sure I know how I can extract the secrets processing in my apps -- ultimately I need to send a token in every request. I guess that would mean that we don't consider a token a secret because it's short lived? Again, I'm not an expert so maybe that's obvious to experts, but we might want to spell this out explicitly to make it clear what the scope is we're trying to solve here.

[blowdart]

I don't think so. What is secret is context specific and is up to the reader to define as it's domain knowledge. If we list things people will go "It's not on the list so it's fine"

[terrajobst]

The point isn't to be exhaustive. The point is to enumerate common secrets to ensure we have sensible end-to-end across the APIs/technologies that need we need to (1) get them into the process, (2) passing them around and (3) handle them to the correct receiver.

[blowdart]

Experience shows that's not how it will be read. It will become a check list, in an audit, just like SecureString use is now. I don't want to see that happen.

To take your token example, it's still context sensitive, based on what the token is, what it's sent over, how it's created, and if it's backed by token binding. Simply saying token is not enough, and to be enough you have to be exhaustive, and suddenly the paragraph becomes bloated and unwieldy, and will take constant maintenance as new secrets come along.

[GrabYourPitchforks]

I think we can provide an example showing something Immo mentioned in another fork: how do I read from the environment variables or config and populate an instance of this type? It's also a good opportunity for us to show very clearly "it's ok to materialize a temporary array or string; we're not trying to prevent this." It doesn't really answer the question of "what is a secret?", but TBH that question isn't really important. What is important is how to use this as an exchange type.

[terrajobst]

I buy that ToString() is called by logging and other code paths that don't need to unshroud the buffer; but to me this doesn't mean we can (or shouldn't) provide a method like UnshroudToString() or UnshroudToArray(). Otherwise it will be hard to insert the type into existing code bases where virtually everyone uses strings today. The benefit of using a well-known method like UnshroudToString() is that it can be audited. If everyone just does the string.Create trick it's harder to audit (and harder to tool with an analyzer).

[ericsampson]

hmmm I can see both sides of that argument :)

[GrabYourPitchforks]

Providing a well-known method would be fine in spirit IMO. The point of this type is to prevent accidental materialization of the string / array. If somebody is intentionally doing it, who are we to stop them?

Though to be honest the reason I didn't add such methods in the first place is that I assumed people would be nitpicky about how they wanted the information to be materialized. "I want it in a pinned array!" or whatever. CopyTo is intended to be a compromise which allows the caller to allocate the buffer themselves, then the secret is copied into that buffer. So CopyTo becomes the auditable method call. But if we need a simple accelerator, we could add one.

[blowdart]

I'd much rather not. A single CopyTo is auditable, adding more methods means more noise, and more chance for a bad audit to miss things. Keeping it to a single method is better in terms of auditing, there's less things to miss.

[terrajobst]

If the backing buffer for the sensitive data is stored in-proc, we should attempt to use a separate unmanaged heap for storing these values, and we should not hand out direct references to this heap.

It's not clear to me how we'd deal with how this data gets into the process in the first place, such as:

• Microsoft.Extensions.Configuration
• Environment variables

For example, we should consider a method like SensitiveData<char> Environment.GetSensitveEnvironmentVariable(string name).

[GrabYourPitchforks]

What's wrong with the following? It's short and simple and doesn't require API changes beyond what's proposed here.

string envVar = Enviroment.GetEnvironmentVariable(name);
SensitiveData<char> sensitiveData = new SensitiveData<char>(envVar);

Remember, the point of this type is not to prevent the initial materialization of strings / arrays, nor is it to hide the contents of this buffer from an active memory dump. The point is to mark a piece of data as "sensitive" and to prevent accidental re-materialization of this string / array. The code sample above fits with this spirit.

[bartonjs]

Remember, the point of this type is not to prevent the initial materialization of strings / arrays, nor is it to hide the contents of this buffer from an active memory dump.

Which is good, since the entire environment block is already in dumpable memory 😄

[terrajobst]

Remember, the point of this type is not to prevent the initial materialization of strings / arrays, nor is it to hide the contents of this buffer from an active memory dump. The point is to mark a piece of data as "sensitive" and to prevent accidental re-materialization of this string / array. The code sample above fits with this spirit.

If we believe that to be the case, then this certainly simplifies the design.

[danmoseley]

Which is good, since the entire environment block is already in dumpable memory

there is prior art for redacting dumps, eg., the triage dumps that blank out much of the data leaving only the stack. in theory, if they don't already, they could clean the environment block. but they couldn't clean an arbitrary part of the GC heap. so I'm not sure the analogy with the env block holds entirely?

[bartonjs]

without first creating a clear and modern alternative.

The problem with coming up with any alternative is first describing what the type is supposed to defend against.

1.1. Reading user input.

I feel like PowerShell does have a mode that builds a SecureString correctly as part of input (Read-Host -AsSecureString). But I've seen quite a number of scripts that use Convert-ToSecureString... which means they already had the text in a .NET System.String, and (depending on what you're defending against) you've already lost the battle... or at least a non-predictable amount of time in a "you've lost" state.

1.2. Keeping internally

Again, what the right answer would be depends on what you think the threats are, and what costs you're willing to pay to defend against them. Also, how long are you keeping it, and how long does it stay loaded but unused?

If you have a SecureString that's being used for loading a PFX and you load that PFX 1000 times per second across parallel threads, you may as well have just kept it as a System.String, because there's going to be at least one plaintext copy in memory at any given time. On the other hand, if you only use the password once every 14 days, and you loaded it correctly, then you have pretty well minimized the risk that it shows up in a memory dump.

1.3. Storing in an vault store

You'll have had to upload it to the vault store, which probably makes it a plaintext message (then encrypted as part of a TLS message). Or it'll be encrypted using something like CMS enveloping... but had to be plaintext input (as bytes) to that. So, again, it had to exist as plaintext, why are you bothering with encrypt a character, decrypt it, add another character, re-encrypt the two characters, re-decrypt them, add a third, re-encrypt the three, etc?

If the upload to the vault was plaintext, have the contents in all the various buffers between you and the TLS encrypt operation been erased? Probably not.

Lets look at the other direction, reading it from a vault store. All the same things apply, plus the value possibly got loaded into a System.String instance, so now there's another ghost somewhere waiting for the memory to be reclaimed, reissued, and overwritten.

1.4. Passing to an OS API

SecureString isn't understood by the OS. Any time that a system call is made the secure string is decrypted. That's why if you're doing 1000 PFX loads per second there's going to be at least one plaintext copy in memory.

1.5. Passing to remote system/service

Same as the vault store... it has to be plaintext in memory somewhere.

Can we use two factor authentication in console applications? Biometric? Windows Hello? Certificates/SmartCard tokens?

Certs and SmartCards, totally. X509Certificate2 and X509Store.

---

Basically, we have a type. It has the word Secure in the name, so it must be good, right? Does it defend against anything you care about? Strong Maybe. Does it defend against it well? Weak Maybe. Does it defend against it in practice? Rarely (because the context where it got used bypassed the protections). Does it, on net, provide positive value? My opinion (which I believe is shared with the rest of .NET Security) is no: it provides marginal value some of the time, a false sense of blind confidence too much of the time, and is too cumbersome all of the time (and can't be made less cumbersome or it essentially never provides value).

What's a good replacement? I don't think there is one. The "SensitiveData" type in the proposal is largely just a code review marker, and a blocker for the ToString method. It's not a "replacement" for SecureString, but rather an acknowledgment that there can't be a good/universal one, but that sometimes "use good hygiene here" is good enough.

[danmoseley]

Just curious, is there a recommended pattern in native code - if you zero out your password buffer before deleting it, can you generally assume that theOS or library API you pass the buffer to will consistently do the same?

[KevinCathcart]

One thing I would like to see is that the documentation not only not make any claims of special protections, but also to explicitly disclaim that any such measure implemented will be available on all platforms or remain the same going forward.

In order to do that one would need to mention that additional measures may be taken while being extremely vague about what is being done. I would propose wording like "The implementation might take additional steps to reduce the probability that the secret could be accidentally exposed, but the details are not necessarily the same on all platforms, and are subject to change".

I think that is sufficiently vague to not be promising anything, and is intended as basically a warning that you cannot look inside implementation and take the current behavior as a guarantee of what will always be done. New platforms may not be capable of everything, or a more efficient or smaller code size implementations could potentially be used at a later point that don't provide identical protections.

It also properly frames the intended purpose of any such protections. Even if measures like encrypting the secret or storing secrets out of process were to be used, they would be better understood as a imperfect mitigation against potential bugs like heartbleed then as protection against malicious use of memory dumps.

If one has stringent requirements for handling of a particular secret, one would be best off writing custom code to ensure that those requirements are being met.

[Joe4evr]

public void RevealAndUse<TArg>(TArg arg, System.Buffers.ReadOnlySpanAction<T, TArg> spanAction) { }

Isn't the convention for methods like this to name the type parameter TState? That's how it is on String.Create.

[filipnavara]

The reviewed API still has public System.Security.Secret<T> Clone() { throw null; } which should use the new namespace instead.

Same for public static string RevealToString(this System.Security.Secret<char> secret) { throw null; }.

[filipnavara]

What is the int return value of RevealInto? I assume it's the number of elements written but I could not find a definite answer.

[ericsampson]

The type being "System.Secret" seems like we're going to set up for too much conflict

I still think that Sensitive might be a better choice than Secret to help avoid the whole issue of interpretation that Secure caused. You'd just hate for all this work to amount to little because the same thing ends up happening again.
But I won't harp on this any more.

[danmoseley]

@GrabYourPitchforks now this is approved, do you expect it will be in 6.0 or miss that? cc @shirhatti

[GrabYourPitchforks]

@danmoseley I'm not optimistic about meeting 6.0. Last I heard there was a push to get this in for 6.0 but not at the expense of other features. If there aren't any objections from your end I'd feel more comfortable punting this to 7.0.

[danmoseley]

That's fine with me -- I asked because @shirhatti was interested in adding API in Microsoft.Extensions that consume it. @shirhatti is 7.0 OK?

[shirhatti]

If it were happening in 6.0, I wanted to make sure we had enough time to react in Extensions.
7.0 is fine by me.

[GSPP]

Is there precedence for putting extension methods on non *Extensions named types?

[stephentoub]

Is there precedence for putting extension methods on non *Extensions named types?

Enumerable

[eerhardt]

Is there precedence for putting extension methods on non *Extensions named types?

Also, it is preferred in a lot of cases to not name the type “*Extensions”. Once you name it that, the type is now locked to only contain extension methods.

[AraHaan]

I would prefer SecureString to face the same fate as BinaryFormater and the replacement type known as Secret<> to be the only way forward.

[Neme12]

I watched the API review (https://www.youtube.com/watch?v=IHRPmHuDQvU) and have a few thoughts.

1. Somebody (I think @GrabYourPitchforks) said that the type explicitly won't make any guarantees, it should just serve as a marker and encourage hygienic use and consumers shouldn't rely on any guarantees. I think the idea about hygienic usage makes a lot of sense - you are very explicit about revealing the underyling data with each method and as it primarily deals with spans, you can copy the data into your own buffer that you instantiate and whose lifetime you control, which means that you can (if needed) create a pinned buffer and then clear it appropriately when no longer used. It makes it very explicit that you're getting the underlying data and does encourage hygienic use. But I don't think is useful without any guarantees at all, because if the type doesn't at least guarantee that the data is cleared from memory after calling Dispose(), you may always copy the data into a buffer that you use hygienically and pin and clear properly, but that's useless if the type itself doesn't clear the data after you're done with it. I think there should be at least 1 guarantee of this type (as opposed to it being an implemention detail), and it is that calling Dispose will clear the data (including all copies possibly made by compaction) from memory, and there won't be any copy left.

   This could be implemented by clearing the buffer on Dispose and either pinning the underyling buffer on creation, or if there's ever the runtime feature about clearing data when compacting (GC Heap Compaction should clear the source data runtime#10480) and the implementation detects that this feature is turned on, it wouldn't even have to pin the data, just make sure to clear it on Dispose, since it will clear the only remaining copy of it. If you're not calling Dispose, I think it's fine if it doesn't get cleared when garbage collected, because the consumer didn't explicitly ask for it to be cleared. But I really feel like this is the 1 guarantee that the type should make. I don't think this is in any way problematic unlike the guarantees that SecureString made - encryption turned out to be hard to do an all platforms. But I think clearing the data is something completely different - that's something that can be done on every conceivable future platform and is easy to do. It doesn't have to guarantee that it won't appear in a crash dump, it doesn't have to guarantee anything about encryption or do any encryption at all. But without at least guaranteeing that the data will be cleared after Dispose and no copies will remain in memory, any hygienic use would be practically useless because the user can be hygienic all they want if the type itself isn't and defeats every purpose of it. Also, then it couldn't even be used as a building block with the PCI requirement that data has to be cleared from memory when no longer needed. And if the implementation does clear it, people might rely on that behavior anyway.

2. Couldn't Secret<T> be a struct? It seems like it's really just a small and immutable wrapper type around a buffer so it might make sense to make it a struct to avoid allocations and make it more efficient. On the one hand, you could say that this is more of a security-oriented type, the implementation is not efficient anyway and security is a much more important concern than performance. But if making it a struct doesn't actually harm security at all, why not make it slightly more efficient that way. The idea that because a type is associated with slow and inefficient usage, an allocation isn't a concern, has already backfired for types like Task - in some cases, it does matter. In the future, this type might become really common and it could add up. Also, some libraries might avoid it because of the allocation.

   EDIT: Later I realized that this is actually problematic because the type is disposable and therefore not actually immutable. I still think it might be worth thinking about this a bit and if there's a way to make it a struct while making sure that people don't use it incorrectly, maybe even with a built-in analyzer.

3. @terrajobst said that the problem with SecureString is that the name explicitly claims that it is secure and consumers might then have the wrong assumptions (which I agree with), and that the name Secret is much better because it doesn't imply anything being secure, it just identifies something as a secret. I don't really agree with this. To me, Secret also has some implications - it kind of implies that the data is stored secretly, suggesting that nobody else could have access to it. I think this name is almost as misleading as SecureString. That's why I think a name like SensitiveData or SensitiveBuffer would be much better as that's the only name that actually doesn't imply anything being secure or stored in a secret way and just identifies something as a sensitive piece of data. Also, Secret<T> doesn't say that it's actually a buffer of data - with that name, I would naively expect to be able to pass any T in there and that it would wrap a single value, not make a buffer.

4. I think there should also be a constructor that takes T[] in addition to the Create method. Yes, the Create method was added because it's more convenient as the type argument can be inferred. But in some cases, you might prefer to create this using a constructor rather than the factory method, especially where you can use target type new - private Secret<T> _secret = new(...) is better than private Secret<T> _secret = Secret.Create(...). That's why I think there should be parity between the factory methods and constructors where there can be. It would of course be nice if the constructor could take string as well but that can't be done because it's a specialization of T.

5. I feel like there should be an overload of RevealAndUse without TArg. Of course the overload with TArg is important as it lets you pass state into the lambda, but in some cases, you might not need any state at all - like when you're just calling static methods. With the arg being required, users will have to supply something like null, and then further realize that now they have to specify the type argument explicitly. I think it would be really nice if there was an overload that didn't take the argument.

6. Should there be a RevealAndUseAsync method as well? That one could have a delegate that takes ReadOnlyMemory<T> instead of ReadOnlySpan<T> as a parameter. I'm sure there will be a request for this variant at some point if it's not there from the beginning.

7. It seems that with the current shape of the type, the data will always have to be copied into the Secret<T>. This makes sense of course - the type wants to make sure that the data is a) really immutable, and b) allocated and stored in the way that the type itself controls. But I think it can use the same approach that this string.Create overload uses to make it possible to avoid the copy:

   public static String Create<TState>(int length, TState state, SpanAction<char, TState> action);

   the equivalent method on Secret would be:

   public static Secret<T> Create<T, TState>(int length, TState state, SpanAction<T, TState> action) where T : unmanaged;

   (I would probably also add a constructor equivalent of this, although that might not make sense if an async version of this is added as well. BTW, seeing that string.Create uses TState as the name of the type argument makes me think that RevealAndUse should also use TState as opposed to TArg.)

   This would allow Secret<T> to allocate the memory in whatever way it does that - pinned or in a custom heap etc, and then have the user fill the data in-place. This could allow users to use this type without making a copy whenever creating it.

8. Regarding the 2 APIs added to SecureString and how they can only be added for .NET vNext and not made portable, because that would mean adding these method to Secret<T> and it would be unfortunate to have the new thing refer to the old thing, I think this alternative to make it portable might make a lot of sense:

   namespace System.Security
   {
       public static class SecureStringSecretExtensions
       {
           public static SecureString ToSecureString(this Secret<char> secret);
           public static Secret<char> ToSecret(this SecureString secureString);
       }
   }

   This would make it so that users of Secret<T> won't see the ToSecureString method unless they already have a using for the namespace in which SecureString (and not a lot of other things) resides, and users of SecureString will always see ToSecret. I think this is a nice solution and IMHO it would be a bit unfortunate if it's not made portable even though it could actually be done in a way that's nice and usable like this.

Those are my 2 (or it seems more like 200) cents. I hope some of these ideas might be useful. 😅

[KLuuKer]

what about the IDataProtector api's?
maybe I'm storing some "sensitive" data like

• passwords for unlocking PFX cert containers
• api token's that are transported encrypted from a (more) secured backend

having the 1 byte at a time would also be very nice to limit what can be leaked from memory dumping attempts

[KLuuKer]

i always try to defense in depth whenever possible
we never know when we end up with the next version of Heartbleed
so the least amount of time data is spent (partially) unencrypted the better

[Neme12]

Was this idea abandoned? :(

[danmoseley]

This would make it so that users of Secret won't see the ToSecureString method unless they already have a using for the namespace in which SecureString (and not a lot of other things) resides, and users of SecureString will always see ToSecret.

I may be missing something but the proposal seems to be to put the new type in System.Security, so the using will inevitably be there?

[Neme12]

@danmoseley The latest approved proposal in #147 (comment) places it in System.Buffers.

[HHobeck]

Hi there.

Rather, this design is intended to prevent inadvertent materialization of the sensitive data. This means that having the data in plaintext before it gets into a SensitiveData - or in plaintext once you unshroud the SensitiveData - is perfectly fine. The point of this type is to serve as an annotation to say "hey, this needs some extra care" - and to avoid people inadvertently materializing it as part of logging, serialization, or similar facilities.

The design goal of @GrabYourPitchforks is exactly what I'm missing in the framework. As an application developer I can think of dozen of use cases. One has been described in the following proposal:

• [API Proposal]: Support of secured value decryption configuration options #95899

Anyway what I'm wondering because it was also mentioned multiply times from other community members but without answering the question: Why do we not introducing an attribute type to get a better integration in tooling and IDE or other use cases when it is not possible to use this type? I'm not saying to replace the Secret/ShroudedBuffer/SensitiveData (the last term gets a thumb up from me ;)) but to have it side by side. The intention remains independent of the fact if you use a data type or an annotation type. We not know what the use case might be and should not limit the user to use just a data type. The design goal can be extended to an annotation attribute as well. We not tarnish somebody's glory. Are we?

Edit: It could be also a best practice if you return a secret also to annotate the method or property with a deserves protection or deserve security attribute.