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.
This commit is contained in:
ryneeverett 2024-01-21 18:16:25 -05:00 committed by GitHub
parent fa21835001
commit 082b6084fa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 11 additions and 17 deletions

View file

@ -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

View file

@ -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)
}

View file

@ -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,

View file

@ -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))

View file

@ -1 +1,2 @@
k_Я╔uз▄x9ФmЙшg╦└┬%j╜[с5щ+╧k≤В╓╪/│
IµNκuΘ
;“Ζ<E2809C>^¤Ό<C2A4>Ά9Iκ±<CEBA>Δ‰·οη©<CEB7>iο

View file

@ -1 +1 @@
[<5B>+Æ¢BÞÞYök­Ch58-éJóCî¢<Ø<>š¬ô)Å›
ذ<EFBFBD>h<>م.<2E>ٌEI<45> /ا جاB<D8A7>ً<EFBFBD>ش <1D>حو

View file

@ -1 +1 @@
ιΏYΖΐ#π_D<15><>υ>Ο½ά<C2BD><6A>Ο3FσΖ#<>BΔQ
YÎ|<7C>z JuâŃľKąçzFń]Ŕáň¨bCxÎŢ<19>

View file

@ -1,2 +1 @@
テP*トレヲ\嵓搖`SナUワ<55>キ~<7E>Wcサ`ン戛ニン
Ss΄ίL΄|π:=<3D>“Λ";ό—Ρ<>²¥5ο JK™=ν

View file

@ -1,2 +1 @@
cl<EFBFBD>;<3B>L+ѩ<><D1A9>'p<>f?
jn<0E><><EFBFBD><EFBFBD><EFBFBD><EFBFBD>v<EFBFBD><1D>O\<5C><>
czاyةنّ3ىâ2في­ˆج6رLzôعAà};„Bإ6@

View file

@ -1 +1 @@
Êx-÷ãàLh¿Aí4?A¸°T3çQ¦Ñe‰R«VW•±
 t<>`&<12>ار_<>ه<EFBFBD><D987>ا<18><>g<>r}<7D>-<2D>Z<EFBFBD><5A>s