From 18a8b12f7afe69903d35f5d5d55e1e6c2c00d816 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 3 May 2024 12:37:01 -0400 Subject: [PATCH 1/3] attestations: improve authentication techniques Signed-off-by: William Woodruff --- Library/Homebrew/attestation.rb | 19 ++++++++++++++++++- Library/Homebrew/formula_installer.rb | 10 ++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/attestation.rb b/Library/Homebrew/attestation.rb index 2dc57ad57a..45412b82a8 100644 --- a/Library/Homebrew/attestation.rb +++ b/Library/Homebrew/attestation.rb @@ -33,6 +33,12 @@ module Homebrew # @api private class InvalidAttestationError < RuntimeError; end + # Raised if attestation verification cannot continue due to missing + # credentials. + # + # @api private + class GhAuthNeeded < RuntimeError; end + # Returns a path to a suitable `gh` executable for attestation verification. # # @api private @@ -56,6 +62,7 @@ module Homebrew # `https://github/OWNER/REPO/.github/workflows/WORKFLOW.yml@REF` format. # # @return [Hash] the JSON-decoded response. + # @raise [GhAuthNeeded] on any authentication failures # @raise [InvalidAttestationError] on any verification failures # # @api private @@ -69,9 +76,18 @@ module Homebrew cmd += ["--cert-identity", signing_workflow] if signing_workflow.present? + # Fail early if we have no credentials. The command below invariably + # fails without them, so this saves us a network roundtrip before + # presenting the user with the same error. + credentials = GitHub::API.credentials + raise GhAuthNeeded, "missing credentials" if credentials.blank? + begin - output = Utils.safe_popen_read(*cmd) + output = Utils.safe_popen_read({ "GH_TOKEN" => credentials }, *cmd) rescue ErrorDuringExecution => e + # Even if we have credentials, they may be invalid or malformed. + raise GhAuthNeeded, "invalid credentials" if e.status.exitstatus == 4 + raise InvalidAttestationError, "attestation verification failed: #{e}" end @@ -100,6 +116,7 @@ module Homebrew # This is a specialization of `check_attestation` for homebrew-core. # # @return [Hash] the JSON-decoded response + # @raise [GhAuthNeeded] on any authentication failures # @raise [InvalidAttestationError] on any verification failures # # @api private diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index e038dd8f50..d41673003e 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -1261,6 +1261,16 @@ on_request: installed_on_request?, options:) ohai "Verifying attestation for #{formula.name}" begin Homebrew::Attestation.check_core_attestation formula.bottle + rescue Homebrew::Attestation::GhAuthNeeded + raise CannotInstallFormulaError, <<~EOS + The bottle for #{formula.name} could not be verified. + + This typically indicates a missing GitHub API token, which you + can resolve either by setting `HOMEBREW_GITHUB_API_TOKEN` or + by running: + + gh auth login + EOS rescue Homebrew::Attestation::InvalidAttestationError => e raise CannotInstallFormulaError, <<~EOS The bottle for #{formula.name} has an invalid build provenance attestation. From 2aa3d77f72e308aa6659a99eaa541014b02065b0 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 3 May 2024 13:01:02 -0400 Subject: [PATCH 2/3] attestation_spec: fixup Signed-off-by: William Woodruff --- Library/Homebrew/test/attestation_spec.rb | 42 ++++++++++++++++++----- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/test/attestation_spec.rb b/Library/Homebrew/test/attestation_spec.rb index e24664bd8d..d7d8e6e9cd 100644 --- a/Library/Homebrew/test/attestation_spec.rb +++ b/Library/Homebrew/test/attestation_spec.rb @@ -4,6 +4,8 @@ require "diagnostic" RSpec.describe Homebrew::Attestation do let(:fake_gh) { Pathname.new("/extremely/fake/gh") } + let(:fake_gh_creds) { "fake-gh-api-token" } + let(:fake_error_status) { instance_double(Process::Status, exitstatus: 1, termsig: nil) } 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_url) { "https://example.com/#{fake_bottle_filename}" } @@ -61,11 +63,24 @@ RSpec.describe Homebrew::Attestation do .and_return(fake_gh) end + it "raises without any gh credentials" do + expect(GitHub::API).to receive(:credentials) + .and_return(nil) + + expect do + described_class.check_attestation fake_bottle, + described_class::HOMEBREW_CORE_REPO + end.to raise_error(described_class::GhAuthNeeded) + end + it "raises when gh subprocess fails" do + expect(GitHub::API).to receive(:credentials) + .and_return(fake_gh_creds) + expect(Utils).to receive(:safe_popen_read) - .with(fake_gh, "attestation", "verify", cached_download, "--repo", + .with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", described_class::HOMEBREW_CORE_REPO, "--format", "json") - .and_raise(ErrorDuringExecution.new(["foo"], status: 1)) + .and_raise(ErrorDuringExecution.new(["foo"], status: fake_error_status)) expect do described_class.check_attestation fake_bottle, @@ -74,8 +89,11 @@ RSpec.describe Homebrew::Attestation do end it "raises when gh returns invalid JSON" do + expect(GitHub::API).to receive(:credentials) + .and_return(fake_gh_creds) + expect(Utils).to receive(:safe_popen_read) - .with(fake_gh, "attestation", "verify", cached_download, "--repo", + .with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", described_class::HOMEBREW_CORE_REPO, "--format", "json") .and_return("\"invalid json") @@ -86,8 +104,11 @@ RSpec.describe Homebrew::Attestation do end it "raises when gh returns other subjects" do + expect(GitHub::API).to receive(:credentials) + .and_return(fake_gh_creds) + expect(Utils).to receive(:safe_popen_read) - .with(fake_gh, "attestation", "verify", cached_download, "--repo", + .with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", described_class::HOMEBREW_CORE_REPO, "--format", "json") .and_return(fake_json_resp_wrong_sub) @@ -102,11 +123,14 @@ RSpec.describe Homebrew::Attestation do before do allow(described_class).to receive(:gh_executable) .and_return(fake_gh) + + allow(GitHub::API).to receive(:credentials) + .and_return(fake_gh_creds) 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", + .with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", described_class::HOMEBREW_CORE_REPO, "--format", "json", "--cert-identity", described_class::HOMEBREW_CORE_CI_URI) .and_return(fake_json_resp) @@ -116,14 +140,14 @@ RSpec.describe Homebrew::Attestation do 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", + .with({ "GH_TOKEN" => fake_gh_creds }, 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", + .with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", described_class::BACKFILL_REPO, "--format", "json") .and_return(fake_json_resp_backfill) @@ -132,14 +156,14 @@ RSpec.describe Homebrew::Attestation do 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", + .with({ "GH_TOKEN" => fake_gh_creds }, 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", + .with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", described_class::BACKFILL_REPO, "--format", "json") .and_return(fake_json_resp_too_new) From 7e43e5aafe9b945b16b0bfef0dbd5d6e85ea7e47 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 3 May 2024 13:17:31 -0400 Subject: [PATCH 3/3] attestation_spec: another auth case Signed-off-by: William Woodruff --- Library/Homebrew/test/attestation_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Library/Homebrew/test/attestation_spec.rb b/Library/Homebrew/test/attestation_spec.rb index d7d8e6e9cd..c9af5ad182 100644 --- a/Library/Homebrew/test/attestation_spec.rb +++ b/Library/Homebrew/test/attestation_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Homebrew::Attestation do let(:fake_gh) { Pathname.new("/extremely/fake/gh") } let(:fake_gh_creds) { "fake-gh-api-token" } let(:fake_error_status) { instance_double(Process::Status, exitstatus: 1, termsig: nil) } + let(:fake_auth_status) { instance_double(Process::Status, exitstatus: 4, termsig: nil) } 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_url) { "https://example.com/#{fake_bottle_filename}" } @@ -88,6 +89,21 @@ RSpec.describe Homebrew::Attestation do end.to raise_error(described_class::InvalidAttestationError) end + it "raises auth error when gh subprocess fails with auth exit code" do + expect(GitHub::API).to receive(:credentials) + .and_return(fake_gh_creds) + + expect(Utils).to receive(:safe_popen_read) + .with({ "GH_TOKEN" => fake_gh_creds }, fake_gh, "attestation", "verify", cached_download, "--repo", + described_class::HOMEBREW_CORE_REPO, "--format", "json") + .and_raise(ErrorDuringExecution.new(["foo"], status: fake_auth_status)) + + expect do + described_class.check_attestation fake_bottle, + described_class::HOMEBREW_CORE_REPO + end.to raise_error(described_class::GhAuthNeeded) + end + it "raises when gh returns invalid JSON" do expect(GitHub::API).to receive(:credentials) .and_return(fake_gh_creds)