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

Gemfile

@@ -11,6 +11,7 @@ gem 'bloomfilter-rb'
 gem 'figaro'
 gem 'health_check'
 gem 'identity-hostdata', github: '18F/identity-hostdata', branch: 'master'
+gem 'mini_cache'
 gem 'newrelic_rpm'
 gem 'pg'
 gem 'puma', '~> 3.7'

Gemfile.lock

@@ -759,6 +759,7 @@ GEM
     memory_profiler (0.9.10)
     method_source (0.9.0)
     mimemagic (0.3.2)
+    mini_cache (1.1.0)
     mini_mime (1.0.0)
     mini_portile2 (2.3.0)
     minitest (5.11.3)
@@ -937,6 +938,7 @@ DEPENDENCIES
   guard-rspec
   health_check
   identity-hostdata!
+  mini_cache
   newrelic_rpm
   overcommit
   pg

app/models/certificate.rb

@@ -3,6 +3,8 @@ class Certificate
 
   attr_accessor :x509_cert
 
+  REVOCATION_CACHE_EXPIRATION = 5.minutes
+
   def initialize(x509_cert)
     @x509_cert = x509_cert
   end
@@ -14,9 +16,13 @@ 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 revoked?
+  def calculate_revocation_status
     ocsp_response = OCSPService.new(self).call
     if !ocsp_response.successful?
       CertificateLoggerService.log_ocsp_response(ocsp_response)
@@ -32,6 +38,16 @@ def revoked?
     end
   end
 
+  def self.revocation_status?(certificate, &block)
+    @revocation_cache ||= MiniCache::Store.new
+    key = [certificate.issuer, certificate.subject, certificate.serial].map(&:to_s).inspect
+    @revocation_cache.get_or_set(key, expires_in: REVOCATION_CACHE_EXPIRATION, &block)
+  end
+
+  def self.clear_revocation_cache
+    @revocation_cache = nil
+  end
+
   def ==(other)
     subject == other.subject &&
       serial == other.serial &&
@@ -91,7 +107,7 @@ def to_pem
   def signature_verified?
     signing_cert = CertificateStore.instance[signing_key_id]
     UnrecognizedCertificateAuthority.find_or_create_for_certificate(self) unless signing_cert
-    signing_cert && verify(signing_cert.public_key)
+    signing_cert && verify(signing_cert.public_key) && signing_cert.valid?
   end
 
   def ca_capable?

app/services/ocsp_service.rb

@@ -6,6 +6,7 @@ class OCSPService
   attr_reader :subject, :authority, :request
 
   NO_AUTHORITY_RESPONSE = OpenStruct.new(revoked?: nil).freeze
+  OCSP_RESPONSE_CACHE_EXPIRATION = 5.minutes
 
   def initialize(subject)
     @subject = subject
@@ -16,16 +17,35 @@ def initialize(subject)
 
   def call
     return NO_AUTHORITY_RESPONSE unless @authority&.certificate && request.present?
-    response = make_http_request(ocsp_url_for_subject, request.to_der)
-    OCSPResponse.new(self, response)
+
+    # 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
+      response = make_http_request(ocsp_url_for_subject, request.to_der)
+      OCSPResponse.new(self, response)
+    end
+  end
+
+  def self.ocsp_response(url, issuer, subject, &block)
+    @ocsp_response_cache ||= MiniCache::Store.new
+    key = [issuer.subject, subject.subject, subject.serial, url].map(&:to_s).inspect
+    @ocsp_response_cache.get_or_set(key, expires_in: OCSP_RESPONSE_CACHE_EXPIRATION, &block)
+  end
+
+  def self.clear_ocsp_response_cache
+    @ocsp_response_cache = nil
   end
 
   private
 
+  def certificate_id
+    @certificate_id ||= begin
+      issuer = authority.certificate
+      digest = OpenSSL::Digest::SHA1.new
+      OpenSSL::OCSP::CertificateId.new(subject.x509_cert, issuer.x509_cert, digest)
+    end
+  end
+
   def build_request
-    issuer = authority.certificate
-    digest = OpenSSL::Digest::SHA1.new
-    certificate_id = OpenSSL::OCSP::CertificateId.new(subject.x509_cert, issuer.x509_cert, digest)
     request.add_certid(certificate_id)
     request.add_nonce
   end
@@ -56,7 +76,7 @@ def handle_response(response, limit)
     end
   end
 
