daemon/lua/trust_anchors: don't crash when dealing with unknown algorhitm

Closes #449 (closed)

• Depends on deckard!159 (merged)
• Depends on respdiff!52 (merged)

added 6 commits

• e915b527 - trust_anchors: get rid of double negation in add_file()
• 4a7aaafc - trust_anchors: do not bootstrap if root TA exists
• a45b09d8 - trust_anchors: add distrust function to remove TA
• 641414e0 - trust_anchors: use cleaner interface between ta_update and trust_anchors module
• 912a9a2a - ta_update: remove parameter refresh_plan(is_initial)
• 93ccd8aa - trust_anchors: document distrust and polish related docs

Compare with previous version

@tkrizek Can you have a look at my changes? I think it is mostly ready but I might do couple small cleanups here and there before merge.

added 15 commits

• a7b3a5df - trust_anchors: get rid of double negation in add_file()
• 70a054f5 - trust_anchors: do not bootstrap if root TA exists
• 952128ec - trust_anchors: add distrust function to remove TA
• a0c083fd - trust_anchors: use cleaner interface between ta_update and trust_anchors module
• a193ef8a - ta_update: remove parameter refresh_plan(is_initial)
• a1568808 - trust_anchors: document distrust and polish related docs
• 8e7a88f2 - trust_anchors: rename distrust to remove
• e5ce2004 - ta_update: polish test
• 77fc73f5 - trust_anchrors/bootstrap.test: fix test
• 8ae8975d - scripts/launch-test-instance: remove obsolete script
• ddde9722 - daemon: remove -k/-K options
• eca5d490 - trust_anchors: make sure to stop tracking managed key when overriding it
• ad873aa6 - trust_anchors: always load keyfile_default
• 2dd4f9e4 - trust_anchors: remove syntactic sugar and duplicity
• 66fd6588 - WIP: test/integration: update deckard

Compare with previous version

added 2 commits

• 9bc063de - doc/upgrading: document removal of -k and -K
• 83fee194 - WIP: test/integration: update deckard

Compare with previous version

added 1 commit

• 5bf4054d - fixup! ta_update: polish test

Compare with previous version

added 1 commit

• f0df08eb - ta_update: remove useless initialization

Compare with previous version

added 1 commit

• c7f61176 - fixup! ta_update: polish test

Compare with previous version

added 1 commit

• 9aa84142 - WIP: trust_anchors: avoid cancelling callback in progress

Compare with previous version

added 1 commit

• cd7e619a - WIP: add debug prints

Compare with previous version

added 1 commit

• 23259985 - WIP: add debug prints

Compare with previous version

added 1 commit

• 7d0e0c5f - ta_update.test: increase times to avoid CI race condition

Compare with previous version

added 1 commit

• 32d0eff9 - ta_update: abort update if keyset is no longer managed

Compare with previous version

added 1 commit

• 2925ef2c - ta_update: abort update if keyset is no longer managed

Compare with previous version

added 12 commits

• a7bd353e - ta_update: polish test
• 2ad14e0c - trust_anchrors/bootstrap.test: fix test
• 85e74a3c - scripts/launch-test-instance: remove obsolete script
• 2284e0fa - daemon: remove -k/-K options
• 10c7edc4 - trust_anchors: make sure to stop tracking managed key when overriding it
• da3cf771 - trust_anchors: always load keyfile_default
• 583c95a4 - trust_anchors: remove syntactic sugar and duplicity
• a65c5a0d - doc/upgrading: document removal of -k and -K
• 2da76c26 - WIP: test/integration: update deckard
• 00908cf6 - ta_update: remove useless initialization
• 84211574 - WIP: add debug prints
• ccb9308e - ta_update: abort update if keyset is no longer managed

Compare with previous version

added 1 commit

• b660dd8a - ta_update.test: increase time for testing in CI

Compare with previous version

added 4 commits

