From 7af38b8e78522a31d72da2c938d96ed8f9bf46fb Mon Sep 17 00:00:00 2001
From: Daniel Salzman <daniel.salzman@nic.cz>
Date: Thu, 31 Mar 2022 12:56:03 +0200
Subject: [PATCH] conf: change 'semantic-checks' type from boolean to option,
 add 'soft' mode

---
 doc/man/knot.conf.5in                         | 29 +++++++++++-----
 doc/reference.rst                             | 26 ++++++++++-----
 src/knot/conf/schema.c                        | 11 ++++++-
 src/knot/conf/schema.h                        |  6 ++++
 src/knot/ctl/commands.c                       |  2 +-
 src/knot/events/handlers/refresh.c            |  4 +--
 src/knot/updates/zone-update.c                |  9 +++--
 src/knot/updates/zone-update.h                |  3 +-
 src/knot/zone/semantic-check.c                | 29 ++++++++++------
 src/knot/zone/semantic-check.h                | 12 ++++---
 src/knot/zone/zone-load.c                     |  5 ++-
 src/utils/kzonecheck/main.c                   |  4 +--
 .../zone/semcheck_soft/data/example.com.zone  | 16 +++++++++
 .../semcheck_soft/data/example.com.zone.1     | 20 +++++++++++
 tests-extra/tests/zone/semcheck_soft/test.py  | 33 +++++++++++++++++++
 tests-extra/tools/dnstest/server.py           |  5 ++-
 16 files changed, 167 insertions(+), 47 deletions(-)
 create mode 100644 tests-extra/tests/zone/semcheck_soft/data/example.com.zone
 create mode 100644 tests-extra/tests/zone/semcheck_soft/data/example.com.zone.1
 create mode 100644 tests-extra/tests/zone/semcheck_soft/test.py

diff --git a/doc/man/knot.conf.5in b/doc/man/knot.conf.5in
index 84afcfa44a..821c42b41c 100644
--- a/doc/man/knot.conf.5in
+++ b/doc/man/knot.conf.5in
@@ -1761,7 +1761,7 @@ zone:
     ddns\-master: remote_id
     notify: remote_id | remotes_id ...
     acl: acl_id ...
-    semantic\-checks: BOOL
+    semantic\-checks: BOOL | soft
     zonefile\-sync: TIME
     zonefile\-load: none | difference | difference\-no\-serial | whole
     journal\-content: none | changes | all
@@ -1864,22 +1864,33 @@ or disallow zone transfers, updates or incoming notifies.
 \fIDefault:\fP not set
 .SS semantic\-checks
 .sp
-If enabled, extra zone semantic checks are turned on.
+Selects if extra zone semantic checks are used or impacts of the mandatory checks.
 .sp
