From 082b6084fa5227956aec26b2eda62e9ebd8dbbe4 Mon Sep 17 00:00:00 2001
From: ryneeverett <ryneeverett@gmail.com>
Date: Sun, 21 Jan 2024 18:16:25 -0500
Subject: [PATCH] sync server: Don't hash client_id for salt (#3250)

We don't know why we're doing this step so we probably shouldn't.
Cryptography isn't magic and extra steps are harmful in that they
obscure the important parts.
---
 taskchampion/docs/src/http.md                                | 2 +-
 taskchampion/taskchampion/src/server/encryption.rs           | 5 +----
 taskchampion/taskchampion/src/server/generate-test-data.py   | 2 +-
 taskchampion/taskchampion/src/server/sync/mod.rs             | 4 +---
 taskchampion/taskchampion/src/server/test-bad-app-id.data    | 3 ++-
 taskchampion/taskchampion/src/server/test-bad-client-id.data | 2 +-
 taskchampion/taskchampion/src/server/test-bad-secret.data    | 2 +-
 .../taskchampion/src/server/test-bad-version-id.data         | 3 +--
 taskchampion/taskchampion/src/server/test-bad-version.data   | 3 +--
 taskchampion/taskchampion/src/server/test-good.data          | 2 +-
 10 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/taskchampion/docs/src/http.md b/taskchampion/docs/src/http.md
index e0c504bc0..90b93753d 100644
--- a/taskchampion/docs/src/http.md
+++ b/taskchampion/docs/src/http.md
@@ -6,7 +6,7 @@ The `origin` *should* be an HTTPS endpoint on general principle, but nothing in
 The replica identifies itself to the server using a `client_id` in the form of a UUID.
 This value is passed with every request in the `X-Client-Id` header, in its dashed-hex format.
 
-The salt used in key derivation is the SHA256 hash of the 16-byte form of the client ID.
+The salt used in key derivation is the 16-byte client ID.
 
 ## AddVersion
 
