diff --git a/Gemfile.lock b/Gemfile.lock index aaa2d9a94..210819a75 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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 diff --git a/app/controllers/repp/v1/certificates/p12_controller.rb b/app/controllers/repp/v1/certificates/p12_controller.rb index 13f5283fd..c35d73566 100644 --- a/app/controllers/repp/v1/certificates/p12_controller.rb +++ b/app/controllers/repp/v1/certificates/p12_controller.rb @@ -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 diff --git a/app/controllers/repp/v1/certificates_controller.rb b/app/controllers/repp/v1/certificates_controller.rb index dccee8850..aa59f8346 100644 --- a/app/controllers/repp/v1/certificates_controller.rb +++ b/app/controllers/repp/v1/certificates_controller.rb @@ -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? diff --git a/app/jobs/p12_generator_job.rb b/app/jobs/p12_generator_job.rb new file mode 100644 index 000000000..9bf4fa5ec --- /dev/null +++ b/app/jobs/p12_generator_job.rb @@ -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 diff --git a/app/models/certificate.rb b/app/models/certificate.rb index a885f462a..bd4e964a7 100644 --- a/app/models/certificate.rb +++ b/app/models/certificate.rb @@ -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 - - @cached_status = SIGNED - - if certificate_expired? - @cached_status = EXPIRED - elsif certificate_revoked? - @cached_status = REVOKED - end - - @cached_status + return REVOKED if revoked? + return EXPIRED if expires_at && expires_at < Time.current + + 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 diff --git a/app/models/concerns/certificate/certificate_concern.rb b/app/models/concerns/certificate/certificate_concern.rb index b56e92e43..f407d8d10 100644 --- a/app/models/concerns/certificate/certificate_concern.rb +++ b/app/models/concerns/certificate/certificate_concern.rb @@ -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? diff --git a/app/services/certificates/certificate_generator.rb b/app/services/certificates/certificate_generator.rb index a1f9a6613..270d0000a 100644 --- a/app/services/certificates/certificate_generator.rb +++ b/app/services/certificates/certificate_generator.rb @@ -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 \ No newline at end of file diff --git a/db/migrate/20250319104749_change_p12_password_digest_to_p12_password_in_certificates.rb b/db/migrate/20250319104749_change_p12_password_digest_to_p12_password_in_certificates.rb new file mode 100644 index 000000000..3a594e415 --- /dev/null +++ b/db/migrate/20250319104749_change_p12_password_digest_to_p12_password_in_certificates.rb @@ -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 diff --git a/db/structure.sql b/db/structure.sql index 4d1270cd1..02f8f6d61 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -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'); diff --git a/lib/serializers/repp/certificate.rb b/lib/serializers/repp/certificate.rb index 4b40b9e47..4d2bca41d 100644 --- a/lib/serializers/repp/certificate.rb +++ b/lib/serializers/repp/certificate.rb @@ -79,7 +79,8 @@ module Serializers def p12_data(obj) { body: obj.p12, - type: 'PKCS12' + type: 'PKCS12', + password: obj.p12_password } end diff --git a/test/fixtures/files/test_ca/crl/crl.pem b/test/fixtures/files/test_ca/crl/crl.pem index e0e265c99..4acbebfec 100644 --- a/test/fixtures/files/test_ca/crl/crl.pem +++ b/test/fixtures/files/test_ca/crl/crl.pem @@ -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----- diff --git a/test/jobs/p12_generator_job_test.rb b/test/jobs/p12_generator_job_test.rb new file mode 100644 index 000000000..b102361ff --- /dev/null +++ b/test/jobs/p12_generator_job_test.rb @@ -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 diff --git a/test/models/certificate_test.rb b/test/models/certificate_test.rb index 2319b4058..08a0dac3d 100644 --- a/test/models/certificate_test.rb +++ b/test/models/certificate_test.rb @@ -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