fix!: detect SHA‐1 collision attacks

Fix [GHSA-2frx-2596-x5r6].

[GHSA-2frx-2596-x5r6]: GHSA-2frx-2596-x5r6

This uses the `sha1-checked` crate from the RustCrypto project. It’s
a pure Rust implementation, with no SIMD or assembly code.

Raw hashing is somewhere around 0.25× to 0.65× the speed of the
previous implementation, depending on the feature configuration
and whether the CPU supports hardware‐accelerated hashing. (The
more portable assembly in `sha1-asm` that doesn’t require the SHA
instruction set doesn’t seem to speed things up that much; in fact,
`sha1_smol` somehow regularly beats the assembly code used by `sha1`
on my i9‐9880H MacBook Pro! Presumably this is why that path was
removed in newer versions of the `sha1` crate.)

Performance on an end‐to‐end `gix no-repo pack verify` benchmark
using pack files from the Linux kernel Git server measures around
0.41× to 0.44× compared to the base commit on an M2 Max and a
Ryzen 7 5800X, both of which have hardware instructions for SHA‐1
acceleration that the previous implementation uses but this one does
not. On the i9‐9880H, it’s around 0.58× to 0.60× the speed;
the slowdown is reduced by the older hardware’s lack of SHA‐1
instructions.

The `sha1collisiondetection` crate from the Sequoia PGP project,
based on a modified C2Rust translation of the library used by Git,
was also considered; although its raw hashing performance seems
to measure around 1.12–1.15× the speed of `sha1-checked` on
x86, it’s indistinguishable from noise on the end‐to‐end
benchmark, and on an M2 Max `sha1-checked` is consistently
around 1.03× the speed of `sha1collisiondetection` on that
benchmark. The `sha1collisiondetection` crate has also had a
soundness issue in the past due to the automatic C translation,
whereas `sha1-checked` has only one trivial `unsafe` block. On the
other hand, `sha1collisiondetection` is used by both Sequoia itself
and the `gitoid` crate, whereas rPGP is the only major user of
`sha1-checked`. I don’t think there’s a clear winner here.

The performance regression is very unfortunate, but the [SHAttered]
attack demonstrated a collision back in 2017, and the 2020 [SHA‐1 is
a Shambles] attack demonstrated a practical chosen‐prefix collision
that broke the use of SHA‐1 in OpenPGP, costing $75k to perform,
with an estimate of $45k to replicate at the time of publication and
$11k for a classical collision.

[SHAttered]: https://shattered.io/
[SHA‐1 is a Shambles]: https://sha-mbles.github.io/

Given the increase in GPU performance and production since then,
that puts the Git object format squarely at risk. Git mitigated this
attack in 2017; the algorithm is fairly general and detects all the
existing public collisions. My understanding is that an entirely new
cryptanalytic approach would be required to develop a collision attack
for SHA‐1 that would not be detected with very high probability.

I believe that the speed penalty could be mitigated, although not
fully eliminated, by implementing a version of the hardened SHA‐1
function that makes use of SIMD. For instance, the assembly code used
by `openssl speed sha1` on my i9‐9880H measures around 830 MiB/s,
compared to the winning 580 MiB/s of `sha1_smol`; adding collision
detection support to that would surely incur a performance penalty,
but it is likely that it could be much more competitive with
the performance before this commit than the 310 MiB/s I get with
`sha1-checked`. I haven’t been able to find any existing work on
this; it seems that more or less everyone just uses the original
C library that Git does, presumably because nothing except Git and
OpenPGP is still relying on SHA‐1 anyway…

The performance will never compete with the >2 GiB/s that can
be achieved with the x86 SHA instruction set extension, as the
`SHA1RNDS4` instruction sadly runs four rounds at a time while the
collision detection algorithm requires checks after every round,
but I believe SIMD would still offer a significant improvement,
and the AArch64 extension seems like it may be more flexible.

I know that these days the Git codebase has an additional faster
unsafe API without these checks that it tries to carefully use only
for operations that do not depend on hashing results for correctness
or safety. I personally believe that’s not a terribly good idea,
as it seems easy to misuse in a case where correctness actually does
matter, but maybe that’s just my Rust safety bias talking. I think
it would be better to focus on improving the performance of the safer
algorithm, as I think that many of the operations where the performance
penalty is the most painful are dealing with untrusted input anyway.