diff --git a/taskchampion/taskchampion/src/server/encryption.rs b/taskchampion/taskchampion/src/server/encryption.rs
index 326ffaa37..610f046ae 100644
--- a/taskchampion/taskchampion/src/server/encryption.rs
+++ b/taskchampion/taskchampion/src/server/encryption.rs
@@ -205,7 +205,6 @@ impl From<Sealed> for Vec<u8> {
 mod test {
     use super::*;
     use pretty_assertions::assert_eq;
-    use ring::digest;
 
     fn make_salt() -> Vec<u8> {
         Cryptor::gen_salt().unwrap()
@@ -333,9 +332,7 @@ mod test {
             let version_id = Uuid::parse_str("b0517957-f912-4d49-8330-f612e73030c4").unwrap();
             let encryption_secret = b"b4a4e6b7b811eda1dc1a2693ded".to_vec();
             let client_id = Uuid::parse_str("0666d464-418a-4a08-ad53-6f15c78270cd").unwrap();
-            let salt = dbg!(digest::digest(&digest::SHA256, client_id.as_ref()))
-                .as_ref()
-                .to_vec();
+            let salt = client_id.as_ref().to_vec();
             (version_id, salt, encryption_secret)
         }
 
diff --git a/taskchampion/taskchampion/src/server/generate-test-data.py b/taskchampion/taskchampion/src/server/generate-test-data.py
index 7d52dce77..d4f818190 100644
--- a/taskchampion/taskchampion/src/server/generate-test-data.py
+++ b/taskchampion/taskchampion/src/server/generate-test-data.py
@@ -20,7 +20,7 @@ def gen(
     version_id=version_id, client_id=client_id, encryption_secret=encryption_secret,
     app_id=1, version=1):
     # first, generate the encryption key
-    salt = hashlib.sha256(uuid.UUID(client_id).bytes).digest()
+    salt = uuid.UUID(client_id).bytes
     key = pbkdf2.PBKDF2(
         encryption_secret,
         salt,
diff --git a/taskchampion/taskchampion/src/server/sync/mod.rs b/taskchampion/taskchampion/src/server/sync/mod.rs
index a7724fe6e..c42db7499 100644
--- a/taskchampion/taskchampion/src/server/sync/mod.rs
+++ b/taskchampion/taskchampion/src/server/sync/mod.rs
@@ -3,7 +3,6 @@ use crate::server::{
     AddVersionResult, GetVersionResult, HistorySegment, Server, Snapshot, SnapshotUrgency,
     VersionId,
 };
-use ring::digest;
 use std::time::Duration;
 use uuid::Uuid;
 
@@ -29,11 +28,10 @@ impl SyncServer {
     /// identify this client to the server.  Multiple replicas synchronizing the same task history
     /// should use the same client_id.
     pub fn new(origin: String, client_id: Uuid, encryption_secret: Vec<u8>) -> Result<SyncServer> {
-        let salt = dbg!(digest::digest(&digest::SHA256, client_id.as_ref()));
         Ok(SyncServer {
             origin,
             client_id,
-            cryptor: Cryptor::new(salt, &Secret(encryption_secret.to_vec()))?,
+            cryptor: Cryptor::new(client_id, &Secret(encryption_secret.to_vec()))?,
             agent: ureq::AgentBuilder::new()
                 .timeout_connect(Duration::from_secs(10))
                 .timeout_read(Duration::from_secs(60))
diff --git a/taskchampion/taskchampion/src/server/test-bad-app-id.data b/taskchampion/taskchampion/src/server/test-bad-app-id.data
index 5f64b922a..e0dbb5f69 100644
--- a/taskchampion/taskchampion/src/server/test-bad-app-id.data
+++ b/taskchampion/taskchampion/src/server/test-bad-app-id.data
@@ -1 +1,2 @@
-
k_�uڌx9�m��g���%j�[�5�+�k����/�
\ No newline at end of file
+
I�N�u�
+;�Ɛ^����9I걏ĉ����i�b��
\ No newline at end of file
diff --git a/taskchampion/taskchampion/src/server/test-bad-client-id.data b/taskchampion/taskchampion/src/server/test-bad-client-id.data
index db8c3262f..10885b533 100644
--- a/taskchampion/taskchampion/src/server/test-bad-client-id.data
+++ b/taskchampion/taskchampion/src/server/test-bad-client-id.data
@@ -1 +1 @@
-
[�+ƢB��Y�k��Ch58-�J�C�<؁���)ś
\ No newline at end of file
+
��h�X��.��EI�/�
��B���
���
\ No newline at end of file
diff --git a/taskchampion/taskchampion/src/server/test-bad-secret.data b/taskchampion/taskchampion/src/server/test-bad-secret.data
index 14c3f6ba3..435585345 100644
--- a/taskchampion/taskchampion/src/server/test-bad-secret.data
+++ b/taskchampion/taskchampion/src/server/test-bad-secret.data
@@ -1 +1 @@
-
�Y��#�_D���>Ͻ܊jޟ�3F��#��B�Q
\ No newline at end of file
+
Y�|�

zJu�ѾK��zF�]���bCx���
\ No newline at end of file
diff --git a/taskchampion/taskchampion/src/server/test-bad-version-id.data b/taskchampion/taskchampion/src/server/test-bad-version-id.data
index 91ef1556e..3b14c54db 100644
--- a/taskchampion/taskchampion/src/server/test-bad-version-id.data
+++ b/taskchampion/taskchampion/src/server/test-bad-version-id.data
@@ -1,2 +1 @@
-
�P*�ڦ\픝�`S�U���
�~��Wc�`ݜ���
-�
\ No newline at end of file
+
Ss���L��|�:=���";��ў��5�JK�=�
\ No newline at end of file
diff --git a/taskchampion/taskchampion/src/server/test-bad-version.data b/taskchampion/taskchampion/src/server/test-bad-version.data
index 50be9e675..7a1380c57 100644
--- a/taskchampion/taskchampion/src/server/test-bad-version.data
+++ b/taskchampion/taskchampion/src/server/test-bad-version.data
@@ -1,2 +1 @@
-cl�;�L+ѩ��'p�f?
-jn������v��O\��
\ No newline at end of file
+cz�y���3��2������6�Lz��A�};�B�6@
\ No newline at end of file
diff --git a/taskchampion/taskchampion/src/server/test-good.data b/taskchampion/taskchampion/src/server/test-good.data
index 453515430..afe7678ba 100644
--- a/taskchampion/taskchampion/src/server/test-good.data
+++ b/taskchampion/taskchampion/src/server/test-good.data
@@ -1 +1 @@
-
�x-��
�Lh�A�4?A��T3�Q��e�R�VW��
\ No newline at end of file
+
t�`&���_)Ӊ������g�r}�-�Z��s
\ No newline at end of file