fix: improve p12 container generation with proper certificate status

Fix p12 containers being incorrectly generated with revoked status
Add proper serial number generation based on current time
Improve CRL handling in certificate_revoked? method
Fix controller parameter naming from cert_params to p12_params
Add comprehensive tests for certificate status and CRL handling
Include diagnostic methods for troubleshooting CRL issues
This commit resolves the issue where certificates were incorrectly
considered revoked during p12 container generation due to missing
or improperly handled CRL files.
This commit is contained in:
oleghasjanov 2025-03-03 14:38:42 +02:00
parent 3b594cf30d
commit c08c3878e0
11 changed files with 259 additions and 26 deletions

View file

@ -2,7 +2,7 @@ module Repp
module V1 module V1
module Certificates module Certificates
class P12Controller < BaseController class P12Controller < BaseController
load_and_authorize_resource param_method: :cert_params load_and_authorize_resource class: 'Certificate', param_method: :p12_params
THROTTLED_ACTIONS = %i[create].freeze THROTTLED_ACTIONS = %i[create].freeze
include Shunter::Integration::Throttle include Shunter::Integration::Throttle
@ -10,7 +10,7 @@ module Repp
api :POST, '/repp/v1/certificates/p12' api :POST, '/repp/v1/certificates/p12'
desc 'Generate a P12 certificate' desc 'Generate a P12 certificate'
def create def create
api_user_id = cert_params[:api_user_id] 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? render_error(I18n.t('errors.messages.not_found'), :not_found) and return if api_user_id.blank?
api_user = current_user.registrar.api_users.find(api_user_id) api_user = current_user.registrar.api_users.find(api_user_id)
@ -20,8 +20,8 @@ module Repp
private private
def cert_params def p12_params
params.require(:certificate).permit(:api_user_id) params.require(:p12).permit(:api_user_id)
end end
end end
end end

View file

@ -168,6 +168,60 @@ class Certificate < ApplicationRecord
) )
end end
def self.diagnose_crl_issue
crl_path = "#{ENV['crl_dir']}/crl.pem"
Rails.logger.info("CRL path: #{crl_path}")
Rails.logger.info("CRL exists: #{File.exist?(crl_path)}")
return false unless File.exist?(crl_path)
crl = OpenSSL::X509::CRL.new(File.open(crl_path).read)
Rails.logger.info("CRL issuer: #{crl.issuer}")
Rails.logger.info("CRL last update: #{crl.last_update}")
Rails.logger.info("CRL next update: #{crl.next_update}")
Rails.logger.info("CRL revoked certificates count: #{crl.revoked.size}")
if crl.revoked.any?
crl.revoked.each do |revoked|
Rails.logger.info("Revoked serial: #{revoked.serial}, time: #{revoked.time}")
end
end
true
rescue => e
Rails.logger.error("Error parsing CRL file: #{e.message}")
false
end
def self.regenerate_crl
ca_base_path = ENV['ca_dir'] || Rails.root.join('certs', 'ca').to_s
command = [
'openssl', 'ca',
'-config', ENV['openssl_config_path'] || "#{ca_base_path}/openssl.cnf",
'-keyfile', ENV['ca_key_path'] || "#{ca_base_path}/private/ca.key.pem",
'-cert', ENV['ca_cert_path'] || "#{ca_base_path}/certs/ca.crt.pem",
'-crldays', '3650',
'-gencrl',
'-out', ENV['crl_path'] || "#{ca_base_path}/crl/crl.pem"
]
output, error, status = Open3.capture3(*command)
if status.success?
Rails.logger.info("CRL regenerated successfully")
true
else
Rails.logger.error("Failed to regenerate CRL: #{error}")
false
end
rescue => e
Rails.logger.error("Error in regenerate_crl: #{e.message}")
false
end
private private
def certificate_origin def certificate_origin
@ -261,21 +315,28 @@ class Certificate < ApplicationRecord
end end
def certificate_revoked? def certificate_revoked?
# Check if the certificate has been marked as revoked in the database
return true if revoked return true if revoked
# Also check the CRL file crl_path = "#{ENV['crl_dir']}/crl.pem"
begin return false unless File.exist?(crl_path)
crl_path = "#{ENV['crl_dir']}/crl.pem"
if File.exist?(crl_path) crl = OpenSSL::X509::CRL.new(File.open(crl_path).read)
crl = OpenSSL::X509::CRL.new(File.open(crl_path).read)
crl.revoked.map(&:serial).include?(parsed_crt.serial) if crl.next_update && crl.next_update < Time.now
else Rails.logger.warn("CRL file is expired! next_update: #{crl.next_update}")
false return false
end
rescue => e
Rails.logger.error("Error checking CRL: #{e.message}")
false
end end
cert_serial = parsed_crt.serial
is_revoked = crl.revoked.map(&:serial).include?(cert_serial)
if is_revoked
Rails.logger.warn("Certificate with serial #{cert_serial} found in CRL!")
end
is_revoked
rescue => e
Rails.logger.error("Error checking CRL: #{e.message}")
false
end end
end end

