Merge branch 'master' into stages/rc-2019-01-31

app/models/certificate.rb

@@ -4,38 +4,24 @@ class Certificate
   attr_accessor :x509_cert
 
   REVOCATION_CACHE_EXPIRATION = 5.minutes
+  ANY_POLICY = '2.5.29.32.0'.freeze
 
   def initialize(x509_cert)
     @x509_cert = x509_cert
+    @cert_policies = CertificatePolicies.new(self)
   end
 
   def_delegators :x509_cert, :not_before, :not_after, :subject, :issuer, :verify,
                  :public_key, :serial, :to_text
 
+  def_delegators :@cert_policies, :allowed_by_policy?, :critical_policies_recognized?
+
   def trusted_root?
     CertificateStore.trusted_ca_root_identifiers.include?(key_id)
   end
 
   def revoked?
-    Certificate.revocation_status?(self) { calculate_revocation_status }
-  end
-
-  # :reek:DuplicateMethodCall
-  # :reek:TooManyStatements
-  def calculate_revocation_status
-    ocsp_response = OCSPService.new(self).call
-    if !ocsp_response.successful?
-      CertificateLoggerService.log_ocsp_response(ocsp_response)
-      CertificateAuthority.revoked?(self)
-    elsif ocsp_response.revoked?
-      CertificateLoggerService.log_ocsp_response(ocsp_response)
-      # save serial number as revoked
-      # temporarily not caching it while we investigate some reported wrong revocations via OCSP
-      # ocsp_response.authority&.certificate_revocations&.create!(serial: serial) if revoked_status
-      true
-    else
-      false
-    end
+    Certificate.revocation_status?(self) { OCSPService.new(self).call.revoked? }
   end
 
   def self.revocation_status?(certificate, &block)
@@ -63,15 +49,6 @@ def self_signed?
     signing_key_id == key_id
   end
 
-  def allowed_by_policy?
-    # if at least one policy in the cert matches one of the "required policies", then we're good
-    # otherwise, we want to allow it for now, but log the cert so we can see what policies are
-    # coming up
-    # This policy check is only on the leaf certificate - not used by CAs
-    expected_policies = required_policies
-    policies.any? { |policy| expected_policies.include?(policy) }
-  end
-
   def validate_cert
     if expired?
       'expired'
@@ -138,8 +115,8 @@ def aia
   end
 
   def token(extra)
+    maybe_log_certificate
     if valid?
-      CertificateLoggerService.log_certificate(self) unless allowed_by_policy?
       token_for_valid_certificate(extra)
     else
       token_for_invalid_certificate(extra)
@@ -163,12 +140,6 @@ def issuer_metadata
     }
   end
 
-  def policies
-    (get_extension('certificatePolicies') || '').split(/\n/).map do |line|
-      line.sub(/^Policy:\s+/, '')
-    end
-  end
-
   def logging_filename
     [key_id, signing_key_id, serial].join('::')
   end
@@ -179,6 +150,11 @@ def logging_content
 
   private
 
+  def maybe_log_certificate
+    return if valid? && critical_policies_recognized? && allowed_by_policy?
+    CertificateLoggerService.log_certificate(self)
+  end
+
   # :reek:UtilityFunction
   def get_extension(oid)
     @x509_cert.extensions.detect { |record| record.oid == oid }&.value
@@ -208,12 +184,6 @@ def token_for_invalid_certificate(extra)
     reason = validate_cert
 
     Rails.logger.warn("Certificate invalid: #{reason}")
-    CertificateLoggerService.log_certificate(self)
     TokenService.box(extra.merge(error: "certificate.#{reason}"))
   end
-
-  # :reek:UtilityFunction
-  def required_policies
-    JSON.parse(Figaro.env.required_policies || '[]')
-  end
 end

app/models/finite_policy_mapping_depth.rb

@@ -0,0 +1,29 @@
+class FinitePolicyMappingDepth
+  attr_reader :value
+
+  # :reek:FeatureEnvy
+  def initialize(value)
+    @value = value.to_i
+  end
+
+  def negative?
+    @value.negative?
+  end
+
+  def any?
+    false
+  end
+
+  # :reek:FeatureEnvy
+  def <=>(other)
+    if other.any?
+      -1
+    else
+      value <=> other.value
+    end
+  end
+
+  def -(other)
+    FinitePolicyMappingDepth.new(value - other)
+  end
+end

