Ignore unsigned files (only read and hash files contained in signature manifest) (#501)

* Add support for ignoring files that are not part of signature/manifest

Add support for command line option --ignore-unsigned-files that allows to
ignore files that are not part of the signature (= not listed in the
manifest). This allows to ignore files that for example were added after a
signature was created and those files' presence would make the signature
verification fail. Another use case for this is the presence of multiple
signatures in the same directory where it is necessary to ignore the
files that are not covered by each signature.

Add support for this option for all verification methods. If this
option is set, then create a list of files_to_hash that is derived from
the list of files in the signature manifest.

Signed-off-by: Stefan Berger <[email protected]>

* tests: Add test cases for ignoring unsigned files

Add test cases covering the cases of a symlink and an additional file
created after a signature was created. Test expected failures and
passes with and without the new option --ignore_unsigned_files.

Signed-off-by: Stefan Berger <[email protected]>

---------

Signed-off-by: Stefan Berger <[email protected]>

Author: stefanberger

CHANGELOG.md

@@ -25,6 +25,7 @@ All versions prior to 1.0.0 are untracked.
 - Add warning for older verification material formats (e.g., raw public key bytes) during verification, recommending re-signing
 - Added guidance to `README.md` on how to install `model-signing` with PKCS#11 support.
 - Added support trace sigstore sign and verify operations using OpenTelemetry.
+- cli: Added support for `--ignore_unsigned_files` option
 
 ## [1.0.1] - 2024-04-18
 

scripts/tests/test-sign-verify

@@ -159,6 +159,40 @@ if [ "${res}" != "${exp}" ]; then
 fi
 check_model_name "${sigfile}" "$(basename "${TMPDIR}")"
 
+echo
+echo "Creating a symlink, that is not part of the signature, to make signature verification fail (1)"
+
+echo "foo" > symlinked
+ln -s symlinked symlink
+
+if python -m model_signing \
+	verify certificate \
+	--signature "$(basename "${sigfile}")" \
+	--certificate_chain "${DIR}/keys/certificate/ca-cert.pem" \
+	--ignore-paths "$(basename "${ignorefile}")" \
+	. ; then
+	echo "Error: 'verify certificate' succeeded after new file (symlink) was created"
+	exit 1
+fi
+
+# This should pass without having to pass --allow_symlinks since the symlink
+# will be ignored
+echo
+echo "Pass signature verification by ignoring any unsigned files or symlinks"
+if ! python -m model_signing \
+	verify certificate \
+	--signature "$(basename "${sigfile}")" \
+	--certificate_chain "${DIR}/keys/certificate/ca-cert.pem" \
+	--ignore-paths "$(basename "${ignorefile}")" \
+	--ignore_unsigned_files \
+	. ; then
+	echo "Error: 'verify certificate' failed with --ignore_unsigned_files while passing --allow_symlinks"
+	exit 1
+fi
+
+
+rm -f symlinked symlink
+
 echo
 echo "Testing 'sign/verify' certificate when in subdir of model directory and using '..'"
 
@@ -208,4 +242,71 @@ if [ "${res}" != "${exp}" ]; then
 fi
 check_model_name "${sigfile}" "$(basename "${TMPDIR}")"
 
+echo
+echo "Creating a symlink, that is not part of the signature, to make signature verification fail (2)"
+
+# Create another symlinked file
+ln -s subdir/symlinked symlink2
+
+echo
+echo "Fail signature verification by NOT ignoring any unsigned (symlinks)"
+if python -m model_signing \
+	verify certificate \
+	--signature "$(basename "${sigfile}")" \
+	--certificate_chain "${DIR}/keys/certificate/ca-cert.pem" \
+	--ignore-paths "$(basename "${ignorefile}")" \
+	--allow_symlinks \
+	. ; then
+	echo "Error: 'verify certificate' succeeded after new file (symlink) was created"
+	exit 1
+fi
+
+echo
+echo "Pass signature verification by ignoring any unsigned files"
+if ! python -m model_signing \
+	verify certificate \
+	--signature "$(basename "${sigfile}")" \
+	--certificate_chain "${DIR}/keys/certificate/ca-cert.pem" \
+	--ignore-paths "$(basename "${ignorefile}")" \
+	--allow_symlinks \
+	--ignore_unsigned_files \
+	. ; then
+	echo "Error: 'verify certificate' failed with --ignore_unsigned_files to ignore symlink"
+	exit 1
+fi
+
+rm symlink2
+
+# Create a simple new file
+echo
+echo "Creating a regular file that is not part of the signature"
+touch newfile
+
+echo
+echo "Fail signature verification by NOT ignoring any unsigned files (symlinks)"
+if python -m model_signing \
+	verify certificate \
+	--signature "$(basename "${sigfile}")" \
+	--certificate_chain "${DIR}/keys/certificate/ca-cert.pem" \
+	--ignore-paths "$(basename "${ignorefile}")" \
+	--allow_symlinks \
+	. ; then
+	echo "Error: 'verify certificate' succeeded after new file was created"
+	exit 1
+fi
+
+echo
+echo "Pass signature verification by ignoring any unsigned files"
+if ! python -m model_signing \
+	verify certificate \
+	--signature "$(basename "${sigfile}")" \
+	--certificate_chain "${DIR}/keys/certificate/ca-cert.pem" \
+	--ignore-paths "$(basename "${ignorefile}")" \
+	--allow_symlinks \
+	--ignore_unsigned_files \
+	. ; then
+	echo "Error: 'verify certificate' failed with --ignore_unsigned_files to ignore regular file"
+	exit 1
+fi
+
 popd 1>/dev/null || exit 1

src/model_signing/_cli.py

@@ -86,6 +86,14 @@ def set_attribute(self, key, value):
     help="Ignore git-related files when signing or verifying.",
 )
 
