From 7a84a8fe89e244bf667acc23a6a639fdf977a4e4 Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Sat, 30 Dec 2023 01:59:39 +0100
Subject: fix(nix-compat/nix-hash): relax padding requirements

Nix is quite tolerant when it comes to parsing SRI hashes and their
padding (and only for SRI hashes, it funnily is strict about that in the
non-SRI-hash case).

Nix essentially accepts any number of padding characters, no matter if
it's too much or too little. So we do the only sane thing - simply strip
all padding characters, and parse it with BASE64_NOPAD and the length
the algo uses.

Change-Id: I6a721aa289b06cc36741589792b9dd4c4f930b86
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10468
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
---
 tvix/nix-compat/src/nixhash/mod.rs | 51 ++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 22 deletions(-)

(limited to 'tvix/nix-compat/src/nixhash/mod.rs')

diff --git a/tvix/nix-compat/src/nixhash/mod.rs b/tvix/nix-compat/src/nixhash/mod.rs
index aa1ce3bbd341..91f47ecc6320 100644
--- a/tvix/nix-compat/src/nixhash/mod.rs
+++ b/tvix/nix-compat/src/nixhash/mod.rs
@@ -215,7 +215,9 @@ pub fn from_nix_str(s: &str) -> Result<NixHash> {
 /// Contrary to the SRI spec, Nix doesn't support SRI strings with multiple hashes,
 /// only supports sha256 and sha512 from the spec, and supports sha1 and md5
 /// additionally.
-/// It also accepts SRI strings where the base64 has an with invalid padding.
+/// It's also dealing with padding in a very funny way - accepting SRI strings
+/// with an arbitrary amount of padding at the end - including *more* padding
+/// characters.
 pub fn from_sri_str(s: &str) -> Result<NixHash> {
     // try to find the first occurence of "-"
     let idx = s.as_bytes().iter().position(|&e| e == b'-');
@@ -232,27 +234,17 @@ pub fn from_sri_str(s: &str) -> Result<NixHash> {
     // the rest should be the digest (as Nix doesn't support more than one hash in an SRI string).
     let encoded_digest = &s[idx + 1..];
 
-    // decode the digest and algo into a [NixHash]
-    match decode_digest(encoded_digest.as_bytes(), algo) {
-        // If decoding was successful, pass along
-        Ok(nixhash) => Ok(nixhash),
-        // For SRI hashes (only), BASE64_NOPAD is also tolerated,
-        // so try to parse for this, too.
-        // NOTE: As of now, we reject SRI hashes containing additional
-        // characters (which upstream Nix seems to simply truncate), as
-        // there's no occurence of this is in nixpkgs.
-        // It most likely should also be a bug in Nix.
-        Err(Error::InvalidEncodedDigestLength(digest_len, hash_algo)) => {
-            if encoded_digest.len() == BASE64_NOPAD.encode_len(algo.digest_length()) {
-                let digest = BASE64_NOPAD
-                    .decode(encoded_digest.as_bytes())
-                    .map_err(Error::InvalidBase64Encoding)?;
-                Ok(from_algo_and_digest(algo, &digest).unwrap())
-            } else {
-                Err(Error::InvalidEncodedDigestLength(digest_len, hash_algo))?
-            }
-        }
-        Err(e) => Err(e)?,
+    // strip all padding characters.
+    let encoded_digest = encoded_digest.trim_end_matches('=');
+
+    if encoded_digest.len() == BASE64_NOPAD.encode_len(algo.digest_length()) {
+        let digest = BASE64_NOPAD
+            .decode(encoded_digest.as_bytes())
+            .map_err(Error::InvalidBase64Encoding)?;
+
+        Ok(from_algo_and_digest(algo, &digest).unwrap())
+    } else {
+        Err(Error::InvalidEncodedDigestLength(idx, algo))?
     }
 }
 
@@ -429,6 +421,21 @@ mod tests {
         )
     }
 
+    /// Test parsing sha512 SRI hash with various paddings, Nix accepts all of them.
+    #[test_case("sha512-7g91TBvYoYQorRTqo+rYD/i5YnWvUBLnqDhPHxBJDaBW7smuPMeRp6E6JOFuVN9bzN0QnH1ToUU0u9c2CjALEQ"; "no padding")]
+    #[test_case("sha512-7g91TBvYoYQorRTqo+rYD/i5YnWvUBLnqDhPHxBJDaBW7smuPMeRp6E6JOFuVN9bzN0QnH1ToUU0u9c2CjALEQ="; "wrong padding")]
+    #[test_case("sha512-7g91TBvYoYQorRTqo+rYD/i5YnWvUBLnqDhPHxBJDaBW7smuPMeRp6E6JOFuVN9bzN0QnH1ToUU0u9c2CjALEQ="; "correct padding")]
+    #[test_case("sha512-7g91TBvYoYQorRTqo+rYD/i5YnWvUBLnqDhPHxBJDaBW7smuPMeRp6E6JOFuVN9bzN0QnH1ToUU0u9c2CjALEQ="; "too much padding")]
+    fn from_sri_str_sha512_paddings(sri_str: &str) {
+        let nix_hash = nixhash::from_sri_str(sri_str).expect("must succeed");
+
+        assert_eq!(HashAlgo::Sha512, nix_hash.algo());
+        assert_eq!(
+            &hex!("ee0f754c1bd8a18428ad14eaa3ead80ff8b96275af5012e7a8384f1f10490da056eec9ae3cc791a7a13a24e16e54df5bccdd109c7d53a14534bbd7360a300b11"),
+            nix_hash.digest_as_bytes()
+        )
+    }
+
     /// Ensure we detect truncated base64 digests, where the digest size
     /// doesn't match what's expected from that hash function.
     #[test]
-- 
cgit 1.4.1