dnssec: Fixes in changeset merging, freeing

- Added new field to dnssec policy structure (SOA serial increment
  policy)
- Removed debug code
- Added some info messages after succcesful signing

Refs #4

src/knot/server/zones.c

@@ -48,37 +48,6 @@ static const size_t XFRIN_CHANGESET_BINARY_SIZE = 100;
 static const size_t XFRIN_CHANGESET_BINARY_STEP = 100;
 static const size_t XFRIN_BOOTSTRAP_DELAY = 2000; /*!< AXFR bootstrap avg. delay */
 
-
-#include "libknot/rrset-dump.h"
-
-static void knot_zone_diff_dump_changeset(knot_changeset_t *ch)
-{
-	if (ch == NULL) {
-		return;
-	}
-	printf("Changeset FROM: %d\n", ch->serial_from);
-	knot_rrset_dump(ch->soa_from);
-	printf("\n");
-	printf("Changeset TO: %d\n", ch->serial_to);
-	knot_rrset_dump(ch->soa_to);
-	printf("\n");
-
-	printf("ADD section:\n");
-	printf("**********************************************\n");
-	char buf[1024];
-	knot_rr_ln_t *rr_node;
-	WALK_LIST(rr_node, ch->add) {
-		knot_rrset_txt_dump(rr_node->rr, buf, 1024, &KNOT_DUMP_STYLE_DNSSEC);
-		printf("%s\n", buf);
-	}
-	printf("REMOVE section:\n");
-	printf("**********************************************\n");
-	WALK_LIST(rr_node, ch->remove) {
-		knot_rrset_txt_dump(rr_node->rr, buf, 1024, &KNOT_DUMP_STYLE_DNSSEC);
-		printf("%s\n", buf);
-	}
-}
-
 /* Forward declarations. */
 static int zones_dump_zone_text(knot_zone_contents_t *zone,  const char *zf);
 