-Several checks are enabled by default and cannot be turned off. An error in
-mandatory checks causes zone not to be loaded. An error in extra checks is
-logged only.
+There are several mandatory checks which are always enabled and cannot be turned
+off. An error in a mandatory check causes the zone not to be loaded. Some of
+the mandatory checks can be weakened by setting \fBsoft\fP, when the zone isn\(aqt
+prevented from loading.
+.sp
+If enabled, extra checks are used. These checks don\(aqt prevent the zone from loading.
 .sp
 Mandatory checks:
 .INDENT 0.0
 .IP \(bu 2
-SOA record missing in the zone (\fI\%RFC 1034\fP)
+Missing SOA record at the zone apex (\fI\%RFC 1034\fP)
+.UNINDENT
+.sp
+Mandatory checks affected by the soft mode:
+.INDENT 0.0
+.IP \(bu 2
+An extra record exists together with a CNAME record except for RRSIG and DS (\fI\%RFC 1034\fP)
+.IP \(bu 2
+Multiple CNAME records with the same owner exist (\fI\%RFC 1034\fP)
 .IP \(bu 2
-An extra record together with CNAME record except for RRSIG and DS (\fI\%RFC 1034\fP)
+DNAME record having a record under it (\fI\%RFC 6672\fP)
 .IP \(bu 2
-Multiple CNAME record with the same owner
+Multiple DNAME records with the same owner exist (\fI\%RFC 6672\fP)
 .IP \(bu 2
-DNAME record having a record under it (\fI\%RFC 2672\fP)
+NS record exists together with a DNAME record (\fI\%RFC 6672\fP)
 .UNINDENT
 .sp
 Extra checks:
diff --git a/doc/reference.rst b/doc/reference.rst
index b1595d64e5..6cef282107 100644
--- a/doc/reference.rst
+++ b/doc/reference.rst
@@ -1924,7 +1924,7 @@ Definition of zones served by the server.
      ddns-master: remote_id
      notify: remote_id | remotes_id ...
      acl: acl_id ...
-     semantic-checks: BOOL
+     semantic-checks: BOOL | soft
      zonefile-sync: TIME
      zonefile-load: none | difference | difference-no-serial | whole
      journal-content: none | changes | all
@@ -2050,18 +2050,26 @@ or disallow zone transfers, updates or incoming notifies.
 semantic-checks
 ---------------
 
-If enabled, extra zone semantic checks are turned on.
+Selects if extra zone semantic checks are used or impacts of the mandatory checks.
 
-Several checks are enabled by default and cannot be turned off. An error in
-mandatory checks causes zone not to be loaded. An error in extra checks is
-logged only.
+There are several mandatory checks which are always enabled and cannot be turned
+off. An error in a mandatory check causes the zone not to be loaded. Some of
+the mandatory checks can be weakened by setting ``soft``, when the zone isn't
+prevented from loading.
+
+If enabled, extra checks are used. These checks don't prevent the zone from loading.
 
 Mandatory checks:
 
-- SOA record missing in the zone (:rfc:`1034`)
-- An extra record together with CNAME record except for RRSIG and DS (:rfc:`1034`)
-- Multiple CNAME record with the same owner
-- DNAME record having a record under it (:rfc:`2672`)
+- Missing SOA record at the zone apex (:rfc:`1034`)
+
+Mandatory checks affected by the soft mode:
+
+- An extra record exists together with a CNAME record except for RRSIG and DS (:rfc:`1034`)
+- Multiple CNAME records with the same owner exist (:rfc:`1034`)
+- DNAME record having a record under it (:rfc:`6672`)
+- Multiple DNAME records with the same owner exist (:rfc:`6672`)
+- NS record exists together with a DNAME record (:rfc:`6672`)
 
 Extra checks:
 
diff --git a/src/knot/conf/schema.c b/src/knot/conf/schema.c
index 4a23a4f8c8..7409abf590 100644
--- a/src/knot/conf/schema.c
+++ b/src/knot/conf/schema.c
@@ -124,6 +124,15 @@ static const knot_lookup_t serial_policies[] = {
 	{ 0, NULL }
 };
 
+static const knot_lookup_t semantic_checks[] = {
+	{ SEMCHECKS_OFF,  "off" },
+	{ SEMCHECKS_OFF,  "false" },
+	{ SEMCHECKS_ON,   "on" },
+	{ SEMCHECKS_ON,   "true" },
+	{ SEMCHECKS_SOFT, "soft" },
+	{ 0, NULL }
+};
+
 static const knot_lookup_t zone_digest[] = {
 	{ ZONE_DIGEST_NONE,   "none" },
 	{ ZONE_DIGEST_SHA384, "zonemd-sha384" },
@@ -418,7 +427,7 @@ static const yp_item_t desc_policy[] = {
 	{ C_DDNS_MASTER,         YP_TREF,  YP_VREF = { C_RMT }, YP_FNONE, { check_ref } }, \
 	{ C_NOTIFY,              YP_TREF,  YP_VREF = { C_RMT, C_RMTS }, YP_FMULTI, { check_ref } }, \
 	{ C_ACL,                 YP_TREF,  YP_VREF = { C_ACL }, YP_FMULTI, { check_ref } }, \
-	{ C_SEM_CHECKS,          YP_TBOOL, YP_VNONE, FLAGS }, \
+	{ C_SEM_CHECKS,          YP_TOPT,  YP_VOPT = { semantic_checks, SEMCHECKS_OFF }, FLAGS }, \
 	{ C_ZONEFILE_SYNC,       YP_TINT,  YP_VINT = { -1, INT32_MAX, 0, YP_STIME } }, \
 	{ C_ZONEFILE_LOAD,       YP_TOPT,  YP_VOPT = { zonefile_load, ZONEFILE_LOAD_WHOLE } }, \
 	{ C_JOURNAL_CONTENT,     YP_TOPT,  YP_VOPT = { journal_content, JOURNAL_CONTENT_CHANGES }, FLAGS }, \
diff --git a/src/knot/conf/schema.h b/src/knot/conf/schema.h
index 9ff258ac44..43ac1f9b24 100644
--- a/src/knot/conf/schema.h
+++ b/src/knot/conf/schema.h
@@ -208,6 +208,12 @@ enum {
 	SERIAL_POLICY_DATESERIAL = 3,
 };
 
+enum {
+	SEMCHECKS_OFF  = 0,
+	SEMCHECKS_ON   = 1,
+	SEMCHECKS_SOFT = 2,
+};
+
 enum {
 	ZONE_DIGEST_NONE   = 0,
 	ZONE_DIGEST_SHA384 = 1,
diff --git a/src/knot/ctl/commands.c b/src/knot/ctl/commands.c
index 8c290a885a..9bdc2192c0 100644
--- a/src/knot/ctl/commands.c
+++ b/src/knot/ctl/commands.c
@@ -746,7 +746,7 @@ static int zone_txn_commit(zone_t *zone, _unused_ ctl_args_t *args)
 		return KNOT_TXN_ENOTEXISTS;
 	}
 
-	int ret = zone_update_semcheck(zone->control_update);
+	int ret = zone_update_semcheck(conf(), zone->control_update);
 	if (ret != KNOT_EOK) {
 		return ret; // Recoverable error.
 	}
diff --git a/src/knot/events/handlers/refresh.c b/src/knot/events/handlers/refresh.c
index ab724034d2..f1392676f0 100644
--- a/src/knot/events/handlers/refresh.c
+++ b/src/knot/events/handlers/refresh.c
@@ -285,7 +285,7 @@ static int axfr_finalize(struct refresh_data *data)
 	// Seized by zone_update. Don't free the contents again in axfr_cleanup.
 	data->axfr.zone = NULL;
 
-	ret = zone_update_semcheck(&up);
+	ret = zone_update_semcheck(data->conf, &up);
 	if (ret == KNOT_EOK) {
 		ret = zone_update_verify_digest(data->conf, &up);
 	}
@@ -559,7 +559,7 @@ static int ixfr_finalize(struct refresh_data *data)
 		}
 	}
 
-	ret = zone_update_semcheck(&up);
+	ret = zone_update_semcheck(data->conf, &up);
 	if (ret == KNOT_EOK) {
 		ret = zone_update_verify_digest(data->conf, &up);
 	}
diff --git a/src/knot/updates/zone-update.c b/src/knot/updates/zone-update.c
index e0a26e6302..a648be91da 100644
--- a/src/knot/updates/zone-update.c
+++ b/src/knot/updates/zone-update.c
@@ -852,7 +852,7 @@ static void discard_adds_tree(zone_update_t *update)
 	update->new_cont->adds_tree = NULL;
 }
 
-int zone_update_semcheck(zone_update_t *update)
+int zone_update_semcheck(conf_t *conf, zone_update_t *update)
 {
 	if (update == NULL) {
 		return KNOT_EINVAL;
@@ -872,8 +872,11 @@ int zone_update_semcheck(zone_update_t *update)
 		.cb = err_handler_logger
 	};
 
-	ret = sem_checks_process(update->new_cont, SEMCHECK_MANDATORY_ONLY,
-	                         &handler, time(NULL));
+	conf_val_t val = conf_zone_get(conf, C_SEM_CHECKS, update->zone->name);
+	semcheck_optional_t mode = (conf_opt(&val) == SEMCHECKS_SOFT) ?
+	                           SEMCHECK_MANDATORY_SOFT : SEMCHECK_MANDATORY_ONLY;
+
+	ret = sem_checks_process(update->new_cont, mode, &handler, time(NULL));
 	if (ret != KNOT_EOK) {
 		// error is logged by the error handler
 		return ret;
diff --git a/src/knot/updates/zone-update.h b/src/knot/updates/zone-update.h
index 1cda54b262..09e36ecf32 100644
--- a/src/knot/updates/zone-update.h
+++ b/src/knot/updates/zone-update.h
@@ -256,11 +256,12 @@ int zone_update_increment_soa(zone_update_t *update, conf_t *conf);
 /*!
  * \brief Executes mandatory semantic checks on the zone contents.
  *
+ * \param conf    Configuration.
  * \param update  Update to be checked.
  *
  * \return KNOT_E*
  */
-int zone_update_semcheck(zone_update_t *update);
+int zone_update_semcheck(conf_t *conf, zone_update_t *update);
 
 /*!
  * \brief If configured, verify ZONEMD and log the result.
diff --git a/src/knot/zone/semantic-check.c b/src/knot/zone/semantic-check.c
index 710adb1c62..08c4b6a2ed 100644
--- a/src/knot/zone/semantic-check.c
+++ b/src/knot/zone/semantic-check.c
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2021 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
@@ -144,9 +144,10 @@ const char *sem_error_msg(sem_error_t code)
 
 typedef enum {
 	MANDATORY = 1 << 0,
-	OPTIONAL =  1 << 1,
-	NSEC =      1 << 2,
-	NSEC3 =     1 << 3,
+	SOFT      = 1 << 1,
+	OPTIONAL  = 1 << 2,
+	NSEC      = 1 << 3,
+	NSEC3     = 1 << 4,
 } check_level_t;
 
 typedef struct {
@@ -179,9 +180,9 @@ struct check_function {
 /* List of function callbacks for defined check_level */
 static const struct check_function CHECK_FUNCTIONS[] = {
 	{ check_soa,            MANDATORY },
-	{ check_cname,          MANDATORY },
-	{ check_dname,          MANDATORY },
-	{ check_delegation,     MANDATORY }, // mandatory for apex, optional for others
+	{ check_cname,          MANDATORY | SOFT },
+	{ check_dname,          MANDATORY | SOFT },
+	{ check_delegation,     MANDATORY | SOFT }, // mandatory for apex, optional for others
 	{ check_ds,             OPTIONAL },
 	{ check_submission,     NSEC | NSEC3 },
 	{ check_rrsig,          NSEC | NSEC3 },
@@ -1172,10 +1173,14 @@ static int do_checks_in_tree(zone_node_t *node, void *data)
 	for (int i = 0; ret == KNOT_EOK && i < CHECK_FUNCTIONS_LEN; ++i) {
 		if (CHECK_FUNCTIONS[i].level & s_data->level) {
 			ret = CHECK_FUNCTIONS[i].function(node, s_data);
+			if (s_data->handler->fatal_error &&
+			    (CHECK_FUNCTIONS[i].level & SOFT) &&
+			    (s_data->level & SOFT)) {
+				s_data->handler->fatal_error = false;
+			}
 		}
 	}
 
-
 	return ret;
 }
 
@@ -1272,10 +1277,14 @@ int sem_checks_process(zone_contents_t *zone, semcheck_optional_t optional, sem_
 		.time = time,
 	};
 
+	if (optional == SEMCHECKS_SOFT) {
+		data.level |= SOFT;
+	}
+
 	if (optional != SEMCHECK_MANDATORY_ONLY) {
 		data.level |= OPTIONAL;
-		if (optional == SEMCHECK_DNSSEC ||
-		    (optional == SEMCHECK_AUTO_DNSSEC && zone->dnssec)) {
+		if (optional == SEMCHECK_DNSSEC_ON ||
+		    (optional == SEMCHECK_DNSSEC_AUTO && zone->dnssec)) {
 			knot_rdataset_t *nsec3param = node_rdataset(zone->apex,
 			                                            KNOT_RRTYPE_NSEC3PARAM);
 			if (nsec3param != NULL) {
diff --git a/src/knot/zone/semantic-check.h b/src/knot/zone/semantic-check.h
index 6c9d2b3825..2face8d36d 100644
--- a/src/knot/zone/semantic-check.h
+++ b/src/knot/zone/semantic-check.h
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2021 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
@@ -16,14 +16,16 @@
 
 #pragma once
 
+#include "knot/conf/schema.h"
 #include "knot/zone/node.h"
 #include "knot/zone/contents.h"
 
 typedef enum {
-	SEMCHECK_MANDATORY_ONLY,
-	SEMCHECK_NO_DNSSEC,
-	SEMCHECK_AUTO_DNSSEC,
-	SEMCHECK_DNSSEC,
+	SEMCHECK_MANDATORY_ONLY = SEMCHECKS_OFF,
+	SEMCHECK_MANDATORY_SOFT = SEMCHECKS_SOFT,
+	SEMCHECK_DNSSEC_AUTO    = SEMCHECKS_ON,
+	SEMCHECK_DNSSEC_OFF,
+	SEMCHECK_DNSSEC_ON,
 } semcheck_optional_t;
 
 /*!
diff --git a/src/knot/zone/zone-load.c b/src/knot/zone/zone-load.c
index ced8830505..1db52971bd 100644
--- a/src/knot/zone/zone-load.c
+++ b/src/knot/zone/zone-load.c
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2021 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
@@ -35,8 +35,7 @@ int zone_load_contents(conf_t *conf, const knot_dname_t *zone_name,
 	conf_val_t val = conf_zone_get(conf, C_SEM_CHECKS, zone_name);
 
 	zloader_t zl;
-	int ret = zonefile_open(&zl, zonefile, zone_name,
-				conf_bool(&val) ? SEMCHECK_AUTO_DNSSEC : SEMCHECK_MANDATORY_ONLY, time(NULL));
+	int ret = zonefile_open(&zl, zonefile, zone_name, conf_opt(&val), time(NULL));
 	free(zonefile);
 	if (ret != KNOT_EOK) {
 		return ret;
diff --git a/src/utils/kzonecheck/main.c b/src/utils/kzonecheck/main.c
index 443c13a4dc..b0276a5f41 100644
--- a/src/utils/kzonecheck/main.c
+++ b/src/utils/kzonecheck/main.c
@@ -66,7 +66,7 @@ int main(int argc, char *argv[])
 {
 	const char *origin = NULL;
 	bool verbose = false;
-	semcheck_optional_t optional = SEMCHECK_AUTO_DNSSEC; // default value for --dnssec
+	semcheck_optional_t optional = SEMCHECK_DNSSEC_AUTO; // default value for --dnssec
 	knot_time_t check_time = (knot_time_t)time(NULL);
 
 	/* Long options. */
@@ -100,7 +100,7 @@ int main(int argc, char *argv[])
 			print_version(PROGRAM_NAME);
 			return EXIT_SUCCESS;
 		case 'd':
-			optional = str2bool(optarg) ? SEMCHECK_DNSSEC : SEMCHECK_NO_DNSSEC;
+			optional = str2bool(optarg) ? SEMCHECK_DNSSEC_ON : SEMCHECK_DNSSEC_OFF;
 			break;
 		case 't':
 			if (knot_time_parse("YMDhms|#|+-#U|+-#",
diff --git a/tests-extra/tests/zone/semcheck_soft/data/example.com.zone b/tests-extra/tests/zone/semcheck_soft/data/example.com.zone
new file mode 100644
index 0000000000..7edf362595
--- /dev/null
+++ b/tests-extra/tests/zone/semcheck_soft/data/example.com.zone
@@ -0,0 +1,16 @@
+$ORIGIN example.com.
+$TTL 3600
+
+@	SOA	dns1 hostmaster 1 10800 3600 1209600 7200
+	NS	dns1
+dns1	A	192.0.2.1
+
+labs	A	0.0.0.0
+labs	DNAME	dname1.example.com.
+
+labs	DNAME	dname2.example.com.
+
+labs	NS	dns1.example.com.
+labs	MX	1 dns1.example.com.
+
+www.labs A	0.0.0.0
diff --git a/tests-extra/tests/zone/semcheck_soft/data/example.com.zone.1 b/tests-extra/tests/zone/semcheck_soft/data/example.com.zone.1
new file mode 100644
index 0000000000..9b0c4aae48
--- /dev/null
+++ b/tests-extra/tests/zone/semcheck_soft/data/example.com.zone.1
@@ -0,0 +1,20 @@
+$ORIGIN example.com.
+$TTL 3600
+
+@	SOA	dns1 hostmaster 2 10800 3600 1209600 7200
+	NS	dns1
+dns1	A	192.0.2.1
+
+labs	A	0.0.0.0
+labs	DNAME	dname1.example.com.
+
+labs	DNAME	dname2.example.com.
+
+labs	NS	dns1.example.com.
+labs	MX	1 dns1.example.com.
+
+www.labs A	0.0.0.0
+
+cname	A	0.0.0.0
+cname	CNAME	example.net.
+cname	CNAME	example.org.
diff --git a/tests-extra/tests/zone/semcheck_soft/test.py b/tests-extra/tests/zone/semcheck_soft/test.py
new file mode 100644
index 0000000000..fd286bdb18
--- /dev/null
+++ b/tests-extra/tests/zone/semcheck_soft/test.py
@@ -0,0 +1,33 @@
+#!/usr/bin/env python3
+
+'''Test that a defective zone is loaded and transfered with soft semantic checks.'''
+
+from dnstest.test import Test
+import dnstest.utils
+
+t = Test()
+
+master = t.server("knot")
+slave = t.server("knot")
+
+zone = t.zone("example.com.", storage=".")
+
+t.link(zone, master, slave, ixfr=True)
+
+master.semantic_check = "soft"
+slave.semantic_check = "soft"
+
+t.start()
+
+serial_init = master.zones_wait(zone)
+slave.zones_wait(zone)
+t.xfr_diff(master, slave, zone)
+
+master.update_zonefile(zone[0], version=1)
+master.reload()
+
+master.zones_wait(zone, serial_init)
+slave.zones_wait(zone, serial_init)
+t.xfr_diff(master, slave, zone, serial_init)
+
+t.end()
diff --git a/tests-extra/tools/dnstest/server.py b/tests-extra/tools/dnstest/server.py
index 8a3b73cd37..baf41e5329 100644
--- a/tests-extra/tools/dnstest/server.py
+++ b/tests-extra/tools/dnstest/server.py
@@ -1471,7 +1471,10 @@ class Knot(Server):
             s.item_str("zonemd-generate", self.zonemd_generate)
         s.item_str("journal-max-usage", self.journal_max_usage)
         s.item_str("adjust-threads", str(random.randint(1,4)))
-        s.item_str("semantic-checks", "on" if self.semantic_check else "off")
+        if self.semantic_check == "soft":
+            self._str(s, "semantic-checks", self.semantic_check)
+        else:
+            self._bool(s, "semantic-checks", self.semantic_check)
         if len(self.modules) > 0:
             modules = ""
             for module in self.modules:
-- 
GitLab