From 990766fe06ed5fa57378dbc27ad7e38e4c49298d Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Sat, 5 Jun 2021 01:27:22 -0700 Subject: [PATCH] [0+UTILS] add docs to utils.rs - requires the `unstable_internals` feature to be enabled for doctests to work --- rust/protocol/src/lib.rs | 2 +- rust/protocol/src/utils.rs | 45 ++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/rust/protocol/src/lib.rs b/rust/protocol/src/lib.rs index 9ac6dbe1b0..fbfacc50d8 100644 --- a/rust/protocol/src/lib.rs +++ b/rust/protocol/src/lib.rs @@ -39,7 +39,7 @@ mod session; mod session_cipher; mod state; mod storage; -mod utils; +pub mod utils; use error::Result; diff --git a/rust/protocol/src/utils.rs b/rust/protocol/src/utils.rs index 67a108c81e..c3465ab160 100644 --- a/rust/protocol/src/utils.rs +++ b/rust/protocol/src/utils.rs @@ -1,8 +1,10 @@ // -// Copyright 2020 Signal Messenger, LLC. +// Copyright 2020-2022 Signal Messenger, LLC. // SPDX-License-Identifier: AGPL-3.0-only // +//! Utility methods. + use std::cmp::Ordering; fn expand_top_bit(a: u8) -> u8 { @@ -31,26 +33,27 @@ fn ct_select(mask: u8, a: u8, b: u8) -> u8 { b ^ (mask & (a ^ b)) } -/* -* If x and y are different lengths, this leaks information about -* their relative sizes. This is irrelevant as we always invoke it -* with two inputs of the same size. -* -* In addition it will leak the final comparison result, when the -* integer is translated to the Ordering enum. This seems unavoidable. -* -* The primary goal of this function is to not leak any additional -* information, besides the ordering, about the value of the two keys, -* say due to an early exit of the loop. -* -* It would be possible to instead have this function SHA-256 hash both -* inputs, then compare the resulting hashes in the usual non-const -* time way. We avoid this approach at the moment since it is not clear -* if applications will rely on public key ordering being defined in -* some particular way or not. - */ - -pub(crate) fn constant_time_cmp(x: &[u8], y: &[u8]) -> Ordering { +/// Compare the contents of `x` and `y` while trying not to leak information to [timing +/// attacks](https://en.wikipedia.org/wiki/Timing_attack). +/// +/// The primary goal of this function is to **not leak any additional information, besides the +/// ordering, about the value of the two keys**, say due to an early exit of the loop. +/// +/// ### Implementation Notes +/// #### Leaked Information +/// 1. If x and y are different lengths, this leaks information about their relative sizes. +/// - *This is currently irrelevant, as we always invoke it with two inputs of the same size.* +/// 2. In addition, it will leak the final comparison result, when the integer is translated to the +/// Ordering enum. +/// - *This seems unavoidable.* +/// +/// #### Possible Improvements +/// - It would be possible to instead have this function SHA-256 hash both +/// inputs, then compare the resulting hashes in the usual non-constant +/// time way. +/// - *We avoid this approach at the moment since it is not clear if applications will rely on +/// public key ordering being defined in some particular way or not.* +pub fn constant_time_cmp(x: &[u8], y: &[u8]) -> Ordering { if x.len() < y.len() { return Ordering::Less; }