@@ -1089,10 +1058,10 @@ static void zones_free_merged_changesets(knot_changesets_t *diff_chs,
 	if (diff_chs == NULL &&
 	    sec_chs == NULL) {
 	} else if (diff_chs == NULL &&
-	    sec_chs != NULL) {
+	           sec_chs != NULL) {
 		knot_changesets_free(&sec_chs);
 	} else if (sec_chs == NULL &&
-	      diff_chs != NULL) {
+	           diff_chs != NULL) {
 		knot_changesets_free(&diff_chs);
 	} else {
 		/* 
@@ -1121,12 +1090,10 @@ static int zones_merge_and_store_changesets(knot_zone_t *zone,
 	}
 	if (!zones_changesets_empty(diff_chs) &&
 	    zones_changesets_empty(sec_chs)) {
-		knot_zone_diff_dump_changeset(knot_changesets_get_last(diff_chs));
 		return zones_store_changesets_to_disk(zone, diff_chs);
 	}
 	if (zones_changesets_empty(diff_chs) &&
 	    !zones_changesets_empty(sec_chs)) {
-		knot_zone_diff_dump_changeset(knot_changesets_get_last(sec_chs));
 		return zones_store_changesets_to_disk(zone, sec_chs);
 	}
 
@@ -1139,41 +1106,21 @@ static int zones_merge_and_store_changesets(knot_zone_t *zone,
 	if (ret != KNOT_EOK) {
 		return ret;
 	}
-
-	/* Rewrite SOAs in 'sec_chs' - we need to use SOAs from 'diff_chs' */
-	/* SOA to */
-	knot_rrset_deep_free(&knot_changesets_get_last(sec_chs)->soa_to, 1, 1);
-	knot_rrset_t *soa_copy = NULL;
-	ret = knot_rrset_deep_copy_no_sig(
-		knot_changesets_get_last(diff_chs)->soa_to, &soa_copy, 1);
-	if (ret != KNOT_EOK) {
-		return ret;
-	}
-	knot_changeset_add_soa(knot_changesets_get_last(sec_chs), soa_copy,
-	                       KNOT_CHANGESET_ADD);
-	knot_changesets_get_last(sec_chs)->serial_to =
-		knot_changesets_get_last(diff_chs)->serial_to;
-
-	/* SOA from */
-	knot_rrset_deep_free(&knot_changesets_get_last(sec_chs)->soa_from, 1, 1);
-	ret = knot_rrset_deep_copy_no_sig(
-		knot_changesets_get_last(diff_chs)->soa_from, &soa_copy, 1);
-	if (ret != KNOT_EOK) {
-		return ret;
-	}
-	knot_changeset_add_soa(knot_changesets_get_last(sec_chs), soa_copy,
-	                       KNOT_CHANGESET_REMOVE);
-	/* This changeset is not supposed to change serial! */
-	knot_changesets_get_last(sec_chs)->serial_from =
-		knot_changesets_get_last(diff_chs)->serial_to;
-
+	
+	/* SOAs in 'sec_chs' should be the same. */
+	assert(knot_changesets_get_last(sec_chs)->serial_from ==
+	       knot_changesets_get_last(sec_chs)->serial_to);
+	/* And they should be the same as the 'to' from 'diff_chs'. */
+	assert(knot_changesets_get_last(diff_chs)->serial_to ==
+	       knot_changesets_get_last(sec_chs)->serial_to);
 	/* First SOA should not be set. */
 	assert(sec_chs->first_soa == NULL);
-	knot_zone_diff_dump_changeset(knot_changesets_get_last(diff_chs));
 
-	/* Store ALL changes to disk. */
+	/* Store *ALL* changes to disk. (and only apply 'sec_chs', but not here. */
 	ret = zones_store_changesets_to_disk(zone, diff_chs);
 	if (ret != KNOT_EOK) {
+		log_zone_error("Could not store changesets to journal (%s)!",
+		               knot_strerror(ret));
 		return ret;
 	}
 
@@ -1460,8 +1407,11 @@ static int zones_insert_zone(conf_zone_t *z, knot_zone_t **dst,
 				return KNOT_ENOMEM;
 			}
 
-			/* Sign the zone. */
-			int ret = knot_dnssec_zone_sign(zone, sec_ch);
+			/* Sign the zone. Do not incr. serial if diff did that. */
+			knot_update_serial_t soa_up = 
+				zones_changesets_empty(diff_chs) ?
+				KNOT_SOA_SERIAL_INC : KNOT_SOA_SERIAL_KEEP;
+			int ret = knot_dnssec_zone_sign(zone, sec_ch, soa_up);
 			if (ret != KNOT_EOK) {
 				knot_changesets_free(&diff_chs);
 				knot_changesets_free(&sec_chs);
@@ -1480,8 +1430,7 @@ static int zones_insert_zone(conf_zone_t *z, knot_zone_t **dst,
 			rcu_read_unlock();
 			return ret;
 		}
-
-		knot_zone_diff_dump_changeset(sec_ch);
+	
 		/* Apply DNSSEC changeset. */
 		if (!knot_changeset_is_empty(sec_ch)) {
 			ret = xfrin_apply_changesets(zone, sec_chs,
@@ -1508,6 +1457,13 @@ static int zones_insert_zone(conf_zone_t *z, knot_zone_t **dst,
 			// No changes
 			ret = KNOT_ENODIFF;
 		}
+		
+		if (!zones_changesets_empty(sec_chs)) {
+			char *zname = knot_dname_to_str(zone->name);
+			log_zone_info("Zone %s was successfully resigned.\n",
+			              zname);
+			free(zname);
+		}
 
 		zones_free_merged_changesets(diff_chs, sec_chs);
 		rcu_read_unlock();
@@ -3419,7 +3375,7 @@ static int zones_dnssec_ev(event_t *event, bool force)
 	if (force) {
 		ret = knot_dnssec_zone_sign_force(zone, ch);
 	} else {
-		ret = knot_dnssec_zone_sign(zone, ch);
+		ret = knot_dnssec_zone_sign(zone, ch, KNOT_SOA_SERIAL_INC);
 	}
 	if (ret != KNOT_EOK) {
 		knot_changesets_free(&chs);
@@ -3428,8 +3384,6 @@ static int zones_dnssec_ev(event_t *event, bool force)
 		return ret;
 	}
 
-	knot_zone_diff_dump_changeset(ch);
-
 	knot_zone_contents_t *new_c = NULL;
 	ret = zones_store_and_apply_chgsets(chs, zone, &new_c, "DNSSEC",
 	                                    XFR_TYPE_UPDATE);
@@ -3447,6 +3401,10 @@ static int zones_dnssec_ev(event_t *event, bool force)
 	// cleanup
 	evsched_event_free(event->parent, event);
 	zd->dnssec_timer = NULL;
+	
+	char *zname = knot_dname_to_str(zone->name);
+	log_zone_info("Zone %s forced signed successfully.\n", zname);
+	free(zname);
 
 	return KNOT_EOK;
 }

src/libknot/dnssec/policy.h

@@ -27,17 +27,25 @@
 #ifndef _KNOT_DNSSEC_POLICY_H_
 #define _KNOT_DNSSEC_POLICY_H_
 
+typedef enum knot_update_serial {
+	KNOT_SOA_SERIAL_INC = 1 << 0,
+	KNOT_SOA_SERIAL_KEEP = 1 << 1
+} knot_update_serial_t;
+
 typedef struct {
-	uint32_t now;           //! Current time.
-	uint32_t sign_lifetime; //! Signature life time.
-	uint32_t sign_refresh;  //! Signature refresh time before expiration.
-	bool forced_sign;       //! Drop valid signatures as well.
+	uint32_t now;               //! Current time.
+	uint32_t sign_lifetime;     //! Signature life time.
+	uint32_t sign_refresh;      //! Sig. refresh time before expiration.
+	bool forced_sign;           //! Drop valid signatures as well.
+	knot_update_serial_t soa_up;//! Policy for serial updating.
 } knot_dnssec_policy_t;
 
 #define DEFAULT_DNSSEC_POLICY { .now = time_now(), .sign_lifetime = 2592000, \
-				.sign_refresh = 7200, .forced_sign = false }
+				.sign_refresh = 7200, .forced_sign = false, \
+				.soa_up = KNOT_SOA_SERIAL_INC }
 #define FORCED_DNSSEC_POLICY { .now = time_now(), .sign_lifetime = 2592000, \
-				.sign_refresh = 7200, .forced_sign = true }
+				.sign_refresh = 7200, .forced_sign = true, \
+				.soa_up = KNOT_SOA_SERIAL_INC }
 
 #endif // _KNOT_DNSSEC_POLICY_H_
 

