Skip to main content
Code Review

Return to Answer

added 1048 characters in body
Source Link
Maarten Bodewes
Maarten Bodewes
  • 5.8k
  • 18
  • 52

"With this implementation multiple clients can communicate in a chat. But they'll only be able to read the messages sent if they all entered the same password. Someone trying to join the chat with a different password will only see encrypted garbage. Also the server only sees encrypted garbage."

That's a claim that isn't true at all. I don't see how the server is supposed to authenticate & trust the clients. Nor do I see how the client is supposed to authenticate & trust the server. The protocol only partially protects against eavesdropping by an adversary, and that is not considered enough security by anyone.

It is always a good idea to create a good description of the protocol. Currently I don't see any way to e.g. update the iteration count, which is set at 600000 (using a literal, which is also not great).

Websockets are perfectly able to send out binary data. Funny enough we see that hexlify is used followed by a "decoding" to a string. If the HTTP protocol & API is followed then it is easy to see that binary can be send without converting to text.

Conclusion

Sorry to be a bit harsh here, but this protocol doesn't add much on top of TLS. The code is easy to read. It should however be considered a learning exercise and nothing more.

Websockets are perfectly able to send out binary data. Funny enough we see that hexlify is used followed by a "decoding" to a string. If the HTTP protocol & API is followed then it is easy to see that binary can be send without converting to text.

"With this implementation multiple clients can communicate in a chat. But they'll only be able to read the messages sent if they all entered the same password. Someone trying to join the chat with a different password will only see encrypted garbage. Also the server only sees encrypted garbage."

That's a claim that isn't true at all. I don't see how the server is supposed to authenticate & trust the clients. Nor do I see how the client is supposed to authenticate & trust the server. The protocol only partially protects against eavesdropping by an adversary, and that is not considered enough security by anyone.

It is always a good idea to create a good description of the protocol. Currently I don't see any way to e.g. update the iteration count, which is set at 600000 (using a literal, which is also not great).

Websockets are perfectly able to send out binary data. Funny enough we see that hexlify is used followed by a "decoding" to a string. If the HTTP protocol & API is followed then it is easy to see that binary can be send without converting to text.

Conclusion

Sorry to be a bit harsh here, but this protocol doesn't add much on top of TLS. The code is easy to read. It should however be considered a learning exercise and nothing more.

Source Link
Maarten Bodewes
Maarten Bodewes
  • 5.8k
  • 18
  • 52

Is this a good implementation for encryption in a chat?

No.

I'd argue that most chat implementations are not dependent on the server to decrypt / reencrypt the information (this is called a proxy in cryptography). This is not the end-to-end encryption that e.g. Signal or even WhatsApp uses.

Step 3 is insecure. There is a need to trust the public key of the server. This is why your browser contain a list of trusted (root) certificates. Although most users are unaware, we trust the CA's that issued those root certificates by trusting the producers of the browser (and this is how many browser manufacturers also try and get some money).

You have used the correct primitives such as PBKDF2, AES and RSA. However, you do use AES in ECB mode, which is known to be insecure for encrypting arbitrary messages.

I used RSA because I read that RSA was a good way send the derived key from client to server. But I don't understand: The AES key is generated from the derived key. The password entered by the client is send encrypted to the server. But the derived key used for the AES is send unencrypted from the server to the client. Couldn't someone with bad intentions not just intercept this derived key and use that to generate the same AES-key?

Yes, they could and this is not a good protocol.

It is possible derive a master / session key using key agreement (DH). Generally it is best to only use the password for authentication.

It is said that in real life TLS is used, but I'd argue that in that case you might as well send the password unencrypted from client to server. That is what HTTP basic authentication does - as there is already an established secure channel where the server is trusted.

I first thought it would make more sense to let the client generate a derived key and set it encrypted to the server. But each client would have a different salt?

Salts can be send to the other side unencrypted; they don't need to be secret.

If my implementation is wrong, what to change?

I don't know, but if I read this text then I'm pretty sure that you are a novice at this. In that case you'd start by studying what is already out there.

Other things to consider?

Yeah, don't be proud and don't think you can create a secure chat from scratch. To do that it is important to first learn what is out there.

Crypto remarks

AES should be using an authenticated mode such as AES-GCM. Currently the protocol uses ECB which is not secure for a transport mode. Actually, it seems the implementation doesn't even protect the confidentiality of messages as it fails to protect against plaintext and particularly padding oracle attacks.

Salts are usually just 16 to 32 random bytes. There is no need to depend on code of bcrypt if PBKDF2 is used for the actual implementation.

Other remarks

Websockets are perfectly able to send out binary data. Funny enough we see that hexlify is used followed by a "decoding" to a string. If the HTTP protocol & API is followed then it is easy to see that binary can be send without converting to text.