+# Decorator for the commonly used option to ignore all unsigned files
+_ignore_unsigned_files_option = click.option(
+    "--ignore_unsigned_files/--no-ignore_unsigned_files",
+    type=bool,
+    show_default=True,
+    help="Ignore all files that were not originally signed.",
+)
+
 # Decorator for the commonly used option to set the path to the private key
 # (when using non-Sigstore PKI).
 _private_key_option = click.option(
@@ -598,6 +606,7 @@ def _verify() -> None:
     required=True,
     help="The expected identity provider (e.g., https://accounts.example.com).",
 )
+@_ignore_unsigned_files_option
 def _verify_sigstore(
     model_path: pathlib.Path,
     signature: pathlib.Path,
@@ -607,6 +616,7 @@ def _verify_sigstore(
     identity: str,
     identity_provider: str,
     use_staging: bool,
+    ignore_unsigned_files: bool,
 ) -> None:
     """Verify using Sigstore (DEFAULT verification method).
 
@@ -637,7 +647,9 @@ def _verify_sigstore(
                     ignore_git_paths=ignore_git_paths,
                 )
                 .set_allow_symlinks(allow_symlinks)
-            ).verify(model_path, signature)
+            ).set_ignore_unsigned_files(ignore_unsigned_files).verify(
+                model_path, signature
+            )
         except Exception as err:
             click.echo(f"Verification failed with error: {err}", err=True)
             sys.exit(1)
@@ -658,13 +670,15 @@ def _verify_sigstore(
     required=True,
     help="Path to the public key used for verification.",
 )
+@_ignore_unsigned_files_option
 def _verify_private_key(
     model_path: pathlib.Path,
     signature: pathlib.Path,
     ignore_paths: Iterable[pathlib.Path],
     ignore_git_paths: bool,
     allow_symlinks: bool,
     public_key: pathlib.Path,
+    ignore_unsigned_files: bool,
 ) -> None:
     """Verity using a public key (paired with a private one).
 
@@ -689,7 +703,9 @@ def _verify_private_key(
                 ignore_git_paths=ignore_git_paths,
             )
             .set_allow_symlinks(allow_symlinks)
-        ).verify(model_path, signature)
+        ).set_ignore_unsigned_files(ignore_unsigned_files).verify(
+            model_path, signature
+        )
     except Exception as err:
         click.echo(f"Verification failed with error: {err}", err=True)
         sys.exit(1)
@@ -712,6 +728,7 @@ def _verify_private_key(
     show_default=True,
     help="Log SHA256 fingerprints of all certificates.",
 )