The `Hasher` struct gets a lot bigger; I don’t know if this is
an issue or not, but if it is, it could potentially be boxed.

Closes: #585

Author: emilazy

.github/workflows/ci.yml

@@ -391,7 +391,7 @@ jobs:
       - name: features of gix-features
         run: |
           set +x
-          for feature in progress fs-walkdir-parallel parallel io-pipe crc32 zlib zlib-rust-backend fast-sha1 rustsha1 cache-efficiency-debug; do
+          for feature in progress fs-walkdir-parallel parallel io-pipe crc32 zlib zlib-rust-backend cache-efficiency-debug; do
             (cd gix-features && cargo build --features "$feature" --target "$TARGET")
           done
       - name: crates with 'wasm' feature

.github/workflows/release.yml

@@ -137,8 +137,7 @@ jobs:
             os: windows-latest
           - target: aarch64-pc-windows-msvc
             os: windows-latest
-        # on linux we build with musl which causes trouble with open-ssl. For now, just build max-pure there
-        # even though we could also build with `--features max-control,http-client-reqwest,gitoxide-core-blocking-client,gix-features/fast-sha1` for fast hashing.
+        # on linux we build with musl which causes trouble with open-ssl. For now, just build max-pure there.
         # It's a TODO.
         exclude:
           - target: x86_64-unknown-linux-musl

Cargo.lock

