about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2023-10-05T07·59+0300
committerflokli <flokli@flokli.de>2023-10-05T09·52+0000
commit1f03a520a9d0d7d49de59f8a7b8dc314281642fa (patch)
tree895b4f0c132896b0abfa10945cdf49a38d7f4aa0
parent7706a8f224d6277b7c4f32ffaf869dc7f28e643d (diff)
feat(tvix/store): add validation for references r/6706
This validates the size of reference digests in the PathInfo message,
as well as inside the narinfo submessage. If narinfo is set, they need
to parse to StorePath, and have the same digest there as in the PathInfo
message.

`proto::tests::pathinfo::validate_references` needed to be updated,
because we actually did not populate the proper references before.

Change-Id: I9545b2487aab9fe0d229c26aceba5ddc5e6daafd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9545
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
-rw-r--r--tvix/store/src/proto/mod.rs62
-rw-r--r--tvix/store/src/proto/tests/pathinfo.rs4
2 files changed, 60 insertions, 6 deletions
diff --git a/tvix/store/src/proto/mod.rs b/tvix/store/src/proto/mod.rs
index 56a0225685..3bc4c2114e 100644
--- a/tvix/store/src/proto/mod.rs
+++ b/tvix/store/src/proto/mod.rs
@@ -1,4 +1,5 @@
 #![allow(clippy::derive_partial_eq_without_eq, non_snake_case)]
+use data_encoding::BASE64;
 // https://github.com/hyperium/tonic/issues/1056
 use nix_compat::store_path::{self, StorePath};
 use thiserror::Error;
@@ -22,6 +23,10 @@ mod tests;
 /// Errors that can occur during the validation of PathInfo messages.
 #[derive(Debug, Error, PartialEq)]
 pub enum ValidatePathInfoError {
+    /// Invalid length of a reference
+    #[error("Invalid length of digest at position {}, expected {}, got {}", .0, store_path::DIGEST_SIZE, .1)]
+    InvalidReferenceDigestLen(usize, usize),
+
     /// No node present
     #[error("No node present")]
     NoNodePresent(),
@@ -36,8 +41,21 @@ pub enum ValidatePathInfoError {
 
     /// The number of references in the narinfo.reference_names field does not match
     /// the number of references in the .references field.
-    #[error("Inconsistent Number of References: {0} (references) vs {0} (narinfo)")]
+    #[error("Inconsistent Number of References: {0} (references) vs {1} (narinfo)")]
     InconsistentNumberOfReferences(usize, usize),
+
+    /// A string in narinfo.reference_names does not parse to a StorePath.
+    #[error("Invalid reference_name at position {0}: {1}")]
+    InvalidNarinfoReferenceName(usize, String),
+
+    /// The digest in the parsed `.narinfo.reference_names[i]` does not match
+    /// the one in `.references[i]`.`
+    #[error("digest in reference_name at position {} does not match digest in PathInfo, expected {}, got {}", .0, BASE64.encode(.1), BASE64.encode(.2))]
+    InconsistentNarinfoReferenceNameDigest(
+        usize,
+        [u8; store_path::DIGEST_SIZE],
+        [u8; store_path::DIGEST_SIZE],
+    ),
 }
 
 /// Parses a root node name.
@@ -59,18 +77,54 @@ impl PathInfo {
     /// Returning either a [StorePath] of the root node, or a
     /// [ValidatePathInfoError].
     pub fn validate(&self) -> Result<StorePath, ValidatePathInfoError> {
+        // ensure the references have the right number of bytes.
+        for (i, reference) in self.references.iter().enumerate() {
+            if reference.len() != store_path::DIGEST_SIZE {
+                return Err(ValidatePathInfoError::InvalidReferenceDigestLen(
+                    i,
+                    reference.len(),
+                ));
+            }
+        }
+
         // If there is a narinfo field populated, ensure the number of references there
         // matches PathInfo.references count.
         if let Some(narinfo) = &self.narinfo {
             if narinfo.reference_names.len() != self.references.len() {
                 return Err(ValidatePathInfoError::InconsistentNumberOfReferences(
-                    narinfo.reference_names.len(),
                     self.references.len(),
+                    narinfo.reference_names.len(),
                 ));
             }
+
+            // parse references in reference_names.
+            for (i, reference_name_str) in narinfo.reference_names.iter().enumerate() {
+                // ensure thy parse as (non-absolute) store path
+                let reference_names_store_path =
+                    StorePath::from_bytes(reference_name_str.as_bytes()).map_err(|_| {
+                        ValidatePathInfoError::InvalidNarinfoReferenceName(
+                            i,
+                            reference_name_str.to_owned(),
+                        )
+                    })?;
+
+                // ensure their digest matches the one at self.references[i].
+                {
+                    // This is safe, because we ensured the proper length earlier already.
+                    let reference_digest = self.references[i].to_vec().try_into().unwrap();
+
+                    if reference_names_store_path.digest != reference_digest {
+                        return Err(
+                            ValidatePathInfoError::InconsistentNarinfoReferenceNameDigest(
+                                i,
+                                reference_digest,
+                                reference_names_store_path.digest,
+                            ),
+                        );
+                    }
+                }
+            }
         }
-        // FUTUREWORK: parse references in reference_names. ensure they start
-        // with storeDir, and use the same digest as in self.references.
 
         // Ensure there is a (root) node present, and it properly parses to a [StorePath].
         let root_nix_path = match &self.node {
diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs
index a74b7c3f7c..3a9ce2e7e3 100644
--- a/tvix/store/src/proto/tests/pathinfo.rs
+++ b/tvix/store/src/proto/tests/pathinfo.rs
@@ -161,7 +161,7 @@ fn validate_references() {
                 size: 0,
             })),
         }),
-        references: vec![DUMMY_DIGEST_2.clone().into()],
+        references: vec![DUMMY_OUTPUT_HASH.clone().into()],
         narinfo: None,
     };
     assert!(path_info.validate().is_ok());
@@ -190,7 +190,7 @@ fn validate_references() {
             nar_size: 0,
             nar_sha256: DUMMY_DIGEST.clone().into(),
             signatures: vec![],
-            reference_names: vec![format!("/nix/store/{}", DUMMY_NAME)],
+            reference_names: vec![DUMMY_NAME.to_string()],
         }),
         ..path_info
     };