src/libknot/dnssec/zone-events.c

@@ -31,19 +31,24 @@ static uint32_t time_now(void)
 	return (uint32_t)time(NULL);
 }
 
-static void init_default_policy(knot_dnssec_policy_t *p)
+static void init_default_policy(knot_dnssec_policy_t *p,
+                                knot_update_serial_t soa_up)
 {
 	knot_dnssec_policy_t p_image = DEFAULT_DNSSEC_POLICY;
 	memcpy(p, &p_image, sizeof(knot_dnssec_policy_t));
+	p->soa_up = soa_up;
 }
 
-static void init_forced_policy(knot_dnssec_policy_t *p)
+static void init_forced_policy(knot_dnssec_policy_t *p,
+                               knot_update_serial_t soa_up)
 {
 	knot_dnssec_policy_t p_image = FORCED_DNSSEC_POLICY;
 	memcpy(p, &p_image, sizeof(knot_dnssec_policy_t));
+	p->soa_up = soa_up;
 }
 
-static int zone_sign(knot_zone_t *zone, knot_changeset_t *out_ch, bool force)
+static int zone_sign(knot_zone_t *zone, knot_changeset_t *out_ch, bool force,
+                     knot_update_serial_t soa_up)
 {
 		if (zone == NULL) {
 		return KNOT_EINVAL;
@@ -82,9 +87,9 @@ static int zone_sign(knot_zone_t *zone, knot_changeset_t *out_ch, bool force)
 	// Create sign policy
 	knot_dnssec_policy_t policy;
 	if (force) {
-		init_forced_policy(&policy);
+		init_forced_policy(&policy, soa_up);
 	} else {
-		init_default_policy(&policy);
+		init_default_policy(&policy, soa_up);
 	}
 
 	// generate NSEC records
@@ -142,19 +147,18 @@ static int zone_sign(knot_zone_t *zone, knot_changeset_t *out_ch, bool force)
 	free_sign_contexts(&zone_keys);
 	free_zone_keys(&zone_keys);
 
-	printf("OK:%d %d\n", list_size(&out_ch->add), list_size(&out_ch->remove));
-
 	return KNOT_EOK;
 }
 
 int knot_dnssec_zone_sign(knot_zone_t *zone,
-                          knot_changeset_t *out_ch)
+                          knot_changeset_t *out_ch,
+                          knot_update_serial_t soa_up)
 {
-	return zone_sign(zone, out_ch, false);
+	return zone_sign(zone, out_ch, false, soa_up);
 }
 
 int knot_dnssec_zone_sign_force(knot_zone_t *zone,
                                 knot_changeset_t *out_ch)
 {
-	return zone_sign(zone, out_ch, true);
+	return zone_sign(zone, out_ch, true, KNOT_SOA_SERIAL_INC);
 }

