Merge pull request #1915 from emilazy/push-qvyqmopsoltr

feat!: detect SHA‐1 collision attacks

Author: Byron

Diff for: .github/workflows/ci.yml

@@ -391,7 +391,7 @@ jobs:
       - name: features of gix-features
         run: |
           set +x
-          for feature in progress parallel io-pipe crc32 zlib zlib-rust-backend fast-sha1 rustsha1 cache-efficiency-debug; do
+          for feature in progress 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

Diff for: .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

Diff for: 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

Diff for: 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).

Diff for: 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

Diff for: crate-status.md

@@ -78,11 +78,11 @@ enum Error {
     },
     #[error("Object didn't verify after right after writing it")]
     Verify(#[from] objs::data::verify::Error),
-    #[error("{kind} object {expected} wasn't re-encoded without change - new hash is {actual}")]
+    #[error("{kind} object wasn't re-encoded without change")]
     ObjectEncodeMismatch {
+        #[source]
+        source: gix::hash::verify::Error,
         kind: object::Kind,
-        actual: ObjectId,
-        expected: ObjectId,
     },
     #[error("The recently written file for loose object {id} could not be found")]
     WrittenFileMissing { id: ObjectId },
@@ -201,17 +201,16 @@ pub fn pack_or_pack_index(
                         kind: object_kind,
                         id: index_entry.oid,
                     })?;
-                    if written_id != index_entry.oid {
+                    if let Err(err) = written_id.verify(&index_entry.oid) {
                         if let object::Kind::Tree = object_kind {
                             progress.info(format!(
                                 "The tree in pack named {} was written as {} due to modes 100664 and 100640 rewritten as 100644.",
                                 index_entry.oid, written_id
                             ));
                         } else {
                             return Err(Error::ObjectEncodeMismatch {
+                                source: err,
                                 kind: object_kind,
-                                actual: index_entry.oid,
-                                expected: written_id,
                             });
                         }
                     }

Diff for: gitoxide-core/src/pack/explode.rs

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

Diff for: gix-commitgraph/Cargo.toml

@@ -28,11 +28,8 @@ pub enum Error<E: std::error::Error + 'static> {
     Filename(String),
     #[error("commit {id} has invalid generation {generation}")]
     Generation { generation: u32, id: gix_hash::ObjectId },
-    #[error("checksum mismatch: expected {expected}, got {actual}")]
-    Mismatch {
-        actual: gix_hash::ObjectId,
-        expected: gix_hash::ObjectId,
-    },
+    #[error(transparent)]
+    Checksum(#[from] checksum::Error),
     #[error("{0}")]
     Processor(#[source] E),
     #[error("commit {id} has invalid root tree ID {root_tree_id}")]
@@ -42,6 +39,19 @@ pub enum Error<E: std::error::Error + 'static> {
     },
 }
 
+///
+pub mod checksum {
+    /// The error used in [`super::File::verify_checksum()`].
+    #[derive(thiserror::Error, Debug)]
+    #[allow(missing_docs)]
+    pub enum Error {
+        #[error("failed to hash commit graph file")]
+        Hasher(#[from] gix_hash::hasher::Error),
+        #[error(transparent)]
+        Verify(#[from] gix_hash::verify::Error),
+    }
+}
+
 /// The positive result of [`File::traverse()`] providing some statistical information.
 #[derive(Clone, Debug, Eq, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
@@ -73,8 +83,7 @@ impl File {
         E: std::error::Error + 'static,
         Processor: FnMut(&file::Commit<'a>) -> Result<(), E>,
     {
-        self.verify_checksum()
-            .map_err(|(actual, expected)| Error::Mismatch { actual, expected })?;
+        self.verify_checksum()?;
         verify_split_chain_filename_hash(&self.path, self.checksum()).map_err(Error::Filename)?;
 
         let null_id = self.object_hash().null_ref();
@@ -138,23 +147,18 @@ impl File {
     /// Assure the [`checksum`][File::checksum()] matches the actual checksum over all content of this file, excluding the trailing
     /// checksum itself.
     ///
-    /// Return the actual checksum on success or `(actual checksum, expected checksum)` if there is a mismatch.
-    pub fn verify_checksum(&self) -> Result<gix_hash::ObjectId, (gix_hash::ObjectId, gix_hash::ObjectId)> {
-        // Even though we could use gix_features::hash::bytes_of_file(…), this would require using our own
-        // Error type to support io::Error and Mismatch. As we only gain progress, there probably isn't much value
+    /// Return the actual checksum on success or [`checksum::Error`] if there is a mismatch.
+    pub fn verify_checksum(&self) -> Result<gix_hash::ObjectId, checksum::Error> {
+        // Even though we could use gix_hash::bytes_of_file(…), this would require extending our
+        // Error type to support io::Error. As we only gain progress, there probably isn't much value
         // as these files are usually small enough to process them in less than a second, even for the large ones.
         // But it's possible, once a progress instance is passed.
         let data_len_without_trailer = self.data.len() - self.hash_len;
-        let mut hasher = gix_features::hash::hasher(self.object_hash());
+        let mut hasher = gix_hash::hasher(self.object_hash());
         hasher.update(&self.data[..data_len_without_trailer]);
-        let actual = gix_hash::ObjectId::from_bytes_or_panic(hasher.digest().as_ref());
-
-        let expected = self.checksum();
-        if actual == expected {
-            Ok(actual)
-        } else {
-            Err((actual, expected.into()))
-        }
+        let actual = hasher.try_finalize()?;
+        actual.verify(self.checksum())?;
+        Ok(actual)
     }
 }
 

Diff for: gix-commitgraph/src/file/verify.rs

@@ -164,9 +164,7 @@ impl Graph {
                         file::verify::Error::RootTreeId { id, root_tree_id } => {
                             file::verify::Error::RootTreeId { id, root_tree_id }
                         }
-                        file::verify::Error::Mismatch { actual, expected } => {
-                            file::verify::Error::Mismatch { actual, expected }
-                        }
+                        file::verify::Error::Checksum(err) => file::verify::Error::Checksum(err),
                         file::verify::Error::Generation { generation, id } => {
                             file::verify::Error::Generation { generation, id }
                         }