More logic moved to X509 wrapper

lib/sigstore/internal/x509.rb

@@ -18,20 +18,89 @@ module Sigstore
   module Internal
     module X509
       class Certificate
+        extend Forwardable
+
+        attr_reader :openssl
+
         def initialize(x509_certificate)
-          @x509_certificate = x509_certificate
+          unless x509_certificate.is_a?(OpenSSL::X509::Certificate)
+            raise ArgumentError,
+                  "Invalid certificate: #{x509_certificate.inspect}"
+          end
+
+          @openssl = x509_certificate
+
+          raise Error::InvalidCertificate, "invalid X.509 version: #{version.inspect}" if version != 2 # v3
+        end
+
+        def self.read(certificate_bytes)
+          new(OpenSSL::X509::Certificate.new(certificate_bytes))
         end
 
         def tbs_certificate_der
           raise NotImplementedError
         end
 
         def extension(cls)
-          @x509_certificate.extensions.each do |ext|
+          openssl.extensions.each do |ext|
             return cls.new(ext) if ext.oid == cls.oid || ext.oid == cls.oid.short_name
           end
           nil
         end
+
+        def_delegators :openssl, :version, :not_after, :not_before, :to_pem, :to_der,
+                       :public_key, :to_text
+
+        def leaf?
+          return false if ca?
+
+          key_usage = extension(Extension::KeyUsage) ||
+                      raise(Error::InvalidCertificate,
+                            "no keyUsage in #{@x509_certificate.extensions.map(&:to_h)}")
+
+          unless key_usage.digital_signature
+            raise Error::InvalidCertificate,
+                  "invalid certificate for Sigstore purposes: missing digital signature usage: #{key_usage.to_h}"
+          end
+
+          extended_key_usage = extension(Extension::ExtendedKeyUsage)
+          return false unless extended_key_usage
+
+          extended_key_usage.code_signing?
+        end
+
+        def ca?
+          basic_constraints = extension(Extension::BasicConstraints)
+          return false unless basic_constraints
+
+          unless basic_constraints.critical?
+            raise Error::InvalidCertificate,
+                  "invalid X.509 certificate: non-critical BasicConstraints in CA"
+          end
+
+          key_usage = extension(Extension::KeyUsage)
+          raise Error::InvalidCertificate, "no keyUsage in #{openssl.inspect}" unless key_usage
+
+          ca = basic_constraints.ca
+          key_cert_sign = key_usage.key_cert_sign
+
+          return true if ca && key_cert_sign
+
+          return false unless key_cert_sign || ca
+
+          raise Error::InvalidCertificate,
+                "invalid X.509 certificate: inconsistent CA/KeyCertSign in BasicConstraints/KeyUsage " \
+                "(#{ca.inspect}, #{key_cert_sign.inspect}):" \
+                "\n#{openssl.extensions.map(&:to_h).pretty_inspect}" \
+                "\n#{key_usage.pretty_inspect}"
+        end
+
+        def preissuer?
+          extended_key_usage = extension(Extension::ExtendedKeyUsage)
+          return false unless extended_key_usage
+
+          extended_key_usage.purposes.include?(OpenSSL::ASN1::ObjectId.new("1.3.6.1.4.1.11129.2.4.4"))
+        end
       end
 
       class Extension
@@ -59,6 +128,10 @@ def initialize(extension)
           parse_value(OpenSSL::ASN1.decode(contents))
         end
 
+        def critical?
+          @extension.critical?
+        end
+
         def shift_value(value, klass)
           v = value.shift
           raise ArgumentError, "Invalid extension: #{v} is not a #{klass}" unless v.is_a?(klass)
@@ -86,7 +159,7 @@ def shift_bitstring(value)
           value.value.each_byte.flat_map do |byte|
             [byte & 0b1000_0000 != 0, byte & 0b0100_0000 != 0, byte & 0b0010_0000 != 0, byte & 0b0001_0000 != 0,
              byte & 0b0000_1000 != 0, byte & 0b0000_0100 != 0, byte & 0b0000_0010 != 0, byte & 0b0000_0001 != 0]
-          end[..-(value.unused_bits - 1)]
+          end[..-value.unused_bits.succ]
         end
 
         class SubjectKeyIdentifier < Extension
@@ -134,6 +207,12 @@ def parse_value(value)
             raise ArgumentError,
                   "Invalid extended key usage: #{value.inspect}"
           end
+
+          CODE_SIGNING = OpenSSL::ASN1::ObjectId.new("1.3.6.1.5.5.7.3.3")
+
+          def code_signing?
+            purposes.any? { |oid| oid == CODE_SIGNING }
+          end
         end
 
         class BasicConstraints < Extension
@@ -142,20 +221,24 @@ class BasicConstraints < Extension
           attr_reader :ca, :path_len_constraint
 
           def parse_value(value)
+            value = shift_value([value], OpenSSL::ASN1::Sequence)
+
             @ca = false
             @path_len_constraint = nil
 
-            @ca = shift_value(value, OpenSSL::ASN1::Boolean).value if value.first.is_a?(OpenSSL::ASN1::Boolean)
+            @ca = shift_value(value, OpenSSL::ASN1::Boolean) if value.first.is_a?(OpenSSL::ASN1::Boolean)
 
             return unless value.first.is_a?(OpenSSL::ASN1::Integer)
 
-            @path_len_constraint = shift_value(value, OpenSSL::ASN1::Integer).value
+            @path_len_constraint = shift_value(value, OpenSSL::ASN1::Integer)
           end
         end
 
         class SubjectAlternativeName < Extension
           self.oid = OpenSSL::ASN1::ObjectId.new("2.5.29.17")
 
+          attr_reader :general_names
+
           #  id-ce-subjectAltName OBJECT IDENTIFIER ::=  { id-ce 17 }
 
           #  SubjectAltName ::= GeneralNames
@@ -180,6 +263,22 @@ class SubjectAlternativeName < Extension
           #  EDIPartyName ::= SEQUENCE {
           #       nameAssigner            [0]     DirectoryString OPTIONAL,
           #       partyName               [1]     DirectoryString }
+
+          def parse_value(value)
+            value = shift_value([value], OpenSSL::ASN1::Sequence)
+
+            @general_names = value.map do |general_name|
+              tag = general_name.tag
+
+              case tag
+              when 6
+                [:uniformResourceIdentifier, general_name.value]
+              else
+                raise Error::Unimplemented,
+                      "Unhandled general name tag: #{tag}"
+              end
+            end
+          end
         end
 
         class PrecertificateSignedCertificateTimestamps < Extension

lib/sigstore/models.rb

@@ -75,7 +75,7 @@ def initialize(input:, cert_pem:, **kwargs)
       digest = OpenSSL::Digest.new("SHA256")
       digest.update(input_bytes)
       hashed_input = digest
-      certificate = OpenSSL::X509::Certificate.new(cert_pem)
+      certificate = Internal::X509::Certificate.read(cert_pem)
 
       super(hashed_input: hashed_input, certificate: certificate, input_bytes: input_bytes, offline: offline, **kwargs)
 
@@ -138,18 +138,18 @@ def self.from_bundle(input:, bundle:, offline:)
 
       case media_type
       when BundleType::BUNDLE_0_3
-        leaf_cert = OpenSSL::X509::Certificate.new(bundle.verification_material.certificate.raw_bytes)
+        leaf_cert = Internal::X509::Certificate.read(bundle.verification_material.certificate.raw_bytes)
       when BundleType::BUNDLE_0_1, BundleType::BUNDLE_0_2
         certs = bundle.verification_material.x509_certificate_chain.certificates.map do |cert|
-          OpenSSL::X509::Certificate.new(cert.raw_bytes)
+          Internal::X509::Certificate.read(cert.raw_bytes)
         end
         raise Error::InvalidBundle, "Expected certificate chain" if certs.empty?
 
         leaf_cert = certs.shift
-        raise Error::InvalidBundle, "Expected leaf certificate" unless cert_is_leaf?(leaf_cert)
+        raise Error::InvalidBundle, "Expected leaf certificate" unless leaf_cert.leaf?
 
         certs.each do |cert|
-          raise Error::InvalidBundle, "Root CA in chain" if cert_is_root_ca?(cert)
+          raise Error::InvalidBundle, "Root CA in chain" if cert.ca?
         end
       else
         raise Error::InvalidBundle, "Unsupported bundle format: #{media_type}"
@@ -209,65 +209,5 @@ def self.from_bundle(input:, bundle:, offline:)
         rekor_entry: entry
       )
     end