-  def make_single_http_request(uri, request, retries = 3)
+  def make_single_http_request(uri, request, retries = 1)
     make_single_http_request!(uri, request)
   rescue Errno::ECONNRESET
     retries -= 1

config/initializers/certificate_store.rb

@@ -7,12 +7,4 @@
       cert_store.add_pem_file(file)
     end
   end
-
-  cert_store.remove_untrusted_certificates
-
-  unless cert_store.all_certificates_valid?
-    raise 'Not all certificates in the certificate store can be trusted'
-  end
-
-  raise 'There are no trusted certificates available' if cert_store.empty? && Rails.env.production?
 end

spec/controllers/identify_controller_spec.rb

@@ -121,6 +121,7 @@
 
         before(:each) do
           # create signing cert
+          Certificate.clear_revocation_cache
           allow(IO).to receive(:binread).with(ca_file_path).and_return(ca_file_content)
           allow(Figaro.env).to receive(:trusted_ca_root_identifiers).and_return(
             root_cert_key_ids.join(',')

spec/models/certificate_spec.rb

@@ -31,6 +31,8 @@
   let(:valid_ocsp) { true }
 
   before(:each) do
+    described_class.clear_revocation_cache
+    OCSPService.clear_ocsp_response_cache
     stub_request(:post, 'http://ocsp.example.com/').
       with(
         headers: {
@@ -75,6 +77,75 @@
     end
   end
 
+  describe '#signature_verified?' do
+    let(:x509_cert) { leaf_cert }
+
+    let(:ca_file_path) { data_file_path('certs.pem') }
+
+    let(:root_cert_key_ids) { root_certs.map(&:key_id) }
+    let(:intermediate_cert_key_ids) { intermediate_certs.map(&:key_id) }
+
+    let(:ca_file_content) do
+      cert_collection.map { |info| info[:certificate] }.map(&:to_pem).join("\n\n")
+    end
+
+    let(:cert_identifiers) do
+      mapping = {}
+      leaf_certs.each do |cert|
+        issuer = CertificateStore.instance[cert.signing_key_id]
+        certificate_id = OpenSSL::OCSP::CertificateId.new(
+          cert.x509_cert, issuer.x509_cert, OpenSSL::Digest::SHA1.new
+        )
+        mapping[certificate_id] = {
+          subject: cert,
+          issuer: issuer,
+        }
+      end
+      mapping
+    end
+
+    before(:each) do
+      allow(IO).to receive(:binread).with(ca_file_path).and_return(ca_file_content)
+      allow(Figaro.env).to receive(:trusted_ca_root_identifiers).and_return(
+        root_cert_key_ids.join(',')
+      )
+      certificate_store.clear_trusted_ca_root_identifiers
+      certificate_store.add_pem_file(ca_file_path)
+    end
+
+    context 'for an expired intermediate certificate' do
+      let(:cert_collection) do
+        create_certificate_set(
+          root_count: 2,
+          intermediate_count: 2,
+          leaf_count: 2,
+          intermediate_options: {
+            not_after: Time.zone.now - 1.day,
+            not_before: Time.zone.now - 1.week,
+          }
+        )
+      end
+
+      it 'has invalid intermediates' do
+        expect(intermediate_certs.any?(&:valid?)).to be_falsey
+      end
+
+      it 'is invalid' do
+        expect(certificate.signature_verified?).to be_falsey
+      end
+    end
+
+    context 'for a valid intermediate certificate' do
+      it 'has valid intermediates' do
+        expect(intermediate_certs.all?(&:valid?)).to be_truthy
+      end
+
+      it 'is valid' do
+        expect(certificate.signature_verified?).to be_truthy
+      end
+    end
+  end
+
   describe 'to_pem' do
     let(:pem) { certificate.to_pem.split(/\n/) }
     let(:x509_cert) { leaf_cert }

spec/services/ocsp_service_spec.rb

@@ -17,6 +17,47 @@
   let(:leaf_certs) { certificates_in_collection(cert_collection, :type, :leaf) }
   let(:cert) { leaf_certs.first }
 
+  describe 'OCSP response caching' do
+    let(:ca_file_path) { data_file_path('certs.pem') }
+
+    let(:ca_file_content) do
+      cert_collection.map { |info| info[:certificate] }.map(&:to_pem).join("\n\n")
+    end
+
+    let(:root_cert_key_ids) { root_certs.map(&:key_id) }
+    let(:status) { :valid }
+
+    before(:each) do
+      allow(IO).to receive(:binread).with(ca_file_path).and_return(ca_file_content)
+      allow(Figaro.env).to receive(:trusted_ca_root_identifiers).and_return(
+        root_cert_key_ids.join(',')
+      )
+      certificate_store.clear_trusted_ca_root_identifiers
+      certificate_store.add_pem_file(ca_file_path)
+
+      stub_request(:post, 'http://ocsp.example.com/').
+        with(
+          headers: {
+            'Content-Type' => 'application/ocsp-request',
+          }
+        ).
+        to_return do |request|
+        {
+          status: 200,
+          body: create_ocsp_response(request.body, cert_collection, status),
+          headers: {},
+        }
+      end
+    end
+
+    it 'makes a request to the OCSP server once' do
+      described_class.clear_ocsp_response_cache
+      expect(ocsp_service).to receive(:make_http_request).once
+      ocsp_service.call
+      ocsp_service.call
+    end
+  end
+
   context 'with valid OCSP responses' do
     let(:ca_file_path) { data_file_path('certs.pem') }
 
@@ -27,6 +68,7 @@
     let(:root_cert_key_ids) { root_certs.map(&:key_id) }
 
     before(:each) do
+      described_class.clear_ocsp_response_cache
       allow(IO).to receive(:binread).with(ca_file_path).and_return(ca_file_content)
       allow(Figaro.env).to receive(:trusted_ca_root_identifiers).and_return(
         root_cert_key_ids.join(',')
@@ -76,6 +118,7 @@
     let(:root_cert_key_ids) { root_certs.map(&:key_id) }
 
     before(:each) do
+      described_class.clear_ocsp_response_cache
       allow(IO).to receive(:binread).with(ca_file_path).and_return(ca_file_content)
       allow(Figaro.env).to receive(:trusted_ca_root_identifiers).and_return(
         root_cert_key_ids.join(',')
@@ -138,6 +181,7 @@
     let(:root_cert_key_ids) { root_certs.map(&:key_id) }
 
     before(:each) do
+      described_class.clear_ocsp_response_cache
       allow(IO).to receive(:binread).with(ca_file_path).and_return(ca_file_content)
       allow(Figaro.env).to receive(:trusted_ca_root_identifiers).and_return(
         root_cert_key_ids.join(',')
@@ -183,6 +227,7 @@
     let(:root_cert_key_ids) { root_certs.map(&:key_id) }
 
     before(:each) do
+      described_class.clear_ocsp_response_cache
       allow(IO).to receive(:binread).with(ca_file_path).and_return(ca_file_content)
       allow(Figaro.env).to receive(:trusted_ca_root_identifiers).and_return(
         root_cert_key_ids.join(',')
@@ -253,6 +298,7 @@
 
     context 'with bad data' do
       before(:each) do
+        described_class.clear_ocsp_response_cache
         allow(IO).to receive(:binread).with(ca_file_path).and_return(ca_file_content)
         allow(Figaro.env).to receive(:trusted_ca_root_identifiers).and_return(
           root_cert_key_ids.join(',')

spec/support/x509.rb

@@ -219,10 +219,14 @@ def create_certificate_set(**options)
     root_certs = []
     intermediate_certs = []
     leaf_certs = []
+    root_options = options[:root_options] || {}
+    intermediate_options = options[:intermediate_options] || {}
+    leaf_options = options[:leaf_options] || {}
     options[:root_count].times do |root_index|
       root, root_key = create_root_certificate(
         dn: "DC=com, DC=example, OU=ca, CN=Root #{root_index + 1}",
-        serial: root_index + 1
+        serial: root_index + 1,
+        **root_options
       )
       root_cert_id = OpenSSL::OCSP::CertificateId.new(
         root, root, OpenSSL::Digest::SHA1.new
@@ -244,7 +248,8 @@ def create_certificate_set(**options)
           ].join(' '),
           serial: intermediate_index + 1,
           ca: root,
-          ca_key: root_key
+          ca_key: root_key,
+          **intermediate_options
         )
         intermediate_cert_id = OpenSSL::OCSP::CertificateId.new(
           intermediate, root, OpenSSL::Digest::SHA1.new
@@ -270,7 +275,7 @@ def create_certificate_set(**options)
             serial: leaf_index + 1,
             ca: intermediate,
             ca_key: intermediate_key,
-            **(options[:leaf_options] || {})
+            **leaf_options
           )
           leaf_cert_id = OpenSSL::OCSP::CertificateId.new(
             leaf, intermediate, OpenSSL::Digest::SHA1.new