Skip to content

Switch the 64 bit hash implementation from 3DES to DES#5529

Merged
sougou merged 1 commit intovitessio:masterfrom
planetscale:jacques_fix_hash
Dec 9, 2019
Merged

Switch the 64 bit hash implementation from 3DES to DES#5529
sougou merged 1 commit intovitessio:masterfrom
planetscale:jacques_fix_hash

Conversation

@aquarapid
Copy link
Copy Markdown
Contributor

DES, for null (\0 bytes) keys like we use they are the same
because 3DES is then just DES in an encrypt-decrypt-encrypt
cycle with a 8 byte \0 key.

Signed-off-by: Jacques Grove aquarapid@gmail.com

DES, for null (\0 bytes) keys like we use they are the same
because 3DES is then just DES in an encrypt-decrypt-encrypt
cycle with a 8 byte \0 key.

Signed-off-by: Jacques Grove <aquarapid@gmail.com>
@aquarapid aquarapid requested a review from sougou as a code owner December 8, 2019 02:25
@deepthi deepthi changed the title Switch the 64 bit hash implementation from 3DES to Switch the 64 bit hash implementation from 3DES to DES Dec 8, 2019
@morgo
Copy link
Copy Markdown
Contributor

morgo commented Dec 8, 2019

@aquarapid If we switch the hash implementation, does that not mean that values will hash differently? i.e. is there an upgrade problem where in a previous version "value" hashed to shard 3, and now it hashes to shard 6?

@aquarapid
Copy link
Copy Markdown
Contributor Author

aquarapid commented Dec 8, 2019

No, the behavior remains unchanged (e.g. the current hash_test.go tests are unchanged), because the values hash exactly the same, viz.:

DES and 3DES use the same encrypt/decrypt primitive functions that operates using 8 byte keys. However, 3DES uses 3 of these 8 byte keys, viz.:

DES: encrypt(input, key) = output

3DES: encrypt(decrypt(encrypt(input, key1), key2), key3) = output

These are obviously equivalent when key == "\0\0\0\0\0\0\0\0" and key1 == key2 == key3 == "\0\0\0\0\0\0\0\0" (as in our case)

@sougou
Copy link
Copy Markdown
Contributor

sougou commented Dec 9, 2019

Verified that this change is safe: https://en.wikipedia.org/wiki/Triple_DES.

@sougou sougou merged commit b3a147c into vitessio:master Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants