mirror of
https://github.com/internetee/registry.git
synced 2025-07-27 13:06:18 +02:00
feat: Implement P12 certificate generation improvements
- Replace hardcoded P12 password with randomly generated one - Add p12_password column to certificates table - Update certificate serializer to include p12 password in response - Remove deprecated certificate revocation logic - Add tests for certificate revocation functionality - Implement async P12 generation via Sidekiq job - Add job uniqueness to prevent parallel certificate generation Migration changes: - Replace p12_password_digest with p12_password column - Add safety measures for column removal
This commit is contained in:
parent
072f4440e2
commit
ae96863b88
13 changed files with 157 additions and 64 deletions
|
@ -517,6 +517,10 @@ GEM
|
|||
connection_pool (>= 2.3.0)
|
||||
rack (>= 2.2.4)
|
||||
redis-client (>= 0.14.0)
|
||||
sidekiq-unique-jobs (8.0.10)
|
||||
concurrent-ruby (~> 1.0, >= 1.0.5)
|
||||
sidekiq (>= 7.0.0, < 8.0.0)
|
||||
thor (>= 1.0, < 3.0)
|
||||
simplecov (0.17.1)
|
||||
docile (~> 1.1)
|
||||
json (>= 1.8, < 3)
|
||||
|
@ -655,6 +659,7 @@ DEPENDENCIES
|
|||
selectize-rails (= 0.12.6)
|
||||
selenium-webdriver (~> 4.26)
|
||||
sidekiq (~> 7.0)
|
||||
sidekiq-unique-jobs (~> 8.0)
|
||||
simplecov (= 0.17.1)
|
||||
simpleidn (= 0.2.3)
|
||||
spy
|
||||
|
|
|
@ -13,8 +13,8 @@ module Repp
|
|||
api_user_id = p12_params[:api_user_id]
|
||||
render_error(I18n.t('errors.messages.not_found'), :not_found) and return if api_user_id.blank?
|
||||
|
||||
certificate = ::Certificates::CertificateGenerator.new(api_user_id: api_user_id, interface: 'registrar').execute
|
||||
render_success(data: { certificate: certificate })
|
||||
P12GeneratorJob.perform_later(api_user_id)
|
||||
render_success(message: 'P12 certificate generation started. Please refresh the page in a few seconds.')
|
||||
end
|
||||
|
||||
private
|
||||
|
|
|
@ -98,7 +98,6 @@ module Repp
|
|||
end
|
||||
|
||||
def notify_admins
|
||||
# Simply use AdminUser model to get all admin emails
|
||||
admin_users_emails = AdminUser.pluck(:email).reject(&:blank?)
|
||||
|
||||
return if admin_users_emails.empty?
|
||||
|
|
17
app/jobs/p12_generator_job.rb
Normal file
17
app/jobs/p12_generator_job.rb
Normal file
|
@ -0,0 +1,17 @@
|
|||
class P12GeneratorJob < ApplicationJob
|
||||
queue_as :default
|
||||
|
||||
sidekiq_options(
|
||||
unique: :until_executed,
|
||||
lock_timeout: 1.hour
|
||||
)
|
||||
|
||||
def perform(api_user_id)
|
||||
api_user = ApiUser.find(api_user_id)
|
||||
|
||||
Certificates::CertificateGenerator.new(
|
||||
api_user_id: api_user_id,
|
||||
interface: 'registrar'
|
||||
).execute
|
||||
end
|
||||
end
|
|
@ -4,6 +4,8 @@ class Certificate < ApplicationRecord
|
|||
include Versions
|
||||
include Certificate::CertificateConcern
|
||||
|
||||
self.ignored_columns = ["p12_password_digest"]
|
||||
|
||||
belongs_to :api_user
|
||||
|
||||
SIGNED = 'signed'.freeze
|
||||
|
@ -20,6 +22,13 @@ class Certificate < ApplicationRecord
|
|||
scope 'registrar', -> { where(interface: REGISTRAR) }
|
||||
scope 'unrevoked', -> { where(revoked: false) }
|
||||
|
||||
# Базовые причины отзыва (самые частые)
|
||||
REVOCATION_REASONS = {
|
||||
unspecified: 0,
|
||||
key_compromise: 1,
|
||||
cessation_of_operation: 5
|
||||
}.freeze
|
||||
|
||||
validate :validate_csr_and_crt_presence
|
||||
def validate_csr_and_crt_presence
|
||||
return if csr.try(:scrub).present? || crt.try(:scrub).present?
|
||||
|
@ -65,14 +74,14 @@ class Certificate < ApplicationRecord
|
|||
return nil if p12.blank?
|
||||
|
||||
decoded_p12 = Base64.decode64(p12)
|
||||
OpenSSL::PKCS12.new(decoded_p12, Certificates::CertificateGenerator::P12_PASSWORD)
|
||||
OpenSSL::PKCS12.new(decoded_p12, p12_password)
|
||||
rescue OpenSSL::PKCS12::PKCS12Error => e
|
||||
Rails.logger.error("Failed to parse PKCS12: #{e.message}")
|
||||
nil
|
||||
end
|
||||
|
||||
def revoked?
|
||||
status == REVOKED
|
||||
revoked
|
||||
end
|
||||
|
||||
def revokable?
|
||||
|
@ -81,17 +90,10 @@ class Certificate < ApplicationRecord
|
|||
|
||||
def status
|
||||
return UNSIGNED if crt.blank?
|
||||
return @cached_status if @cached_status
|
||||
return REVOKED if revoked?
|
||||
return EXPIRED if expires_at && expires_at < Time.current
|
||||
|
||||
@cached_status = SIGNED
|
||||
|
||||
if certificate_expired?
|
||||
@cached_status = EXPIRED
|
||||
elsif certificate_revoked?
|
||||
@cached_status = REVOKED
|
||||
end
|
||||
|
||||
@cached_status
|
||||
SIGNED
|
||||
end
|
||||
|
||||
def sign!(password:)
|
||||
|
@ -106,18 +108,14 @@ class Certificate < ApplicationRecord
|
|||
false
|
||||
end
|
||||
|
||||
def revoke!(password:)
|
||||
crt_file = create_tempfile('client_crt', crt)
|
||||
def revoke!(password:, reason: :unspecified)
|
||||
return false unless password == ENV['ca_key_password']
|
||||
|
||||
err_output = execute_openssl_revoke_command(password, crt_file.path)
|
||||
|
||||
if revocation_successful?(err_output)
|
||||
update_revocation_status
|
||||
self.class.update_crl
|
||||
return self
|
||||
end
|
||||
|
||||
handle_revocation_failure(err_output)
|
||||
update!(
|
||||
revoked: true,
|
||||
revoked_at: Time.current,
|
||||
revoked_reason: REVOCATION_REASONS[reason]
|
||||
)
|
||||
end
|
||||
|
||||
private
|
||||
|
|
|
@ -3,17 +3,6 @@ module Certificate::CertificateConcern
|
|||
extend ActiveSupport::Concern
|
||||
|
||||
class_methods do
|
||||
def tostdout(message)
|
||||
time = Time.zone.now.utc
|
||||
$stdout << "#{time} - #{message}\n" unless Rails.env.test?
|
||||
end
|
||||
|
||||
def update_crl
|
||||
tostdout('Running crlupdater')
|
||||
system('/bin/bash', ENV['crl_updater_path'].to_s)
|
||||
tostdout('Finished running crlupdater')
|
||||
end
|
||||
|
||||
def parse_md_from_string(crt)
|
||||
return if crt.blank?
|
||||
|
||||
|
|
|
@ -3,15 +3,14 @@ module Certificates
|
|||
attribute :api_user_id, Types::Coercible::Integer
|
||||
attribute? :interface, Types::String.optional
|
||||
|
||||
P12_PASSWORD = 'todo-change-me'
|
||||
|
||||
def execute
|
||||
api_user = ApiUser.find(api_user_id)
|
||||
password = generate_random_password
|
||||
|
||||
private_key = generate_user_key
|
||||
csr = generate_user_csr(private_key)
|
||||
certificate = sign_user_certificate(csr)
|
||||
p12 = create_user_p12(private_key, certificate)
|
||||
p12 = create_user_p12(private_key, certificate, password)
|
||||
|
||||
certificate_record = api_user.certificates.build(
|
||||
private_key: private_key.to_pem,
|
||||
|
@ -20,7 +19,7 @@ module Certificates
|
|||
p12: Base64.strict_encode64(p12),
|
||||
expires_at: certificate.not_after,
|
||||
interface: interface || 'registrar',
|
||||
p12_password_digest: P12_PASSWORD,
|
||||
p12_password: password,
|
||||
serial: certificate.serial.to_s,
|
||||
common_name: api_user.username
|
||||
)
|
||||
|
@ -124,7 +123,7 @@ module Certificates
|
|||
cert
|
||||
end
|
||||
|
||||
def create_user_p12(key, cert, password = P12_PASSWORD)
|
||||
def create_user_p12(key, cert, password)
|
||||
ca_cert = OpenSSL::X509::Certificate.new(File.read(ca_cert_path))
|
||||
|
||||
p12 = OpenSSL::PKCS12.create(
|
||||
|
@ -137,5 +136,11 @@ module Certificates
|
|||
|
||||
p12.to_der
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def generate_random_password
|
||||
SecureRandom.hex(8)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,21 @@
|
|||
class ChangeP12PasswordDigestToP12PasswordInCertificates < ActiveRecord::Migration[6.1]
|
||||
def up
|
||||
add_column :certificates, :p12_password, :string
|
||||
|
||||
Certificate.find_each do |cert|
|
||||
cert.update_column(:p12_password, cert.p12_password_digest)
|
||||
end
|
||||
|
||||
safety_assured { remove_column :certificates, :p12_password_digest }
|
||||
end
|
||||
|
||||
def down
|
||||
add_column :certificates, :p12_password_digest, :string
|
||||
|
||||
Certificate.find_each do |cert|
|
||||
cert.update_column(:p12_password_digest, cert.p12_password)
|
||||
end
|
||||
|
||||
safety_assured { remove_column :certificates, :p12_password }
|
||||
end
|
||||
end
|
|
@ -590,11 +590,11 @@ CREATE TABLE public.certificates (
|
|||
revoked boolean DEFAULT false NOT NULL,
|
||||
private_key bytea,
|
||||
p12 bytea,
|
||||
p12_password_digest character varying,
|
||||
expires_at timestamp without time zone,
|
||||
serial character varying,
|
||||
revoked_at timestamp without time zone,
|
||||
revoked_reason integer
|
||||
revoked_reason integer,
|
||||
p12_password character varying
|
||||
);
|
||||
|
||||
|
||||
|
@ -5727,6 +5727,7 @@ INSERT INTO "schema_migrations" (version) VALUES
|
|||
('20241206085817'),
|
||||
('20250204094550'),
|
||||
('20250219102811'),
|
||||
('20250313122119');
|
||||
('20250313122119'),
|
||||
('20250319104749');
|
||||
|
||||
|
||||
|
|
|
@ -79,7 +79,8 @@ module Serializers
|
|||
def p12_data(obj)
|
||||
{
|
||||
body: obj.p12,
|
||||
type: 'PKCS12'
|
||||
type: 'PKCS12',
|
||||
password: obj.p12_password
|
||||
}
|
||||
end
|
||||
|
||||
|
|
34
test/fixtures/files/test_ca/crl/crl.pem
vendored
34
test/fixtures/files/test_ca/crl/crl.pem
vendored
|
@ -1,22 +1,22 @@
|
|||
-----BEGIN X509 CRL-----
|
||||
MIIDkjCCAXoCAQEwDQYJKoZIhvcNAQELBQAwgZUxCzAJBgNVBAYTAkVFMREwDwYD
|
||||
MIIDuzCCAaMCAQEwDQYJKoZIhvcNAQELBQAwgZUxCzAJBgNVBAYTAkVFMREwDwYD
|
||||
VQQIDAhIYXJqdW1hYTEQMA4GA1UEBwwHVGFsbGlubjEjMCEGA1UECgwaRWVzdGkg
|
||||
SW50ZXJuZXRpIFNpaHRhc3V0dXMxGjAYBgNVBAMMEWVwcF9wcm94eSB0ZXN0IGNh
|
||||
MSAwHgYJKoZIhvcNAQkBFhFoZWxsb0BpbnRlcm5ldC5lZRcNMTkwNzI5MDc1NTA5
|
||||
WhcNMjkwNzI2MDc1NTA5WjB+MBMCAhACFw0xOTA1MjkwNjM5MTJaMBMCAhADFw0x
|
||||
OTA1MjkwODQxMDJaMBMCAhAEFw0xOTA1MzExMTI0NTJaMBMCAhAFFw0xOTA1MzEx
|
||||
MTQyMjJaMBMCAhAGFw0xOTA1MzExMjQzNDlaMBMCAhAHFw0xOTA3MjkwNzU0MzRa
|
||||
oDAwLjAfBgNVHSMEGDAWgBT9d+ZKc72lPGWzuc+1FZVZCGRoEDALBgNVHRQEBAIC
|
||||
EAkwDQYJKoZIhvcNAQELBQADggIBAEk9pyZjqyYUdnA0Sv7RyevRUQGKbbf3EXdv
|
||||
JLDyvI9rpoyuWPkMT6vPsYght0cf/wO7oaEK/uustvFEYQiJss60jI0XuczWypk9
|
||||
paKu3LhIy6Drm3locY2k0ESrgP9IwNzS5Xr0FiaWRIozbkcawte8M4Nqe8BO5prk
|
||||
/5sLjv3eFnD7E445tZhu3vmXkD50FT3PLHVBEz4yS6Fx6nTiv+9QUu8NGf+bc6+o
|
||||
YKPMy6Lh/wGC7p6sZJCOCjfzLAcqWfB2EW6XU8WeQcQCZ0au7zvZjQownCS9CeJV
|
||||
KVsC4QiUt97FxR2gcEN2GJesywIF11X9o8s1K/Hz3+rrtU1ymoMLeumaRW24z35A
|
||||
zVsdNwRfSPmt1qHlyaJaFhKG6jw5/nws+/wGFycIjWK0DSORiGCYdKD0cCjKJbNO
|
||||
2QJnJlNOaCUUj8ULyiFOtZvdadc4JVW42NI/F+AFy/bnBK0uH6CenK5XwX3kEMme
|
||||
KD8b5reUcVRhQdVJdAABFJlihIg05yENI7hlH1CKfy4vmlBKl+M2mW9cmNO8O6uS
|
||||
KMH8/wLuLga9gYziNT1RmVNFbnpF0hc6CFtSnlVXXTlU/TrxheH8ykrHQhKEkQj+
|
||||
3krObDFDCUMKmaGu2nxRYZwLXzUe3wVl1SAxw0eEGyON/N83sLYlcrwWTVzRG3Z7
|
||||
RqRHPn+h
|
||||
WhcNMjkwNzI2MDc1NTA5WjCBpjASAgEAFw0yNTAzMTgxMDI5MzdaMBICAQAXDTI1
|
||||
MDMxODEwMjkzN1owEwICEAIXDTE5MDUyOTA2MzkxMlowEwICEAMXDTE5MDUyOTA4
|
||||
NDEwMlowEwICEAQXDTE5MDUzMTExMjQ1MlowEwICEAUXDTE5MDUzMTExNDIyMlow
|
||||
EwICEAYXDTE5MDUzMTEyNDM0OVowEwICEAcXDTE5MDcyOTA3NTQzNFqgMDAuMB8G
|
||||
A1UdIwQYMBaAFP135kpzvaU8ZbO5z7UVlVkIZGgQMAsGA1UdFAQEAgIQCTANBgkq
|
||||
hkiG9w0BAQsFAAOCAgEAST2nJmOrJhR2cDRK/tHJ69FRAYptt/cRd28ksPK8j2um
|
||||
jK5Y+QxPq8+xiCG3Rx//A7uhoQr+66y28URhCImyzrSMjRe5zNbKmT2loq7cuEjL
|
||||
oOubeWhxjaTQRKuA/0jA3NLlevQWJpZEijNuRxrC17wzg2p7wE7mmuT/mwuO/d4W
|
||||
cPsTjjm1mG7e+ZeQPnQVPc8sdUETPjJLoXHqdOK/71BS7w0Z/5tzr6hgo8zLouH/
|
||||
AYLunqxkkI4KN/MsBypZ8HYRbpdTxZ5BxAJnRq7vO9mNCjCcJL0J4lUpWwLhCJS3
|
||||
3sXFHaBwQ3YYl6zLAgXXVf2jyzUr8fPf6uu1TXKagwt66ZpFbbjPfkDNWx03BF9I
|
||||
+a3WoeXJoloWEobqPDn+fCz7/AYXJwiNYrQNI5GIYJh0oPRwKMols07ZAmcmU05o
|
||||
JRSPxQvKIU61m91p1zglVbjY0j8X4AXL9ucErS4foJ6crlfBfeQQyZ4oPxvmt5Rx
|
||||
VGFB1Ul0AAEUmWKEiDTnIQ0juGUfUIp/Li+aUEqX4zaZb1yY07w7q5Iowfz/Au4u
|
||||
Br2BjOI1PVGZU0VuekXSFzoIW1KeVVddOVT9OvGF4fzKSsdCEoSRCP7eSs5sMUMJ
|
||||
QwqZoa7afFFhnAtfNR7fBWXVIDHDR4QbI4383zewtiVyvBZNXNEbdntGpEc+f6E=
|
||||
-----END X509 CRL-----
|
||||
|
|
25
test/jobs/p12_generator_job_test.rb
Normal file
25
test/jobs/p12_generator_job_test.rb
Normal file
|
@ -0,0 +1,25 @@
|
|||
require 'test_helper'
|
||||
|
||||
class P12GeneratorJobTest < ActiveJob::TestCase
|
||||
test "ensures only one job runs at a time" do
|
||||
Sidekiq::Testing.inline!
|
||||
|
||||
api_user = users(:api_bestnames)
|
||||
|
||||
thread1 = Thread.new do
|
||||
P12GeneratorJob.perform_later(api_user.id)
|
||||
end
|
||||
|
||||
sleep(2)
|
||||
|
||||
thread2 = Thread.new do
|
||||
P12GeneratorJob.perform_later(api_user.id)
|
||||
end
|
||||
|
||||
thread1.join
|
||||
thread2.join
|
||||
|
||||
ensure
|
||||
Sidekiq::Testing.fake!
|
||||
end
|
||||
end
|
|
@ -9,4 +9,36 @@ class CertificateTest < ActiveSupport::TestCase
|
|||
def test_certificate_sign_returns_false
|
||||
assert_not @certificate.sign!(password: ENV['ca_key_password']), 'false'
|
||||
end
|
||||
|
||||
# Revocation tests
|
||||
def test_revoke_with_valid_password
|
||||
assert @certificate.revoke!(password: ENV['ca_key_password'])
|
||||
assert @certificate.revoked?
|
||||
assert_not_nil @certificate.revoked_at
|
||||
assert_equal Certificate::REVOCATION_REASONS[:unspecified], @certificate.revoked_reason
|
||||
end
|
||||
|
||||
def test_revoke_with_invalid_password
|
||||
assert_not @certificate.revoke!(password: 'wrong_password')
|
||||
assert_not @certificate.revoked?
|
||||
assert_nil @certificate.revoked_at
|
||||
assert_nil @certificate.revoked_reason
|
||||
end
|
||||
|
||||
def test_revoke_updates_certificate_status
|
||||
assert_equal Certificate::SIGNED, @certificate.status
|
||||
@certificate.revoke!(password: ENV['ca_key_password'])
|
||||
assert_equal Certificate::REVOKED, @certificate.status
|
||||
end
|
||||
|
||||
def test_revokable_for_different_interfaces
|
||||
@certificate.update!(interface: Certificate::REGISTRAR)
|
||||
assert @certificate.revokable?
|
||||
|
||||
@certificate.update!(interface: Certificate::API)
|
||||
assert_not @certificate.revokable?
|
||||
|
||||
@certificate.update!(interface: Certificate::REGISTRAR, crt: nil)
|
||||
assert_not @certificate.revokable?
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue