Commit bff53d60 authored by Roger Meier's avatar Roger Meier Committed by Thong Kuah

Add functionality to revoke a X509Certificate and update related X509CommitSignatures

parent b9ec8706
......@@ -2,6 +2,7 @@
class X509Certificate < ApplicationRecord
include X509SerialNumberAttribute
include AfterCommitQueue
x509_serial_number_attribute :serial_number
......@@ -25,8 +26,14 @@ class X509Certificate < ApplicationRecord
validates :x509_issuer_id, presence: true
after_commit :mark_commit_signatures_unverified
def self.safe_create!(attributes)
create_with(attributes)
.safe_find_or_create_by!(subject_key_identifier: attributes[:subject_key_identifier])
end
def mark_commit_signatures_unverified
X509CertificateRevokeWorker.perform_async(self.id) if revoked?
end
end
# frozen_string_literal: true
class X509CertificateRevokeService
def execute(certificate)
return unless certificate.revoked?
certificate.x509_commit_signatures.update_all(verification_status: :unverified)
end
end
.gpg-popover-certificate-details
%strong= _('Certificate Subject')
- if signature.x509_certificate.revoked?
%strong.cred= _('(revoked)')
%ul
- x509_subject(signature.x509_certificate.subject, ["CN", "O"]).map do |key, value|
%li= key + "=" + value
......
......@@ -1284,3 +1284,10 @@
:resource_boundary: :unknown
:weight: 1
:idempotent:
- :name: x509_certificate_revoke
:feature_category: :source_code_management
:has_external_dependencies:
:urgency: :default
:resource_boundary: :unknown
:weight: 1
:idempotent: true
# frozen_string_literal: true
class X509CertificateRevokeWorker
include ApplicationWorker
feature_category :source_code_management
idempotent!
def perform(certificate_id)
return unless certificate_id
X509Certificate.find_by_id(certificate_id).try do |certificate|
X509CertificateRevokeService.new.execute(certificate)
end
end
end
---
title: Add functionality to revoke a X509Certificate and update related X509CommitSignatures
merge_request: 24889
author: Roger Meier
type: added
......@@ -246,3 +246,5 @@
- 1
- - web_hook
- 1
- - x509_certificate_revoke
- 1
......@@ -184,11 +184,13 @@ module Gitlab
commit_sha: @commit.sha,
project: @commit.project,
x509_certificate_id: certificate.id,
verification_status: verification_status
verification_status: verification_status(certificate)
}
end
def verification_status
def verification_status(certificate)
return :unverified if certificate.revoked?
if verified_signature && certificate_email == @commit.committer_email
:verified
else
......
......@@ -560,6 +560,9 @@ msgstr ""
msgid "(removed)"
msgstr ""
msgid "(revoked)"
msgstr ""
msgid "*"
msgstr ""
......
......@@ -8,5 +8,6 @@ FactoryBot.define do
email { 'gitlab@example.org' }
serial_number { 278969561018901340486471282831158785578 }
x509_issuer
certificate_status { :good }
end
end
......@@ -111,6 +111,22 @@ describe Gitlab::X509::Commit do
expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes)
expect(signature.persisted?).to be_truthy
end
context 'revoked certificate' do
let(:x509_issuer) { create(:x509_issuer, user1_issuer_attributes) }
let!(:x509_certificate) { create(:x509_certificate, user1_certificate_attributes.merge(x509_issuer_id: x509_issuer.id, certificate_status: :revoked)) }
it 'returns an unverified signature' do
expect(signature).to have_attributes(
commit_sha: commit_sha,
project: project,
verification_status: 'unverified'
)
expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes)
expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes)
expect(signature.persisted?).to be_truthy
end
end
end
context 'without trusted certificate within store' do
......
......@@ -43,6 +43,28 @@ RSpec.describe X509Certificate do
expect(certificate.subject).to eq(subject)
expect(certificate.email).to eq(email)
end
it 'calls mark_commit_signatures_unverified' do
expect_any_instance_of(described_class).to receive(:mark_commit_signatures_unverified)
described_class.safe_create!(attributes)
end
context 'certificate revocation handling' do
let(:x509_certificate) { create(:x509_certificate) }
it 'starts a revoke worker if certificate is revoked' do
expect(X509CertificateRevokeWorker).to receive(:perform_async).with(x509_certificate.id)
x509_certificate.revoked!
end
it 'does not starts a revoke worker for good certificates' do
expect(X509CertificateRevokeWorker).not_to receive(:perform_async).with(x509_certificate.id)
x509_certificate
end
end
end
describe 'validators' do
......
# frozen_string_literal: true
require 'spec_helper'
describe X509CertificateRevokeService do
describe '#execute' do
let(:service) { described_class.new }
let!(:x509_signature_1) { create(:x509_commit_signature, x509_certificate: x509_certificate, verification_status: :verified ) }
let!(:x509_signature_2) { create(:x509_commit_signature, x509_certificate: x509_certificate, verification_status: :verified ) }
context 'for revoked certificates' do
let(:x509_certificate) { create(:x509_certificate, certificate_status: :revoked ) }
it 'update all commit signatures' do
expect do
service.execute(x509_certificate)
x509_signature_1.reload
x509_signature_2.reload
end
.to change(x509_signature_1, :verification_status).from('verified').to('unverified')
.and change(x509_signature_2, :verification_status).from('verified').to('unverified')
end
end
context 'for good certificates' do
RSpec::Matchers.define_negated_matcher :not_change, :change
let(:x509_certificate) { create(:x509_certificate) }
it 'do not update any commit signature' do
expect do
service.execute(x509_certificate)
x509_signature_1.reload
x509_signature_2.reload
end
.to not_change(x509_signature_1, :verification_status)
.and not_change(x509_signature_2, :verification_status)
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe X509CertificateRevokeWorker do
describe '#perform' do
context 'with a revoked certificate' do
subject { described_class.new }
let(:x509_certificate) { create(:x509_certificate, certificate_status: :revoked) }
let(:job_args) { x509_certificate.id }
include_examples 'an idempotent worker' do
it 'executes the revoke service' do
spy_service = X509CertificateRevokeService.new
allow(X509CertificateRevokeService).to receive(:new) { spy_service }
expect(spy_service).to receive(:execute)
.exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES).times
.with(x509_certificate)
.and_call_original
subject
end
end
it 'executes the revoke service' do
spy_service = X509CertificateRevokeService.new
allow(X509CertificateRevokeService).to receive(:new) { spy_service }
expect_next_instance_of(X509CertificateRevokeService) do |service|
expect(service).to receive(:execute).with(x509_certificate)
end
subject
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment