From 7796e635d1fd1cf8ed0966bbddae7c3f7b232e8c Mon Sep 17 00:00:00 2001
From: Jan Kadlec <jan.kadlec@nic.cz>
Date: Wed, 23 Apr 2014 18:29:54 +0200
Subject: [PATCH] ttl-check: Fixed wrong TTL checks.

- in DDNS, the check would not work if the node was previously somehow
  changed, the check in changeset application should take care of this.

- check in changeset application would not work because it was triggered
  only when adding RRs into empty node.
---
 src/knot/nameserver/update.c |  6 +++-
 src/knot/updates/ddns.c      | 14 +--------
 src/knot/updates/xfr-in.c    | 59 +++++++++++++-----------------------
 src/knot/zone/zone-create.c  |  9 +++---
 src/knot/zone/zone-create.h  | 24 +++++++++++++++
 5 files changed, 55 insertions(+), 57 deletions(-)

diff --git a/src/knot/nameserver/update.c b/src/knot/nameserver/update.c
index 3c034c43e..d676d2975 100644
--- a/src/knot/nameserver/update.c
+++ b/src/knot/nameserver/update.c
@@ -281,7 +281,11 @@ static int zones_process_update_auth(struct query_data *qdata)
 		ret = xfrin_apply_changesets(zone, chgsets, &new_contents);
 		if (ret != KNOT_EOK) {
 			log_zone_notice("%s: Failed to process: %s.\n", msg, knot_strerror(ret));
-			qdata->rcode = KNOT_RCODE_SERVFAIL;
+			if (ret == KNOT_ETTL) {
+				qdata->rcode = KNOT_RCODE_REFUSED;
+			} else {
+				qdata->rcode = KNOT_RCODE_SERVFAIL;
+			}
 			knot_changesets_free(&chgsets);
 			free(msg);
 			return ret;
diff --git a/src/knot/updates/ddns.c b/src/knot/updates/ddns.c
index 0c9ade64d..059f4a725 100644
--- a/src/knot/updates/ddns.c
+++ b/src/knot/updates/ddns.c
@@ -667,18 +667,6 @@ static int process_add_normal(const zone_node_t *node,
 		return KNOT_EOK;
 	}
 
-	/* First check if the TTL of the new RR is equal to that of the first
-	 * RR in the node's RRSet. If not, refuse the UPDATE.
-	 */
-	knot_rrset_t rr_in_zone = node_rrset(node, rr->type);
-	if (node_rrtype_exists(node, rr->type)) {
-		const knot_rdata_t *add_data = knot_rdataset_at(&rr->rrs, 0);
-		const knot_rdata_t *zone_data = knot_rdataset_at(&rr_in_zone.rrs, 0);
-		if (knot_rdata_ttl(add_data) != knot_rdata_ttl(zone_data)) {
-			return KNOT_ETTL;
-		}
-	}
-
 	const bool apex_ns = node_rrtype_exists(node, KNOT_RRTYPE_SOA) &&
 	                     rr->type == KNOT_RRTYPE_NS;
 	return add_rr_to_chgset(rr, changeset, apex_ns ? apex_ns_rem : NULL);
@@ -931,7 +919,7 @@ static uint16_t ret_to_rcode(int ret)
 {
 	if (ret == KNOT_EMALF) {
 		return KNOT_RCODE_FORMERR;
-	} else if (ret == KNOT_EDENIED || ret == KNOT_ETTL) {
+	} else if (ret == KNOT_EDENIED) {
 		return KNOT_RCODE_REFUSED;
 	} else {
 		return KNOT_RCODE_SERVFAIL;
diff --git a/src/knot/updates/xfr-in.c b/src/knot/updates/xfr-in.c
index 46a2b9dd4..2829a3d67 100644
--- a/src/knot/updates/xfr-in.c
+++ b/src/knot/updates/xfr-in.c
@@ -826,7 +826,7 @@ static int xfrin_apply_remove(knot_zone_contents_t *contents,
 }
 
 static int add_rr(zone_node_t *node, const knot_rrset_t *rr,
-                  knot_changeset_t *chset)
+                  knot_changeset_t *chset, bool master)
 {
 	knot_rrset_t changed_rrset = node_rrset(node, rr->type);
 	if (!knot_rrset_empty(&changed_rrset)) {
@@ -843,37 +843,23 @@ static int add_rr(zone_node_t *node, const knot_rrset_t *rr,
 			clear_new_rrs(node, rr->type);
 			return ret;
 		}
-
-		// Extract copy, merge into it
-		knot_rdataset_t *changed_rrs = node_rdataset(node, rr->type);
-		ret = knot_rdataset_merge(changed_rrs, &rr->rrs, NULL);
-		if (ret != KNOT_EOK) {
-			clear_new_rrs(node, rr->type);
-			return ret;
-		}
-	} else {
-		// Inserting new RRSet, data will be copied.
-		bool ttl_err = false;
-		int ret = node_add_rrset(node, rr, &ttl_err);
+	}
+	// Insert new RR to RRSet, data will be copied.
+	bool ttl_err = false;
+	int ret = node_add_rrset(node, rr, &ttl_err);
+	if (ret != KNOT_EOK) {
+		return ret;
+	}
+	if (ttl_err) {
+		ret = log_ttl_error(node, rr, master);
 		if (ret != KNOT_EOK) {
 			return ret;
 		}
-
-		if (ttl_err) {
-			char type_str[16] = { '\0' };
-			knot_rrtype_to_string(rr->type, type_str, sizeof(type_str));
-			char *name = knot_dname_to_str(rr->owner);
-			char *zname = knot_dname_to_str(chset->soa_from->owner);
-			log_zone_warning("Changes application to zone %s: TTL mismatch"
-			                 " in %s, type %s\n", zname, name, type_str);
-			free(name);
-			free(zname);
-		}
 	}
 
 	// Get changed RRS and store for possible rollback.
 	knot_rdataset_t *rrs = node_rdataset(node, rr->type);
-	int ret = add_new_data(chset, rrs->data);
+	ret = add_new_data(chset, rrs->data);
 	if (ret != KNOT_EOK) {
 		knot_rdataset_clear(rrs, NULL);
 		return ret;
@@ -883,7 +869,7 @@ static int add_rr(zone_node_t *node, const knot_rrset_t *rr,
 }
 
 static int xfrin_apply_add(knot_zone_contents_t *contents,
-                           knot_changeset_t *chset)
+                           knot_changeset_t *chset, bool master)
 {
 	knot_rr_ln_t *rr_node = NULL;
 	WALK_LIST(rr_node, chset->add) {
@@ -895,7 +881,7 @@ static int xfrin_apply_add(knot_zone_contents_t *contents,
 			return KNOT_ENOMEM;
 		}
 
-		int ret = add_rr(node, rr, chset);
+		int ret = add_rr(node, rr, chset, master);
 		if (ret != KNOT_EOK) {
 			return ret;
 		}
@@ -917,14 +903,13 @@ static int xfrin_apply_replace_soa(knot_zone_contents_t *contents,
 
 	assert(!node_rrtype_exists(contents->apex, KNOT_RRTYPE_SOA));
 
-	return add_rr(contents->apex, chset->soa_to, chset);
+	return add_rr(contents->apex, chset->soa_to, chset, false);
 }
 
 /*----------------------------------------------------------------------------*/
 
-static int xfrin_apply_changeset(list_t *old_rrs, list_t *new_rrs,
-                                 knot_zone_contents_t *contents,
-                                 knot_changeset_t *chset)
+static int xfrin_apply_changeset(knot_zone_contents_t *contents,
+                                 knot_changeset_t *chset, bool master)
 {
 	/*
 	 * Applies one changeset to the zone. Checks if the changeset may be
@@ -948,7 +933,7 @@ static int xfrin_apply_changeset(list_t *old_rrs, list_t *new_rrs,
 		return ret;
 	}
 
-	ret = xfrin_apply_add(contents, chset);
+	ret = xfrin_apply_add(contents, chset, master);
 	if (ret != KNOT_EOK) {
 		return ret;
 	}
@@ -1160,9 +1145,8 @@ int xfrin_apply_changesets_directly(knot_zone_contents_t *contents,
 
 	knot_changeset_t *set = NULL;
 	WALK_LIST(set, chsets->sets) {
-		int ret = xfrin_apply_changeset(&set->old_data,
-		                                &set->new_data,
-		                                contents, set);
+		const bool master = true; // Only DNSSEC changesets are applied directly.
+		int ret = xfrin_apply_changeset(contents, set, master);
 		if (ret != KNOT_EOK) {
 			return ret;
 		}
@@ -1243,10 +1227,9 @@ int xfrin_apply_changesets(zone_t *zone,
 	dbg_xfrin_verb("Old contents apex: %p, new apex: %p\n",
 		       old_contents->apex, contents_copy->apex);
 	knot_changeset_t *set = NULL;
+	const bool master = (zone_master(zone) == NULL);
 	WALK_LIST(set, chsets->sets) {
-		ret = xfrin_apply_changeset(&set->old_data,
-		                            &set->new_data,
-		                            contents_copy, set);
+		ret = xfrin_apply_changeset(contents_copy, set, master);
 		if (ret != KNOT_EOK) {
 			xfrin_rollback_update(chsets, &contents_copy);
 			dbg_xfrin("Failed to apply changesets to zone: "
diff --git a/src/knot/zone/zone-create.c b/src/knot/zone/zone-create.c
index 70f7d076e..5e339e7cb 100644
--- a/src/knot/zone/zone-create.c
+++ b/src/knot/zone/zone-create.c
@@ -76,8 +76,7 @@ static bool handle_err(zcreator_t *zc,
 	}
 }
 
-static int log_ttl(const zcreator_t *zc, const zone_node_t *node,
-                   const knot_rrset_t *rr)
+int log_ttl_error(const zone_node_t *node, const knot_rrset_t *rr, bool master)
 {
 	err_handler_t err_handler;
 	err_handler_init(&err_handler);
@@ -91,11 +90,11 @@ static int log_ttl(const zcreator_t *zc, const zone_node_t *node,
 		*info_str = '\0';
 	}
 
-	if (zc->master) {
+	if (master) {
 		/*!< \todo REPLACE WITH FATAL ERROR */
 		err_handler_handle_error(&err_handler, node,
 		                         ZC_ERR_TTL_MISMATCH, info_str);
-		return KNOT_EMALF;
+		return KNOT_ETTL;
 	} else {
 		err_handler_handle_error(&err_handler, node,
 		                         ZC_ERR_TTL_MISMATCH, info_str);
@@ -129,7 +128,7 @@ int zcreator_step(zcreator_t *zc, const knot_rrset_t *rr)
 	assert(node);
 
 	if (ttl_err) {
-		ret = log_ttl(zc, node, rr);
+		ret = log_ttl_error(node, rr, zc->master);
 		if (ret != KNOT_EOK) {
 			return ret;
 		}
diff --git a/src/knot/zone/zone-create.h b/src/knot/zone/zone-create.h
index 8cb15d4ed..051b04658 100644
--- a/src/knot/zone/zone-create.h
+++ b/src/knot/zone/zone-create.h
@@ -82,10 +82,34 @@ knot_zone_contents_t *zonefile_load(zloader_t *loader);
  */
 void zonefile_close(zloader_t *loader);
 
+/*!
+ * \brief Adds one RR into zone.
+ *
+ * \param zl  Zone loader.
+ * \param rr  RR to add.
+ *
+ * \return KNOT_E*
+ */
 int zcreator_step(zcreator_t *zl, const knot_rrset_t *rr);
 
+/*!
+ * \brief Scanner error processing function.
+ * \param scanner  Scanner to use.
+ */
 void process_error(zs_scanner_t *scanner);
 
+/*!
+ * \brief Logs TTL mismatch error.
+ *
+ * \param node    Node with TTL mismatch.
+ * \param rr      RR that caused the mismatch.
+ * \param master  Master/slave switch.
+ *
+ * \retval KNOT_EOK if slave.
+ * \retval KNOT_ETTL if master.
+ */
+int log_ttl_error(const zone_node_t *node, const knot_rrset_t *rr, bool master);
+
 #endif /* _KNOTD_ZONELOAD_H_ */
 
 /*! @} */
-- 
GitLab