From e2b5d9319867537eb58f536069769f993f7aa068 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 11 Apr 2024 13:39:13 -0400 Subject: [PATCH] more attestation coverage Signed-off-by: William Woodruff --- Library/Homebrew/attestation.rb | 16 ++- Library/Homebrew/software_spec.rb | 5 + Library/Homebrew/test/attestation_spec.rb | 124 +++++++++++++++++++--- 3 files changed, 128 insertions(+), 17 deletions(-) diff --git a/Library/Homebrew/attestation.rb b/Library/Homebrew/attestation.rb index 1e28eae035..1b4dee3dcb 100644 --- a/Library/Homebrew/attestation.rb +++ b/Library/Homebrew/attestation.rb @@ -78,14 +78,22 @@ module Homebrew end begin - data = JSON.parse(output) + attestations = JSON.parse(output) rescue JSON::ParserError raise InvalidAttestationError, "attestation verification returned malformed JSON" end - raise InvalidAttestationError, "attestation output is empty" if data.blank? + # `gh attestation verify` returns a JSON array of one or more results, + # for all attestations that match the input's digest. We want to additionally + # filter these down to just the attestation whose subject matches the bottle's name. + bottle_name = bottle.filename.to_s + attestation = attestations.find do |a| + a.dig("verificationResult", "statement", "subject", 0, "name") == bottle_name + end - data + raise InvalidAttestationError, "no attestation matches subject" if attestation.blank? + + attestation end # Verifies the given bottle against a cryptographic attestation of build provenance @@ -105,7 +113,7 @@ module Homebrew rescue InvalidAttestationError odebug "falling back on backfilled attestation for #{bottle}" backfill_attestation = check_attestation bottle, BACKFILL_REPO, BACKFILL_REPO_CI_URI - timestamp = backfill_attestation.dig(0, "verificationResult", "verifiedTimestamps", + timestamp = backfill_attestation.dig("verificationResult", "verifiedTimestamps", 0, "timestamp") raise InvalidAttestationError, "backfill attestation is missing verified timestamp" if timestamp.nil? diff --git a/Library/Homebrew/software_spec.rb b/Library/Homebrew/software_spec.rb index 42d2477bb4..fffd5116a4 100644 --- a/Library/Homebrew/software_spec.rb +++ b/Library/Homebrew/software_spec.rb @@ -423,6 +423,11 @@ class Bottle github_packages_manifest_resource_tab(github_packages_manifest_resource) end + sig { returns(Filename) } + def filename + Filename.create(resource.owner, @tag, @spec.rebuild) + end + private def github_packages_manifest_resource_tab(github_packages_manifest_resource) diff --git a/Library/Homebrew/test/attestation_spec.rb b/Library/Homebrew/test/attestation_spec.rb index 04e897e077..5ac6f29e2d 100644 --- a/Library/Homebrew/test/attestation_spec.rb +++ b/Library/Homebrew/test/attestation_spec.rb @@ -3,40 +3,138 @@ require "diagnostic" RSpec.describe Homebrew::Attestation do - subject(:attestation) { described_class } - let(:fake_gh) { Pathname.new("/extremely/fake/gh") } - let(:fake_json_resp) { JSON.dump({ foo: "bar" }) } let(:cached_download) { "/fake/cached/download" } - let(:fake_bottle) { instance_double(Bottle, cached_download:) } + let(:fake_bottle_filename) { instance_double(Bottle::Filename, to_s: "fakebottle--1.0.faketag.bottle.tar.gz") } + let(:fake_bottle) { instance_double(Bottle, cached_download:, filename: fake_bottle_filename) } + let(:fake_json_resp) do + JSON.dump([ + { verificationResult: { + verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }], + statement: { subject: [{ name: fake_bottle_filename.to_s }] }, + } }, + ]) + end + let(:fake_json_resp_too_new) do + JSON.dump([ + { verificationResult: { + verifiedTimestamps: [{ timestamp: "2024-03-15T00:00:00Z" }], + statement: { subject: [{ name: fake_bottle_filename.to_s }] }, + } }, + ]) + end + let(:fake_json_resp_wrong_sub) do + JSON.dump([ + { verificationResult: { + verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }], + statement: { subject: [{ name: "wrong-subject.tar.gz" }] }, + } }, + ]) + end describe "::gh_executable" do it "calls ensure_executable" do - expect(attestation).to receive(:ensure_executable!) + expect(described_class).to receive(:ensure_executable!) .with("gh") .and_return(fake_gh) - attestation.gh_executable + described_class.gh_executable + end + end + + describe "::check_attestation" do + before do + allow(described_class).to receive(:gh_executable) + .and_return(fake_gh) + end + + it "raises when gh subprocess fails" do + expect(Utils).to receive(:safe_popen_read) + .with(fake_gh, "attestation", "verify", cached_download, "--repo", + described_class::HOMEBREW_CORE_REPO, "--format", "json") + .and_raise(ErrorDuringExecution.new(["foo"], status: 1)) + + expect do + described_class.check_attestation fake_bottle, + described_class::HOMEBREW_CORE_REPO + end.to raise_error(described_class::InvalidAttestationError) + end + + it "raises when gh returns invalid JSON" do + expect(Utils).to receive(:safe_popen_read) + .with(fake_gh, "attestation", "verify", cached_download, "--repo", + described_class::HOMEBREW_CORE_REPO, "--format", "json") + .and_return("\"invalid json") + + expect do + described_class.check_attestation fake_bottle, + described_class::HOMEBREW_CORE_REPO + end.to raise_error(described_class::InvalidAttestationError) + end + + it "raises when gh returns other subjects" do + expect(Utils).to receive(:safe_popen_read) + .with(fake_gh, "attestation", "verify", cached_download, "--repo", + described_class::HOMEBREW_CORE_REPO, "--format", "json") + .and_return(fake_json_resp_wrong_sub) + + expect do + described_class.check_attestation fake_bottle, + described_class::HOMEBREW_CORE_REPO + end.to raise_error(described_class::InvalidAttestationError) end end describe "::check_core_attestation" do before do - allow(attestation).to receive(:gh_executable) + allow(described_class).to receive(:gh_executable) .and_return(fake_gh) - - allow(Utils).to receive(:safe_popen_read) - .and_return(fake_json_resp) end it "calls gh with args for homebrew-core" do expect(Utils).to receive(:safe_popen_read) .with(fake_gh, "attestation", "verify", cached_download, "--repo", - attestation::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity", - attestation::HOMEBREW_CORE_CI_URI) + described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity", + described_class::HOMEBREW_CORE_CI_URI) .and_return(fake_json_resp) - attestation.check_core_attestation fake_bottle + described_class.check_core_attestation fake_bottle + end + + it "calls gh with args for backfill when homebrew-core fails" do + expect(Utils).to receive(:safe_popen_read) + .with(fake_gh, "attestation", "verify", cached_download, "--repo", + described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity", + described_class::HOMEBREW_CORE_CI_URI) + .once + .and_raise(described_class::InvalidAttestationError) + + expect(Utils).to receive(:safe_popen_read) + .with(fake_gh, "attestation", "verify", cached_download, "--repo", + described_class::BACKFILL_REPO, "--format", "json", "--cert-identity", + described_class::BACKFILL_REPO_CI_URI) + .and_return(fake_json_resp) + + described_class.check_core_attestation fake_bottle + end + + it "raises when the backfilled attestation is too new" do + expect(Utils).to receive(:safe_popen_read) + .with(fake_gh, "attestation", "verify", cached_download, "--repo", + described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity", + described_class::HOMEBREW_CORE_CI_URI) + .once + .and_raise(described_class::InvalidAttestationError) + + expect(Utils).to receive(:safe_popen_read) + .with(fake_gh, "attestation", "verify", cached_download, "--repo", + described_class::BACKFILL_REPO, "--format", "json", "--cert-identity", + described_class::BACKFILL_REPO_CI_URI) + .and_return(fake_json_resp_too_new) + + expect do + described_class.check_core_attestation fake_bottle + end.to raise_error(described_class::InvalidAttestationError) end end end