skycoin / dmsg

dmsg
7 stars 17 forks source link

Dmsg server optimization #33

Open Darkren opened 3 years ago

Darkren commented 3 years ago

While trying to make encryption optional I noticed one thing. What I'm going to write here is just a suggestion, and I might be totally wrong in my judgements, since I'm not that familiar with the code. This func here: https://github.com/skycoin/dmsg/blob/f50a25191ba9d3ab8b048348de4a5a42c1130fbe/server_session.go#L68 . This func is called on each client->client packet if I get it right. What looks wrong to me in this func:

  1. We decrypt incoming package, perform all sorts of checks, encrypt and resend to the responding client. Would it hurt if we removed decryption/encryption part from here? This way during handshake server must transfer pub keys of clients to them each other. So client A encrypts, sends to the server, server sends to client B and client B decrypts. No ecnryption/decryption on the server side
  2. This func also verifies request, which includes verifiying the object signature. Do we really need the signature?
  3. We also calculate and compare request hash there. Is this needed too? If it's intended to check for data loss during transfer, I guess we shouldn't care about it as long as we use TCP

Probably we could also use something like SSL handshake. During handshake clients via server exchange their pub keys, generate 2 halves of a common symetric key, exchange it with PK-encrypted messages. After that point all the messaging may be encrypted with a single symmetric key which should be more performant. But not sure about this part, just a suggestion

evanlinjin commented 3 years ago

This func is called on each client->client packet if I get it right.

I agree with everything you are saying except this statement. This function is only called to setup a stream. After the stream is set up, client->client communication (via the stream) is handled elsewhere.

jdknives commented 3 years ago

@Darkren @evanlinjin in this case, it might make sense to make separate tickets for these proposals so we can tackle them.