• 41b87dd2 - ta_update: remove useless initialization
• 4351b7df - ta_update: abort update if keyset is no longer managed
• a1e89d1a - ta_update.test: increase time for testing in CI
• 3d8de751 - WIP: test/integration: update deckard

Compare with previous version

added 10 commits

• 65994267 - daemon: remove -k/-K options
• 4780b926 - trust_anchors: make sure to stop tracking managed key when overriding it
• d7c8b32d - trust_anchors: always load keyfile_default
• e77c83bf - trust_anchors: remove syntactic sugar and duplicity
• c656bef7 - doc/upgrading: document removal of -k and -K
• b28c3f81 - ta_update: remove useless initialization
• 4a72361c - ta_update: abort update if keyset is no longer managed
• 5e2635d6 - ta_update.test: increase time for testing in CI
• c9490185 - ci: fix luacheck
• 0dcf0951 - WIP: test/integration: update deckard

Compare with previous version

added 34 commits

• 0dcf0951...3775bbc4 - 3 commits from branch master
• a31ac25b - daemon/lua/trust_anchors.test.integr: test key rollover to unsupported algorhitm
• cc2c656c - daemon/lua/trust_anchors: don't crash when dealing with unknown algorhitm
• d9908250 - daemon/lua/trust_anchors: bootstrap TA immediately after startup
• 45df0acf - modules/ta_update: move RFC5011 to a separate module
• 55f23a12 - daemon/lua/trust_anchors: write keyset after bootstrap
• 6275e688 - lua/trust_anchors: use tabs everywhere
• 91d170ab - tests/integration: update kresd config for deckard
• e27525e9 - modules/ta_update: remove all asserts
• 90f1272a - nitpick: modules/ta_update - unify log message format
• 114a2ce8 - ci: luacheckrc - organize, add ta_update
• dc2cadd5 - trust_anchors: get rid of double negation in add_file()
• 59bfdcce - trust_anchors: do not bootstrap if root TA exists
• b5bf685b - trust_anchors: add distrust function to remove TA
• e8d549cd - trust_anchors: use cleaner interface between ta_update and trust_anchors module
• aefae8a3 - ta_update: remove parameter refresh_plan(is_initial)
• 3401a945 - trust_anchors: document distrust and polish related docs
• f7062485 - trust_anchors: rename distrust to remove
• 2e1e7b21 - ta_update: polish test
• 5667c78d - trust_anchrors/bootstrap.test: fix test
• 9667b69c - scripts/launch-test-instance: remove obsolete script
• cf67fb05 - daemon: remove -k/-K options
• e2e49e1f - trust_anchors: make sure to stop tracking managed key when overriding it
• 84fafaa5 - trust_anchors: always load keyfile_default
• 19c09644 - trust_anchors: remove syntactic sugar and duplicity
• 3f72fa6e - doc/upgrading: document removal of -k and -K
• 072a69b4 - ta_update: remove useless initialization
• 578984fd - ta_update: abort update if keyset is no longer managed
• 3c84e223 - ta_update.test: increase time for testing in CI
• 5be1a301 - ci: fix luacheck
• e19559fd - ci: remove testlog before running tests
• 1bf5fb01 - WIP: test/integration: update deckard

Compare with previous version

added 1 commit

• ee979bb5 - ci: archive ass testlogs

Compare with previous version

changed the description

added 33 commits

