From ba84c7d2777b112005d155ef0f8416b6936cde16 Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Fri, 25 Oct 2024 17:04:33 -0400 Subject: [PATCH] Improve error handling Signed-off-by: Samuel Giddins --- bin/sigstore-ruby | 2 +- lib/sigstore/internal/x509.rb | 4 +++- lib/sigstore/models.rb | 24 +++++++++++++++++++----- lib/sigstore/verifier.rb | 13 +++++++++++-- test/sigstore/models_test.rb | 21 +++++++++++++++++++++ 5 files changed, 55 insertions(+), 9 deletions(-) diff --git a/bin/sigstore-ruby b/bin/sigstore-ruby index 87b4762..e2b8595 100755 --- a/bin/sigstore-ruby +++ b/bin/sigstore-ruby @@ -239,7 +239,7 @@ module Sigstore b64_sig = Gem.read_binary(inputs[:sig]) signature = b64_sig.unpack1("m") - verification_input.bundle = Sigstore::SBundle.for_cert_bytes_and_signature(cert_pem, signature) + verification_input.bundle = Sigstore::SBundle.for_cert_bytes_and_signature(cert_pem, signature).__getobj__ end say "Verifying #{file}..." diff --git a/lib/sigstore/internal/x509.rb b/lib/sigstore/internal/x509.rb index 60f8014..e81269a 100644 --- a/lib/sigstore/internal/x509.rb +++ b/lib/sigstore/internal/x509.rb @@ -38,6 +38,8 @@ def initialize(x509_certificate) def self.read(certificate_bytes) new(OpenSSL::X509::Certificate.new(certificate_bytes)) + rescue OpenSSL::X509::CertificateError => e + raise Error::InvalidCertificate, e.message end def tbs_certificate_der @@ -104,7 +106,7 @@ def leaf? key_usage = extension(Extension::KeyUsage) || raise(Error::InvalidCertificate, - "no keyUsage in #{@x509_certificate.extensions.map(&:to_h)}") + "no keyUsage in #{openssl.extensions.map(&:to_h)}") unless key_usage.digital_signature raise Error::InvalidCertificate, diff --git a/lib/sigstore/models.rb b/lib/sigstore/models.rb index a609384..74cc80c 100644 --- a/lib/sigstore/models.rb +++ b/lib/sigstore/models.rb @@ -80,6 +80,12 @@ class VerificationInput < DelegateClass(Verification::V1::Input) def initialize(*) super + + unless bundle.is_a?(Bundle::V1::Bundle) + raise ArgumentError, + "bundle must be a #{Bundle::V1::Bundle}, is #{bundle.class}" + end + @trusted_root = TrustedRoot.new(artifact_trust_root) @sbundle = SBundle.new(bundle) if sbundle.message_signature? && !artifact @@ -138,13 +144,19 @@ def expected_tlog_entry(hashed_input) expected_hashed_rekord_tlog_entry(hashed_input) when :dsse_envelope rekor_entry = verification_material.tlog_entries.first - case JSON.parse(rekor_entry.canonicalized_body).values_at("kind", "apiVersion") + canonicalized_body = begin + JSON.parse(rekor_entry.canonicalized_body) + rescue JSON::ParserError + raise Error::InvalidBundle, "expected canonicalized_body to be JSON" + end + + case kind_version = canonicalized_body.values_at("kind", "apiVersion") when %w[dsse 0.0.1] expected_dsse_0_0_1_tlog_entry when %w[intoto 0.0.2] expected_intoto_0_0_2_tlog_entry else - raise Error::InvalidRekorEntry, "Unhandled rekor entry kind/version: #{t.inspect}" + raise Error::InvalidRekorEntry, "Unhandled rekor entry kind/version: #{kind_version.inspect}" end else raise Error::InvalidBundle, "expected either message_signature or dsse_envelope" @@ -154,6 +166,8 @@ def expected_tlog_entry(hashed_input) private def validate_version! + raise Error::InvalidBundle, "bundle requires verification material" unless verification_material + case bundle_type when BundleType::BUNDLE_0_1 unless verification_material.tlog_entries.all?(&:inclusion_promise) @@ -169,7 +183,7 @@ def validate_version! raise Error::InvalidBundle, "must contain an inclusion proof" end - unless verification_material.tlog_entries.all? { |t| t.inclusion_proof.checkpoint.envelope } + unless verification_material.tlog_entries.all? { |t| t.inclusion_proof.checkpoint&.envelope } raise Error::InvalidBundle, "inclusion proof must contain a checkpoint" end @@ -192,9 +206,9 @@ def validate_version! when :certificate @leaf_certificate = Internal::X509::Certificate.read(verification_material.certificate.raw_bytes) else - raise Error::InvalidBundle, "Unsupported bundle content: #{content}" + raise Error::InvalidBundle, "Unsupported bundle content: #{content.inspect}" end - raise Error::InvalidBundle, "Expected leaf certificate" unless @leaf_certificate.leaf? + raise Error::InvalidBundle, "expected certificate to be leaf" unless @leaf_certificate.leaf? end def expected_hashed_rekord_tlog_entry(hashed_input) diff --git a/lib/sigstore/verifier.rb b/lib/sigstore/verifier.rb index ac122a4..ee14e6d 100644 --- a/lib/sigstore/verifier.rb +++ b/lib/sigstore/verifier.rb @@ -183,7 +183,12 @@ def verify(input:, policy:, offline:) case bundle.dsse_envelope.payloadType when "application/vnd.in-toto+json" - verify_in_toto(input, JSON.parse(bundle.dsse_envelope.payload)) + in_toto = begin + JSON.parse(bundle.dsse_envelope.payload) + rescue JSON::ParserError + raise Error::InvalidBundle, "invalid JSON for in-toto statement in DSSE payload" + end + verify_in_toto(input, in_toto) else raise Sigstore::Error::Unimplemented, "unsupported DSSE payload type: #{bundle.dsse_envelope.payloadType.inspect}" @@ -432,7 +437,11 @@ def find_rekor_entry(bundle, hashed_input, offline:) logger.debug { "Found rekor entry: #{entry}" } - actual_body = JSON.parse(entry.canonicalized_body) + actual_body = begin + JSON.parse(entry.canonicalized_body) + rescue JSON::ParserError + raise Error::InvalidRekorEntry, "invalid JSON in rekor entry canonicalized_body" + end if bundle.dsse_envelope? # since the hash is over the uncanonicalized envelope, we need to remove it # diff --git a/test/sigstore/models_test.rb b/test/sigstore/models_test.rb index c649a26..3dc9848 100644 --- a/test/sigstore/models_test.rb +++ b/test/sigstore/models_test.rb @@ -17,4 +17,25 @@ def test_from_media_type Sigstore::BundleType.from_media_type("application/vnd.dev.sigstore.bundle+json;version=0.0") end end + + def test_verification_input_no_bundle + verification_input = Sigstore::Verification::V1::Input.new + e = assert_raise(ArgumentError) { Sigstore::VerificationInput.new(verification_input) } + assert_equal("bundle must be a Sigstore::Bundle::V1::Bundle, is NilClass", e.message) + end + + def test_verification_input_bundle_missing_media_type + verification_input = Sigstore::Verification::V1::Input.new + verification_input.bundle = Sigstore::Bundle::V1::Bundle.new + e = assert_raise(Sigstore::Error::InvalidBundle) { Sigstore::VerificationInput.new(verification_input) } + assert_equal("Unsupported bundle format: \"\"", e.message) + end + + def test_verification_input_bundle_missing_verification_material + verification_input = Sigstore::Verification::V1::Input.new + verification_input.bundle = Sigstore::Bundle::V1::Bundle.new + verification_input.bundle.media_type = Sigstore::BundleType::BUNDLE_0_3.media_type + e = assert_raise(Sigstore::Error::InvalidBundle) { Sigstore::VerificationInput.new(verification_input) } + assert_equal("bundle requires verification material", e.message) + end end