Package: opendnssec-signer
Version: 1:1.4.6-6
Severity: important
Dear Debian DNS Maintainers,
[upstream bug https://issues.opendnssec.org/browse/SUPPORT-197
discovered on Debian production opendnssec setup.]
ods-signerd fails to correctly process its zonelist.xml configuration file.
This results in zones not being properly signed as they should.
This issue is security (as in availability) critical. Failing to sign its
zones, RRSIG records will expire and validating clients will refuse to use
returned records, making all hostnames in affected zones unavailable after some
time.
Posted patch
https://issues.opendnssec.org/secure/attachment/11811/0001-zonelist-don-t-read-elements-from-other-zones.patch
tested on Debian amd64 install by rebuilding and installing the package with
the patch listed in debian/patches. Note the same issue exists in
ods-enforcerd and ods-ksmutil.
Description as posted upstream:
The Zonelist parser (i.e. parse_zonelist_zones()) assumes that
xmlTextReaderRead / xmlTextReaderExpand only load one XML node, which is the
current <Zone> to be processed. This assumption is incorrect, especially if the
XML is very short in its serialised form. (e.g. no extra whitespace, short zone
filenames, etc.)
This, in turn, makes the later XPath lookups match nodes from both the current
and the next <Zone> element, which caused the following two behaviours in our
setup:
- one zone had the next zone's input file applied to it. Resulting error
(because it was loading the wrong zonefile):
"[adapter] unable to add rr to zone: soa record has invalid owner name"
"[adapter] error adding RR at line 11: @ 86400 IN SOA <...>"
- another zone used the empty string as output filename (because the XML node
for that zone was not fully loaded, the attribute was still empty/in
processing). Resulting error:
"[adapter] unable to write file: failed to rename .tmp to (No such file or
directory)"
Attached fix is against 1.4.6 and only changes signer code; enforcer and
ksmutil can potentially exhibit the same issue. Signer patch is tested &
confirmed to fix the issues.
Note this bug hard-breaks operation since some zones will fail signing;
signatures will expire and the zone's records will start being rejected by
validating clients.
-- System Information:
Debian Release: 8.4
APT prefers stable-updates
APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)
Kernel: Linux 3.16.0-4-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
Versions of packages opendnssec-signer depends on:
ii dpkg 1.17.26
ii libc6 2.19-18+deb8u4
ii libldns1 1.6.17-5+b1
ii libssl1.0.0 1.0.1t-1+deb8u2
ii libxml2 2.9.1+dfsg1-5+deb8u2
ii opendnssec-common 1:1.4.6-6
Versions of packages opendnssec-signer recommends:
ii opendnssec 1:1.4.6-6
ii opendnssec-enforcer 1:1.4.6-6
ii softhsm 1.3.7-2+deb8u1
opendnssec-signer suggests no packages.
-- no debconf information
>From 2c4af2a62f8d109b6150b226231d69dccfc67753 Mon Sep 17 00:00:00 2001
From: David Lamparter <equi...@diac24.net>
Date: Thu, 14 Jul 2016 05:29:09 +0200
Subject: [PATCH] zonelist: don't read elements from other zones
The TextReader interface is not guaranteed to only load 1 node; it may
well load the next 2 nodes, i.e. more than one <Zone> element.
The XPath lookups used to find data for the current zone MUST happen
relative to the XML node currently being processed.
Signed-off-by: David Lamparter <equi...@diac24.net>
---
signer/src/parser/zonelistparser.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/signer/src/parser/zonelistparser.c b/signer/src/parser/zonelistparser.c
index 2368e1b..119637c 100644
--- a/signer/src/parser/zonelistparser.c
+++ b/signer/src/parser/zonelistparser.c
@@ -153,8 +153,8 @@ parse_zonelist_adapter(xmlXPathContextPtr xpathCtx, xmlChar* expr,
static void
parse_zonelist_adapters(xmlXPathContextPtr xpathCtx, zone_type* zone)
{
- xmlChar* i_expr = (xmlChar*) "//Zone/Adapters/Input";
- xmlChar* o_expr = (xmlChar*) "//Zone/Adapters/Output";
+ xmlChar* i_expr = (xmlChar*) "Adapters/Input";
+ xmlChar* o_expr = (xmlChar*) "Adapters/Output";
if (!xpathCtx || !zone) {
return;
@@ -179,10 +179,11 @@ parse_zonelist_zones(void* zlist, const char* zlfile)
int error = 0;
xmlTextReaderPtr reader = NULL;
xmlDocPtr doc = NULL;
+ xmlNodePtr node = NULL;
xmlXPathContextPtr xpathCtx = NULL;
xmlChar* name_expr = (unsigned char*) "name";
- xmlChar* policy_expr = (unsigned char*) "//Zone/Policy";
- xmlChar* signconf_expr = (unsigned char*) "//Zone/SignerConfiguration";
+ xmlChar* policy_expr = (unsigned char*) "Policy";
+ xmlChar* signconf_expr = (unsigned char*) "SignerConfiguration";
if (!zlist || !zlfile) {
return ODS_STATUS_ASSERT_ERR;
@@ -213,12 +214,13 @@ parse_zonelist_zones(void* zlist, const char* zlfile)
continue;
}
/* Expand this node to get the rest of the info */
- xmlTextReaderExpand(reader);
+ node = xmlTextReaderExpand(reader);
doc = xmlTextReaderCurrentDoc(reader);
if (doc) {
xpathCtx = xmlXPathNewContext(doc);
}
- if (doc == NULL || xpathCtx == NULL) {
+ if (doc == NULL || xpathCtx == NULL || node == NULL ||
+ xmlXPathSetContextNode(node, xpathCtx)) {
ods_log_alert("[%s] unable to read zone %s, skipping...",
parser_str, zone_name);
ret = xmlTextReaderRead(reader);
--
2.7.3
