Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store revoked state in Certificate model #1578

Merged
merged 7 commits into from
May 14, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/api/repp/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class API < Grape::API
webclient_cert_name = ENV['webclient_cert_common_name'] || 'webclient'
error! "Webclient #{message} #{webclient_cert_name}", 401 if webclient_cert_name != request_name
else
unless @current_user.api_pki_ok?(request.env['HTTP_SSL_CLIENT_CERT'], request.env['HTTP_SSL_CLIENT_S_DN_CN'])
unless @current_user.pki_ok?(request.env['HTTP_SSL_CLIENT_CERT'],
request.env['HTTP_SSL_CLIENT_S_DN_CN'])
error! "#{message} #{@current_user.username}", 401
end
end
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/epp/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ def login
end

if !Rails.env.development? && (!webclient_request && @api_user)
unless @api_user.api_pki_ok?(request.env['HTTP_SSL_CLIENT_CERT'], request.env['HTTP_SSL_CLIENT_S_DN_CN'])
unless @api_user.pki_ok?(request.env['HTTP_SSL_CLIENT_CERT'],
request.env['HTTP_SSL_CLIENT_S_DN_CN'])
epp_errors << {
msg: 'Authentication error; server closing connection (certificate is not valid)',
code: '2501'
Expand Down
5 changes: 3 additions & 2 deletions app/controllers/registrar/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ def create
end

if @depp_user.pki
unless @api_user.registrar_pki_ok?(request.env['HTTP_SSL_CLIENT_CERT'], request.env['HTTP_SSL_CLIENT_S_DN_CN'])
unless @api_user.pki_ok?(request.env['HTTP_SSL_CLIENT_CERT'],
request.env['HTTP_SSL_CLIENT_S_DN_CN'], api: false)
@depp_user.errors.add(:base, :invalid_cert)
end
end
Expand Down Expand Up @@ -205,4 +206,4 @@ def show_error
redirect_to new_registrar_user_session_url, alert: @depp_user.errors.full_messages.first
end
end
end
end
32 changes: 16 additions & 16 deletions app/models/api_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,24 +64,14 @@ def unread_notifications
registrar.notifications.unread
end

def registrar_pki_ok?(crt, cn)
return false if crt.blank? || cn.blank?
crt = crt.split(' ').join("\n")
crt.gsub!("-----BEGIN\nCERTIFICATE-----\n", "-----BEGIN CERTIFICATE-----\n")
crt.gsub!("\n-----END\nCERTIFICATE-----", "\n-----END CERTIFICATE-----")
cert = OpenSSL::X509::Certificate.new(crt)
md5 = OpenSSL::Digest::MD5.new(cert.to_der).to_s
certificates.registrar.exists?(md5: md5, common_name: cn)
end
def pki_ok?(crt, com, api: true)
return false if crt.blank? || com.blank?

def api_pki_ok?(crt, cn)
return false if crt.blank? || cn.blank?
crt = crt.split(' ').join("\n")
crt.gsub!("-----BEGIN\nCERTIFICATE-----\n", "-----BEGIN CERTIFICATE-----\n")
crt.gsub!("\n-----END\nCERTIFICATE-----", "\n-----END CERTIFICATE-----")
cert = OpenSSL::X509::Certificate.new(crt)
origin = api ? certificates.api : certificates.registrar
cert = machine_readable_certificate(crt)
md5 = OpenSSL::Digest::MD5.new(cert.to_der).to_s
certificates.api.exists?(md5: md5, common_name: cn)

origin.exists?(md5: md5, common_name: com, revoked: false)
end

def linked_users
Expand All @@ -93,4 +83,14 @@ def linked_users
def linked_with?(another_api_user)
another_api_user.identity_code == self.identity_code
end

private

def machine_readable_certificate(cert)
cert = cert.split(' ').join("\n")
cert.gsub!("-----BEGIN\nCERTIFICATE-----\n", "-----BEGIN CERTIFICATE-----\n")
cert.gsub!("\n-----END\nCERTIFICATE-----", "\n-----END CERTIFICATE-----")

OpenSSL::X509::Certificate.new(cert)
end
end
1 change: 1 addition & 0 deletions app/models/certificate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ def revoke!
-revoke #{crt_file.path} -key '#{ENV['ca_key_password']}' -batch")

if err.match(/Data Base Updated/) || err.match(/ERROR:Already revoked/)
self.revoked = true
save!
@cached_status = REVOKED
else
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20200505103316_add_revoked_to_certificate.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddRevokedToCertificate < ActiveRecord::Migration[5.2]
def change
add_column :certificates, :revoked, :boolean, null: false, default: false
end
end
7 changes: 5 additions & 2 deletions db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,8 @@ CREATE TABLE public.certificates (
updated_at timestamp without time zone,
common_name character varying,
md5 character varying,
interface character varying
interface character varying,
revoked boolean DEFAULT false NOT NULL
);


Expand Down Expand Up @@ -4463,5 +4464,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20200130092113'),
('20200203143458'),
('20200204103125'),
('20200311114649');
('20200311114649'),
('20200505103316');


7 changes: 7 additions & 0 deletions test/fixtures/certificates.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
one:
api_user: api_bestnames
common_name: registry.test
crt: "-----BEGIN CERTIFICATE-----\nMIICYjCCAcugAwIBAgIBADANBgkqhkiG9w0BAQ0FADBNMQswCQYDVQQGEwJ1czEO\nMAwGA1UECAwFVGV4YXMxFjAUBgNVBAoMDVJlZ2lzdHJ5IHRlc3QxFjAUBgNVBAMM\nDXJlZ2lzdHJ5LnRlc3QwIBcNMjAwNTA1MTIzNzQxWhgPMjEyMDA0MTExMjM3NDFa\nME0xCzAJBgNVBAYTAnVzMQ4wDAYDVQQIDAVUZXhhczEWMBQGA1UECgwNUmVnaXN0\ncnkgdGVzdDEWMBQGA1UEAwwNcmVnaXN0cnkudGVzdDCBnzANBgkqhkiG9w0BAQEF\nAAOBjQAwgYkCgYEAyn+GCkUJIhdXVBOPrZH+Zj2B/tQfL5TLZwVYZQt38x6GQT+4\n6ndty467IJvKSUlHej7uMpsCzC8Ffmda4cZm16jO1vUb4hXIrmeKP84zLrrUpKag\ngZR4rBDbG2+uL4SzMyy3yeQysYuTiQ4N1i4vdhvkKYPSWIht/QFvuzdFq+0CAwEA\nAaNQME4wHQYDVR0OBBYEFD6B5j6NnMCDBnfbtjBYKBJM7sCRMB8GA1UdIwQYMBaA\nFD6B5j6NnMCDBnfbtjBYKBJM7sCRMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEN\nBQADgYEArtCR6VOabD3nM/KlZTmHMZVT4ntenYlNTM9FS0RatzPmdh4REhykvmZs\nOlBcpoV5tN5Y8bHOVRqY9V2e903QEhQgoccQhbt0Py6uFwfLv+WLKAUbeGnPqK9d\ndL3wXN9BQs0hJA6IZNFyz2F/gSTURrD1zWW2na3ipRzhupW5+98=\n-----END CERTIFICATE-----\n"
md5: e6771ed5dc857a1dbcc1e0a36baa1fee
interface: api
revoked: false
15 changes: 15 additions & 0 deletions test/models/api_user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,21 @@ def test_finds_user_by_id_card
assert_nil ApiUser.find_by_id_card(id_card)
end

def test_verifies_pki_status
certificate = certificates(:one)

assert @user.pki_ok?(certificate.crt, certificate.common_name, api: true)
assert_not @user.pki_ok?(certificate.crt, 'invalid-cn', api: true)

certificate.update(interface: 'registrar')

assert @user.pki_ok?(certificate.crt, certificate.common_name, api: false)
assert_not @user.pki_ok?(certificate.crt, 'invalid-cn', api: false)

certificate.update(revoked: true)
assert_not @user.pki_ok?(certificate.crt, certificate.common_name, api: false)
end

private

def valid_user
Expand Down