Merge branch 'feat/x509_issuer_crl_check' into 'master'

Add certification revocation list download and certificate revoke

Closes #122159

See merge request gitlab-org/gitlab!28336

app/models/x509_certificate.rb

@@ -26,6 +26,8 @@ class X509Certificate < ApplicationRecord
 
   validates :x509_issuer_id, presence: true
 
+  scope :by_x509_issuer, ->(issuer) { where(x509_issuer_id: issuer.id) }
+
   after_commit :mark_commit_signatures_unverified
 
   def self.safe_create!(attributes)
@@ -33,6 +35,10 @@ class X509Certificate < ApplicationRecord
       .safe_find_or_create_by!(subject_key_identifier: attributes[:subject_key_identifier])
   end
 
+  def self.serial_numbers(issuer)
+    by_x509_issuer(issuer).pluck(:serial_number)
+  end
+
   def mark_commit_signatures_unverified
     X509CertificateRevokeWorker.perform_async(self.id) if revoked?
   end

app/workers/all_queues.yml

@@ -269,6 +269,13 @@
   :resource_boundary: :unknown
   :weight: 1
   :idempotent: 
+- :name: cronjob:x509_issuer_crl_check
+  :feature_category: :source_code_management
+  :has_external_dependencies: true
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
 - :name: deployment:deployments_finished
   :feature_category: :continuous_delivery
   :has_external_dependencies: 

app/workers/x509_issuer_crl_check_worker.rb

+# frozen_string_literal: true
+
+class X509IssuerCrlCheckWorker
+  include ApplicationWorker
+  include CronjobQueue
+
+  feature_category :source_code_management
+  urgency :low
+
+  idempotent!
+  worker_has_external_dependencies!
+
+  attr_accessor :logger
+
+  def perform
+    @logger = Gitlab::GitLogger.build
+
+    X509Issuer.all.find_each do |issuer|
+      with_context(related_class: X509IssuerCrlCheckWorker) do
+        update_certificates(issuer)
+      end
+    end
+  end
+
+  private
+
+  def update_certificates(issuer)
+    crl = download_crl(issuer)
+    return unless crl
+
+    serials = X509Certificate.serial_numbers(issuer)
+    return if serials.empty?
+
+    revoked_serials = serials & crl.revoked.map(&:serial).map(&:to_i)
+
+    revoked_serials.each_slice(1000) do |batch|
+      certs = issuer.x509_certificates.where(serial_number: batch, certificate_status: :good) # rubocop: disable CodeReuse/ActiveRecord
+
+      certs.find_each do |cert|
+        logger.info(message: "Certificate revoked",
+          id: cert.id,
+          email: cert.email,
+          subject: cert.subject,
+          serial_number: cert.serial_number,
+          issuer: cert.x509_issuer.id,
+          issuer_subject: cert.x509_issuer.subject,
+          issuer_crl_url: cert.x509_issuer.crl_url)
+      end
+
+      certs.update_all(certificate_status: :revoked)
+    end
+  end
+
+  def download_crl(issuer)
+    response = Gitlab::HTTP.try_get(issuer.crl_url)
+
+    if response&.code == 200
+      OpenSSL::X509::CRL.new(response.body)
+    else
+      logger.warn(message: "Failed to download certificate revocation list",
+        issuer: issuer.id,
+        issuer_subject: issuer.subject,
+        issuer_crl_url: issuer.crl_url)
+
+      nil
+    end
+
+  rescue OpenSSL::X509::CRLError
+    logger.warn(message: "Failed to parse certificate revocation list",
+      issuer: issuer.id,
+      issuer_subject: issuer.subject,
+      issuer_crl_url: issuer.crl_url)
+
+    nil
+  end
+end

changelogs/unreleased/feat-x509_issuer_crl_check.yml

+---
+title: Add certification revocation list download and certificate revoke
+merge_request: 28336
+author: Roger Meier
+type: added

config/initializers/1_settings.rb

