Merge pull request #5706 from coolreader18/constant_time_eq

Use the `constant_time_eq` crate instead of our bespoke implementation

Cargo.lock

@@ -439,6 +439,12 @@ dependencies = [
  "wasm-bindgen",
 ]
 
+[[package]]
+name = "constant_time_eq"
+version = "0.4.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3d52eff69cd5e647efe296129160853a42795992097e8af39800e1060caeea9b"
+
 [[package]]
 name = "core-foundation"
 version = "0.9.4"
@@ -2348,7 +2354,6 @@ dependencies = [
  "rustpython-wtf8",
  "siphasher",
  "unicode_names2",
- "volatile",
  "widestring",
  "windows-sys 0.59.0",
 ]
@@ -2556,6 +2561,7 @@ dependencies = [
  "caseless",
  "cfg-if",
  "chrono",
+ "constant_time_eq",
  "crossbeam-utils",
  "errno",
  "exitcode",
@@ -3279,12 +3285,6 @@ version = "0.9.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a"
 
-[[package]]
-name = "volatile"
-version = "0.3.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f8e76fae08f03f96e166d2dfda232190638c10e0383841252416f9cfe2ae60e6"
-
 [[package]]
 name = "walkdir"
 version = "2.5.0"

Cargo.toml

@@ -155,6 +155,7 @@ bitflags = "2.4.2"
 bstr = "1"
 cfg-if = "1.0"
 chrono = "0.4.39"
+constant_time_eq = "0.4"
 criterion = { version = "0.5", features = ["html_reports"] }
 crossbeam-utils = "0.8.21"
 flame = "0.2.2"

common/Cargo.toml

@@ -34,7 +34,6 @@ unicode_names2 = { workspace = true }
 lock_api = "0.4"
 radium = "1.1"
 siphasher = "1"
-volatile = "0.3"
 
 [target.'cfg(windows)'.dependencies]
 widestring = { workspace = true }

common/src/cmp.rs

@@ -1,48 +0,0 @@
-use volatile::Volatile;
-
-/// Compare 2 byte slices in a way that ensures that the timing of the operation can't be used to
-/// glean any information about the data.
-#[inline(never)]
-#[cold]
-pub fn timing_safe_cmp(a: &[u8], b: &[u8]) -> bool {
-    // we use raw pointers here to keep faithful to the C implementation and
-    // to try to avoid any optimizations rustc might do with slices
-    let len_a = a.len();
-    let a = a.as_ptr();
-    let len_b = b.len();
-    let b = b.as_ptr();
-    /* The volatile type declarations make sure that the compiler has no
-     * chance to optimize and fold the code in any way that may change
-     * the timing.
-     */
-    let mut result: u8 = 0;
-    /* loop count depends on length of b */
-    let length: Volatile<usize> = Volatile::new(len_b);
-    let mut left: Volatile<*const u8> = Volatile::new(std::ptr::null());
-    let mut right: Volatile<*const u8> = Volatile::new(b);
-
-    /* don't use else here to keep the amount of CPU instructions constant,
-     * volatile forces re-evaluation
-     *  */
-    if len_a == length.read() {
-        left.write(Volatile::new(a).read());
-        result = 0;
-    }
-    if len_a != length.read() {
-        left.write(b);
-        result = 1;
-    }
-
-    for _ in 0..length.read() {
-        let l = left.read();
-        left.write(l.wrapping_add(1));
-        let r = right.read();
-        right.write(r.wrapping_add(1));
-        // safety: the 0..length range will always be either:
-        // * as long as the length of both a and b, if len_a and len_b are equal
-        // * as long as b, and both `left` and `right` are b
-        result |= unsafe { l.read_volatile() ^ r.read_volatile() };
-    }
-
-    result == 0
-}

common/src/lib.rs

@@ -10,7 +10,6 @@ pub mod atomic;
 pub mod borrow;
 pub mod boxvec;
 pub mod cformat;
-pub mod cmp;
 #[cfg(any(unix, windows, target_os = "wasi"))]
 pub mod crt_fd;
 pub mod encodings;

vm/Cargo.toml

@@ -51,6 +51,7 @@ bstr = { workspace = true }
 cfg-if = { workspace = true }
 crossbeam-utils = { workspace = true }
 chrono = { workspace = true, features = ["wasmbind"] }
+constant_time_eq = { workspace = true }
 flame = { workspace = true, optional = true }
 getrandom = { workspace = true }
 hex = { workspace = true }

vm/src/stdlib/operator.rs

@@ -2,7 +2,6 @@ pub(crate) use _operator::make_module;
 
 #[pymodule]
 mod _operator {
-    use crate::common::cmp;
     use crate::{
         AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
         builtins::{PyInt, PyIntRef, PyStr, PyStrRef, PyTupleRef, PyTypeRef},
@@ -13,6 +12,7 @@ mod _operator {
         recursion::ReprGuard,
         types::{Callable, Constructor, PyComparisonOp, Representable},
     };
+    use constant_time_eq::constant_time_eq;
 
     #[pyfunction]
     fn lt(a: PyObjectRef, b: PyObjectRef, vm: &VirtualMachine) -> PyResult {
@@ -328,11 +328,9 @@ mod _operator {
                         "comparing strings with non-ASCII characters is not supported".to_owned(),
                     ));
                 }
-                cmp::timing_safe_cmp(a.as_bytes(), b.as_bytes())
-            }
-            (Either::B(a), Either::B(b)) => {
-                a.with_ref(|a| b.with_ref(|b| cmp::timing_safe_cmp(a, b)))
+                constant_time_eq(a.as_bytes(), b.as_bytes())
             }
+            (Either::B(a), Either::B(b)) => a.with_ref(|a| b.with_ref(|b| constant_time_eq(a, b))),
             _ => {
                 return Err(vm.new_type_error(
                     "unsupported operand types(s) or combination of types".to_owned(),