app/models/infinite_policy_mapping_depth.rb

@@ -0,0 +1,22 @@
+class InfinitePolicyMappingDepth
+  def negative?
+    false
+  end
+
+  def any?
+    true
+  end
+
+  # :reek:UtilityFunction
+  def <=>(other)
+    if other.any?
+      0
+    else
+      1
+    end
+  end
+
+  def -(_other)
+    self
+  end
+end

app/models/ocsp_response.rb

@@ -6,6 +6,7 @@ class OCSPResponse
   def initialize(ocsp_request, response)
     @ocsp_request = ocsp_request
     @response = response
+    @revoked = nil
   end
 
   def_delegators :@ocsp_request, :subject, :request, :authority
@@ -15,8 +16,18 @@ def successful?
   end
 
   def revoked?
-    return unless successful? && verified? && valid_nonce?
-    any_revoked?
+    return @revoked unless @revoked.nil?
+
+    @revoked = calculate_revocation
+    log_if_interesting
+    cache_revocation
+    @revoked
+  end
+
+  def calculate_revocation
+    return CertificateAuthority.revoked?(subject) unless successful? && verified? && valid_nonce?
+
+    any_revoked? || false
   end
 
   def verified?
@@ -80,6 +91,17 @@ def to_text
       response.basic.status.map { |status| status_description(status) }.join('')
   end
 
+  def log_if_interesting
+    return if successful? && !revoked?
+    CertificateLoggerService.log_ocsp_response(self)
+  end
+
+  def cache_revocation
+    return unless revoked?
+    # save serial number as revoked
+    authority&.certificate_revocations&.create(serial: subject.serial)
+  end
+
   private
 
   def general_text_description

app/policies/certificate_policies.rb

@@ -0,0 +1,90 @@
+class CertificatePolicies
+  KNOWN_POLICIES = [
+    # Policies we've seen marked critical
+    'basicConstraints',
+    'inhibitAnyPolicy',
+    'keyUsage',
+    'policyConstraints',
+    # Policies we use that could be marked critical
+    'authorityInfoAccess',
+    'authorityKeyIdentifier',
+    'certificatePolicies',
+    'crlDistributionPoints',
+    'policyMappings',
+    'subjectKeyIdentifier',
+    # Policies we don't use
+    'nameConstraints',
+    'subjectAltName',
+    'subjectInfoAccess',
+  ].freeze
+
+  def initialize(cert)
+    @certificate = cert
+  end
+
+  def allowed_by_policy?
+    # if at least one policy in the cert matches one of the "required policies", then we're good
+    # otherwise, we want to allow it for now, but log the cert so we can see what policies are
+    # coming up
+    # This policy check is only on the leaf certificate - not used by CAs
+    mapping = PolicyMappingService.new(@certificate).call
+    expected_policies = required_policies
+    cert_policies = policies.map { |policy| mapping[policy] }
+    (cert_policies & expected_policies).any?
+  end
+
+  def policies
+    (get_extension('certificatePolicies') || '').split(/\n/).map do |line|
+      line.sub(/^Policy:\s+/, '')
+    end
+  end
+
+  def critical_policies_recognized?
+    (certificate.x509_cert.extensions.select(&:critical?).map(&:oid) - KNOWN_POLICIES).empty?
+  end
+
+  # provides a mapping of policy OIDs seen in child certificates to policy OIDs expected by the
+  # issuing certificate
+  def policy_mappings
+    get_extension('policyMappings')&.
+    split(/\s*,\s*/)&.
+    map { |mapping| mapping.split(/:/).reverse }.
+      to_h
+  end
+
+  ANY_POLICY_MAPPING_DEPTH = InfinitePolicyMappingDepth.new
+
+  def policy_mappings_allowed(previous_allowed = ANY_POLICY_MAPPING_DEPTH)
+    [inhibit_policy_mapping, previous_allowed - 1].min
+  end
+
+  def policy_constraints
+    get_extension('policyConstraints')&.
+    split(/\s*,\s*/)&.
+    map { |mapping| mapping.split(/:/) }.
+      to_h
+  end
+
+  def inhibit_policy_mapping
+    value = policy_constraints.fetch('Inhibit Policy Mapping') { :any }
+    if value == :any
+      ANY_POLICY_MAPPING_DEPTH
+    else
+      FinitePolicyMappingDepth.new(value)
+    end
+  end
+
+  private
+
+  attr_reader :certificate
+
+  # :reek:UtilityFunction
+  def get_extension(oid)
+    certificate.x509_cert.extensions.detect { |record| record.oid == oid }&.value
+  end
+
+  # :reek:UtilityFunction
+  def required_policies
+    JSON.parse(Figaro.env.required_policies || '[]')
+  end
+end