src/libknot/dnssec/zone-events.h

@@ -30,8 +30,10 @@
 
 #include "libknot/zone/zone.h"
 #include "libknot/updates/changesets.h"
+#include "libknot/dnssec/policy.h"
 
-int knot_dnssec_zone_sign(knot_zone_t *zone, knot_changeset_t *out_ch);
+int knot_dnssec_zone_sign(knot_zone_t *zone, knot_changeset_t *out_ch,
+                          knot_update_serial_t soa_up);
 int knot_dnssec_zone_sign_force(knot_zone_t *zone, knot_changeset_t *out_ch);
 
 #endif // _KNOT_DNSSEC_ZONE_EVENTS_H_

src/libknot/dnssec/zone-sign.c

@@ -654,7 +654,7 @@ bool knot_zone_sign_soa_expired(const knot_zone_contents_t *zone,
 int knot_zone_sign_update_soa(const knot_zone_contents_t *zone,
                               const knot_zone_keys_t *zone_keys,
                               const knot_dnssec_policy_t *policy,
-			      knot_changeset_t *changeset)
+                              knot_changeset_t *changeset)
 {
 	knot_node_t *apex = knot_zone_contents_get_apex(zone);
 	knot_rrset_t *soa = knot_node_get_rrset(apex, KNOT_RRTYPE_SOA);
@@ -684,7 +684,12 @@ int knot_zone_sign_update_soa(const knot_zone_contents_t *zone,
 		return KNOT_EINVAL;
 
 	// TODO: proper increment, no check
-	uint32_t new_serial = serial + 1;
+	uint32_t new_serial = serial;
+	if (policy->soa_up == KNOT_SOA_SERIAL_INC) {
+		new_serial += 1;
+	} else {
+		assert(policy->soa_up == KNOT_SOA_SERIAL_KEEP);
+	}
 
 	// create SOA and new SOA with updated serial
 	knot_rrset_t *soa_from = NULL;
@@ -708,7 +713,7 @@ int knot_zone_sign_update_soa(const knot_zone_contents_t *zone,
 	if (result != KNOT_EOK) {
 		return result;
 	}
-
+	
 	// save the result
 
 	changeset->soa_from = soa_from;

src/libknot/updates/changesets.c

@@ -290,8 +290,8 @@ int knot_changeset_merge(knot_changeset_t *ch1, knot_changeset_t *ch2)
 	}
 
 	// Connect lists in changesets together
-	add_tail(&ch1->add, HEAD(ch2->add));
-	add_tail(&ch1->remove, HEAD(ch2->remove));
+	add_tail_list(&ch1->add, &ch2->add);
+	add_tail_list(&ch1->remove, &ch2->remove);
 
 	return KNOT_EOK;
 }