Wrong IV calculation
Closed this issue · 12 comments
In https://github.com/medooze/sframe/blob/master/lib/IV.js the IV is constructed from the fromeCounter || KeyId. Unfortunately this will cause IV reuse at the AES-CTR block level. The IV needed to be padded with zeros to avoid this. I suggest to remove the KeyId from the IV and replace it with zeros
In my implementation, the keyId is used as a unique senderId (i.e it should be unique per conference per participant). I will double check the buffer operations, but I think they added padding.
@eomara in which scenario this would be duplicated? (given that the keyId is unique)
Looking at the code again, I think it is fine as long as the counter is in the right 8 bytes and not the other-way around.
@murillo128 @eomara "the counter is in the right 8 bytes and not the other-way around" is exactly the issue leading to IV reuse and making the encryption protocol effectively not E2E.
For example, you can see in https://jsfiddle.net/d1uaqm4t/ that if the IV is generated in the current way with two consecutive value of the counter, then bytes Utils.fromHex("0123456789abcdef0123456789abcdef"); placed in positions 16 and 0 respectively and encrypted with these IVs are exactly the same. This clearly shows, that IV, used to encrypt second 16 bytes with the first counter, is exactly the same as IV, used to encrypt first 16 bytes with the second counter. I.e. the IV is reused, making the protocol completely unsafe.
@levlam would reversing the keyId andd counter solve the issue?
export const IV =
{
generate : function(keyId,counter,salt)
{
//128 bits
const iv = new Uint8Array (16);
//Get view
const view = new DataView(iv.buffer);
//Set keyId
view.setBigUint64(0, BigInt(counter));
//Set coutner
view.setBigUint64(8, BigInt(keyId));
//Xor with salt key
for (let i=0; i<iv.byteLength; ++i)
//xor
view.setUint8(i,iv[i]^salt[i]);
//return buffer
return iv;
}
};
```
@murillo128 No, if the counter can be the same for different keyId.
@murillo128 No, it will lead to exactly the same issue, if keyId are close enough.
(I have deleted previous comment because it was inexact)
keyId is used as senderId, each sender should have different keys, so iv will not be reused for same sender/key
IV contains the keyId and the frame counter to ensure uniquenes when using same encryption key for all participants.
When all participants use the same encryption key and different but close enough keyId, there will be exactly the same issue.
While using the same encryption key for all users is allowed by the library, using a key per participant is strongly recommended.
If you don't want to support the same encryption key for all participants, you don't need keyId at all.
If the protocol is vulnerable when the same encryption key is used by all participants, then the protocol doesn't support the same encryption key for all participants and this should be clearly documented.
I still have pending the update to the latest draft (I will do when new version is released to ensure not having to implement intermediate versions).
I have removed the comment about single key for all participants, as it has proven to be insecure.