app/services/ocsp_service.rb

@@ -5,7 +5,6 @@
 class OCSPService
   attr_reader :subject, :authority, :request
 
-  NO_AUTHORITY_RESPONSE = OpenStruct.new(revoked?: nil).freeze
   OCSP_RESPONSE_CACHE_EXPIRATION = 5.minutes
 
   def initialize(subject)
@@ -16,7 +15,7 @@ def initialize(subject)
   end
 
   def call
-    return NO_AUTHORITY_RESPONSE unless @authority&.certificate && request.present?
+    return OpenStruct.new(revoked?: CertificateAuthority.revoked?(subject)) if no_request
 
     # we want to cache the call for a few minutes so we don't hammer on the same request
     OCSPService.ocsp_response(ocsp_url_for_subject, authority.certificate, subject) do
@@ -37,6 +36,10 @@ def self.clear_ocsp_response_cache
 
   private
 
+  def no_request
+    !@authority&.certificate || request.blank?
+  end
+
   def certificate_id
     @certificate_id ||= begin
       issuer = authority.certificate

app/services/policy_mapping_service.rb

@@ -0,0 +1,57 @@
+class PolicyMappingService
+  def initialize(certificate)
+    @certificate = certificate
+  end
+
+  def call
+    policy_mapping
+  end
+
+  private
+
+  attr_reader :certificate
+
+  def chain(set = [])
+    # walk from the cert to a root - we can do this safely because we've already
+    # constructed a path from the leaf cert to a trusted root elsewhere
+    store = CertificateStore.instance
+    @chain ||= begin
+      signer = store[certificate.signing_key_id]
+      while signer
+        set << signer
+        signer = !signer.self_signed? && store[signer.signing_key_id]
+      end
+      set.reverse
+    end
+  end
+
+  # :reek:UtilityFunction
+  def new_mapping
+    Hash.new { |_, key| key }
+  end
+
+  # ultimately maps OIDs seen in child certs to OIDs we expect at the top level
+  def policy_mapping
+    return new_mapping if chain.empty?
+    allowed_depth = CertificatePolicies.new(chain.first).policy_mappings_allowed
+
+    chain.each_with_object(new_mapping) do |cert, mapping|
+      next if allowed_depth != :any && allowed_depth.negative?
+
+      allowed_depth = import_mapping(mapping, cert, allowed_depth)
+    end
+  end
+
+  # :reek:UtilityFunction
+  def import_mapping(mapping, cert, allowed_depth)
+    policy = CertificatePolicies.new(cert)
+
+    policy.policy_mappings.each do |(key, value)|
+      # RFC 5280, section 4.2.1.5 requires that no mapping can be to or from
+      # the value anyPolicy.
+      next if ([key, value] & ['X509v3 Any Policy', Certificate::ANY_POLICY]).any?
+      mapping[key] = mapping[value]
+    end
+    policy.policy_mappings_allowed(allowed_depth)
+  end
+end

config/application.yml.example

@@ -24,7 +24,13 @@ required_policies: |
     "2.16.840.1.101.2.1.11.43",
     "2.16.840.1.101.2.1.11.44",
     "2.16.840.1.101.3.2.1.3.7",
-    "2.16.840.1.101.3.2.1.3.13"
+    "2.16.840.1.101.3.2.1.3.13",
+    "2.16.840.1.101.3.2.1.12.2",
+    "2.16.840.1.101.3.2.1.12.3",
+    "2.16.840.1.101.3.2.1.12.5",
+    "2.16.840.1.101.3.2.1.12.6",
+    "2.16.840.1.101.3.2.1.12.10",
+    "2.16.840.1.114027.200.3.10.7.2"
   ]
 
 development: