search

TokTok / spec

Tox Protocol Specification
https://toktok.ltd/spec
GNU General Public License v3.0
32 stars 13 forks source link

TCP Nonce use is opposite to what's described in the spec #63

Closed gjedeer closed 4 years ago

gjedeer commented 6 years ago

I'm implementing a toy tox tcp client in python and found a little mismatch between the spec and reality.

Nonce sent in initial by client is later used in encrypting client->server messages. Similarly, nonce sent by the server is later used in encrypting server->client messages.

Proof: https://github.com/TokTok/c-toxcore/blob/bb5f8a17103537caa09bcf14130debe3210fb61f/toxcore/TCP_server.c#L545

memcpy(con->recv_nonce, plain + CRYPTO_PUBLIC_KEY_SIZE, CRYPTO_NONCE_SIZE);

The nonce used by the TCP server for messages received from client is copied from the initial packet received from the client.

This change is Reviewable

CLAassistant commented 6 years ago

CLA assistant check
All committers have signed the CLA.

gjedeer commented 6 years ago

Rebased this on top of master and fixed the CLA situation. The only thing missing is an LGTM

zugz commented 6 years ago

I'm implementing a toy tox tcp client in python and found a little mismatch between the spec and reality.

Nonce sent in initial by client is later used in encrypting client->server messages. Similarly, nonce sent by the server is later used in encrypting server->client messages.

You're right. The patch is good, except:

@@ -1557,8 +1557,8 @@ TCP client uses along with the secret key associated with the public key in the first 32 bytes of the packet to encrypt the rest of this 'packet'. The encrypted part of this packet contains a temporary public key that will be used for encryption during the connection and will be discarded after. It also -contains a base nonce which will be used later for encrypting packets sent to -the TCP client. +contains a base nonce which will be used later for encrypting packets received +from the TCP client.

If the server decrypts successfully the encrypted data in the handshake packet and responds with the following handshake response of length 96 bytes:

This one should be "decrypting packets received from the TCP client".

gjedeer commented 6 years ago

@zugz fixed

zugz commented 6 years ago

LGTM

zugz commented 6 years ago
:lgtm_strong:
iphydf commented 6 years ago

@gjedeer I'm updating the spec with the latest text from hs-toxcore. Have you made this change in hs-toxcore as well?

gjedeer commented 6 years ago

I have no idea about how to use hs-toxcore or Haskell in general.

iphydf commented 6 years ago

@gjedeer https://github.com/TokTok/hs-toxcore/blob/master/src/tox/Network/Tox.lhs#L900 edit this file, then we can regenerate the spec from that.

kpp commented 6 years ago

There is also a nice diagram on how it works: https://github.com/tox-rs/tox-spec/blob/master/spec.md#handshake-diagram

iphydf commented 4 years ago

I've edited the source file in https://github.com/TokTok/hs-toxcore/pull/164, so I can merge this now. Thanks!