Implement KeyObfuscator for Deterministic Encryption of storage keys.

[G8XSU]

Add KeyObfuscator to provide a helper object for client-side key obfuscation.
This implementation uses ChaCha20-Poly1305 for deterministic encryption, enhancing privacy and preventing common pitfalls (foot-guns) in client-side encryption.

Approach:

* We generate a nonce N from the plaintext storage key P, where basically N = hash(P).
* We encrypt the plaintext P using the nonce N, getting the ciphertext C.
* We encrypt that nonce N using the nonce N2 = hash (ciphertext-C) to obtain encrypted-nonce 'E'.
* We append ciphertext C and encrypted-nonce E , to store them together in vss-server.

Main reason for using ChaCha20-Poly1305:

• Deterministic encryption
• Already in dependency
• We can generate synthetic iv ourselves.

Comparison with AES-256-SIV:

• Deterministic encryption. (we need this for get operation to work, when client only knows about key.)
• Designed for synthetic IV instead of random nonce. (we need this for get operation to work, when client only knows about key.)
• Nonce misuse resistant, can even work without it.
• Designed for key-wrapping problem in the first place. (we use this for nonce wrapping.)
• Unlike just hashing, it is also reversible, this is needed for list operation to work, when client doesn't have any information about the keys.

[tnull]

Hmm, interesting. Obfuscating keys def. makes sense, but before I get into a detailed review, could you clarify why ChaCha20Poly1305 isn't an option here, in particular given that we already have working reviewed code for it and the explicity security warning given by the introduced aes-siv dependency? Or, if we go that route, why not go aes-gcm-siv which should be generally more secure and has a less scary security warning?

Also, it may be noteworthy that ChaCha20Poly1305 will likely soon get added to bitcoin_hashes (cf. rust-bitcoin/rust-bitcoin#2960) at which point we can drop our implementations here and in rust-lightning.

[G8XSU]

why ChaCha20Poly1305 isn't an option here

For chacha20, main problem was of nonce re-use and compromise of security guarantees when there is nonce re-use. Both chacha20 and aes-gcm were meant to work with random nonces. (aes-gcm-siv has more safety margin for nonce misuse but afaiu it is still vulnerable to nonce reuse and actively discourages it, whereas aes-siv was designed for key-wrapping problem and can even work without nonces. We are currently using key-wrapping to wrap nonces, it is secure as long as adversary doesn't know about the underlying plaintext and the key being wrapped isn't guessable.)

It is possible for the adversary to guess plaintext of certain keys(e.g. channel-manager).
I was worried about security implications of using a deterministic method for encryption and nonce generation, with known nonce and guessable plaintext. (But on reconsideration, I think it should be fine if we use another derived-key as nonce for wrapping nonce. And nonce re-use risk is more applicable when we use it for different plaintexts iiuc.)

less scary security warning

Fwiw, both aes-gcm-siv and aes-siv have similar security warnings.
Underlying aes-gcm and aes implementation is reviewed/audited by NCC group. They do have the testvectors acc. to rfc, but aes-siv has additional warning regarding constant-time evaluation, which isn't as relevant for client-side encryption in this case but more so for secure enclave environment or similar.

[tnull]

I was worried about security implications of using a deterministic method for encryption and nonce generation, with known nonce and guessable plaintext. (But on reconsideration, I think it should be fine if we use another derived-key as nonce for wrapping nonce. And nonce re-use risk is more applicable when we use it for different plaintexts iiuc.)

Right, I def. follow these concerns. But, if I'm not missing something, above sketched method of deterministically deriving the wrapped nonce's encryption nonce from the ciphertext and some key material should work to derive per-storage-key nonces allowing to use ChaCha20Poly1305.

In particular, we would have one nonce per plaintext (the storage key), i.e., if the adversary knows the plaintext they already know what we're trying to protect and will be able to determine the associated nonce but won't be able to learn more about nonces of other storage keys.

[tnull]

Thanks for switching to ChaCha20Poly1305. Generally looks reasonable, a few comments.

[tnull]

This needs a rebase as we now have a duplicate dependency in Cargo.toml.

[tnull]

Mostly looks good to me, some comments/nits.

Would be great to see proptest support here to uncover and test edge cases.

[tnull]

Looks good from my side I think, should be ready for a second pair of eyes (cc @arik-so ?)

One question though: what is the upgrade story here? Do we need to ship this and default to it before any users are going to use VSS really? Should we (otherwise on in general) prepend the ciphertext with some magic sentinel bytes indicating that it has been obfuscated and proceed without deobfuscation if the keys don't start with the correct sequence?

[G8XSU]

what is the upgrade story here? Do we need to ship this and default to it before any users are going to use VSS really?

Yes the default is obfuscated keys for all users, this is a non-backward compatible change hence important to do before release.

Should we (otherwise on in general) prepend the ciphertext with some magic sentinel bytes

No, we cant do this because in 'get' operation we don't know whether key was obfuscated or not during 'put', hence all keys must be obfuscated.

[arik-so]

So just to recap (simplified) what happened here so far:

1. We generate a nonce N from the plaintext storage key P, where basically N = hash(P).
2. We encrypt the plaintext P using the nonce N, getting the ciphertext C.
3. We encrypt that nonce N using the ciphertext C as the nonce.

Is this approach cryptographically sound? Are there any risks associated with it?

[G8XSU]

Yes,
with minor correction, we use N2 = hash(ciphertext) to encrypt nonce N.

* We generate a nonce N from the plaintext storage key P, where basically N = hash(P).
* We encrypt the plaintext P using the nonce N, getting the ciphertext C.
* We encrypt that nonce N using the nonce N2 = hash (ciphertext-C) to obtain encrypted-nonce 'E'.
* We append ciphertext C and encrypted-nonce E , to store them together by sending in request to vss-server.

With using ChaCha20Poly1305 here, we need to take care of two things mainly:

1. Don't re-use nonces for different plaintext. (Addressing Concern 1: We don't re-use it for different plaintexts, we do re-use it for same plaintext.)
2. Since it is possible to guess some of the plaintexts like channel-manager, server shouldn't have access to enough information to be able to reverse the deterministic encryption.

For e.g. if we store unencrypted nonce N with cipher-text, and known plain text it might be possible to reverse it. Hence we store the encrypted nonce.

Addressing Concern 2: For nonce encryption we use hash(ciphertext) as nonce which isn't stored alongside data in server hence it should be safe.

[arik-so]

All right, thanks for the response! As far as I can tell, it looks good, too, and I am also slightly less concerned about the cryptography considering it's merely key obfuscation.