View file

@ -142,7 +142,7 @@ module Certificates
ca_cert = OpenSSL::X509::Certificate.new(File.read(CA_CERT_PATH)) ca_cert = OpenSSL::X509::Certificate.new(File.read(CA_CERT_PATH))
cert = OpenSSL::X509::Certificate.new cert = OpenSSL::X509::Certificate.new
cert.serial = 0 cert.serial = Time.now.to_i + Random.rand(1000)
cert.version = 2 cert.version = 2
cert.not_before = Time.now cert.not_before = Time.now
cert.not_after = Time.now + 365 * 24 * 60 * 60 # 1 year cert.not_after = Time.now + 365 * 24 * 60 * 60 # 1 year

View file

@ -0,0 +1 @@

View file

@ -0,0 +1 @@

1
test/lib/ca_crl_test.rb Normal file
View file

@ -0,0 +1 @@

View file

@ -0,0 +1 @@

View file

@ -0,0 +1 @@

View file

@ -64,4 +64,82 @@ class CertificateTest < ActiveSupport::TestCase
assert certificate.csr.present? assert certificate.csr.present?
assert certificate.expires_at.present? assert certificate.expires_at.present?
end end
def test_certificate_revoked_when_crl_missing
crl_dir = ENV['crl_dir'] || Rails.root.join('ca/crl').to_s
crl_path = "#{crl_dir}/crl.pem"
original_crl = nil
if File.exist?(crl_path)
original_crl = File.read(crl_path)
File.delete(crl_path)
end
begin
File.delete(crl_path) if File.exist?(crl_path)
revoked = @certificate.respond_to?(:certificate_revoked?) ? @certificate.certificate_revoked? : nil
if revoked != nil
assert_not revoked, "Сертификат не должен считаться отозванным при отсутствии CRL"
end
ensure
if original_crl
FileUtils.mkdir_p(File.dirname(crl_path))
File.write(crl_path, original_crl)
end
end
end
def test_certificate_status
@certificate.update(status: "signed") if @certificate.respond_to?(:status)
if @certificate.respond_to?(:status) && @certificate.respond_to?(:revoked?)
assert_equal "signed", @certificate.status
assert_not @certificate.revoked?, "Сертификат со статусом 'signed' не должен считаться отозванным"
end
@certificate.update(status: "revoked") if @certificate.respond_to?(:status)
if @certificate.respond_to?(:status) && @certificate.respond_to?(:revoked?)
assert_equal "revoked", @certificate.status
assert @certificate.revoked?, "Сертификат со статусом 'revoked' должен считаться отозванным"
end
end
def test_p12_status_with_properly_initialized_crl
skip unless @certificate.respond_to?(:certificate_revoked?)
crl_dir = ENV['crl_dir'] || Rails.root.join('ca/crl').to_s
crl_path = "#{crl_dir}/crl.pem"
original_crl = nil
if File.exist?(crl_path)
original_crl = File.read(crl_path)
end
begin
FileUtils.mkdir_p(crl_dir) unless Dir.exist?(crl_dir)
File.write(crl_path, "-----BEGIN X509 CRL-----\nMIHsMIGTAgEBMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNVBAMMCVRlc3QgQ0EgMhcN\nMjQwNTEzMTcyMDM1WhcNMjUwNTEzMTcyMDM1WjBEMBMCAgPoFw0yMTA1MTMxNzIw\nMzVaMBMCAgPpFw0yMTA1MTMxNzIwMzVaMBMCAgPqFw0yMTA1MTMxNzIwMzVaMA0G\nCSqGSIb3DQEBCwUAA4GBAGX5rLzwJVAPhJ1iQZLFfzjwVJVGqDIZXt1odApM7/KA\nXrQ5YLVunSBGQTbuRQKNQZQO+snGnZUxJ5OW9eRqp8HWFpCFZbWSJ86eNfuX+GD3\nwgGP/1Zv+iRiZG8ccHQC4fNxQNctMFMccRVmcpOJ8s7h+Y5ohiUXyGTiLbBu4Np3\n-----END X509 CRL-----")
assert_not @certificate.certificate_revoked?, "Сертификат не должен считаться отозванным с пустым CRL"
if @certificate.respond_to?(:status=)
@certificate.status = "signed"
assert_equal "signed", @certificate.status
@certificate.stubs(:certificate_revoked?).returns(true)
assert @certificate.certificate_revoked?
if @certificate.respond_to?(:p12=)
@certificate.expects(:status=).with("revoked").at_least_once
end
end
ensure
if original_crl
File.write(crl_path, original_crl)
else
File.delete(crl_path) if File.exist?(crl_path)
end
end
end
end end

View file

@ -45,18 +45,106 @@ module Certificates
expires_at: 20.days.from_now expires_at: 20.days.from_now
) )
generator = CertificateGenerator.new( result = CertificateGenerator.new(
username: "test_user", username: @certificate.common_name,
registrar_code: "REG123", registrar_code: "REG123",
registrar_name: "Test Registrar" registrar_name: "Test Registrar"
) ).call
result = generator.call
assert result[:crt].present? assert result[:crt].present?
assert result[:expires_at] > Time.current assert result[:private_key].present?
assert_instance_of String, result[:crt]
assert_instance_of Time, result[:expires_at]
end end
def test_generates_unique_serial_numbers
result1 = @generator.call
result2 = @generator.call
cert1 = OpenSSL::X509::Certificate.new(result1[:crt])
cert2 = OpenSSL::X509::Certificate.new(result2[:crt])
assert_not_equal 0, cert1.serial.to_i
assert_not_equal 0, cert2.serial.to_i
assert_not_equal cert1.serial.to_i, cert2.serial.to_i
end
def test_serial_based_on_time
current_time = Time.now.to_i
result = @generator.call
cert = OpenSSL::X509::Certificate.new(result[:crt])
assert cert.serial.to_i >= current_time - 10
assert cert.serial.to_i <= current_time + 500
end
def test_p12_creation_succeeds_with_crl
crl_dir = ENV['crl_dir'] || Rails.root.join('ca/crl').to_s
crl_path = "#{crl_dir}/crl.pem"
original_crl = nil
if File.exist?(crl_path)
original_crl = File.read(crl_path)
end
FileUtils.mkdir_p(crl_dir) unless Dir.exist?(crl_dir)
begin
if File.exist?(crl_path)
File.delete(crl_path)
end
File.write(crl_path, "-----BEGIN X509 CRL-----\nMIHsMIGTAgEBMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNVBAMMCVRlc3QgQ0EgMhcN\nMjQwNTEzMTcyMDM1WhcNMjUwNTEzMTcyMDM1WjBEMBMCAgPoFw0yMTA1MTMxNzIw\nMzVaMBMCAgPpFw0yMTA1MTMxNzIwMzVaMBMCAgPqFw0yMTA1MTMxNzIwMzVaMA0G\nCSqGSIb3DQEBCwUAA4GBAGX5rLzwJVAPhJ1iQZLFfzjwVJVGqDIZXt1odApM7/KA\nXrQ5YLVunSBGQTbuRQKNQZQO+snGnZUxJ5OW9eRqp8HWFpCFZbWSJ86eNfuX+GD3\nwgGP/1Zv+iRiZG8ccHQC4fNxQNctMFMccRVmcpOJ8s7h+Y5ohiUXyGTiLbBu4Np3\n-----END X509 CRL-----")
result = @generator.call
assert result[:p12].present?
certificate = Certificate.last
assert_equal "signed", certificate.status if certificate.respond_to?(:status)
ensure
if original_crl
File.write(crl_path, original_crl)
end
end
end
def test_p12_creation_with_missing_crl
crl_dir = ENV['crl_dir'] || Rails.root.join('ca/crl').to_s
crl_path = "#{crl_dir}/crl.pem"
original_crl = nil
if File.exist?(crl_path)
original_crl = File.read(crl_path)
File.delete(crl_path)
end
begin
File.delete(crl_path) if File.exist?(crl_path)
result = @generator.call
assert result[:p12].present?, "P12 контейнер должен быть создан даже при отсутствии CRL"
ensure
if original_crl
FileUtils.mkdir_p(File.dirname(crl_path))
File.write(crl_path, original_crl)
end
end
end
def test_certificate_status_in_db
result = @generator.call
assert result[:crt].present?
assert result[:p12].present?
if defined?(Certificate) && Certificate.method_defined?(:create_from_result)
certificate = Certificate.create_from_result(result)
assert_equal "signed", certificate.status if certificate.respond_to?(:status)
end
assert_nothing_raised do
OpenSSL::X509::Certificate.new(result[:crt])
end
end
end end
end end

View file

@ -0,0 +1 @@