From faa00c8c79f253337b8cbb00e8d548f80f1cacb2 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 11 Apr 2024 16:44:57 -0400 Subject: [PATCH] handle backfilled attestation subjects correctly Signed-off-by: William Woodruff --- Library/Homebrew/attestation.rb | 19 ++++++++++++++----- Library/Homebrew/test/attestation_spec.rb | 17 +++++++++++++++-- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/attestation.rb b/Library/Homebrew/attestation.rb index 1b4dee3dcb..63678dc1db 100644 --- a/Library/Homebrew/attestation.rb +++ b/Library/Homebrew/attestation.rb @@ -63,9 +63,9 @@ module Homebrew # @api private sig { params(bottle: Bottle, signing_repo: String, - signing_workflow: T.nilable(String)).returns(T::Hash[T.untyped, T.untyped]) + signing_workflow: T.nilable(String), subject: T.nilable(String)).returns(T::Hash[T.untyped, T.untyped]) } - def self.check_attestation(bottle, signing_repo, signing_workflow = nil) + def self.check_attestation(bottle, signing_repo, signing_workflow = nil, subject = nil) cmd = [gh_executable, "attestation", "verify", bottle.cached_download, "--repo", signing_repo, "--format", "json"] @@ -86,9 +86,9 @@ module Homebrew # `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 + subject = bottle.filename.to_s if subject.blank? attestation = attestations.find do |a| - a.dig("verificationResult", "statement", "subject", 0, "name") == bottle_name + a.dig("verificationResult", "statement", "subject", 0, "name") == subject end raise InvalidAttestationError, "no attestation matches subject" if attestation.blank? @@ -112,7 +112,16 @@ module Homebrew return attestation rescue InvalidAttestationError odebug "falling back on backfilled attestation for #{bottle}" - backfill_attestation = check_attestation bottle, BACKFILL_REPO, BACKFILL_REPO_CI_URI + + # Our backfilled attestation is a little unique: the subject is not just the bottle + # filename, but also has the bottle's hosted URL hash prepended to it. + # This was originally unintentional, but has a virtuous side effect of further + # limiting domain separation on the backfilled signatures (by committing them to + # their original bottle URLs). + url_sha256 = Digest::SHA256.hexdigest(bottle.url) + subject = "#{url_sha256}--#{bottle.filename}" + + backfill_attestation = check_attestation bottle, BACKFILL_REPO, BACKFILL_REPO_CI_URI, subject timestamp = backfill_attestation.dig("verificationResult", "verifiedTimestamps", 0, "timestamp") diff --git a/Library/Homebrew/test/attestation_spec.rb b/Library/Homebrew/test/attestation_spec.rb index 5ac6f29e2d..b04b0946a7 100644 --- a/Library/Homebrew/test/attestation_spec.rb +++ b/Library/Homebrew/test/attestation_spec.rb @@ -6,7 +6,10 @@ RSpec.describe Homebrew::Attestation do let(:fake_gh) { Pathname.new("/extremely/fake/gh") } let(:cached_download) { "/fake/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_bottle_url) { "https://example.com/#{fake_bottle_filename}" } + let(:fake_bottle) do + instance_double(Bottle, cached_download:, filename: fake_bottle_filename, url: fake_bottle_url) + end let(:fake_json_resp) do JSON.dump([ { verificationResult: { @@ -15,6 +18,16 @@ RSpec.describe Homebrew::Attestation do } }, ]) end + let(:fake_json_resp_backfill) do + JSON.dump([ + { verificationResult: { + verifiedTimestamps: [{ timestamp: "2024-03-13T00:00:00Z" }], + statement: { + subject: [{ name: "#{Digest::SHA256.hexdigest(fake_bottle_url)}--#{fake_bottle_filename}" }], + }, + } }, + ]) + end let(:fake_json_resp_too_new) do JSON.dump([ { verificationResult: { @@ -113,7 +126,7 @@ RSpec.describe Homebrew::Attestation do .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) + .and_return(fake_json_resp_backfill) described_class.check_core_attestation fake_bottle end