@@ -45,7 +45,7 @@ max = ["max-control", "fast", "gitoxide-core-tools-query", "gitoxide-core-tools-
 ## transports as it uses Rust's HTTP implementation.
 ##
 ## As fast as possible, with TUI progress, progress line rendering with auto-configuration, all transports available but less mature pure Rust HTTP implementation, all `ein` tools, CLI colors and local-time support, JSON output, regex support for rev-specs.
-max-pure = ["max-control", "gix-features/rustsha1", "gix-features/zlib-rust-backend", "http-client-reqwest", "gitoxide-core-blocking-client"]
+max-pure = ["max-control", "gix-features/zlib-rust-backend", "http-client-reqwest", "gitoxide-core-blocking-client"]
 
 ## Like `max`, but with more control for configuration. See the *Package Maintainers* headline for more information.
 max-control = ["tracing", "fast-safe", "pretty-cli", "gitoxide-core-tools", "prodash-render-line", "prodash-render-tui", "prodash/render-line-autoconfigure", "gix/revparse-regex"]
@@ -60,7 +60,7 @@ lean = ["fast", "tracing", "pretty-cli", "http-client-curl", "gitoxide-core-tool
 ## This build is essentially limited to local operations without any fanciness.
 ##
 ## Optimized for size, no parallelism thus much slower, progress line rendering.
-small = ["pretty-cli", "gix-features/rustsha1", "gix-features/zlib-rust-backend", "prodash-render-line", "is-terminal"]
+small = ["pretty-cli", "gix-features/zlib-rust-backend", "prodash-render-line", "is-terminal"]
 
 ## Like lean, but uses Rusts async implementations for networking.
 ##
@@ -74,7 +74,7 @@ small = ["pretty-cli", "gix-features/rustsha1", "gix-features/zlib-rust-backend"
 lean-async = ["fast", "tracing", "pretty-cli", "gitoxide-core-tools", "gitoxide-core-tools-query", "gitoxide-core-tools-corpus", "gitoxide-core-async-client", "prodash-render-line"]
 
 #! ### Package Maintainers
-#! `*-control` features leave it to you to configure C libraries, involving choices for `zlib`, ! hashing and transport implementation.
+#! `*-control` features leave it to you to configure C libraries, involving choices for `zlib` and transport implementation.
 #!
 #! Additional features *can* be provided with `--features` and are handled by the [`gix-features` crate](https://docs.rs/gix-features/latest).
 #! If nothing else is specified, the Rust implementation is used. ! Note that only one feature of each section can be enabled at a time.
@@ -84,28 +84,25 @@ lean-async = ["fast", "tracing", "pretty-cli", "gitoxide-core-tools", "gitoxide-
 #!     - `gix-features/zlib-ng-compat`
 #!     - `gix-features/zlib-stock`
 #!     - `gix-features/zlib-rust-backend` (*default if no choice is made*)
-#! * **sha1**
-#!     - `gix-features/fast-sha1`
-#!     - `gix-features/rustsha1` (*default if no choice is made*)
 #! * **HTTP** - see the *Building Blocks for mutually exclusive networking* headline
 #!
 #! #### Examples
 #!
 #! * `cargo build --release --no-default-features --features max-control,gix-features/zlib-stock,gitoxide-core-blocking-client,http-client-curl`
 #!     - Create a build just like `max`, but using the stock `zlib` library instead of `zlib-ng`
-#! * `cargo build --release --no-default-features --features max-control,http-client-reqwest,gitoxide-core-blocking-client,gix-features/fast-sha1`
-#!     - Create a build just like `max-pure`, but with faster hashing due to `fast-sha1`.
+#! * `cargo build --release --no-default-features --features max-control,http-client-reqwest,gitoxide-core-blocking-client,gix-features/zlib-ng`
+#!     - Create a build just like `max-pure`, but with faster compression due to `zlib-ng`.
 
 #! ### Building Blocks
 #! Typical combinations of features of our dependencies, some of which are referred to in the `gitoxide` crate's code for conditional compilation.
 
 ## Makes the crate execute as fast as possible by supporting parallel computation of otherwise long-running functions
-## as well as fast, hardware accelerated hashing, along with a faster zlib backend.
+## as well as a faster zlib backend.
 ## If disabled, the binary will be visibly smaller.
 fast = ["gix/max-performance", "gix/comfort"]
 
 ## Makes the crate execute as fast as possible by supporting parallel computation of otherwise long-running functions
-## as well as fast, hardware accelerated hashing, along with a faster zlib backend.
+## as well as a faster zlib backend.
 ## If disabled, the binary will be visibly smaller.
 fast-safe = ["gix/max-performance-safe", "gix/comfort"]
 
@@ -205,8 +202,7 @@ gix-hash = { opt-level = 3 }
 gix-actor = { opt-level = 3 }
 gix-config = { opt-level = 3 }
 miniz_oxide = { opt-level = 3 }
-sha1 = { opt-level = 3 }
-sha1_smol = { opt-level = 3 }
+sha1-checked = { opt-level = 3 }
 
 [profile.release]
 overflow-checks = false

Cargo.toml

@@ -35,7 +35,3 @@ This file is for tracking features that are less well implemented or less powerf
 * **gix-url** _might_ be more restrictive than what git allows as for the most part, it uses a browser grade URL parser.
     * Thus far there is no proof for this, and as _potential remedy_ we could certainly re-implement exactly what git does
       to handle its URLs.
-
-### `gix-features`
-
-* **sha1** isn't hardened (i.e. doesn't have collision detection). Needs [to be contributed](https://github.com/GitoxideLabs/gitoxide/issues/585).

SHORTCOMINGS.md

@@ -894,8 +894,6 @@ See its [README.md](https://github.com/GitoxideLabs/gitoxide/blob/main/gix-lock/
         * `in_parallel`
         * `join`
     * _When off all functions execute serially_
-* **fast-sha1**
-    * provides a faster SHA1 implementation using CPU intrinsics
 * [x] API documentation
 
 ### gix-tui

crate-status.md

@@ -20,7 +20,7 @@ doctest = false
 serde = ["dep:serde", "gix-hash/serde", "bstr/serde"]
 
 [dependencies]
-gix-features = { version = "^0.40.0", path = "../gix-features", features = ["rustsha1"] }
+gix-features = { version = "^0.40.0", path = "../gix-features" }
 gix-hash = { version = "^0.16.0", path = "../gix-hash" }
 gix-chunk = { version = "^0.4.11", path = "../gix-chunk" }
 

gix-commitgraph/Cargo.toml

@@ -80,40 +80,25 @@ zlib-stock = ["zlib", "flate2?/zlib"]
 ## may build in environments where other backends don't.
 zlib-rust-backend = ["zlib", "flate2?/rust_backend"]
 
-#! ### Mutually Exclusive SHA1
-## A fast SHA1 implementation is critical to `gitoxide's` object database performance
-## A multi-crate implementation that can use hardware acceleration, thus bearing the potential for up to 2Gb/s throughput on
-## CPUs that support it, like AMD Ryzen or Intel Core i3, as well as Apple Silicon like M1.
-## Takes precedence over `rustsha1` if both are specified.
-fast-sha1 = ["dep:sha1"]
-## A standard and well performing pure Rust implementation of Sha1. Will significantly slow down various git operations.
-rustsha1 = ["dep:sha1_smol"]
-
 #! ### Other
 
 ## Count cache hits and misses and print that debug information on drop.
 ## Caches implement this by default, which costs nothing unless this feature is enabled
 cache-efficiency-debug = []
 
-[[test]]
-name = "hash"
-path = "tests/hash.rs"
-required-features = ["rustsha1"]
-
 [[test]]
 name = "parallel"
 path = "tests/parallel_threaded.rs"
-required-features = ["parallel", "rustsha1"]
+required-features = ["parallel"]
 
 [[test]]
 name = "multi-threaded"
 path = "tests/parallel_shared_threaded.rs"
-required-features = ["parallel", "rustsha1"]
+required-features = ["parallel"]
 
 [[test]]
 name = "single-threaded"
 path = "tests/parallel_shared.rs"
-required-features = ["rustsha1"]
 
 [[test]]
 name = "pipe"
@@ -133,10 +118,8 @@ parking_lot = { version = "0.12.0", default-features = false, optional = true }
 jwalk = { version = "0.8.1", optional = true }
 walkdir = { version = "2.3.2", optional = true } # used when parallel is off
 
-# hashing and 'fast-sha1' feature
-sha1_smol = { version = "1.0.0", optional = true }
+# hashing
 crc32fast = { version = "1.2.1", optional = true }
-sha1 = { version = "0.10.0", optional = true }
 
 # progress
 prodash = { version = "29.0.1", optional = true }
@@ -159,12 +142,6 @@ libc = { version = "0.2.119" }
 [dev-dependencies]
 bstr = { version = "1.3.0", default-features = false }
 
-
-# Assembly doesn't yet compile on MSVC on windows, but does on GNU, see https://github.com/RustCrypto/asm-hashes/issues/17
-# At this time, only aarch64, x86 and x86_64 are supported.
-[target.'cfg(all(any(target_arch = "aarch64", target_arch = "x86", target_arch = "x86_64"), not(target_os = "windows")))'.dependencies]
-sha1 = { version = "0.10.0", optional = true, features = ["asm"] }
-
 [package.metadata.docs.rs]
 all-features = true
 features = ["document-features"]

gix-features/Cargo.toml

@@ -1,54 +1,4 @@
 //! Hash functions and hash utilities
-//!
-//! With the `fast-sha1` feature, the `Sha1` hash type will use a more elaborate implementation utilizing hardware support
-//! in case it is available. Otherwise the `rustsha1` feature should be set. `fast-sha1` will take precedence.
-//! Otherwise, a minimal yet performant implementation is used instead for a decent trade-off between compile times and run-time performance.
-#[cfg(all(feature = "rustsha1", not(feature = "fast-sha1")))]
-mod _impl {
-    use super::Digest;
-
-    /// A implementation of the Sha1 hash, which can be used once.
-    #[derive(Default, Clone)]
-    pub struct Sha1(sha1_smol::Sha1);
-
-    impl Sha1 {
-        /// Digest the given `bytes`.
-        pub fn update(&mut self, bytes: &[u8]) {
-            self.0.update(bytes);
-        }
-        /// Finalize the hash and produce a digest.
-        pub fn digest(self) -> Digest {
-            self.0.digest().bytes()
-        }
-    }
-}
-
-/// A hash-digest produced by a [`Hasher`] hash implementation.
-#[cfg(any(feature = "fast-sha1", feature = "rustsha1"))]
-pub type Digest = [u8; 20];
-
-#[cfg(feature = "fast-sha1")]
-mod _impl {
-    use sha1::Digest;
-
-    /// A implementation of the Sha1 hash, which can be used once.
-    #[derive(Default, Clone)]
-    pub struct Sha1(sha1::Sha1);
-
-    impl Sha1 {
-        /// Digest the given `bytes`.
-        pub fn update(&mut self, bytes: &[u8]) {
-            self.0.update(bytes);
-        }
-        /// Finalize the hash and produce a digest.
-        pub fn digest(self) -> super::Digest {
-            self.0.finalize().into()
-        }
-    }
-}
-
-#[cfg(any(feature = "rustsha1", feature = "fast-sha1"))]
-pub use _impl::Sha1 as Hasher;
 
 /// Compute a CRC32 hash from the given `bytes`, returning the CRC32 hash.
 ///

gix-features/src/hash.rs

@@ -20,17 +20,18 @@ test = false
 serde = ["dep:serde"]
 
 [dependencies]
-gix-features = { version = "^0.40.0", path = "../gix-features", features = ["rustsha1", "progress"] }
+gix-features = { version = "^0.40.0", path = "../gix-features", features = ["progress"] }
 
 thiserror = "2.0.0"
 faster-hex = { version = "0.9.0" }
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
+sha1-checked = { version = "0.10.0", default-features = false }
 
 document-features = { version = "0.2.0", optional = true }
 
 [dev-dependencies]
 gix-testtools = { path = "../tests/tools" }
-gix-features = { path = "../gix-features", features = ["rustsha1"] }
+gix-features = { path = "../gix-features" }
 
 [package.metadata.docs.rs]
 all-features = true

gix-features/tests/hash.rs

@@ -1,24 +1,65 @@
+use sha1_checked::{CollisionResult, Digest};
+
 /// The error returned by [`Hasher::try_finalize()`].
 #[derive(Debug, thiserror::Error)]
 #[allow(missing_docs)]
-pub enum Error {}
+pub enum Error {
+    #[error("Detected SHA-1 collision attack with digest {digest}")]
+    CollisionAttack { digest: crate::ObjectId },
+}
 
 /// A implementation of the Sha1 hash, which can be used once.
-#[derive(Default, Clone)]
-pub struct Hasher(gix_features::hash::Hasher);
+///
+/// We use [`sha1_checked`] to implement the same collision detection
+/// algorithm as Git.
+#[derive(Clone)]
+pub struct Hasher(sha1_checked::Sha1);
+
+impl Default for Hasher {
+    #[inline]
+    fn default() -> Self {
+        // This matches the configuration used by Git, which only uses
+        // the collision detection to bail out, rather than computing
+        // alternate “safe hashes” for inputs where a collision attack
+        // was detected.
+        Self(sha1_checked::Builder::default().safe_hash(false).build())
+    }
+}
 
 impl Hasher {
     /// Digest the given `bytes`.
     pub fn update(&mut self, bytes: &[u8]) {
         self.0.update(bytes);
     }
     /// Finalize the hash and produce an object ID.
+    ///
+    /// Returns [`Error`] if a collision attack is detected.
+    #[inline]
     pub fn try_finalize(self) -> Result<crate::ObjectId, Error> {
-        Ok(self.0.digest().into())
+        match self.0.try_finalize() {
+            CollisionResult::Ok(digest) => Ok(crate::ObjectId::Sha1(digest.into())),
+            CollisionResult::Mitigated(_) => {
+                // SAFETY: `CollisionResult::Mitigated` is only
+                // returned when `safe_hash()` is on. `Hasher`’s field
+                // is private, and we only construct it in the
+                // `Default` instance, which turns `safe_hash()` off.
+                //
+                // As of Rust 1.84.1, the compiler can’t figure out
+                // this function cannot panic without this.
+                #[allow(unsafe_code)]
+                unsafe {
+                    std::hint::unreachable_unchecked()
+                }
+            }
+            CollisionResult::Collision(digest) => Err(Error::CollisionAttack {
+                digest: crate::ObjectId::Sha1(digest.into()),
+            }),
+        }
     }
 }
 
 /// Produce a hasher suitable for the given kind of hash.
+#[inline]
 pub fn hasher(kind: crate::Kind) -> Hasher {
     match kind {
         crate::Kind::Sha1 => Hasher::default(),

gix-hash/Cargo.toml

@@ -1,5 +1,6 @@
 use gix_hash::ObjectId;
 
+mod hasher;
 mod kind;
 mod object_id;
 mod oid;

gix-hash/src/hasher/mod.rs

@@ -0,0 +1,6 @@
+use gix_hash::Hasher;
+
+#[test]
+fn size_of_sha1() {
+    assert_eq!(std::mem::size_of::<Hasher>(), 824);
+}