uber / ringpop-go

Scalable, fault-tolerant application-layer sharding for Go applications
http://www.uber.com
MIT License
835 stars 83 forks source link

[Labels] Prevent checksum collisions for labels #174

Closed thanodnl closed 8 years ago

thanodnl commented 8 years ago

This PR hardens the checksumming algorithm behind labels by preventing the easy forging of checksum collisions. The labels checksum is calculated by hash(key + "=" + value). By disallowing applications to use the = in their keys it is non-trivial to create a hash collision.

Since the hashes are xorred for order independence having the same hash twice effectively negates a certain value in the checksum.

coveralls commented 8 years ago

Coverage Status

Changes Unknown when pulling 55be11d0a9b3a4f2893f9ef54f7aa3e46d819006 on feature/labels-checksum-hardening into \ on feature/labels**.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.1%) to 92.269% when pulling 55be11d0a9b3a4f2893f9ef54f7aa3e46d819006 on feature/labels-checksum-hardening into a285bbc7fac089eb1f9f437b677075ce4b1d9364 on feature/labels.

mennopruijssers commented 8 years ago

Looks good! The only thing missing is a test to make sure they're validated when set through SetLocalLabel / SetLocalLabels (but you're probably adding it after the tests are added and merged in from #167)

thanodnl commented 8 years ago

I have change checksum calculation to write the size (in bytes) of the key and value prior to their respective bytes in the buffer that is fingerprinted. This prevents hash collisions that would have been caused by the same input. This diff is now not needed anymore.