From 12aee772f02acb1aee81f44f174326931b496ad6 Mon Sep 17 00:00:00 2001
From: Daniel Salzman <daniel.salzman@nic.cz>
Date: Tue, 31 May 2022 19:45:54 +0200
Subject: [PATCH] ixfr: add check on the last journal SOA serial and current
 zone SOA serial equality

---
 src/knot/nameserver/ixfr.c                    | 18 ++++++--
 src/knot/nameserver/ixfr.h                    |  3 +-
 .../data/example.com.zone                     | 16 +++++++
 .../data/example.com.zone.1                   | 16 +++++++
 .../data/example.com.zone.2                   | 16 +++++++
 .../tests/ixfr/inconsistent_history/test.py   | 45 +++++++++++++++++++
 6 files changed, 109 insertions(+), 5 deletions(-)
 create mode 100644 tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone
 create mode 100644 tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone.1
 create mode 100644 tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone.2
 create mode 100644 tests-extra/tests/ixfr/inconsistent_history/test.py

diff --git a/src/knot/nameserver/ixfr.c b/src/knot/nameserver/ixfr.c
index 1491e20f5a..7716ec0f9c 100644
--- a/src/knot/nameserver/ixfr.c
+++ b/src/knot/nameserver/ixfr.c
@@ -58,10 +58,14 @@ static int ixfr_put_chg_part(knot_pkt_t *pkt, struct ixfr_proc *ixfr,
 	}
 
 	while (journal_read_rrset(read, &ixfr->cur_rr, true)) {
-		if (ixfr->cur_rr.type == KNOT_RRTYPE_SOA &&
-		    !ixfr->in_remove_section &&
-		    knot_soa_serial(ixfr->cur_rr.rrs.rdata) == ixfr->soa_to) {
-			break;
+		if (ixfr->cur_rr.type == KNOT_RRTYPE_SOA) {
+			if (ixfr->in_remove_section) {
+				if (knot_soa_serial(ixfr->cur_rr.rrs.rdata) == ixfr->soa_to) {
+					break;
+				}
+			} else {
+				ixfr->soa_last = knot_soa_serial(ixfr->cur_rr.rrs.rdata);
+			}
 		}
 
 		if (pkt->size > KNOT_WIRE_PTR_MAX) {
@@ -199,6 +203,7 @@ static int ixfr_answer_init(knotd_qdata_t *qdata, uint32_t *serial_from)
 
 	xfr_stats_begin(&xfer->proc.stats);
 	xfer->state = IXFR_SOA_DEL;
+	xfer->in_remove_section = true;
 	init_list(&xfer->proc.nodes);
 	knot_rrset_init_empty(&xfer->cur_rr);
 	xfer->qdata = qdata;
@@ -207,6 +212,7 @@ static int ixfr_answer_init(knotd_qdata_t *qdata, uint32_t *serial_from)
 
 	xfer->soa_from = knot_soa_serial(their_soa->rrs.rdata);
 	xfer->soa_to = zone_contents_serial(qdata->extra->contents);
+	xfer->soa_last = xfer->soa_from;
 
 	qdata->extra->ext = xfer;
 	qdata->extra->ext_cleanup = &ixfr_answer_cleanup;
@@ -306,6 +312,10 @@ int ixfr_process_query(knot_pkt_t *pkt, knotd_qdata_t *qdata)
 	case KNOT_ESPACE: /* Couldn't write more, send packet and continue. */
 		return KNOT_STATE_PRODUCE; /* Check for more. */
 	case KNOT_EOK:    /* Last response. */
+		if (ixfr->soa_last != ixfr->soa_to) {
+			IXFROUT_LOG(LOG_ERR, qdata, "failed (inconsistent history)");
+			return KNOT_STATE_FAIL;
+		}
 		xfr_stats_end(&ixfr->proc.stats);
 		xfr_log_finished(ZONE_NAME(qdata), LOG_OPERATION_IXFR, LOG_DIRECTION_OUT,
 		                 REMOTE(qdata), false, &ixfr->proc.stats);
diff --git a/src/knot/nameserver/ixfr.h b/src/knot/nameserver/ixfr.h
index 50b88e7d60..3012be123e 100644
--- a/src/knot/nameserver/ixfr.h
+++ b/src/knot/nameserver/ixfr.h
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2019 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+/*  Copyright (C) 2022 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -50,6 +50,7 @@ struct ixfr_proc {
 	knot_mm_t *mm;
 	uint32_t soa_from;
 	uint32_t soa_to;
+	uint32_t soa_last;
 };
 
 /*!
diff --git a/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone b/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone
new file mode 100644
index 0000000000..8a1af1e3ce
--- /dev/null
+++ b/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone
@@ -0,0 +1,16 @@
+$ORIGIN example.com.
+$TTL 3600
+
+@	SOA	dns1 hostmaster 2010111201 10800 3600 1209600 7200
+	NS	dns1
+	NS	dns2
+	MX	10 mail
+
+dns1	A	192.0.2.1
+	AAAA	2001:DB8::1
+
+dns2	A	192.0.2.2
+	AAAA	2001:DB8::2
+
+mail	A	192.0.2.3
+	AAAA	2001:DB8::3
diff --git a/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone.1 b/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone.1
new file mode 100644
index 0000000000..dc0da7af21
--- /dev/null
+++ b/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone.1
@@ -0,0 +1,16 @@
+$ORIGIN example.com.
+$TTL 3600
+
+@	SOA	dns1 hostmaster 2010111202 10800 3600 1209600 7200
+	NS	dns1
+	NS	dns2
+	MX	10 mail
+
+dns1	A	192.0.2.2
+	AAAA	2001:DB8::1
+
+dns2	A	192.0.2.2
+	AAAA	2001:DB8::2
+
+mail	A	192.0.2.3
+	AAAA	2001:DB8::3
diff --git a/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone.2 b/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone.2
new file mode 100644
index 0000000000..4f077599a3
--- /dev/null
+++ b/tests-extra/tests/ixfr/inconsistent_history/data/example.com.zone.2
@@ -0,0 +1,16 @@
+$ORIGIN example.com.
+$TTL 3600
+
+@	SOA	dns1 hostmaster 2010111203 10800 3600 1209600 7200
+	NS	dns1
+	NS	dns2
+	MX	10 mail
+
+dns1	A	192.0.2.3
+	AAAA	2001:DB8::1
+
+dns2	A	192.0.2.2
+	AAAA	2001:DB8::2
+
+mail	A	192.0.2.3
+	AAAA	2001:DB8::3
diff --git a/tests-extra/tests/ixfr/inconsistent_history/test.py b/tests-extra/tests/ixfr/inconsistent_history/test.py
new file mode 100644
index 0000000000..55472ed371
--- /dev/null
+++ b/tests-extra/tests/ixfr/inconsistent_history/test.py
@@ -0,0 +1,45 @@
+#!/usr/bin/env python3
+
+'''Test for failed IXFR with inconsistent history'''
+
+from dnstest.test import Test
+
+t = Test()
+
+master = t.server("knot")
+slave = t.server("knot")
+zone = t.zone("example.com.", storage=".")
+t.link(zone, master, slave, ixfr=True)
+
+master.disable_notify = True
+slave.disable_notify = True
+
+t.start()
+
+# Start both servers with the initial zone
+serial = master.zone_wait(zone)
+slave.zone_wait(zone)
+
+# Add some zone history on the master only
+master.update_zonefile(zone, version=1)
+master.reload()
+serial = master.zone_wait(zone, serial)
+
+# Update the zone in a wrong way (zonefile-load: difference, journal-contents: changes, restart)
+# -> missing a changeset in the journal
+master.update_zonefile(zone, version=2)
+master.stop()
+master.start()
+
+# Try to refresh slave, IXFR should fail, AXFR ok
+slave.ctl("zone-refresh", wait=True)
+
+master.zone_wait(zone, serial)
+slave.zone_wait(zone, serial)
+
+# Check that slave has the actual zone
+resp = slave.dig("dns1.example.com.", "A")
+resp.check()
+resp.check_record(name="dns1.example.com.", rtype="A", rdata="192.0.2.3")
+
+t.end()
-- 
GitLab