Hi Michael. Some comments inline... On Wed, Mar 19, 2014 at 9:01 AM, Michael Powers <[email protected]> wrote: > For a private message, we generate a random 256-bit key and encrypt with > AES. Then for each recipient, we use a hash of the shared ECDH secret and > the message-id to encrypt the key and append it to the message. We're xor > ciphering because the inputs are random-ish keys and we need it to be simple > and fast. All public keys are static, and all public and private messages > are viewable by anyone.
I don't understand what this is trying to accomplish. It generates: (inputKey || Hash(inputKey)) ^ Hash(sharedKey || entryId)) What is that trying to do? It looks like you made up your own attempt at an authenticated cipher mode. That's generally a bad idea. I don't buy the argument that you need to make up your own construction for speed. As is, in this Java code, I think the KeyAgreement and multiple MessageDigest object instantiations will dominate the cost. I'd use AES-CBC and HMAC if you're trying to authenticate and encrypt. Keep it simple and well-understood. Some specific comments & questions: 1. You don't do anything to check that sharedKey is a fixed-length or use an unambiguous delimiter between entryId: https://github.com/TrsstProject/trsst/blob/master/src/main/java/com/trsst/Crypto.java#L72 2. What cipher mode is being used here? It's not using ECB by default is it? https://github.com/TrsstProject/trsst/blob/master/src/main/java/com/trsst/Crypto.java#L176 3. Where is the authentication? https://github.com/TrsstProject/trsst/blob/master/src/main/java/com/trsst/Crypto.java#L180 4. This is padding with zero bytes. Is it expecting fixed-length messages? https://github.com/TrsstProject/trsst/blob/master/src/main/java/com/trsst/Crypto.java#L177 5. This might be shady since it doesn't do any input validation on the token: https://github.com/TrsstProject/trsst/blob/master/src/main/java/com/trsst/Crypto.java#L246 6. Vulnerable to timing attack: https://github.com/TrsstProject/trsst/blob/master/src/main/java/com/trsst/Crypto.java#L263 7. Vulnerable to timing attack: https://github.com/TrsstProject/trsst/blob/master/src/main/java/com/trsst/Crypto.java#L331 8. Why did you decide to use BouncyCastle? 9. You're throwing the wrong SecurityException. Probably want to catch GeneralSecurityException and not Exception: https://github.com/TrsstProject/trsst/blob/master/src/main/java/com/trsst/Crypto.java#L31 I only spent a few minutes looking at this. It needs a lot of work. -- Liberationtech is public & archives are searchable on Google. Violations of list guidelines will get you moderated: https://mailman.stanford.edu/mailman/listinfo/liberationtech. Unsubscribe, change to digest, or change password by emailing moderator at [email protected].
