Memory cleanup of sensitive data

[hug-dev]

From the PSA Crypto API version 1.0.0, section 6.3.3:

Implementations must wipe all sensitive data from memory when it is no longer used. It is recommended that they wipe this sensitive data as soon as possible. All temporary data used during the execution of a function, such as stack buffers, must be wiped before the function returns. All data associated with an object, such as a multi-part operation, must be wiped, at the latest, when the object becomes inactive, for example, when a multi-part operation is aborted.
The rationale for this non-functional requirement is to minimize impact if the system is compromised. If sensitive data is wiped immediately after use, only data that is currently in use can be leaked. It does not compromise past data.

We should make sure that all of our data structures that contain confidential information are cleared once they are no longer used.
An implementation of Drop could handle that.

[ionut-arm]

We should check crates.io for any potential helpful crates. secrecy seems to do what we need, but if it isn't and there's nothing else out there (which I doubt, but who knows), we should create such a crate ourselves. It's a very important utility to have, especially for security-related apps and libraries.

[ionut-arm]

Also, once this is done, some documentation should be added about this.

Do we need to add anything in the threat-model about potentially leaking data (Heartbleed-style)?

[justincormack]

You should probably use https://github.com/iqlusioninc/crates/tree/develop/zeroize - @tarcieri has done a lot of work on secure zeroing and making sure it doesn't get optimised out which is a hard problem.

[tarcieri]

Note that secrecy is built on zeroize, and also tries to avoid things like implicit/unintentional disclosure of secrets (e.g. through Debug)

[ionut-arm]

Thanks for the tips! We'll make sure to get this in before any prod use.

The zeroize code should also provide some interesting reading, then :)

[ionut-arm]

Based on the PSA spec which says

All temporary data used during the execution of a function, such as stack buffers, must be wiped before the function returns

we should wipe out all memory allocated during operations

[ionut-arm]

My proposal here would be to use zeroize and finally-block to create a macro like with_cleanup! that takes in an instruction/a block which computes a value, creates a finally block for that value in which it is zeroized and then returns the (value, _finally_block) tuple.

In this way we do not have to concern ourselves with the cleanup process after the variable creation.

@hug-dev - thoughts on this?

[hug-dev]

That seems like a good idea! We should try and prototype it on a few values. I have a few thoughts:

• zeroize requires a mutable reference, I am wondering how it would look like in the code to respect the ownership rules if a reference is inside the finally block and also in your program
• We should probably go through the code and check which values need zeroizing and when
• If we are using finally-block to zeroize a value when it is no longer needed, isn't it the equivalent of implementing Drop on it? That would require some more new types but I assume a lot of our types are similar, like Vec<u8> and String.

[ionut-arm]

Tbh it seems a lot of the types we handle as temporary values are defined by us so we can just implement Drop or add that to the existing documentation.

I think I'll start with the psa-crypto crate, then with interface, then I can do the service. Oh, and tss-esapi before, I guess 🤡

[hug-dev]

We did implement zeroing on most of our types but I forgot which repos are left out: probably parsec and tss-esapi?

[ionut-arm]

We did implement zeroing on most of our types but I forgot which repos are left out: probably parsec and tss-esapi?

Yeah, and tss-esapi will be a real pain in the bum....

[paulhowardarm]

Can we get a precise definition of exactly what (remaining) work is needed here?

[hug-dev]

I think this one is known: it is about going through all structures of Parsec, checking if they contain any confidential information/secrets and if they do, wrap them in a Zeroize or Secret.
Most of the structures come from the interface (which the zeroing work has already been done) but I'd expect there would be a few only defined in the service.