refactor(tvix/nix-compat/nixhash): validate digest lengths

There was a NixHash::new() before, which didn't perform any validation
of the digest length. We had some length validation when parsing nix
hashes or SRI hashes, but some places didn't perform validation and/or
constructed the struct directly.

Replace NixHash::new() with a
`impl TryFrom<(HashAlgo, Vec<u8>)> for NixHash`,  which does do this
validation, and update constructing code to use that, rather than
populating structs directly. In some rare cases where we're sure the
digest length is correct we still populate the struct manually.

Fixes b/291.

Change-Id: I7a323c5b18d94de0ec15e391b3e7586df42f4229
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9109
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI

tvix/nix-compat/src/derivation/mod.rs

@@ -185,7 +185,13 @@ impl Derivation {
 
             hasher.finalize().to_vec()
         });
-        NixHash::new(crate::nixhash::HashAlgo::Sha256, digest.to_vec())
+
+        // We populate the struct directly, as we know the sha256 digest has the
+        // right size.
+        NixHash {
+            algo: crate::nixhash::HashAlgo::Sha256,
+            digest: digest.to_vec(),
+        }
     }
 
     /// This calculates all output paths of a Derivation and updates the struct.

tvix/nix-compat/src/derivation/tests/mod.rs

@@ -1,6 +1,5 @@
 use crate::derivation::output::Output;
 use crate::derivation::Derivation;
-use crate::nixhash::NixHash;
 use crate::store_path::StorePath;
 use bstr::{BStr, BString};
 use std::collections::{BTreeMap, BTreeSet};
@@ -238,15 +237,19 @@ fn output_path_construction() {
         "out".to_string(),
         Output {
             path: "".to_string(), // will be calculated
-            hash_with_mode: Some(crate::nixhash::NixHashWithMode::Recursive(NixHash {
+            hash_with_mode: Some(crate::nixhash::NixHashWithMode::Recursive(
-                digest: data_encoding::HEXLOWER
+                (
+                    crate::nixhash::HashAlgo::Sha256,
+                    data_encoding::HEXLOWER
                         .decode(
                             "08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba"
                                 .as_bytes(),
                         )
                         .unwrap(),
-                algo: crate::nixhash::HashAlgo::Sha256,
+                )
-            })),
+                    .try_into()
+                    .unwrap(),
+            )),
         },
     );
 

tvix/nix-compat/src/nixhash/mod.rs

@@ -1,6 +1,6 @@
 use crate::nixbase32;
 use data_encoding::{BASE64, BASE64_NOPAD, HEXLOWER};
-use thiserror::Error;
+use thiserror;
 
 mod algos;
 mod with_mode;