• ee979bb5...bb035e0a - 3 commits from branch master
• e54dd3fb - daemon/lua/trust_anchors.test.integr: test key rollover to unsupported algorhitm
• fd070be4 - daemon/lua/trust_anchors: don't crash when dealing with unknown algorhitm
• 4b0080c0 - daemon/lua/trust_anchors: bootstrap TA immediately after startup
• b46428bb - modules/ta_update: move RFC5011 to a separate module
• 87d98c3a - daemon/lua/trust_anchors: write keyset after bootstrap
• b356b40b - lua/trust_anchors: use tabs everywhere
• 30f08399 - tests/integration: update kresd config for deckard
• 89ff2797 - modules/ta_update: remove all asserts
• af141672 - nitpick: modules/ta_update - unify log message format
• 668bfcc8 - ci: luacheckrc - organize, add ta_update
• 3cc825d7 - trust_anchors: get rid of double negation in add_file()
• 83646e82 - trust_anchors: do not bootstrap if root TA exists
• b32bbc3e - trust_anchors: add distrust function to remove TA
• c61c73a2 - trust_anchors: use cleaner interface between ta_update and trust_anchors module
• c0717a9e - ta_update: remove parameter refresh_plan(is_initial)
• 64d6ae70 - trust_anchors: document distrust and polish related docs
• dd506b18 - trust_anchors: rename distrust to remove
• 5e03c2b6 - ta_update: polish test
• cfd6b0c5 - trust_anchrors/bootstrap.test: fix test
• d248416a - scripts/launch-test-instance: remove obsolete script
• 2c2025ef - daemon: remove -k/-K options
• cee077de - trust_anchors: make sure to stop tracking managed key when overriding it
• 146a60ff - trust_anchors: always load keyfile_default
• bf0a8935 - trust_anchors: remove syntactic sugar and duplicity
• 1221a09d - doc/upgrading: document removal of -k and -K
• 7581506c - ta_update: remove useless initialization
• ef6a8ef1 - ta_update: abort update if keyset is no longer managed
• aeba5f87 - ta_update.test: increase time for testing in CI
• 1373d778 - ci: fix luacheck
• 9fa67d3e - WIP: test/integration: update deckard

Compare with previous version

added 28 commits

• 162786b7 - modules/ta_update: move RFC5011 to a separate module
• c40dd8ef - daemon/lua/trust_anchors: write keyset after bootstrap
• b0616f3a - lua/trust_anchors: use tabs everywhere
• 679e36af - tests/integration: update kresd config for deckard
• da1dbffb - modules/ta_update: remove all asserts
• c23331a6 - nitpick: modules/ta_update - unify log message format
• 363e524a - ci: luacheckrc - organize, add ta_update
• 61f214e7 - trust_anchors: get rid of double negation in add_file()
• 241f7f3d - trust_anchors: do not bootstrap if root TA exists
• e119e156 - trust_anchors: add distrust function to remove TA
• 035ee1f5 - trust_anchors: use cleaner interface between ta_update and trust_anchors module
• d2fea3c6 - ta_update: remove parameter refresh_plan(is_initial)
• 6bb70340 - trust_anchors: document distrust and polish related docs
• 9de7c550 - trust_anchors: rename distrust to remove
• dbf44139 - ta_update: polish test
• eaa7cf7c - scripts/launch-test-instance: remove obsolete script
• 7562196e - daemon: remove -k/-K options
• 30c04182 - trust_anchors: make sure to stop tracking managed key when overriding it
• 82011353 - trust_anchors: always load keyfile_default
• 3a9e5113 - trust_anchors: remove syntactic sugar and duplicity
• 396d2eaf - doc/upgrading: document removal of -k and -K
• 3f2211e8 - ta_update: remove useless initialization
• 831f72c6 - ta_update: abort update if keyset is no longer managed
• 35c13d6c - ta_update.test: increase time for testing in CI
• f5cdda78 - ci: fix luacheck
• 865d8a1c - WIP: test/integration: update deckard
• 2c7b272a - trust_anchrors/bootstrap.test: fix test
• 2baffec9 - meson: config_tests - remove obsolete args, retuncode checks

Compare with previous version

unmarked as a Work In Progress

assigned to @pspacek

@pspacek Please check the latest changes.

added 33 commits

