Implement CRC16 for the packet content.

TimonPost / laminar

A simple semi-reliable UDP protocol for multiplayer games

824 stars 66 forks source link

Implement CRC16 for the packet content. #151

Open TimonPost opened 5 years ago

TimonPost commented 5 years ago

Implement a CRC16 checksum for the payload integrity.

Task The CRC16 should be added to the StandardHeader

Create a u16 entry for the StandardHeader, the CRC16 should be the very first 2 bytes of our packet.
Write the CRC16 here
Read the CRC16 here
Update Unittests, because unit tests might expect the protocol version to be on the first 4 bytes.

You could check out the protocol version as a reference which is a CRC32.

daxpedda commented 5 years ago

UDP already has checksum computation integrated: https://en.wikipedia.org/wiki/User_Datagram_Protocol#Packet_structure, https://tools.ietf.org/html/rfc768

I think there are only two other things to discuss here:

UDP checksum on IPv4 is not mandatory, but as far as I know it is enabled by default on all OSs, I dont think we should handle that case
we could still implement a checksum for security reasons, personally I think this doesn't make sense for the following reasons:
- if an attacker can manipulate the checksum in the UDP packet header, then he can also manipulate the checksum in the package itself
- if we want to protect against an attacker, we should implement encryption, checksum is for data integrity and not for security

TimonPost commented 5 years ago

Ideally, you can make a checksum based on some user-defined password. An attacker won't be able to alter that so easily.

OvermindDL1 commented 5 years ago

Ideally, you can make a checksum based on some user-defined password. An attacker won't be able to alter that so easily.

That's not a password then but rather a salt, which is not that hard to reverse for CRC due to its low entropy. If you want to encrypt the or sign packets then full encryption/signing should be performed (this is why RakNet had optional encryption that could be enabled with a single line of code).

jstnlef commented 5 years ago

Just to reiterate here, a checksum is not used as an attack mitigation strategy. It is used exclusively for data integrity. Also, the reason we want to do our own checksum of our payloads would be, in the future, to know whether or not our protocol needs to resend a received packet based on a corrupted payload AND it verifies that the sender is speaking our protocol (or at least attempting to).

ckaran commented 4 years ago

@TimonPost Just to be 100% sure, you don't plan on creating your own CRC16, you just want to add it in, right? https://crates.io/crates/crc will probably be helpful in this.