@@ -18,11 +18,6 @@ pub struct NixHash {
 }
 
 impl NixHash {
-    /// Constructs a new [NixHash] by specifying [HashAlgo] and digest.
-    pub fn new(algo: HashAlgo, digest: Vec<u8>) -> Self {
-        Self { algo, digest }
-    }
-
     /// Formats a [NixHash] in the Nix default hash format,
     /// which is the algo, followed by a colon, then the lower hex encoded digest.
     pub fn to_nix_hash_string(&self) -> String {
@@ -30,8 +25,23 @@ impl NixHash {
     }
 }
 
+impl TryFrom<(HashAlgo, Vec<u8>)> for NixHash {
+    type Error = Error;
+
+    /// Constructs a new [NixHash] by specifying [HashAlgo] and digest.
+    // It can fail if the passed digest length doesn't match what's expected for
+    // the passed algo.
+    fn try_from(value: (HashAlgo, Vec<u8>)) -> Result<Self, Self::Error> {
+        let (algo, digest) = value;
+        if digest.len() != hash_algo_length(&algo) {
+            return Err(Error::InvalidEncodedDigestLength(digest.len(), algo));
+        }
+        Ok(Self { algo, digest })
+    }
+}
+
 /// Errors related to NixHash construction.
-#[derive(Debug, Error)]
+#[derive(Debug, thiserror::Error)]
 pub enum Error {
     #[error("invalid hash algo: {0}")]
     InvalidAlgo(String),

tvix/nix-compat/src/nixhash/with_mode.rs

@@ -1,5 +1,5 @@
 use crate::nixbase32;
-use crate::nixhash::{HashAlgo, NixHash};
+use crate::nixhash::{self, HashAlgo, NixHash};
 use serde::de::Unexpected;
 use serde::ser::SerializeMap;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
@@ -102,7 +102,8 @@ impl NixHashWithMode {
         if let Some(v) = map.get("hashAlgo") {
             if let Some(s) = v.as_str() {
                 match s.strip_prefix("r:") {
-                    Some(rest) => Ok(Some(Self::Recursive(NixHash::new(
+                    Some(rest) => Ok(Some(Self::Recursive(
+                        (
                             HashAlgo::try_from(rest).map_err(|e| {
                                 serde::de::Error::invalid_value(
                                     Unexpected::Other(&e.to_string()),
@@ -110,8 +111,17 @@ impl NixHashWithMode {
                                 )
                             })?,
                             digest,
-                    )))),
+                        )
-                    None => Ok(Some(Self::Flat(NixHash::new(
+                            .try_into()
+                            .map_err(|e: nixhash::Error| {
+                                serde::de::Error::invalid_value(
+                                    Unexpected::Other(&e.to_string()),
+                                    &"a digest with right length",
+                                )
+                            })?,
+                    ))),
+                    None => Ok(Some(Self::Flat(
+                        (
                             HashAlgo::try_from(s).map_err(|e| {
                                 serde::de::Error::invalid_value(
                                     Unexpected::Other(&e.to_string()),
@@ -119,7 +129,15 @@ impl NixHashWithMode {
                                 )
                             })?,
                             digest,
-                    )))),
+                        )
+                            .try_into()
+                            .map_err(|e: nixhash::Error| {
+                                serde::de::Error::invalid_value(
+                                    Unexpected::Other(&e.to_string()),
+                                    &"a digest with right length",
+                                )
+                            })?,
+                    ))),
                 }
             } else {
                 Err(serde::de::Error::invalid_type(

tvix/nix-compat/src/store_path/utils.rs

@@ -54,7 +54,13 @@ pub fn build_text_path<S: AsRef<str>, I: IntoIterator<Item = S>, C: AsRef<[u8]>>
                 let hasher = Sha256::new_with_prefix(content);
                 hasher.finalize()
             };
-            NixHash::new(crate::nixhash::HashAlgo::Sha256, content_digest.to_vec())
+
+            // We populate the struct directly, as we know the sha256 digest has the
+            // right size.
+            NixHash {
+                algo: crate::nixhash::HashAlgo::Sha256,
+                digest: content_digest.to_vec(),
+            }
         },
         name,
     )
@@ -100,7 +106,13 @@ pub fn build_regular_ca_path<S: AsRef<str>, I: IntoIterator<Item = S>>(
                         hasher.update(":");
                         hasher.finalize()
                     };
-                    NixHash::new(crate::nixhash::HashAlgo::Sha256, content_digest.to_vec())
+
+                    // We don't use [NixHash::from_algo_and_digest], as we know [Sha256] has
+                    // the right digest size.
+                    NixHash {
+                        algo: crate::nixhash::HashAlgo::Sha256,
+                        digest: content_digest.to_vec(),
+                    }
                 },
                 name,
             )

tvix/store/src/store_io.rs

@@ -114,9 +114,12 @@ impl TvixStoreIO {
         )
         .expect("error during nar calculation"); // TODO: handle error
 
-        // For given NAR sha256 digest and name, return the new [StorePath] this would have.
+        // We populate the struct directly, as we know the sha256 digest has the
-        let nar_hash_with_mode =
+        // right size.
-            NixHashWithMode::Recursive(NixHash::new(HashAlgo::Sha256, nar_sha256.to_vec()));
+        let nar_hash_with_mode = NixHashWithMode::Recursive(NixHash {
+            algo: HashAlgo::Sha256,
+            digest: nar_sha256.to_vec(),
+        });
 
         let name = path
             .file_name()
@@ -172,8 +175,12 @@ impl TvixStoreIO {
 /// For given NAR sha256 digest and name, return the new [StorePath] this would have.
 #[instrument(skip(nar_sha256_digest), ret, fields(nar_sha256_digest=BASE64.encode(nar_sha256_digest)))]
 fn calculate_nar_based_store_path(nar_sha256_digest: &[u8; 32], name: &str) -> StorePath {
-    let nar_hash_with_mode =
+    // We populate the struct directly, as we know the sha256 digest has the
-        NixHashWithMode::Recursive(NixHash::new(HashAlgo::Sha256, nar_sha256_digest.to_vec()));
+    // right size.
+    let nar_hash_with_mode = NixHashWithMode::Recursive(NixHash {
+        algo: HashAlgo::Sha256,
+        digest: nar_sha256_digest.to_vec(),
+    });
 
     build_regular_ca_path(name, &nar_hash_with_mode, Vec::<String>::new(), false).unwrap()
 }