• 2baffec9...42e86643 - 2 commits from branch master
• b2194402 - daemon/lua/trust_anchors.test.integr: test key rollover to unsupported algorhitm
• 0501da03 - daemon/lua/trust_anchors: don't crash when dealing with unknown algorhitm
• 7ada5a9c - daemon/lua/trust_anchors: bootstrap TA immediately after startup
• a5a4aae8 - modules/ta_update: move RFC5011 to a separate module
• 8e66ef75 - daemon/lua/trust_anchors: write keyset after bootstrap
• 241eb03b - lua/trust_anchors: use tabs everywhere
• 9f5fc0dd - tests/integration: update kresd config for deckard
• b54ff2b9 - modules/ta_update: remove all asserts
• bae0cb95 - nitpick: modules/ta_update - unify log message format
• 783736e4 - ci: luacheckrc - organize, add ta_update
• 8929787a - trust_anchors: get rid of double negation in add_file()
• 34b68cac - trust_anchors: do not bootstrap if root TA exists
• 6c56a278 - trust_anchors: add distrust function to remove TA
• 78d908d9 - trust_anchors: use cleaner interface between ta_update and trust_anchors module
• 19f37e38 - ta_update: remove parameter refresh_plan(is_initial)
• 79ab22eb - trust_anchors: document distrust and polish related docs
• e3f0f1c4 - trust_anchors: rename distrust to remove
• 1d788839 - ta_update: polish test
• ab53b44c - scripts/launch-test-instance: remove obsolete script
• ca3bee85 - daemon: remove -k/-K options
• 816613b2 - trust_anchors: make sure to stop tracking managed key when overriding it
• 35be95a7 - trust_anchors: always load keyfile_default
• b3212fc2 - trust_anchors: remove syntactic sugar and duplicity
• dee13d8e - doc/upgrading: document removal of -k and -K
• e640963e - ta_update: remove useless initialization
• 0c33a616 - ta_update: abort update if keyset is no longer managed
• 58fa35e1 - ta_update.test: increase time for testing in CI
• b3476350 - ci: fix luacheck
• 5086e61d - WIP: test/integration: update deckard
• 410362bd - trust_anchrors/bootstrap.test: fix test
• d324e4f4 - meson: config_tests - remove obsolete args, retuncode checks

Compare with previous version

marked as a Work In Progress from 5086e61d

Leaving this marked as WIP, because we need to merge changes in other repos and update submodules in this MR.

added 18 commits

• 7ed510f7 - trust_anchors: use cleaner interface between ta_update and trust_anchors module
• 6a70b213 - ta_update: remove parameter refresh_plan(is_initial)
• e45f5eb3 - trust_anchors: document distrust and polish related docs
• 230d5444 - trust_anchors: rename distrust to remove
• 5407af9b - ta_update: polish test
• 54b54815 - scripts/launch-test-instance: remove obsolete script
• c9e28c3b - daemon: remove -k/-K options
• c74b3d51 - trust_anchors: make sure to stop tracking managed key when overriding it
• 6692d2f7 - trust_anchors: always load keyfile_default
• 4a6d8628 - trust_anchors: remove syntactic sugar and duplicity
• e9841f5b - doc/upgrading: document removal of -k and -K
• e686e315 - ta_update: remove useless initialization
• fead59fd - ta_update: abort update if keyset is no longer managed
• 867cf8be - ta_update.test: increase time for testing in CI
• 3a826ff2 - ci: fix luacheck
• 045ddc7b - WIP: test/integration: update deckard
• 90eb630f - trust_anchrors/bootstrap.test: fix test
• 0f4ec37b - meson: config_tests - remove obsolete args, retuncode checks

Compare with previous version

added 3 commits

• c6efd2ae - trust_anchors: do not accept add_file() for managed TA without ta_update module
• dbc57a20 - unify error message format between between C and Lua
• ea8749c5 - trust_anchors: add explanatory error messages for removed functions

Compare with previous version

Related issue: yesterday the revoked root DNSKEY was removed and apparently that caused a problem: https://gitter.im/CZ-NIC/knot-resolver?at=5c96030e2fb6800d8068e24f EDIT: it's quite unclear if the removal is causally linked to that problem (or what the problem is).

added 1 commit

