Shouldn't the string Generate function use ProtectedStrings?

[ThisMakesSenseToMe]

string Generate(Options options, PwProfile profile, CryptoRandomStream random)
uses normal Strings to generate a Diceware password.

However, it is used in an override that expects a ProtectedString and there a non-protected string is returned [return new ProtectedString(false, result);]

So the chosen Diceware words are plain to read from memory. I think ProtectedStrings should be used as much and as soon as possible and the normal strings should be zeroed as soon as possible.

[nitz]

I can look into this.

I'm not quite sure what the performance implications are when using ProtectedString where many subsequent modifications are going to be made. As well, at some point each piece of the resulting passphrase is going to need to exist as traditional strings at some point during the creation. My current implementation is based loosely on how the sample password generator used a string, returning the protected version at the end. I'll take a look at some other generators and see how they handle it.

For the last bit: The .NET runtime should clean up unreferenced strings as soon as the GC runs.

[ThisMakesSenseToMe]

Thanks Nitz, appreciate it!

You might be right about the performance implications, but I've seen multiple plugins for KeePass that use a lot of this kind of construction:

var pbResult = new ProtectedBinary(true, result);
MemUtil.ZeroByteArray(result);
return pbResult;

Notice how the normal string is zeroed as soon as possible. I've seen this a lot throughout code of KeePass plugins.

About the GC. The GC runs only when needed, that might be a long time (or you need to call it explicitly). But it will not zero released memory. It will just release it, and leave the bytes as they were (for others to read if they like to).

You might want to have a look at other plugins like LockAssist for instance. Success and thanks again for you great plugin.

[nitz]

Is there a MemUtil.ZeroByteArray() overload I'm missing? I only see one that clears out traditional arrays, not one that deals with strings directly. I'm guessing result in your example there is actually a byte[], given that it's passed to a ProtectedBinary ctor?

My knowledge of the .NET Runtime internals for strings is pretty cursory, but I was under the assumption that they're managed as immutable char sequences and can't be directly interacted with. (Meaning you're stuck waiting on the GC to clean them up, even if you manually run it, since the GC makes no guarentees.) So if the generated phrase (or any part of it) ever exists in a native string type, we'd run into this.

Taking a step back to approach this from a practicality viewpoint for a moment. My assumption is that the generated password must exist as plaintext at some point immediately or soon thereafter generation, right? The only case I imagine where it may never exist (at least in KeePass' working set) in plaintext is if it's generated, never displayed to the user, and then only used in auto-typing. If it's ever displayed to the user at all (e.g.: password isn't hidden in the entry dialog; used from the preview tab of the generator dialog; etc.) then it's going to exist as a native string anyways, and thus be left up to the GC anyways.

Do you feel like that specific workflow (generate while not showing any unprotected string anywhere, then only using auto-type) is worth the added complexity of juggling UTF-8 byte arrays / char arrays?

[ThisMakesSenseToMe]

I've looked into it Chris, and you're right. Using ProtectedString is only useful when one uses it from the start. If one passes a string to the constructor, it doesn't protect the string anymore, no matter what. So, in your code you're passing a ProtectedString back, but it will in fact never become protected (false sense of security if one doesn't know that). One should pass a byte array from the start if one wants that, or just begin with a ProtectedString immediately, and do operations with that. The operations one can use with a ProtectedString are minimal: Equal, Insert, Remove, operator +

Also, you're right that if it displays the password(s) at some point, it's pretty useless to use ProtectedString. I think it's mainly there to protect memory that has to do with the security of the database itself (e.g. the master password, encryption keys etc.).

I'm sorry to have wasted your time.

[nitz]

I'm sorry to have wasted your time.

Not at all!

I back-of-a-napkined a concept for a sort of glorified "protected string builder" type class that would encapsulate the building of the passphrases in a way that would leave an interface similar enough to just manipulating strings, while keeping the data in arrays that are zero-able.

I think it will be fine loading the dictionaries as they are now (assuming an adversary would have access to them anyways), but then switching to this new theoretical container before working on them.

I'm not sure if I'll have the time to build it but I am going to leave this issue open as a reminder that it might be something fun to work on at some point. If nothing else it may be a fun exercise for the future! Might give me an excuse to actually look at directly dealing with UTF-8 strings rather than treating them as "black boxed" like I typically do 😂

With a decent enough interface it might be something I'd use in other projects too, just for that slight added bonus of keeping sensitive data out of traditional strings.

[ThisMakesSenseToMe]

Great idea! Maybe you can use the KeePass implementation of ProtectedString as a starting point. Thanks Chris!

[ThisMakesSenseToMe]

Meanwhile, I've posted the Dutch word list to StrongBox too, and they will include it as well.