trust anchors: improve .add()

These keys will now be more uniformly represented and thus also
shown by .summary().  It's still not perfectly synchronized when
that function fails, but that seems acceptable.

diff --git a/NEWS b/NEWS
index f4d195795fee1f40b50a131f2b9452a687744975..e4899599e89af33bf200719022d75e8b461c6d46 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ Bugfixes
 - predict module: load stats module if config didn't specify period (!755)
 - trust_anchors: don't do 5011-style updates on anchors from files
   that were loaded as unmanaged trust anchors (!753)
+- trust_anchors.add(): include these TAs in .summary() (!753)
 
 
 Knot Resolver 3.2.1 (2019-01-10)

diff --git a/daemon/lua/trust_anchors.lua.in b/daemon/lua/trust_anchors.lua.in
index 7bc330b7e466688d8229f81daf93195d363ee1e7..e8499a015a1178fff4020eeb955c34a7026b1ea7 100644 (file)
--- a/daemon/lua/trust_anchors.lua.in
+++ b/daemon/lua/trust_anchors.lua.in
@@ -437,16 +437,20 @@ local function keyset_read(path, string)
 end
 
 -- Replace current TAs for given owner by the "trusted" ones from passed keyset.
--- Return the number of trusted keys for the owner.
+-- Return true iff no TA errored out and at least one is in VALID state.
 local function keyset_publish(keyset)
        local store = kres.context().trust_anchors
        local count = 0
+       local has_error = false
        C.kr_ta_del(store, keyset.owner)
        for _, ta in ipairs(keyset) do
                -- Key MAY be used as a TA only in these two states (RFC5011, 4.2)
                if ta.state == key_state.Valid or ta.state == key_state.Missing then
                        if C.kr_ta_add(store, ta.owner, ta.type, ta.ttl, ta.rdata, #ta.rdata) == 0 then
                                count = count + 1
+                       else
+                               ta.state = 'ERROR'
+                               has_error = true
                        end
                end
        end
@@ -454,7 +458,7 @@ local function keyset_publish(keyset)
                warn('[ ta ] ERROR: no anchors are trusted for ' ..
                     kres.dname2str(keyset.owner) .. ' !')
        end
-       return count
+       return count > 0 and not has_error
 end
 
 
@@ -507,7 +511,7 @@ update = function (keyset, new_keys, is_initial)
        keyset_write(keyset)
 
        -- Start using the new TAs.
-       if keyset_publish(keyset) == 0 then
+       if not keyset_publish(keyset) then
                -- TODO: try to rebootstrap if for root?
                return false
        elseif verbose() then
@@ -569,7 +573,7 @@ local add_file = function (path, unmanaged)
        trust_anchors.keysets[owner] = keyset
 
        -- Replace the TA store used for validation
-       if keyset_publish(keyset) ~= 0 and verbose() then
+       if keyset_publish(keyset) and verbose() then
                log('[ ta ] installed trust anchors for domain ' .. owner_str .. ' are:\n'
                    .. trust_anchors.summary(owner))
        end
@@ -631,13 +635,37 @@ trust_anchors = {
        config = add_file,
 
        -- Add DS/DNSKEY record(s) (unmanaged)
-       -- FIXME: this function won't update the .keysets,
-       -- so it won't e.g. be shown by .summary() - confusing.
        add = function (keystr)
-               local ret = trustanchor(keystr)
-               if verbose() then log(trust_anchors.summary()) end
-               return ret
+               local keyset, err = keyset_read(nil, keystr)
+               if keyset ~= nil then
+                       local owner = keyset.owner
+                       local owner_str = kres.dname2str(owner)
+                       local keyset_orig = trust_anchors.keysets[owner]
+                       -- Set up trust_anchors.keysets[owner]
+                       if keyset_orig then
+                               warn('[ ta ] warning: extending previously set trust anchors for '
+                                               .. owner_str)
+                               for _, ta in ipairs(keyset) do
+                                       table.insert(keyset_orig, ta)
+                               end
+                               -- we might also add more warning if it's managed, i.e. has .filename,
+                               -- as the next update would overwrite this additional TA
+                       else
+                               trust_anchors.keysets[owner] = keyset
+                       end
+                       -- Replace the TA store used for validation
+                       if not keyset_publish(keyset) then
+                               err = "when publishing the TA set"
+                               -- trust_anchors.keysets[owner] was already updated to the
+                               -- (partially) failing state, but I'm not sure how much to improve this
+                       end
+               end
+               if verbose() or err then log('New TA state:\n' .. trust_anchors.summary()) end
+               if err then
+                       panic('[ ta ] .add() failed: ' .. err)
+               end
        end,
+
        -- Negative TA management
        set_insecure = function (list)
                assert(type(list) == 'table', 'parameter must be list of domain names (e.g. {"a.test", "b.example"})')

diff --git a/daemon/lua/trust_anchors.test/ta.test.lua b/daemon/lua/trust_anchors.test/ta.test.lua
index bdd00fcb97b9c883a24a615cedab978aeb2f97f9..97da60da53cacaa048b7853a0e1199926b0d843a 100644 (file)
--- a/daemon/lua/trust_anchors.test/ta.test.lua
+++ b/daemon/lua/trust_anchors.test/ta.test.lua
@@ -13,7 +13,6 @@ local function test_revoked_key()
                .. 'vN9dlzEheX7ICJBBtuA6G3LQpzW5hOA2hzCTMjJPJ8LbqF6dsV6DoBQzgul0sGIcGOYl7O'
                .. 'yQdXfZ57relSQageu+ipAdTTJ25AsRTAoub8ONGcLmqrAmRLKBP1dfwhYB4N7knNnulqQxA+Uk1ihz0='
        boom(trust_anchors.add, { '. 3600 DNSKEY 385 3 8 ' .. key_crypto }, 'refuse revoked key')
-       same(#trust_anchors.keysets, 0, 'no keysets')
        same(ffi.C.kr_ta_get(ta_c, '\0') == nil, true, 'no TA for root is used')
        -- Test that we don't have another problem in the key
        trust_anchors.add('. 3600 DNSKEY 257 3 8 ' .. key_crypto)