@@ -487,6 +487,9 @@ Settings.cron_jobs['namespaces_prune_aggregation_schedules_worker']['job_class']
 Settings.cron_jobs['container_expiration_policy_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['container_expiration_policy_worker']['cron'] ||= '50 * * * *'
 Settings.cron_jobs['container_expiration_policy_worker']['job_class'] = 'ContainerExpirationPolicyWorker'
+Settings.cron_jobs['x509_issuer_crl_check_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['x509_issuer_crl_check_worker']['cron'] ||= '30 1 * * *'
+Settings.cron_jobs['x509_issuer_crl_check_worker']['job_class'] = 'X509IssuerCrlCheckWorker'
 
 Gitlab.ee do
   Settings.cron_jobs['adjourned_group_deletion_worker'] ||= Settingslogic.new({})

doc/user/project/repository/x509_signed_commits/index.md

@@ -25,7 +25,7 @@ For a commit to be *verified* by GitLab:
   which is usually up to three years.
 - The signing time is equal or later then commit time.
 
-NOTE: **Note:** There is no certificate revocation list check in place at the moment.
+NOTE: **Note:** Certificate revocation lists are checked on a daily basis via background worker.
 
 ## Obtaining an x509 key pair
 

spec/workers/x509_issuer_crl_check_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe X509IssuerCrlCheckWorker do
+  subject(:worker) { described_class.new }
+
+  let(:project) { create(:project, :public, :repository) }
+  let(:x509_signed_commit) { project.commit_by(oid: '189a6c924013fc3fe40d6f1ec1dc20214183bc97') }
+  let(:revoked_x509_signed_commit) { project.commit_by(oid: 'ed775cc81e5477df30c2abba7b6fdbb5d0baadae') }
+
+  describe '#perform' do
+    context 'valid crl' do
+      before do
+        stub_request(:get, "http://ch.siemens.com/pki?ZZZZZZA6.crl")
+          .to_return(status: 200, body: File.read('spec/fixtures/x509/ZZZZZZA6.crl'), headers: {})
+      end
+
+      it 'changes certificate status for revoked certificates' do
+        revoked_x509_commit = Gitlab::X509::Commit.new(revoked_x509_signed_commit)
+        x509_commit = Gitlab::X509::Commit.new(x509_signed_commit)
+        issuer = revoked_x509_commit.signature.x509_certificate.x509_issuer
+
+        expect(issuer).to eq(x509_commit.signature.x509_certificate.x509_issuer)
+        expect(revoked_x509_commit.signature.x509_certificate.good?).to be_truthy
+        expect(x509_commit.signature.x509_certificate.good?).to be_truthy
+
+        worker.perform
+        revoked_x509_commit.signature.reload
+
+        expect(revoked_x509_commit.signature.x509_certificate.revoked?).to be_truthy
+        expect(x509_commit.signature.x509_certificate.revoked?).to be_falsey
+      end
+    end
+
+    context 'invalid crl' do
+      before do
+        stub_request(:get, "http://ch.siemens.com/pki?ZZZZZZA6.crl")
+          .to_return(status: 200, body: "trash", headers: {})
+      end
+
+      it 'does not change certificate status' do
+        revoked_x509_commit = Gitlab::X509::Commit.new(revoked_x509_signed_commit)
+
+        expect(revoked_x509_commit.signature.x509_certificate.good?).to be_truthy
+
+        worker.perform
+        revoked_x509_commit.signature.reload
+
+        expect(revoked_x509_commit.signature.x509_certificate.revoked?).to be_falsey
+      end
+    end
+
+    context 'not found crl' do
+      before do
+        stub_request(:get, "http://ch.siemens.com/pki?ZZZZZZA6.crl")
+          .to_return(status: 404, body: "not found", headers: {})
+      end
+
+      it 'does not change certificate status' do
+        revoked_x509_commit = Gitlab::X509::Commit.new(revoked_x509_signed_commit)
+
+        expect(revoked_x509_commit.signature.x509_certificate.good?).to be_truthy
+
+        worker.perform
+        revoked_x509_commit.signature.reload
+
+        expect(revoked_x509_commit.signature.x509_certificate.revoked?).to be_falsey
+      end
+    end
+
+    context 'unreachable crl' do
+      before do
+        stub_request(:get, "http://ch.siemens.com/pki?ZZZZZZA6.crl")
+          .to_raise(SocketError.new('Some HTTP error'))
+      end
+
+      it 'does not change certificate status' do
+        revoked_x509_commit = Gitlab::X509::Commit.new(revoked_x509_signed_commit)
+
+        expect(revoked_x509_commit.signature.x509_certificate.good?).to be_truthy
+
+        worker.perform
+        revoked_x509_commit.signature.reload
+
+        expect(revoked_x509_commit.signature.x509_certificate.revoked?).to be_falsey
+      end
+    end
+  end
+end