From e7506b6a70d16c5565e147cbfd5b07af8a7ad68a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Kr=C4=99cicki?= Date: Tue, 26 Jun 2018 21:06:55 +0200 Subject: [PATCH 1/4] Fix some issues with large journal entries (cherry picked from commit 0db7130f2b4ae24c3f32e95325673688f16b54f0) (cherry picked from commit 1919f5c93781ac79b8fab2275c1ab6b4b794cb8b) --- lib/dns/journal.c | 25 +++++++++++++++++++++++-- lib/dns/zone.c | 7 +++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/dns/journal.c b/lib/dns/journal.c index da70007f9a..58f0338af6 100644 --- a/lib/dns/journal.c +++ b/lib/dns/journal.c @@ -1013,7 +1013,7 @@ dns_journal_writediff(dns_journal_t *j, dns_diff_t *diff) { dns_difftuple_t *t; isc_buffer_t buffer; void *mem = NULL; - unsigned int size; + size_t size; isc_result_t result; isc_region_t used; @@ -1043,6 +1043,10 @@ dns_journal_writediff(dns_journal_t *j, dns_diff_t *diff) { size += t->rdata.length; } + if (size >= DNS_JOURNAL_SIZE_MAX) { + return (ISC_R_RANGE); + } + mem = isc_mem_get(j->mctx, size); if (mem == NULL) return (ISC_R_NOMEMORY); @@ -1145,6 +1149,18 @@ dns_journal_commit(dns_journal_t *j) { } } + /* + * We currently don't support huge journal entries. + */ + unsigned long long total = j->x.pos[1].offset - j->x.pos[0].offset; + if (total >= DNS_JOURNAL_SIZE_MAX) { + isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, + "transaction too big to be stored in journal :" + "%llub (max is %llub)", total, + (unsigned long long)DNS_JOURNAL_SIZE_MAX); + return (ISC_R_UNEXPECTED); + } + /* * Some old journal entries may become non-addressable * when we increment the current serial number. Purge them @@ -1670,7 +1686,12 @@ read_one_rr(dns_journal_t *j) { journal_xhdr_t xhdr; journal_rrhdr_t rrhdr; - INSIST(j->offset <= j->it.epos.offset); + if (j->offset > j->it.epos.offset) { + isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, + "%s: journal corrupt: possible integer overflow", + j->filename); + return (ISC_R_UNEXPECTED); + } if (j->offset == j->it.epos.offset) return (ISC_R_NOMORE); if (j->it.xpos == j->it.xsize) { diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 7971f7ffa0..ca1c06fda8 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -14281,6 +14281,13 @@ zone_replacedb(dns_zone_t *zone, dns_db_t *db, isc_boolean_t dump) { result = dns_db_diff(zone->mctx, db, ver, zone->db, NULL, zone->journal); + if (result == ISC_R_RANGE) { + dns_zone_log(zone, ISC_LOG_ERROR, + "ixfr-from-differences: failed: " + "difference too big to be stored " + "in journal"); + goto fail; + } if (result != ISC_R_SUCCESS) goto fail; if (dump) From 200a3fef3cedca7ce809195d06721a37ded920b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Kr=C4=99cicki?= Date: Wed, 27 Jun 2018 14:10:04 +0200 Subject: [PATCH 2/4] Fallback to normal procedure if creating of ixfr-from-differences fails (cherry picked from commit b1254430df1a9795ff2afec1face7a4f5f4a78e0) (cherry picked from commit e92d5421c37d03e48d4bcd17cee950cb0e4ce24d) --- lib/dns/journal.c | 15 ++++++++++----- lib/dns/zone.c | 12 ++++++------ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/dns/journal.c b/lib/dns/journal.c index 58f0338af6..82df35b7b3 100644 --- a/lib/dns/journal.c +++ b/lib/dns/journal.c @@ -1013,7 +1013,7 @@ dns_journal_writediff(dns_journal_t *j, dns_diff_t *diff) { dns_difftuple_t *t; isc_buffer_t buffer; void *mem = NULL; - size_t size; + isc_uint64_t size; isc_result_t result; isc_region_t used; @@ -1044,7 +1044,11 @@ dns_journal_writediff(dns_journal_t *j, dns_diff_t *diff) { } if (size >= DNS_JOURNAL_SIZE_MAX) { - return (ISC_R_RANGE); + isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, + "dns_journal_writediff: %s: journal entry " + "too big to be stored: %llu bytes", j->filename, + size); + return (ISC_R_NOSPACE); } mem = isc_mem_get(j->mctx, size); @@ -1100,6 +1104,7 @@ isc_result_t dns_journal_commit(dns_journal_t *j) { isc_result_t result; journal_rawheader_t rawheader; + isc_uint64_t total; REQUIRE(DNS_JOURNAL_VALID(j)); REQUIRE(j->state == JOURNAL_STATE_TRANSACTION || @@ -1152,12 +1157,12 @@ dns_journal_commit(dns_journal_t *j) { /* * We currently don't support huge journal entries. */ - unsigned long long total = j->x.pos[1].offset - j->x.pos[0].offset; + total = j->x.pos[1].offset - j->x.pos[0].offset; if (total >= DNS_JOURNAL_SIZE_MAX) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, - "transaction too big to be stored in journal :" + "transaction too big to be stored in journal: " "%llub (max is %llub)", total, - (unsigned long long)DNS_JOURNAL_SIZE_MAX); + (isc_uint64_t)DNS_JOURNAL_SIZE_MAX); return (ISC_R_UNEXPECTED); } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index ca1c06fda8..85b6008498 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -14281,15 +14281,14 @@ zone_replacedb(dns_zone_t *zone, dns_db_t *db, isc_boolean_t dump) { result = dns_db_diff(zone->mctx, db, ver, zone->db, NULL, zone->journal); - if (result == ISC_R_RANGE) { + if (result != ISC_R_SUCCESS) { + char strbuf[ISC_STRERRORSIZE]; + isc__strerror(errno, strbuf, sizeof(strbuf)); dns_zone_log(zone, ISC_LOG_ERROR, "ixfr-from-differences: failed: " - "difference too big to be stored " - "in journal"); - goto fail; + "%s", strbuf); + goto fallback; } - if (result != ISC_R_SUCCESS) - goto fail; if (dump) zone_needdump(zone, DNS_DUMP_DELAY); else if (zone->journalsize != -1) { @@ -14313,6 +14312,7 @@ zone_replacedb(dns_zone_t *zone, dns_db_t *db, isc_boolean_t dump) { if (zone->type == dns_zone_master && inline_raw(zone)) zone_send_secureserial(zone, serial); } else { + fallback: if (dump && zone->masterfile != NULL) { /* * If DNS_ZONEFLG_FORCEXFER was set we don't want From dd685955eec7d5695efe6af1053f628e317ad5a2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 27 Jun 2018 18:39:51 -0700 Subject: [PATCH 3/4] use ISC_INT32_MAX as maximum (cherry picked from commit 65bf99c85a2221d426a6cadaa09cf0a76d5d92a6) --- lib/dns/journal.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/dns/journal.c b/lib/dns/journal.c index 82df35b7b3..9aab656d41 100644 --- a/lib/dns/journal.c +++ b/lib/dns/journal.c @@ -14,8 +14,6 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: journal.c,v 1.120 2011/12/22 07:32:41 each Exp $ */ - #include #include @@ -1043,7 +1041,7 @@ dns_journal_writediff(dns_journal_t *j, dns_diff_t *diff) { size += t->rdata.length; } - if (size >= DNS_JOURNAL_SIZE_MAX) { + if (size >= ISC_INT32_MAX) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, "dns_journal_writediff: %s: journal entry " "too big to be stored: %llu bytes", j->filename, @@ -1158,11 +1156,11 @@ dns_journal_commit(dns_journal_t *j) { * We currently don't support huge journal entries. */ total = j->x.pos[1].offset - j->x.pos[0].offset; - if (total >= DNS_JOURNAL_SIZE_MAX) { + if (total >= ISC_INT32_MAX) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, "transaction too big to be stored in journal: " "%llub (max is %llub)", total, - (isc_uint64_t)DNS_JOURNAL_SIZE_MAX); + (isc_uint64_t)ISC_INT32_MAX); return (ISC_R_UNEXPECTED); } From c36dc130f5abd9a31c4be7f67840afb9cdb4a725 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 27 Jun 2018 17:34:51 -0700 Subject: [PATCH 4/4] CHANGES, release note (cherry picked from commit 2aee33f412ca7425989af9a249f9ca4292a4d33a) (cherry picked from commit 470b8612b247dba5a5f8e9ff1491fdb53cd4215b) --- CHANGES | 3 +++ doc/arm/notes.xml | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/CHANGES b/CHANGES index 64cced24c8..ba22d0708d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +4984. [bug] Improve handling of very large incremental + zone transfers to prevent journal corruption. [GL #339] + 4979. [bug] Non-libcap builds were not checking whether all requested capabilities are present in the permitted capability set. [GL #321] diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml index b301b5b287..7f2a98b69a 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -90,6 +90,14 @@
Bug Fixes + + + named now rejects excessively large + incremental (IXFR) zone transfers in order to prevent + possible corruption of journal files which could cause + named to abort when loading zones. [GL #339] + + rndc reload could cause named