From 3453b7f4a1ffd0328aaeaee505181ac3c09fdf81 Mon Sep 17 00:00:00 2001 From: Martin Lensment Date: Thu, 9 Oct 2014 13:59:52 +0300 Subject: [PATCH] DNSSEC refactor --- app/helpers/epp/domains_helper.rb | 9 ++- app/models/delegation_signer.rb | 3 + app/models/dnskey.rb | 1 + app/models/epp/epp_domain.rb | 74 +++++++++++++++++-- app/models/setting.rb | 6 ++ app/models/setting_group.rb | 8 +- config/locales/en.yml | 4 + .../20141008134959_add_dnskey_settings.rb | 8 +- ...20141009100818_create_delegation_signer.rb | 10 +++ ...9101337_add_delegation_signer_to_dnskey.rb | 5 ++ db/schema.rb | 10 ++- spec/epp/domain_spec.rb | 16 ++-- spec/fabricators/setting_group_fabricator.rb | 12 +++ spec/support/epp.rb | 14 ++-- 14 files changed, 150 insertions(+), 30 deletions(-) create mode 100644 app/models/delegation_signer.rb create mode 100644 db/migrate/20141009100818_create_delegation_signer.rb create mode 100644 db/migrate/20141009101337_add_delegation_signer_to_dnskey.rb diff --git a/app/helpers/epp/domains_helper.rb b/app/helpers/epp/domains_helper.rb index bea0f17f8..c76fda2d0 100644 --- a/app/helpers/epp/domains_helper.rb +++ b/app/helpers/epp/domains_helper.rb @@ -94,7 +94,14 @@ module Epp::DomainsHelper def validate_domain_create_request @ph = params_hash['epp']['command']['create']['create'] # TODO: Verify contact presence if registrant is juridical - xml_attrs_present?(@ph, [['name'], ['ns'], ['registrant']]) + attrs_present = xml_attrs_present?(@ph, [['name'], ['ns'], ['registrant']]) + return false unless attrs_present + + if parsed_frame.css('dsData').count > 0 && parsed_frame.css('create > keyData').count > 0 + epp_errors << { code: '2306', msg: I18n.t('shared.ds_data_and_key_data_must_not_exists_together') } + return false + end + true end def domain_create_params diff --git a/app/models/delegation_signer.rb b/app/models/delegation_signer.rb new file mode 100644 index 000000000..6fbff83e4 --- /dev/null +++ b/app/models/delegation_signer.rb @@ -0,0 +1,3 @@ +class DelegationSigner < ActiveRecord::Base + has_many :dnskeys +end diff --git a/app/models/dnskey.rb b/app/models/dnskey.rb index 304e2de8e..74dbf7a0a 100644 --- a/app/models/dnskey.rb +++ b/app/models/dnskey.rb @@ -2,6 +2,7 @@ class Dnskey < ActiveRecord::Base include EppErrors belongs_to :domain + belongs_to :delegation_signer validates :alg, :protocol, :flags, :public_key, presence: true validate :validate_algorithm diff --git a/app/models/epp/epp_domain.rb b/app/models/epp/epp_domain.rb index e3f4f515f..71b7e63fb 100644 --- a/app/models/epp/epp_domain.rb +++ b/app/models/epp/epp_domain.rb @@ -174,9 +174,48 @@ class Epp::EppDomain < Domain domain_statuses.delete(to_delete) end - def attach_dnskeys(dnskey_list) - dnskey_list.each do |dnskey_attrs| - dnskeys.build(dnskey_attrs) + def attach_dnskeys(dnssec_data) + sg = SettingGroup.dnskeys + ds_data_allowed = sg.setting(Setting::ALLOW_DS_DATA) == '0' ? false : true + ds_data_with_keys_allowed = sg.setting(Setting::ALLOW_DS_DATA_WITH_KEYS) == '0' ? false : true + key_data_allowed = sg.setting(Setting::ALLOW_KEY_DATA) == '0' ? false : true + + if dnssec_data[:ds_data].any? && !ds_data_allowed + errors.add(:base, :ds_data_not_allowed) + return + end + + dnssec_data[:ds_data].each do |ds_data| + if ds_data[:key_data].any? && !ds_data_with_keys_allowed + errors.add(:base, :ds_data_with_keys_not_allowed) + next + else + attach_ds(ds_data) + end + end + + if dnssec_data[:key_data].any? && !key_data_allowed + errors.add(:base, :key_data_not_allowed) + return + end + + attach_ds({ + keyTag: SecureRandom.hex(5), + alg: 3, + digestType: sg.setting(Setting::DS_ALGORITHM), + key_data: dnssec_data[:key_data] + }) + + # dnskey_list.each do |dnskey_attrs| + # dnskeys.build(dnskey_attrs) + # end + end + + def attach_ds(ds_data) + key_data = ds_data.delete(:key_data) + ds = delegation_signers.build(ds_data) + key_data.each do |x| + ds.dnskeys.build(x) end end @@ -358,16 +397,39 @@ class Epp::EppDomain < Domain end def parse_dnskeys_from_frame(parsed_frame) - res = [] + res = { ds_data: [], key_data: [] } - parsed_frame.css('dnskey').each do |x| - res << { + res[:max_sig_life] = parsed_frame.css('maxSigLife').first.try(:text) + + parsed_frame.css('dsData').each do |x| + keys = [] + x.css('keyData').each do |kd| + keys << { + flags: kd.css('flags').first.try(:text), + protocol: kd.css('protocol').first.try(:text), + alg: kd.css('alg').first.try(:text), + public_key: kd.css('pubKey').first.try(:text) + } + end + + res[:ds_data] << { + key_tag: x.css('keyTag').first.try(:text), + alg: x.css('alg').first.try(:text), + digest_type: x.css('digestType').first.try(:text), + digest: x.css('digest').first.try(:text), + key_data: keys + } + end + + parsed_frame.css('create > keyData').each do |x| + res[:key_data] << { flags: x.css('flags').first.try(:text), protocol: x.css('protocol').first.try(:text), alg: x.css('alg').first.try(:text), public_key: x.css('pubKey').first.try(:text) } end + res end diff --git a/app/models/setting.rb b/app/models/setting.rb index eb4dbe7f5..0c77db866 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -3,4 +3,10 @@ class Setting < ActiveRecord::Base has_many :domain_statuses has_many :domains, through: :domain_statuses validates :code, uniqueness: { scope: :setting_group_id } + + #dnskeys + DS_ALGORITHM = 'ds_algorithm' + ALLOW_DS_DATA = 'allow_ds_data' + ALLOW_DS_DATA_WITH_KEYS = 'allow_ds_data_with_keys' + ALLOW_KEY_DATA = 'allow_key_data' end diff --git a/app/models/setting_group.rb b/app/models/setting_group.rb index 685520c6d..be1656dd3 100644 --- a/app/models/setting_group.rb +++ b/app/models/setting_group.rb @@ -14,12 +14,12 @@ class SettingGroup < ActiveRecord::Base find_by(code: 'domain_validation') end - def domain_statuses - find_by(code: 'domain_statuses') - end - def domain_general find_by(code: 'domain_general') end + + def dnskeys + find_by(code: 'dnskeys') + end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index a37e6eda0..9d2724ea4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -74,6 +74,9 @@ en: base: domain_status_prohibits_operation: 'Domain status prohibits operation' domain_already_belongs_to_the_querying_registrar: 'Domain already belongs to the querying registrar' + ds_data_not_allowed: 'dsData object is not allowed' + ds_data_with_keys_not_allowed: 'dsData object with key data is not allowed' + key_data_not_allowed: 'keyData object is not allowed' name_dirty: invalid: 'Domain name is invalid' reserved: 'Domain name is reserved or restricted' @@ -424,3 +427,4 @@ en: # sorry these need to be refactored - Andres authentication_error: 'Authentication error' + ds_data_and_key_data_must_not_exists_together: 'dsData and keyData objects must not exists together' diff --git a/db/migrate/20141008134959_add_dnskey_settings.rb b/db/migrate/20141008134959_add_dnskey_settings.rb index 19ae9868f..4fb8ccb22 100644 --- a/db/migrate/20141008134959_add_dnskey_settings.rb +++ b/db/migrate/20141008134959_add_dnskey_settings.rb @@ -1,9 +1,9 @@ class AddDnskeySettings < ActiveRecord::Migration def change sg = SettingGroup.create(code: 'dnskeys') - sg.settings << Setting.create(code: 'ds_algorithm', value: 1) - sg.settings << Setting.create(code: 'allow_ds_data', value: 1) - sg.settings << Setting.create(code: 'allow_ds_data_with_keys', value: 1) - sg.settings << Setting.create(code: 'allow_key_data', value: 1) + sg.settings << Setting.create(code: Setting::DS_ALGORITHM, value: 1) + sg.settings << Setting.create(code: Setting::ALLOW_DS_DATA, value: 1) + sg.settings << Setting.create(code: Setting::ALLOW_DS_DATA_WITH_KEYS, value: 1) + sg.settings << Setting.create(code: Setting::ALLOW_KEY_DATA, value: 1) end end diff --git a/db/migrate/20141009100818_create_delegation_signer.rb b/db/migrate/20141009100818_create_delegation_signer.rb new file mode 100644 index 000000000..2a311e397 --- /dev/null +++ b/db/migrate/20141009100818_create_delegation_signer.rb @@ -0,0 +1,10 @@ +class CreateDelegationSigner < ActiveRecord::Migration + def change + create_table :delegation_signers do |t| + t.integer :domain_id + t.string :key_tag + t.integer :digest_type + t.string :digest + end + end +end diff --git a/db/migrate/20141009101337_add_delegation_signer_to_dnskey.rb b/db/migrate/20141009101337_add_delegation_signer_to_dnskey.rb new file mode 100644 index 000000000..8f031ecc5 --- /dev/null +++ b/db/migrate/20141009101337_add_delegation_signer_to_dnskey.rb @@ -0,0 +1,5 @@ +class AddDelegationSignerToDnskey < ActiveRecord::Migration + def change + add_column :dnskeys, :delegation_signer_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 1105d71f5..44531fbe8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20141008134959) do +ActiveRecord::Schema.define(version: 20141009101337) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -91,12 +91,20 @@ ActiveRecord::Schema.define(version: 20141008134959) do t.datetime "updated_at" end + create_table "delegation_signers", force: true do |t| + t.integer "domain_id" + t.string "key_tag" + t.integer "digest_type" + t.string "digest" + end + create_table "dnskeys", force: true do |t| t.integer "domain_id" t.integer "flags" t.integer "protocol" t.integer "alg" t.string "public_key" + t.integer "delegation_signer_id" end create_table "domain_contacts", force: true do |t| diff --git a/spec/epp/domain_spec.rb b/spec/epp/domain_spec.rb index 13c55bbd5..3d75a7b63 100644 --- a/spec/epp/domain_spec.rb +++ b/spec/epp/domain_spec.rb @@ -13,6 +13,7 @@ describe 'EPP Domain', epp: true do Fabricate(:domain_validation_setting_group) Fabricate(:domain_statuses_setting_group) + Fabricate(:dnskeys_setting_group) end it 'returns error if contact does not exists' do @@ -314,7 +315,7 @@ describe 'EPP Domain', epp: true do it 'does not create a domain with invalid period' do xml = domain_create_xml({ - period: {value: '367', attrs: { unit: 'd' } } + period: { value: '367', attrs: { unit: 'd' } } }) response = epp_request(xml, :xml) @@ -324,18 +325,17 @@ describe 'EPP Domain', epp: true do end it 'creates a domain with multiple dnskeys' do - xml = domain_create_xml({ - dnssec: [ - { - dnskey: { + xml = domain_create_xml({}, { + _other: [ + { keyData: { flags: { value: '257' }, protocol: { value: '3' }, - alg: { value: '3' }, + alg: { value: '5' }, pubKey: { value: 'AwEAAddt2AkLfYGKgiEZB5SmIF8EvrjxNMH6HtxWEA4RJ9Ao6LCWheg8' } } }, { - dnskey: { + keyData: { flags: { value: '0' }, protocol: { value: '3' }, alg: { value: '5' }, @@ -343,7 +343,7 @@ describe 'EPP Domain', epp: true do } }, { - dnskey: { + keyData: { flags: { value: '256' }, protocol: { value: '3' }, alg: { value: '254' }, diff --git a/spec/fabricators/setting_group_fabricator.rb b/spec/fabricators/setting_group_fabricator.rb index 8906fe0d2..381ab5b29 100644 --- a/spec/fabricators/setting_group_fabricator.rb +++ b/spec/fabricators/setting_group_fabricator.rb @@ -38,3 +38,15 @@ Fabricator(:domain_general_setting_group, from: :setting_group) do ] end end + +Fabricator(:dnskeys_setting_group, from: :setting_group) do + code 'dnskeys' + settings do + [ + Fabricate(:setting, code: Setting::DS_ALGORITHM, value: 1), + Fabricate(:setting, code: Setting::ALLOW_DS_DATA, value: 1), + Fabricate(:setting, code: Setting::ALLOW_DS_DATA_WITH_KEYS, value: 1), + Fabricate(:setting, code: Setting::ALLOW_KEY_DATA, value: 1) + ] + end +end diff --git a/spec/support/epp.rb b/spec/support/epp.rb index 9555fe976..48e1ffeb9 100644 --- a/spec/support/epp.rb +++ b/spec/support/epp.rb @@ -72,12 +72,14 @@ module Epp xml_params = defaults.deep_merge(xml_params) dsnsec_defaults = { - keyData: [ - flags: { value: '257' }, - protocol: { value: '3' }, - alg: { value: '5' }, - pubKey: { value: 'AwEAAddt2AkLfYGKgiEZB5SmIF8EvrjxNMH6HtxWEA4RJ9Ao6LCWheg8' } - ] + _other: [ + { keyData: { + flags: { value: '257' }, + protocol: { value: '3' }, + alg: { value: '5' }, + pubKey: { value: 'AwEAAddt2AkLfYGKgiEZB5SmIF8EvrjxNMH6HtxWEA4RJ9Ao6LCWheg8' } + } + }] } dnssec_params = dsnsec_defaults.deep_merge(dnssec_params) if dnssec_params != false