Fixed-key AES garbling #8
Conversation
There was a problem hiding this comment.
I haven't checked the actual cryptography, just the Rust code style. In general it looks good to me, but maybe we should wait with the merge until we have the benchmarks, to be able to see the performance difference between the current encryption scheme and this one?
src/garble.rs
Outdated
| bincode::deserialize(&plaintext).map_err(|e| Error::Serde(format!("{e:?}"))) | ||
| let mut triple: (bool, Vec<Option<Mac>>, Label) = (false, vec![], Label(0)); | ||
| let hash = hash_aes(garbling_key, party_num + 1, cipher)?; | ||
| let mut decrypted = xor_dec(&hash[0], &bytes[0])?; |
There was a problem hiding this comment.
The bytes[0] access will panic if bytes is empty, which could be the case if an attacker sends an empty message, correct? I would suggest to rewrite the function in such a way the length of any Vec is checked before we attempt to decrypt, so that the function either errors out or succeeds, but never panics.
There was a problem hiding this comment.
Very true, thank you, I added a check on the length of the bytes for decryption.
src/garble.rs
Outdated
| let hash = hash_aes(garbling_key, party_num + 1, cipher)?; | ||
| let mut result: Vec<Vec<u8>> = vec![]; | ||
| result.push(xor_enc(&hash[0], triple.0 as u128)?); | ||
| for (i, h) in hash.iter().enumerate().take(party_num + 1).skip(1) { |
There was a problem hiding this comment.
We can keep it as it is, but I'm wondering if this can be simplified, in two ways:
for (i, h) in hash.iter().skip(1).take(party_num).enumerate(), so you don't have to doi - 1- should
hash[0]andhash[1..]instead be a tuple(hash, hashes)?
There was a problem hiding this comment.
I included these, also the suggestion to have a tuple. It also made resolving the next comment easier (to serialize/deserialize only once).
src/garble.rs
Outdated
| triple.1.push(None); | ||
| } else { | ||
| decrypted = xor_dec(&hash[i], &bytes[i])?; | ||
| mac = bincode::deserialize(&(decrypted).to_le_bytes()) |
There was a problem hiding this comment.
Instead of serializing / deserializing multiple times inside a function, maybe we could just serialize / deserialize once?
|
Let's maybe close this one for now? We don't know whether we want to keep this and even if we do we would need to update the branch. |
No description provided.