-
-    def self.cert_is_leaf?(cert)
-      raise Error::InvalidCertificate, "invalid X.509 version: #{cert.version.inspect}" if cert.version != 2 # v3
-
-      return false if cert_is_ca?(cert)
-
-      key_usage = cert.find_extension("keyUsage") || raise(Error::InvalidCertificate,
-                                                           "no keyUsage in #{cert.extensions.map(&:to_h)}")
-      digital_signature = key_usage&.value&.include?("Digital Signature") # TODO: proper inclusion checking
-
-      unless digital_signature
-        raise Error::InvalidCertificate,
-              "invalid certificate for Sigstore purposes: missing digital signature usage: #{key_usage.to_h}"
-      end
-
-      extended_key_usage = cert.find_extension("extendedKeyUsage")
-      extended_key_usage&.value&.include?("Code Signing") # TODO: proper inclusion checking
-    end
-
-    def self.cert_is_ca?(cert)
-      raise Error::InvalidCertificate, "invalid X.509 version: #{cert.version.inspect}" if cert.version != 2 # v3
-
-      basic_constraints = cert.find_extension("basicConstraints")
-      return false unless basic_constraints
-
-      unless basic_constraints.critical?
-        raise Error::InvalidCertificate,
-              "invalid X.509 certificate: non-critical BasicConstraints in CA"
-      end
-
-      seq = OpenSSL::ASN1.decode(basic_constraints.value_der)
-      unless seq.is_a?(OpenSSL::ASN1::Sequence)
-        raise Error::InvalidCertificate,
-              "invalid X.509 certificate: BasicConstraints is not a sequence"
-      end
-
-      ca, _path_len = seq.value
-      unless ca.is_a?(OpenSSL::ASN1::Boolean)
-        raise Error::InvalidCertificate,
-              "invalid X.509 certificate: ca is not a boolean"
-      end
-
-      ca = ca.value
-
-      key_usage = cert.find_extension("keyUsage")
-      raise Error::InvalidCertificate, "invalid X.509 certificate: missing keyUsage" unless key_usage
-
-      key_usage_bs = OpenSSL::ASN1.decode(key_usage.value_der)
-      unless key_usage_bs.is_a?(OpenSSL::ASN1::BitString)
-        raise Error::InvalidCertificate, "invalid X.509 certificate: keyUsage is not a bit string"
-      end
-
-      key_sign_cert = key_usage_bs.value.getbyte(0).allbits?(0b00000100) # KeyUsage.keyCertSign, bit 5
-
-      return true if ca && key_sign_cert
-
-      return false unless ca || key_sign_cert
-
-      raise Error::InvalidCertificate, "invalid certificate states: KeyUsage.keyCertSign"
-    end
   end
 end

lib/sigstore/policy.rb

@@ -22,7 +22,7 @@ def initialize(value)
       end
 
       def verify(cert)
-        ext = cert.find_extension(oid)
+        ext = cert.openssl.find_extension(oid)
         unless ext
           return VerificationFailure.new("Certificate does not contain #{self.class.name&.[](/::([^:]+)$/, 1)} " \
                                          "(#{oid}) extension")
@@ -87,24 +87,14 @@ def verify(cert)
         issuer_verified = @issuer.verify(cert)
         return issuer_verified unless issuer_verified.verified?
 
-        san_ext = cert.find_extension("subjectAltName")
-        raise "Certificate does not contain subjectAltName extension" unless san_ext
+        san_ext = cert.extension(Sigstore::Internal::X509::Extension::SubjectAlternativeName)
+        raise Error::InvalidCertificate, "Certificate does not contain subjectAltName extension" unless san_ext
 
-        sequence = OpenSSL::ASN1.decode(san_ext.value_der)
-        raise "subjectAltName is not a sequence" unless sequence.is_a?(OpenSSL::ASN1::Sequence)
-
-        all_sans = sequence.map do |asn1_data|
-          case asn1_data.tag
-          when 6 # URI
-            asn1_data.value
-          else
-            raise "Unknown SAN type: #{asn1_data.tag}"
-          end
-        end.compact
-
-        verified = all_sans.include?(@identity)
+        verified = san_ext.general_names.include?([:uniformResourceIdentifier, @identity])
         unless verified
-          return VerificationFailure.new("Certificate's SANs do not match #{@identity}; actual SANs: #{all_sans}")
+          return VerificationFailure.new(
+            "Certificate's SANs do not match #{@identity}; actual SANs: #{san_ext.general_names}"
+          )
         end
 
         VerificationSuccess.new

lib/sigstore/trusted_root.rb

@@ -61,7 +61,7 @@ def ctfe_keys
     end
 
     def fulcio_cert_chain
-      certs = ca_keys(certificate_authorities, allow_expired: true).flat_map { OpenSSL::X509::Certificate.new(_1) }
+      certs = ca_keys(certificate_authorities, allow_expired: true).flat_map { Internal::X509::Certificate.read(_1) }
       raise "Fulcio certificates not found in trusted root" if certs.empty?
 
       certs