awslabs / tough

Rust libraries and tools for using and generating TUF repositories
191 stars 45 forks source link

tuftool transfer-metadata missing targets output files #673

Closed stmcginnis closed 10 months ago

stmcginnis commented 10 months ago

The new transfer-metadata command successfully generates the targets.json file containing the expected target information. Part of this optimization is exactly for this reason, so there isn't a need to download a large number of files when all that is needed is the metadata.

With the previous download and create workflow that this replaces, it was convenient to be able to validate the locally created repo by doing a download of a repo file with the new root.json to validate it works. But without any target files, there is nothing there to use to validate.

The manifest.json file is a relatively small file that can provide a quick and easy validation. We should consider if we want to include that as perhaps the only target file that gets downloaded when transferring metadata to aid in validation.

Alternatively, and since there are less moving parts with the metadata transfer, it could just be a known thing that when creating a transferred repo there will need to be some other sort of validation performed.

Getting the metadata.json file into a targets/ subdirectory with the expected prepended sha as part of its name can be done manually to verify the new repo is functioning correctly.

stmcginnis commented 10 months ago

As a quick testing hack, locally changed things to download manifest.json to then later use it as a --target-file to validate the local repo.

diff --git a/tuftool/src/transfer_metadata.rs b/tuftool/src/transfer_metadata.rs
index cee4683..e3b5f8f 100644
--- a/tuftool/src/transfer_metadata.rs
+++ b/tuftool/src/transfer_metadata.rs
@@ -10,9 +10,10 @@ use snafu::ResultExt;
 use std::fs::File;
 use std::num::NonZeroU64;
 use std::path::{Path, PathBuf};
+use tough::editor::signed::PathExists;
 use tough::editor::RepositoryEditor;
 use tough::key_source::KeySource;
-use tough::{ExpirationEnforcement, RepositoryLoader};
+use tough::{ExpirationEnforcement, Prefix, RepositoryLoader, TargetName};
 use url::Url;

 #[derive(Debug, Parser)]
@@ -117,16 +118,42 @@ impl TransferMetadataArgs {
             .timestamp_version(self.timestamp_version)
             .timestamp_expires(self.timestamp_expires);

+        println!("Creating output directory {:?}", self.outdir);
+        std::fs::create_dir_all(&self.outdir)
+            .context(error::DirCreateSnafu { path: &self.outdir })?;
+
+        let target_tmpdir = &self.outdir.join("tmp");
+        std::fs::create_dir_all(&target_tmpdir).context(error::DirCreateSnafu {
+            path: &target_tmpdir,
+        })?;
+        let download_target = |name: &TargetName| -> Result<()> {
+            println!("\t-> {}", name.raw());
+            current_repo
+                .save_target(name, &target_tmpdir, Prefix::None)
+                .context(error::MetadataSnafu)?;
+            Ok(())
+        };
+
         let targets = current_repo.targets();
         for (target_name, target) in &targets.signed.targets {
             editor
                 .add_target(target_name.clone(), target.clone())
                 .context(error::DelegationStructureSnafu)?;
+            if target_name.raw() == "manifest.json" {
+                download_target(target_name)?;
+            }
         }

         let signed_repo = editor.sign(&self.keys).context(error::SignRepoSnafu)?;

         let metadata_dir = &self.outdir.join("metadata");
+        let targets_outdir = &self.outdir.join("targets");
+        signed_repo
+            .link_targets(&target_tmpdir, targets_outdir, PathExists::Skip)
+            .context(error::LinkTargetsSnafu {
+                indir: &target_tmpdir,
+                outdir: targets_outdir,
+            })?;
         signed_repo
             .write(metadata_dir)
             .context(error::WriteRepoSnafu {
bcressey commented 10 months ago

manifest.json is not specified by the TUF spec; it's a Bottlerocket-specific target and shouldn't be baked in here.

For Bottlerocket there are typically two URLs - metadata and target - and for "will it download?" validation you should be able to use metadata via file:// and targets via https://.

I wouldn't really want that built into the rotate command; it can be a separate CLI invocation, since the choice of target to download is fundamentally arbitrary.

stmcginnis commented 10 months ago

Was able to perform new repo verification by using the suggestion:

      --metadata-url "file://${PWD}/upload/${path}/metadata/" \
      --targets-url https://updates.example.com/targets/ \

That makes sense to validate the existing targets with the new metadata. So nothing to do here.