• 1b28ae53 - trust_anchors: improve error messages

Compare with previous version

MR looks good to me.

As for the DNSKEY issue, I wasn't able to find the cause in the old code - it behaved as if the upstream was unreachable. However, I don't see how failure to check the TA would prevent resolution of other queries. I think the TA error is probably a symptom of another issue, not the cause.

added 1 commit

• 439ac20f - trust_anchors: improve error messages

Compare with previous version

marked the checklist item Depends on deckard!159 (merged) as completed

marked the checklist item Depends on respdiff!52 (merged) as completed

added 1 commit

• 77d87889 - trust_anchors: update Deckard to take ta_update module into account

Compare with previous version

unmarked as a Work In Progress

added 38 commits

• 77d87889...b2ebd444 - 2 commits from branch master
• 4410fa55 - daemon/lua/trust_anchors.test.integr: test key rollover to unsupported algorhitm
• 088aad9e - daemon/lua/trust_anchors: don't crash when dealing with unknown algorhitm
• 960cc1b3 - daemon/lua/trust_anchors: bootstrap TA immediately after startup
• feac3e94 - modules/ta_update: move RFC5011 to a separate module
• ad351baa - daemon/lua/trust_anchors: write keyset after bootstrap
• 8f3a157a - lua/trust_anchors: use tabs everywhere
• 1651125a - tests/integration: update kresd config for deckard
• 49366242 - modules/ta_update: remove all asserts
• 7ac587af - nitpick: modules/ta_update - unify log message format
• bc526c3f - ci: luacheckrc - organize, add ta_update
• 373b42ed - trust_anchors: get rid of double negation in add_file()
• f57cf735 - trust_anchors: do not bootstrap if root TA exists
• 0ca663dc - trust_anchors: add distrust function to remove TA
• 22b0c6d5 - trust_anchors: use cleaner interface between ta_update and trust_anchors module
• 8f9992c4 - ta_update: remove parameter refresh_plan(is_initial)
• aaff913c - trust_anchors: document distrust and polish related docs
• 1f569a24 - trust_anchors: rename distrust to remove
• 37e60e28 - ta_update: polish test
• 18f2fa74 - scripts/launch-test-instance: remove obsolete script
• 1405517d - daemon: remove -k/-K options
• e2abf7fa - trust_anchors: make sure to stop tracking managed key when overriding it
• 8abc490f - trust_anchors: always load keyfile_default
• 41ba4b4d - trust_anchors: remove syntactic sugar and duplicity
• 0f473f3d - doc/upgrading: document removal of -k and -K
• 56447445 - ta_update: remove useless initialization
• e71446c8 - ta_update: abort update if keyset is no longer managed
• 9db8ffbf - ta_update.test: increase time for testing in CI
• 08427d19 - ci: fix luacheck
• d45c03de - WIP: test/integration: update deckard
• 74098a8b - trust_anchrors/bootstrap.test: fix test
• 8451e991 - meson: config_tests - remove obsolete args, retuncode checks
• ff41bca5 - trust_anchors: do not accept add_file() for managed TA without ta_update module
• 2efa642b - unify error message format between between C and Lua
• b25212ef - trust_anchors: add explanatory error messages for removed functions
• 56070bf9 - trust_anchors: improve error messages
• 2346346b - trust_anchors: update Deckard to take ta_update module into account

Compare with previous version

marked as a Work In Progress from d45c03de

unmarked as a Work In Progress

mentioned in commit 287e06a2

merged

• Before release we should re-check defaults. When starting kresd without any TA setup, I'm getting

  [ ta ] ERROR: write access needed to keyfile dir '/etc/knot-resolver/root.keys'

  EDIT: actually, there seems no config capable of stopping that message (!) (it happens before config is loaded, and the command-line options were removed)

This is caused by incorrect combination of managed_ta and keyfile_default options during compilation, please refer to https://knot-resolver.readthedocs.io/en/latest/build.html#trust-anchors