Improve error handling (#153)

Author: segiddins

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}..."

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,

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)

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
         #

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