+@_ignore_unsigned_files_option
 def _verify_certificate(
     model_path: pathlib.Path,
     signature: pathlib.Path,
@@ -720,6 +737,7 @@ def _verify_certificate(
     allow_symlinks: bool,
     certificate_chain: Iterable[pathlib.Path],
     log_fingerprints: bool,
+    ignore_unsigned_files: bool,
 ) -> None:
     """Verify using a certificate.
 
@@ -748,7 +766,9 @@ def _verify_certificate(
                 ignore_git_paths=ignore_git_paths,
             )
             .set_allow_symlinks(allow_symlinks)
-        ).verify(model_path, signature)
+        ).set_ignore_unsigned_files(ignore_unsigned_files).verify(
+            model_path, signature
+        )
     except Exception as err:
         click.echo(f"Verification failed with error: {err}", err=True)
         sys.exit(1)

src/model_signing/_serialization/file.py

@@ -76,6 +76,7 @@ def serialize(
         model_path: pathlib.Path,
         *,
         ignore_paths: Iterable[pathlib.Path] = frozenset(),
+        files_to_hash: Optional[Iterable[pathlib.Path]] = None,
     ) -> manifest.Manifest:
         """Serializes the model given by the `model_path` argument.
 
@@ -84,6 +85,8 @@ def serialize(
             ignore_paths: The paths to ignore during serialization. If a
               provided path is a directory, all children of the directory are
               ignored.
+            files_to_hash: Optional list of files that are to be hashed;
+              ignore all others
 
         Returns:
             The model's serialized manifest.
@@ -97,7 +100,11 @@ def serialize(
         # Python3.12 is the minimum supported version, the glob can be replaced
         # with `pathlib.Path.walk` for a clearer interface, and some speed
         # improvement.
-        for path in itertools.chain((model_path,), model_path.glob("**/*")):
+        if files_to_hash is None:
+            files_to_hash = itertools.chain(
+                (model_path,), model_path.glob("**/*")
+            )
+        for path in files_to_hash:
             serialization.check_file_or_directory(
                 path, allow_symlinks=self._allow_symlinks
             )

src/model_signing/_serialization/file_shard.py

@@ -110,6 +110,7 @@ def serialize(
         model_path: pathlib.Path,
         *,
         ignore_paths: Iterable[pathlib.Path] = frozenset(),
+        files_to_hash: Optional[Iterable[pathlib.Path]] = None,
     ) -> manifest.Manifest:
         """Serializes the model given by the `model_path` argument.
 
@@ -118,6 +119,8 @@ def serialize(
             ignore_paths: The paths to ignore during serialization. If a
               provided path is a directory, all children of the directory are
               ignored.
+            files_to_hash: Opitonal list of files that are to be hashed;
+              ignore all others
 
         Returns:
             The model's serialized manifest.
@@ -131,7 +134,11 @@ def serialize(
         # Python3.12 is the minimum supported version, the glob can be replaced
         # with `pathlib.Path.walk` for a clearer interface, and some speed
         # improvement.
-        for path in itertools.chain((model_path,), model_path.glob("**/*")):
+        if files_to_hash is None:
+            files_to_hash = itertools.chain(
+                (model_path,), model_path.glob("**/*")
+            )
+        for path in files_to_hash:
             serialization.check_file_or_directory(
                 path, allow_symlinks=self._allow_symlinks
             )

src/model_signing/hashing.py

@@ -141,7 +141,12 @@ def __init__(self):
         self.use_file_serialization()
         self._allow_symlinks = False
 
-    def hash(self, model_path: PathLike) -> manifest.Manifest:
+    def hash(
+        self,
+        model_path: PathLike,
+        *,
+        files_to_hash: Optional[Iterable[PathLike]] = None,
+    ) -> manifest.Manifest:
         """Hashes a model using the current configuration."""
         # All paths in ignored_paths must have model_path as prefix
         ignored_paths = []
@@ -167,7 +172,9 @@ def hash(self, model_path: PathLike) -> manifest.Manifest:
         self._serializer.set_allow_symlinks(self._allow_symlinks)
 
         return self._serializer.serialize(
-            pathlib.Path(model_path), ignore_paths=ignored_paths
+            pathlib.Path(model_path),
+            ignore_paths=ignored_paths,
+            files_to_hash=files_to_hash,
         )
 
     def _build_stream_hasher(

src/model_signing/verifying.py

@@ -73,6 +73,7 @@ def __init__(self):
         self._hashing_config = None
         self._verifier = None
         self._uses_sigstore = False
+        self._ignore_unsigned_files = False
 
     def verify(
         self, model_path: hashing.PathLike, signature_path: hashing.PathLike
@@ -102,7 +103,18 @@ def verify(
                 model_path=model_path,
                 paths=expected_manifest.serialization_type["ignore_paths"],
             )
-        actual_manifest = self._hashing_config.hash(model_path)
+
+        if self._ignore_unsigned_files:
+            files_to_hash = [
+                model_path / rd.identifier
+                for rd in expected_manifest.resource_descriptors()
+            ]
+        else:
+            files_to_hash = None
+
+        actual_manifest = self._hashing_config.hash(
+            model_path, files_to_hash=files_to_hash
+        )
 
         if actual_manifest != expected_manifest:
             diff_message = self._get_manifest_diff(
@@ -164,6 +176,18 @@ def set_hashing_config(self, hashing_config: hashing.Config) -> Self:
         self._hashing_config = hashing_config
         return self
 
+    def set_ignore_unsigned_files(self, ignore_unsigned_files: bool) -> Self:
+        """Sets whether files that were not signed are to be ignored.
+
+        This method allows to ignore those files that are not part of the
+        manifest and therefor were not originally signed.
+
+        Args:
+            ignore_unsigned_files: whether to ignore unsigned files
+        """
+        self._ignore_unsigned_files = ignore_unsigned_files
+        return self
+
     def _guess_hashing_config(self, source_manifest: manifest.Manifest) -> None:
         """Attempts to guess the hashing config from a manifest."""
         args = source_manifest.serialization_type