From: Lukáš Ježek Date: Tue, 25 Aug 2020 10:03:10 +0000 (+0200) Subject: modules/ta_update: warn if there are differences between statically configured keys... X-Git-Tag: v5.2.0~14^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ada83c482056e28b4eea4edd3f1fea727c0af57e;p=thirdparty%2Fknot-resolver.git modules/ta_update: warn if there are differences between statically configured keys and upstream --- diff --git a/NEWS b/NEWS index bd0ffa293..b7db0b88d 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ Improvements - net: split the EDNS buffer size into upstream and downstream (!1026) - lua-http doh: answer to /dns-query endpoint as well as /doh (!1069) - improve resiliency against UDP fragmentation attacks (disable PMTUD) (!1061) +- ta_update: warn if there are differences between statically configured + keys and upstream (#251, !1051) Bugfixes -------- diff --git a/daemon/lua/trust_anchors.lua.in b/daemon/lua/trust_anchors.lua.in index 30d170394..af4421e7f 100644 --- a/daemon/lua/trust_anchors.lua.in +++ b/daemon/lua/trust_anchors.lua.in @@ -311,11 +311,10 @@ end local function add_file(path, unmanaged) local managed = not unmanaged + if not ta_update then + modules.load('ta_update') + end if managed then - if not ta_update then - panic('[ ta ] automatic update for ' .. path .. ' requested, ' - .. 'but required module ta_update is not loaded') - end if not io.open(path .. '.lock', 'w') then error("[ ta ] ERROR: write access needed to keyfile dir '"..path.."'") end @@ -367,9 +366,7 @@ local function add_file(path, unmanaged) end -- TODO: if failed and for root, try to rebootstrap? - if managed then - ta_update.start(owner) - end + ta_update.start(owner, managed) end local function remove(zname) diff --git a/modules/meson.build b/modules/meson.build index 472c29513..c7b62b906 100644 --- a/modules/meson.build +++ b/modules/meson.build @@ -15,7 +15,6 @@ lua_mod_src = [ # add lua modules without separate meson.build files('serve_stale/serve_stale.lua'), files('ta_sentinel/ta_sentinel.lua'), files('ta_signal_query/ta_signal_query.lua'), - files('ta_update/ta_update.lua'), files('watchdog/watchdog.lua'), files('workarounds/workarounds.lua'), ] @@ -24,7 +23,6 @@ lua_mod_src = [ # add lua modules without separate meson.build config_tests += [ ['predict', files('predict/predict.test.lua')], ['dns64', files('dns64/dns64.test.lua')], - ['ta_update', files('ta_update/ta_update.test.lua'), ['snowflake']], ['prefill', files('prefill/prefill.test/prefill.test.lua')], ] @@ -33,7 +31,6 @@ integr_tests += [ ['serve_stale', join_paths(meson.current_source_dir(), 'serve_stale', 'test.integr')], # NOTE: ta_update may pass in cases when it should fail due to race conditions # To ensure reliability, deckard should introduce a time wait - ['ta_update', join_paths(meson.current_source_dir(), 'ta_update', 'ta_update.test.integr')], ] @@ -50,6 +47,7 @@ subdir('nsid') subdir('policy') subdir('refuse_nord') subdir('stats') +subdir('ta_update') subdir('view') # install lua modules diff --git a/modules/ta_update/meson.build b/modules/ta_update/meson.build new file mode 100644 index 000000000..07ade0e07 --- /dev/null +++ b/modules/ta_update/meson.build @@ -0,0 +1,19 @@ +# LUA module: ta_update +# SPDX-License-Identifier: GPL-3.0-or-later + +config_tests += [ + ['ta_update', files('ta_update.test.lua'), ['snowflake']], +] + +integr_tests += [ + ['ta_update', join_paths(meson.current_source_dir(), 'ta_update.test.integr')], + ['ta_update.unmanagedkey', join_paths(meson.current_source_dir(), 'ta_update.unmanagedkey.test.integr')], +] + +lua_mod_src += [ + files('ta_update.lua'), +] + +install_data( + install_dir: join_paths(modules_dir, 'ta_update'), +) diff --git a/modules/ta_update/ta_update.lua b/modules/ta_update/ta_update.lua index 496db8511..84f7a4a87 100644 --- a/modules/ta_update/ta_update.lua +++ b/modules/ta_update/ta_update.lua @@ -200,8 +200,56 @@ local function update(keyset, new_keys) return true end +local function unmanagedkey_change() + warn('[ta_update] you need to update package with trust anchors before it breaks') +end + +local function check_upstream(keyset, new_keys) + local process_keys = {} + + for _, rr in ipairs(new_keys) do + local key_revoked = (rr.type == kres.type.DNSKEY) and C.kr_dnssec_key_revoked(rr.rdata) + local ta = ta_find(keyset, rr) + table.insert(process_keys, ta) + + if rr.type == kres.type.DNSKEY and not C.kr_dnssec_key_ksk(rr.rdata) then + goto continue -- Ignore + end + + if not ta and not key_revoked then + -- I see new key + ta_update.cb_unmanagedkey_change() + end + + if ta and key_revoked then + -- I see revoked key + ta_update.cb_unmanagedkey_change() + end + + ::continue:: + end + + for _, rr in ipairs(keyset) do + local missing_rr = true + for _, rr_old in ipairs(process_keys) do + if (rr.owner == rr_old.owner) and (rr.type == rr_old.type) and (rr.type == kres.type.DNSKEY) then + if C.kr_dnssec_key_match(rr.rdata, #rr.rdata, rr_old.rdata, #rr_old.rdata) == 0 then + missing_rr = false + break + end + end + end + + if missing_rr then + -- This key is missing in the new keyset + ta_update.cb_unmanagedkey_change() + end + end + +end + -- Refresh the DNSKEYs from the packet, and return time to the next check. -local function active_refresh(keyset, pkt) +local function active_refresh(keyset, pkt, managed) local retry = true if pkt:rcode() == kres.rcode.NOERROR then local records = pkt:section(kres.section.ANSWER) @@ -211,7 +259,12 @@ local function active_refresh(keyset, pkt) table.insert(new_keys, rr) end end - update(keyset, new_keys) + + if managed then + update(keyset, new_keys) + else + check_upstream(keyset, new_keys) + end retry = false else warn('[ta_update] active refresh failed for ' .. kres.dname2str(keyset.owner) @@ -226,7 +279,7 @@ local function active_refresh(keyset, pkt) end -- Plan an event for refreshing DNSKEYs and re-scheduling itself -local function refresh_plan(keyset, delay) +local function refresh_plan(keyset, delay, managed) local owner = keyset.owner local owner_str = kres.dname2str(keyset.owner) if not tracked_tas[owner] then @@ -241,11 +294,11 @@ local function refresh_plan(keyset, delay) resolve(owner_str, kres.type.DNSKEY, kres.class.IN, 'NO_CACHE', function (pkt) -- Schedule itself with updated timeout - local delay_new = active_refresh(keyset, pkt) + local delay_new = active_refresh(keyset, pkt, managed) delay_new = keyset.refresh_time or ta_update.refresh_time or delay_new log('[ta_update] next refresh for ' .. owner_str .. ' in ' .. delay_new/hour .. ' hours') - refresh_plan(keyset, delay_new) + refresh_plan(keyset, delay_new, managed) end) end) end @@ -257,20 +310,17 @@ ta_update = { refresh_time = nil, keep_removed = 0, tracked = tracked_tas, -- debug and visibility, should not be changed by hand + cb_unmanagedkey_change = unmanagedkey_change, } -- start tracking (already loaded) TA with given zone name in wire format -- do first refresh immediatelly -function ta_update.start(zname) +function ta_update.start(zname, managed) local keyset = trust_anchors.keysets[zname] if not keyset then panic('[ta_update] TA must be configured first before tracking it') end - if not keyset.managed then - panic('[ta_update] TA is configured as unmanaged; remove it and ' - .. 'add it again as managed using trust_anchors.add_file()') - end - refresh_plan(keyset, 0) + refresh_plan(keyset, 0, managed) end function ta_update.stop(zname) diff --git a/modules/ta_update/ta_update.test.integr/rfc5011/README b/modules/ta_update/ta_update.test.integr/rfc5011/README index f7629564c..9b719878f 100644 --- a/modules/ta_update/ta_update.test.integr/rfc5011/README +++ b/modules/ta_update/ta_update.test.integr/rfc5011/README @@ -1,5 +1,13 @@ -Start with `genkeyszones.sh` and generate DNSSEC keys + signed versions of `unsigned.db`. +Start with `genkeyszones.sh` and generate DNSSEC keys + signed versions of `unsigned_*.db`. Then use `dns2rpl.py` to run Knot DNS server with signed zone and to generate RPL file from server's answers. +Generate RFC5011 test: +`dns2rpl.py`. +`./genkeyszones.sh` + +Generate unmanaged keys tests: +`./genkeyszones.sh <--unmanaged_key-presens|--unmanagedkey-missing|--unmanagedkey-revoke>` +`VARIANT="unmanaged_key" ./dns2rpl.py` + See comments in script headers to further details. diff --git a/modules/ta_update/ta_update.test.integr/rfc5011/dns2rpl.py b/modules/ta_update/ta_update.test.integr/rfc5011/dns2rpl.py index 84f611765..c196d61dc 100755 --- a/modules/ta_update/ta_update.test.integr/rfc5011/dns2rpl.py +++ b/modules/ta_update/ta_update.test.integr/rfc5011/dns2rpl.py @@ -18,6 +18,10 @@ import dns.resolver import pydnstest.scenario +try: + VARIANT = os.environ["VARIANT"] +except KeyError: + VARIANT = "" def store_answer(qname, qtype, template): answ = dns.resolver.query(qname, qtype, raise_on_no_answer=False) @@ -114,6 +118,31 @@ test. IN TXT "it works" ENTRY_END '''.format(id_prefix) +def generate_step_nocheck(id_prefix): + return '''STEP {0}000001 CHECK_ANSWER +ENTRY_BEGIN +REPLY QR RD RA AD +MATCH opcode qname question +SECTION QUESTION +test. IN TXT +SECTION ANSWER +test. IN TXT "it works" +ENTRY_END +'''.format(id_prefix) + +def generate_step_finish_msg(id_prefix): + return '''STEP {0}000001 CHECK_ANSWER +ENTRY_BEGIN +REPLY QR RD RA AA NXDOMAIN +MATCH opcode rcode flags question answer +SECTION QUESTION +test. IN TXT +SECTION AUTHORITY +test. 10800 IN SOA test. nobody.invalid. 1 3600 1200 604800 10800 +SECTION ADDITIONAL +explanation.invalid. 10800 IN TXT "check last answer" +ENTRY_END +'''.format(id_prefix) def generate_step_elapse(tstep, id_prefix): out = '; move time by {0}\n'.format(tstep) @@ -126,6 +155,7 @@ def main(): resolver_init() rng_templ, entry_templ = get_templates() ranges = [] + check_last_msg = False # transform data in zones files into RANGEs files = os.listdir() @@ -143,28 +173,38 @@ def main(): # steps steps = [] tstart = datetime.datetime(year=2017, month=7, day=1) - tend = datetime.datetime(year=2017, month=12, day=31, hour=23, minute=59, second=59) + if VARIANT == "unmanaged_key": + tend = datetime.datetime(year=2017, month=7, day=21, hour=23, minute=59, second=59) + check_last_msg = True + else: + tend = datetime.datetime(year=2017, month=12, day=31, hour=23, minute=59, second=59) tstep = datetime.timedelta(days=1) tcurr = tstart while tcurr < tend: id_prefix = tcurr.strftime('%Y%m%d') steps.append(generate_step_query(tcurr, id_prefix)) - steps.append(generate_step_check(id_prefix)) + if (check_last_msg is True and tcurr + tstep > tend): + steps.append(generate_step_finish_msg(id_prefix)) + elif VARIANT == "unmanaged_key": + steps.append(generate_step_nocheck(id_prefix)) + else: + steps.append(generate_step_check(id_prefix)) steps.append(generate_step_elapse(tstep, id_prefix)) tcurr += tstep # generate output with open('keys/ds') as dsfile: - ta = dsfile.read().strip() + tas = dsfile.read().strip() # constant RPL file header - print("""stub-addr: 2001:503:ba3e::2:30 - trust-anchor: {ta} - val-override-date: 20170701000000 - query-minimization: off - CONFIG_END - - SCENARIO_BEGIN Simulation of successfull RFC 5011 KSK roll-over during 2017 + print("stub-addr: 2001:503:ba3e::2:30") + for ta in tas.split('\n'): + print ("trust-anchor: " + ta) + print("""val-override-date: 20170701000000 +query-minimization: off +CONFIG_END + +SCENARIO_BEGIN Simulation of successfull RFC 5011 KSK roll-over during 2017 """.format(ta=ta)) for rng in ranges: print(rng) @@ -174,7 +214,7 @@ def main(): # constant RPL file footer print(''' - SCENARIO_END +SCENARIO_END ''') diff --git a/modules/ta_update/ta_update.test.integr/rfc5011/genkeyszones.sh b/modules/ta_update/ta_update.test.integr/rfc5011/genkeyszones.sh index bc778865e..4a654695b 100755 --- a/modules/ta_update/ta_update.test.integr/rfc5011/genkeyszones.sh +++ b/modules/ta_update/ta_update.test.integr/rfc5011/genkeyszones.sh @@ -12,7 +12,17 @@ set -o nounset -o errexit -o xtrace GEN="dnssec-keygen -K keys/ -a RSASHA256 -b 2048 -L 21d" -function sign { +function usage { + echo -e "Usage: $0