Re: Unblock request for opendnssec 1.1.3-1
Hi Julien,
I know maybe it's too much to ask, but the upstream have found a grave bug
in opendnssec 1.1.2 and released 1.1.3 today, which fixes just that
one critical bug. (I didn't want to open another bug in Debian BTS
just for formal reasons, but I can do that.)
The bug description is:
OpenDNSSEC 1.1.3 - 2010-09-10
Bugfixes:
* Bugreport #183: Partial zone could get signed if zone transfer failed
when using zone_fetcher
The full description of the bug can be found here:
http://trac.opendnssec.org/ticket/183
The trimmed down patch is attached and the output of diff stat looks like this:
zone_fetcher.c | 54 ++++++++++++++++++++++++++++++++++++++++--------------
zone_fetcher.h | 4 ++--
2 files changed, 42 insertions(+), 16 deletions(-)
It just moves initialization of xfrd structure lower into data
structures, because it cannot be reused and does much more strict data
checking.
Ondrej
On Thu, Sep 9, 2010 at 13:15, Julien Cristau <jcristau@debian.org> wrote:
> On Mon, Sep 6, 2010 at 11:22:31 +0200, Ondřej Surý wrote:
>
>> On Mon, Sep 6, 2010 at 10:48, Julien Cristau <jcristau@debian.org> wrote:
>> > On Mon, Sep 6, 2010 at 09:24:25 +0200, Ondřej Surý wrote:
>> >
>> >> On Sat, Sep 4, 2010 at 15:42, Julien Cristau <jcristau@debian.org> wrote:
>> >> > tools/ods-control.in looks like it should exit with an error if it
>> >> > doesn't like its command line arguments.
>> >>
>> >> I have prepared patch for this issue and I will upload it as 1.1.2-2
>> >> if you say it's ok.
>> >>
>> > ack
>>
>> Thanks. Uploaded.
>>
> Unblocked.
>
> Cheers,
> Julien
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iQIcBAEBCAAGBQJMiMHMAAoJEDEBgAUJBeQMdP0QAMxaqcz2j1fxSCcp1Yk9wdK5
> qNlGXBGp9SzTFMaIu6Xn0ClvWgoArLDS+zzpZGeroCL1Er20cIRfhq7F3dw3Mc0B
> b27jynVjE9rfjmIU6Thj+e7uZU4eBj//Zi1DFLDx/obMHE47u6+YIm2++4Qsf6+f
> Lz+mZijuFOXEtv7EXh8/FIv/vU2i3INfGW+d0AtXre5ybH7aYGD6i1kBxghzJOq1
> jlWcoSeFou+xuHqBhWSIfetYXx08JkuP+i4YtRhgFB9I+lb8zqAJg8Z9uisM9vdx
> 9y2coDYbTnrNhYplED6/Dky0dW3VT3CqEi27JmhLefcEsL8Tzsx/UZe2ZpKPoS7W
> 4P6fj8UrPoNznUIiTGstzXBPiGzBaJtwynxbbvRPvaWjmMUpB9CDRO1KhIJMz8p9
> zGi9MWI59ogZQp4DV82GfPNaUKG1bwrOS9ujOVwZR6RdVqEYoaq/wsdlJMF0lHcs
> zXePFrtyUa4HXkX8OKbu5kZrey5ysH8nr7iKo4mscrKwXGWW0eiTx43MX/6FoL2d
> WGC/QlShNEJ56zcet3ww8dzKrUPMOb1hmVyGlcUxewCI6BsEDCL9rhD8Cx8xakFR
> kNJ0ZBG2nU/fg2DP1F0Qt+JtkFsK1tboLC1pcmSyuvcKJVR/wLsPa+nhJLVCMu8s
> yG2MLre/los5CIJPnzIF
> =T0AN
> -----END PGP SIGNATURE-----
>
>
--
Ondřej Surý <ondrej@sury.org>
diff -urNapw opendnssec-1.1.2/signer/tools/zone_fetcher.c opendnssec-1.1.3/signer/tools/zone_fetcher.c
--- opendnssec-1.1.2/signer/tools/zone_fetcher.c 2010-08-25 10:47:46.000000000 +0200
+++ opendnssec-1.1.3/signer/tools/zone_fetcher.c 2010-09-13 15:09:50.918928154 +0200
@@ -1,5 +1,5 @@
/*
- * $Id: zone_fetcher.c 3653 2010-08-05 14:04:51Z matthijs $
+ * $Id: zone_fetcher.c 3912 2010-09-10 12:35:26Z rb $
*
* Copyright (c) 2009 NLnet Labs. All rights reserved.
*
@@ -37,6 +37,7 @@
#include <getopt.h>
#include <signal.h>
#include <syslog.h>
+#include <unistd.h>
#include <libxml/tree.h>
#include <libxml/parser.h>
@@ -168,7 +169,6 @@ free_config(config_type* cfg)
free_zonelist(cfg->zonelist);
free_serverlist(cfg->serverlist);
free_serverlist(cfg->notifylist);
- ldns_resolver_free(cfg->xfrd);
free((void*) cfg);
}
}
@@ -846,6 +846,7 @@ odd_xfer(zonelist_type* zone, uint32_t s
char dest_file[MAXPATHLEN];
char engine_sign_cmd[MAXPATHLEN + 1024];
int soa_seen = 0;
+ ldns_resolver* xfrd = NULL;
/* soa serial query */
if (!zone || !zone->dname) {
@@ -862,11 +863,21 @@ odd_xfer(zonelist_type* zone, uint32_t s
"Aborting AXFR");
return -1;
}
- status = ldns_resolver_send_pkt(&apkt, config->xfrd, qpkt);
+
+ /* Initialise LDNS resolver for AXFR */
+ xfrd = init_xfrd(config);
+
+ if (!xfrd) {
+ log_msg(LOG_ERR, "Failed to initialise AXFR structure");
+ return -1;
+ }
+
+ status = ldns_resolver_send_pkt(&apkt, xfrd, qpkt);
if (status != LDNS_STATUS_OK) {
log_msg(LOG_ERR, "zone fetcher failed to send SOA query: %s",
ldns_get_errorstr_by_id(status));
ldns_pkt_free(qpkt);
+ ldns_resolver_free(xfrd);
return -1;
}
if (ldns_pkt_ancount(apkt) == 1) {
@@ -881,15 +892,17 @@ odd_xfer(zonelist_type* zone, uint32_t s
"Aborting AXFR");
/* retry? */
ldns_pkt_free(apkt);
+ ldns_resolver_free(xfrd);
return -1;
}
if (DNS_SERIAL_GT(new_serial, serial)) {
- status = ldns_axfr_start(config->xfrd, zone->dname, LDNS_RR_CLASS_IN);
+ status = ldns_axfr_start(xfrd, zone->dname, LDNS_RR_CLASS_IN);
ldns_pkt_free(qpkt);
if (status != LDNS_STATUS_OK) {
log_msg(LOG_ERR, "zone fetcher failed to start axfr: %s",
ldns_get_errorstr_by_id(status));
+ ldns_resolver_free(xfrd);
return -1;
}
@@ -905,17 +918,19 @@ odd_xfer(zonelist_type* zone, uint32_t s
if (!fd) {
log_msg(LOG_ERR, "zone fetcher cannot store AXFR to file %s",
axfr_file);
+ ldns_resolver_free(xfrd);
return -1;
}
}
assert(fd);
- axfr_rr = ldns_axfr_next(config->xfrd);
+ axfr_rr = ldns_axfr_next(xfrd);
if (!axfr_rr) {
log_msg(LOG_ERR, "zone fetcher AXFR for %s failed", zone->name);
fclose(fd);
unlink(axfr_file);
+ ldns_resolver_free(xfrd);
return -1;
}
else {
@@ -929,8 +944,26 @@ odd_xfer(zonelist_type* zone, uint32_t s
ldns_rr_print(fd, axfr_rr);
}
ldns_rr_free(axfr_rr);
- axfr_rr = ldns_axfr_next(config->xfrd);
+ axfr_rr = ldns_axfr_next(xfrd);
+ }
+
+ /* RoRi:
+ * We MUST now check if the AXFR was successful by verifying that
+ * LDNS has seen the SOA record twice. Not doing this can result
+ * in a half-transferred zone if the AXFR is interrupted.
+ */
+ if (!ldns_axfr_complete(xfrd)) {
+ /* The AXFR was not successful, we've received only a partial zone */
+ log_msg(LOG_ERR, "zone fetcher AXFR for %s failed, received only a partial zone", zone->name);
+ fclose(fd);
+ unlink(axfr_file);
+ ldns_resolver_free(xfrd);
+ return -1;
}
+
+ ldns_resolver_free(xfrd);
+ xfrd = NULL;
+
log_msg(LOG_INFO, "zone fetcher transferred zone %s serial %u "
"successfully", zone->name, new_serial);
@@ -964,7 +997,7 @@ odd_xfer(zonelist_type* zone, uint32_t s
return 0;
}
-static ldns_resolver*
+ldns_resolver*
init_xfrd(config_type* config)
{
serverlist_type* servers;
@@ -1431,12 +1464,6 @@ list_settings(config_type* config, const
fprintf(stdout, "\t%s\n", servers->ipaddr);
servers = servers->next;
}
- if (config->xfrd) {
- fprintf(stdout, "transfer daemon available\n");
- }
- else {
- fprintf(stdout, "transfer daemon missing\n");
- }
}
else fprintf(stdout, "no config\n");
}
@@ -1511,7 +1538,6 @@ main(int argc, char **argv)
config->pidfile = strdup(PID_FILENAME_STRING); /* not freed */
c = read_axfr_config(config_file, config);
config->zonelist = read_zonelist(zonelist_file);
- config->xfrd = init_xfrd(config);
xmlCleanupParser();
if (info) {
diff -urNapw opendnssec-1.1.2/signer/tools/zone_fetcher.h opendnssec-1.1.3/signer/tools/zone_fetcher.h
--- opendnssec-1.1.2/signer/tools/zone_fetcher.h 2010-08-25 10:47:46.000000000 +0200
+++ opendnssec-1.1.3/signer/tools/zone_fetcher.h 2010-09-10 15:42:28.000000000 +0200
@@ -1,5 +1,5 @@
/*
- * $Id: zone_fetcher.h 3150 2010-04-08 11:36:13Z jakob $
+ * $Id: zone_fetcher.h 3912 2010-09-10 12:35:26Z rb $
*
* Copyright (c) 2009 NLnet Labs. All rights reserved.
*
@@ -88,7 +88,7 @@ struct config_struct
zonelist_type* zonelist;
serverlist_type* serverlist;
serverlist_type* notifylist;
- ldns_resolver* xfrd;
+ /* ldns_resolver* xfrd; *** RoRi: removed this, structure is NOT suitable for re-use */
};
/**
Reply to: