sem-checks: code cleanup, documentation clarification

doc/man/knot.conf.5in

@@ -1030,13 +1030,13 @@ logged only.
 Mandatory checks:
 .INDENT 0.0
 .IP \(bu 2
-An extra record together with CNAME record (except for RRSIG and DS)
+SOA record missing in the zone (RFC 1034)
 .IP \(bu 2
-Multiple CNAME records with the same owner
+An extra record together with CNAME record except for RRSIG and DS (RFC 1034)
 .IP \(bu 2
-SOA record missing in the zone (RFC 1034)
+Multiple CNAME record with the same owner
 .IP \(bu 2
-DNAME records having records under it (DNAME children) (RFC 2672)
+DNAME record having a record under it (RFC 2672)
 .UNINDENT
 .sp
 Extra checks:
@@ -1044,33 +1044,17 @@ Extra checks:
 .IP \(bu 2
 Missing NS record at the zone apex
 .IP \(bu 2
-Missing glue A or AAAA records
-.IP \(bu 2
-Broken or non\-cyclic NSEC(3) chain
-.IP \(bu 2
-Wrong NSEC(3) type bitmap
-.IP \(bu 2
-Multiple NSEC records at the same node
+Missing glue A or AAAA record
 .IP \(bu 2
-Missing NSEC records at authoritative nodes
+Invalid DNSKEY, DS, or NSEC3PARAM record
 .IP \(bu 2
-NSEC3 insecure delegation that is not part of Opt\-out span
+CDS or CDNSKEY inconsistency
 .IP \(bu 2
-Wrong original TTL value in NSEC3 records
+Missing, invalid, or unverifiable RRSIG record
 .IP \(bu 2
-Wrong RDATA TTL value in RRSIG record
+Invalid NSEC(3) record
 .IP \(bu 2
-Signer name in RRSIG RR not the same as in DNSKEY
-.IP \(bu 2
-Signed RRSIG
-.IP \(bu 2
-Wrong key flags or wrong key in RRSIG record (not the same as ZSK)
-.IP \(bu 2
-Invalid NSEC3PARAM flags or digest algorithm
-.IP \(bu 2
-Invalid DS algorithm or digest length
-.IP \(bu 2
-Not corresponding CDS and CDNSKEY records
+Broken or non\-cyclic NSEC(3) chain
 .UNINDENT
 .sp
 \fIDefault:\fP off

doc/reference.rst

@@ -1183,28 +1183,20 @@ logged only.
 
 Mandatory checks:
 
-- An extra record together with CNAME record (except for RRSIG and DS)
-- Multiple CNAME records with the same owner
 - SOA record missing in the zone (RFC 1034)
-- DNAME records having records under it (DNAME children) (RFC 2672)
+- 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)
 
 Extra checks:
 
 - Missing NS record at the zone apex
-- Missing glue A or AAAA records
+- Missing glue A or AAAA record
+- Invalid DNSKEY, DS, or NSEC3PARAM record
+- CDS or CDNSKEY inconsistency
+- Missing, invalid, or unverifiable RRSIG record
+- Invalid NSEC(3) record
 - Broken or non-cyclic NSEC(3) chain
-- Wrong NSEC(3) type bitmap
-- Multiple NSEC records at the same node
-- Missing NSEC records at authoritative nodes
-- NSEC3 insecure delegation that is not part of Opt-out span
-- Wrong original TTL value in NSEC3 records
-- Wrong RDATA TTL value in RRSIG record
-- Signer name in RRSIG RR not the same as in DNSKEY
-- Signed RRSIG
-- Wrong key flags or wrong key in RRSIG record (not the same as ZSK)
-- Invalid NSEC3PARAM flags or digest algorithm
-- Invalid DS algorithm or digest length
-- Not corresponding CDS and CDNSKEY records
 
 *Default:* off
 

src/knot/events/handlers/refresh.c

@@ -156,11 +156,11 @@ static time_t bootstrap_next(const zone_timers_t *timers)
 
 static int xfr_validate(zone_contents_t *zone, struct refresh_data *data)
 {
-	err_handler_t handler = {
+	sem_handler_t handler = {
 		.cb = err_handler_logger
 	};
 
-	int ret = zone_do_sem_checks(zone, false, &handler, time(NULL));
+	int ret = sem_checks_process(zone, false, &handler, time(NULL));
 	if (ret != KNOT_EOK) {
 		// error is logged by the error handler
 		return ret;

src/knot/zone/semantic-check.h

@@ -20,77 +20,77 @@
 #include "knot/zone/contents.h"
 
 /*!
- *\brief Internal error constants. General errors are added for convenience,
- *       so that code does not have to change if new errors are added.
+ *\brief Internal error constants.
  */
 typedef enum {
-	ZC_ERR_UNKNOWN = -50,
-
-	ZC_ERR_MISSING_SOA,
-	ZC_ERR_MISSING_NS_DEL_POINT,
-
-	ZC_ERR_GENERIC_GENERAL_ERROR, /* Generic error delimiter. */
-
-	ZC_ERR_RRSIG_RDATA_TYPE_COVERED,
-	ZC_ERR_RRSIG_RDATA_TTL,
-	ZC_ERR_RRSIG_RDATA_EXPIRATION,
-	ZC_ERR_RRSIG_RDATA_INCEPTION,
-	ZC_ERR_RRSIG_RDATA_LABELS,
-	ZC_ERR_RRSIG_RDATA_OWNER,
-	ZC_ERR_RRSIG_NO_RRSIG,
-	ZC_ERR_RRSIG_SIGNED,
-	ZC_ERR_RRSIG_TTL,
-	ZC_ERR_RRSIG_UNVERIFIABLE,
-
-	ZC_ERR_RRSIG_GENERAL_ERROR, /* RRSIG error delimiter. */
-
-	ZC_ERR_NSEC_NONE,
-	ZC_ERR_NSEC_RDATA_BITMAP,
-	ZC_ERR_NSEC_RDATA_MULTIPLE,
-	ZC_ERR_NSEC_RDATA_CHAIN,
-
-	ZC_ERR_NSEC_GENERAL_ERROR, /* NSEC error delimiter. */
-
-	ZC_ERR_NSEC3_NONE,
-	ZC_ERR_NSEC3_INSECURE_DELEGATION_OPT,
-	ZC_ERR_NSEC3_EXTRA_RECORD,
-	ZC_ERR_NSEC3_RDATA_TTL,
-	ZC_ERR_NSEC3_RDATA_CHAIN,
-	ZC_ERR_NSEC3_RDATA_BITMAP,
-	ZC_ERR_NSEC3_RDATA_FLAGS,
-	ZC_ERR_NSEC3_RDATA_SALT,
-	ZC_ERR_NSEC3_RDATA_ALG,
-	ZC_ERR_NSEC3_RDATA_ITERS,
-
-	ZC_ERR_NSEC3_PARAM_RDATA_FLAGS,
-	ZC_ERR_NSEC3_PARAM_RDATA_ALG,
-
-	ZC_ERR_NSEC3_GENERAL_ERROR, /* NSEC3 error delimiter. */
-
-	ZC_ERR_CNAME_EXTRA_RECORDS,
-	ZC_ERR_CNAME_MULTIPLE,
-	ZC_ERR_DNAME_CHILDREN,
-
-	ZC_ERR_CNAME_GENERAL_ERROR, /* CNAME/DNAME error delimiter. */
-
-	ZC_ERR_GLUE_RECORD,
-
-	ZC_ERR_DS_RDATA_ALG,
-	ZC_ERR_DS_RDATA_DIGLEN,
-
-	ZC_ERR_INVALID_KEY,
-
-	ZC_ERR_CDS_CDNSKEY,
-
-	ZC_ERR_LAST,
-} zc_error_t;
-
-const char *semantic_check_error_msg(int ecode);
+	// Mandatory checks.
+	SEM_ERR_SOA_NONE,
+
+	SEM_ERR_CNAME_EXTRA_RECORDS,
+	SEM_ERR_CNAME_MULTIPLE,
+
+	SEM_ERR_DNAME_CHILDREN,
+
+	// Optional checks.
+	SEM_ERR_NS_APEX,
+	SEM_ERR_NS_GLUE,
+
+	SEM_ERR_RRSIG_RDATA_TYPE_COVERED,
+	SEM_ERR_RRSIG_RDATA_TTL,
+	SEM_ERR_RRSIG_RDATA_EXPIRATION,
+	SEM_ERR_RRSIG_RDATA_INCEPTION,
+	SEM_ERR_RRSIG_RDATA_LABELS,
+	SEM_ERR_RRSIG_RDATA_OWNER,
+	SEM_ERR_RRSIG_NO_RRSIG,
+	SEM_ERR_RRSIG_SIGNED,
+	SEM_ERR_RRSIG_TTL,
+	SEM_ERR_RRSIG_UNVERIFIABLE,
+
+	SEM_ERR_NSEC_NONE,
+	SEM_ERR_NSEC_RDATA_BITMAP,
+	SEM_ERR_NSEC_RDATA_MULTIPLE,
+	SEM_ERR_NSEC_RDATA_CHAIN,
+
+	SEM_ERR_NSEC3_NONE,
+	SEM_ERR_NSEC3_INSECURE_DELEGATION_OPT,
+	SEM_ERR_NSEC3_EXTRA_RECORD,
+	SEM_ERR_NSEC3_RDATA_TTL,
+	SEM_ERR_NSEC3_RDATA_CHAIN,
+	SEM_ERR_NSEC3_RDATA_BITMAP,
+	SEM_ERR_NSEC3_RDATA_FLAGS,
+	SEM_ERR_NSEC3_RDATA_SALT,
+	SEM_ERR_NSEC3_RDATA_ALG,
+	SEM_ERR_NSEC3_RDATA_ITERS,
+
+	SEM_ERR_NSEC3PARAM_RDATA_FLAGS,
+	SEM_ERR_NSEC3PARAM_RDATA_ALG,
+
+	SEM_ERR_DS_RDATA_ALG,
+	SEM_ERR_DS_RDATA_DIGLEN,
+
+	SEM_ERR_DNSKEY_NONE,
+	SEM_ERR_DNSKEY_INVALID,
+	SEM_ERR_DNSKEY_RDATA_PROTOCOL,
+
+	SEM_ERR_CDS_NONE,
+	SEM_ERR_CDS_MULTIPLE,
+	SEM_ERR_CDS_NOT_MATCH,
+
+	SEM_ERR_CDNSKEY_NONE,
+	SEM_ERR_CDNSKEY_MULTIPLE,
+	SEM_ERR_CDNSKEY_NOT_KSK,
+	SEM_ERR_CDNSKEY_NO_DNSKEY,
+
+	// General error!
+	SEM_ERR_UNKNOWN
+} sem_error_t;
+
+const char *sem_error_msg(sem_error_t code);
 
 /*!
  * \brief Structure for handling semantic errors.
  */
-typedef struct err_handler err_handler_t;
+typedef struct sem_handler sem_handler_t;
 
 /*!
  * \brief Callback for handle error.
@@ -98,11 +98,11 @@ typedef struct err_handler err_handler_t;
  * Return KNOT_EOK to continue in semantic checks.
  * Return other KNOT_E* to stop semantic check with error.
  */
-typedef void (*error_cb) (err_handler_t *ctx, const zone_contents_t *zone,
-                          const zone_node_t *node, zc_error_t error, const char *data);
+typedef void (*sem_callback) (sem_handler_t *ctx, const zone_contents_t *zone,
+                              const zone_node_t *node, sem_error_t error, const char *data);
 
-struct err_handler {
-	error_cb cb;
+struct sem_handler {
+	sem_callback cb;
 	bool fatal_error;
 };
 
@@ -120,5 +120,5 @@ struct err_handler {
  * \retval KNOT_ESEMCHECK found semantic error
  * \retval KNOT_EINVAL or other error
  */
-int zone_do_sem_checks(zone_contents_t *zone, bool optional,
-                       err_handler_t *handler, time_t time);
+int sem_checks_process(zone_contents_t *zone, bool optional, sem_handler_t *handler,
+                       time_t time);

src/knot/zone/zone-load.c

@@ -42,7 +42,7 @@ int zone_load_contents(conf_t *conf, const knot_dname_t *zone_name,
 		return ret;
 	}
 
-	err_handler_t handler = {
+	sem_handler_t handler = {
 		.cb = err_handler_logger
 	};
 

src/knot/zone/zonefile.c

@@ -251,7 +251,7 @@ zone_contents_t *zonefile_load(zloader_t *loader)
 	if (!node_rrtype_exists(loader->creator->z->apex, KNOT_RRTYPE_SOA)) {
 		loader->err_handler->fatal_error = true;
 		loader->err_handler->cb(loader->err_handler, zc->z, NULL,
-		                        ZC_ERR_MISSING_SOA, NULL);
+		                        SEM_ERR_SOA_NONE, NULL);
 		goto fail;
 	}
 
@@ -262,7 +262,7 @@ zone_contents_t *zonefile_load(zloader_t *loader)
 		goto fail;
 	}
 
-	ret = zone_do_sem_checks(zc->z, loader->semantic_checks,
+	ret = sem_checks_process(zc->z, loader->semantic_checks,
 	                         loader->err_handler, loader->time);
 
 	if (ret != KNOT_EOK) {
@@ -347,8 +347,8 @@ void zonefile_close(zloader_t *loader)
 	free(loader->creator);
 }
 
-void err_handler_logger(err_handler_t *handler, const zone_contents_t *zone,
-                        const zone_node_t *node, zc_error_t error, const char *data)
+void err_handler_logger(sem_handler_t *handler, const zone_contents_t *zone,
+                        const zone_node_t *node, sem_error_t error, const char *data)
 {
 	assert(handler != NULL);
 	assert(zone != NULL);
@@ -363,7 +363,7 @@ void err_handler_logger(err_handler_t *handler, const zone_contents_t *zone,
 	             "check%s%s, %s%s%s",
 	             (node != NULL ? ", node " : ""),
 	             (node != NULL ? buff      : ""),
-	             semantic_check_error_msg(error),
+	             sem_error_msg(error),
 	             (data != NULL ? " "  : ""),
 	             (data != NULL ? data : ""));
 }

src/knot/zone/zonefile.h

@@ -37,14 +37,14 @@ typedef struct zcreator {
 typedef struct {
 	char *source;                /*!< Zone source file. */
 	bool semantic_checks;        /*!< Do semantic checks. */
-	err_handler_t *err_handler;  /*!< Semantic checks error handler. */
+	sem_handler_t *err_handler;  /*!< Semantic checks error handler. */
 	zcreator_t *creator;         /*!< Loader context. */
 	zs_scanner_t scanner;        /*!< Zone scanner. */
 	time_t time;                 /*!< time for zone check. */
 } zloader_t;
 
-void err_handler_logger(err_handler_t *handler, const zone_contents_t *zone,
-                        const zone_node_t *node, zc_error_t error, const char *data);
+void err_handler_logger(sem_handler_t *handler, const zone_contents_t *zone,
+                        const zone_node_t *node, sem_error_t error, const char *data);
 
 /*!
  * \brief Open zone file for loading.

src/utils/kzonecheck/zone_check.c

@@ -22,14 +22,14 @@
 #include "utils/kzonecheck/zone_check.h"
 
 typedef struct {
-	err_handler_t handler;
+	sem_handler_t handler;
 	FILE *outfile;
-	unsigned errors[(-ZC_ERR_UNKNOWN) + 1]; /*!< Counting errors by type */
-	unsigned error_count; /*!< Total error count */
+	unsigned errors[SEM_ERR_UNKNOWN + 1]; /*!< Counting errors by type. */
+	unsigned error_count;                 /*!< Total error count. */
 } err_handler_stats_t;
 
-static void err_callback(err_handler_t *handler, const zone_contents_t *zone,
-                         const zone_node_t *node, zc_error_t error, const char *data)
+static void err_callback(sem_handler_t *handler, const zone_contents_t *zone,
+                         const zone_node_t *node, sem_error_t error, const char *data)
 {
 	assert(handler != NULL);
 	assert(zone != NULL);
@@ -40,21 +40,21 @@ static void err_callback(err_handler_t *handler, const zone_contents_t *zone,
 	                        sizeof(buff));
 
 	fprintf(stats->outfile, "[%s] %s%s%s\n",
-	        buff, semantic_check_error_msg(error),
+	        buff, sem_error_msg(error),
 	        (data != NULL ? " "  : ""),
 	        (data != NULL ? data : ""));
 
-	stats->errors[-error]++;
+	stats->errors[error]++;
 	stats->error_count++;
 }
 
 static void print_statistics(err_handler_stats_t *stats)
 {
 	fprintf(stats->outfile, "\nError summary:\n");
-	for(int i = ZC_ERR_UNKNOWN; i < ZC_ERR_LAST; ++i) {
-		if (stats->errors[-i] > 0) {
-			fprintf(stats->outfile, "%4u\t%s\n", stats->errors[-i],
-			        semantic_check_error_msg(i));
+	for (sem_error_t i = 0; i <= SEM_ERR_UNKNOWN; ++i) {
+		if (stats->errors[i] > 0) {
+			fprintf(stats->outfile, "%4u\t%s\n", stats->errors[i],
+			        sem_error_msg(i));
 		}
 	}
 }
@@ -72,7 +72,7 @@ int zone_check(const char *zone_file, const knot_dname_t *zone_name,
 	if (ret != KNOT_EOK) {
 		return ret;
 	}
-	zl.err_handler = (err_handler_t *)&stats;
+	zl.err_handler = (sem_handler_t *)&stats;
 	zl.creator->master = true;
 
 	zone_contents_t *contents = zonefile_load(&zl);

tests/test_semantic_check.in

@@ -40,19 +40,18 @@ if [ ! -x $KZONECHECK ]; then
 	skip_all "kzonecheck is missing or is not executable"
 fi
 
-# error messages
-# exported from knot/src/zone/semantic-check.c
-UNKNOWN="unknown error"
-MISSING_SOA="missing SOA in zone apex"
-MISSING_NS_DEL_POINT="missing NS in zone apex"
+# error messages exported from knot/src/zone/semantic-check.c
+NS_APEX="missing NS at the zone apex"
+NS_GLUE="missing glue record"
 RRSIG_RDATA_TYPE_COVERED="wrong type covered in RRSIG"
 RRSIG_RDATA_TTL="wrong original TTL in RRSIG"
-RRSIG_RDATA_EXPIRATION="expired RRSIG"
+RRSIG_EXPIRED="expired RRSIG"
 RRSIG_RDATA_LABELS="wrong labels in RRSIG"
 RRSIG_RDATA_DNSKEY_OWNER="wrong signer's name in RRSIG"
 RRSIG_NO_RRSIG="missing RRSIG"
 RRSIG_SIGNED="signed RRSIG"
 RRSIG_TTL="wrong RRSIG TTL"
+RRSIG_UNVERIFIABLE="unverifiable signature"
 NSEC_NONE="missing NSEC"
 NSEC_RDATA_BITMAP="incorrect type bitmap in NSEC"
 NSEC_RDATA_MULTIPLE="multiple NSEC records"
@@ -66,15 +65,18 @@ NSEC3_EXTRA_RECORD="invalid record type in NSEC3 chain"
 CNAME_EXTRA_RECORDS="more records exist at CNAME"
 DNAME_CHILDREN="child record exists under DNAME"
 CNAME_MULTIPLE="multiple CNAME records"
-GLUE_RECORD="missing glue record"
-UNVERIFIABLE="unverifiable signature"
-EXPIRED="expired RRSIG"
 NSEC3_ALG="incorrect algorithm in NSEC3"
 NSEC3_ITERS="incorrect number of iterations in NSEC3"
 NSEC3PARAM_FLAGS="invalid flags in NSEC3PARAM"
-INVALIDKEY="invalid DNSKEY"
+DNSKEY_PROTO="invalid protocol in DNSKEY"
 DS_ALG="invalid algorithm in DS"
-CDS="incorrect CDS and CDNSKEY pair"
+CDS_NONE="missing CDS"
+CDS_MULTIPLE="multiple CDS records"
+CDS_NOT_MATCH="CDS not match CDNSKEY"
+CDNSKEY_NONE="missing CDNSKEY"
+CDNSKEY_MULTIPLE="multiple CDNSKEY records"
+CDNSKEY_NOT_KSK="CDNSKEY not match KSK DNSKEY"
+CDNSKEY_NO_DNSKEY="CDNSKEY not match DNSKEY"
 
 plan_lazy
 
@@ -83,12 +85,12 @@ expect_error "cname_extra_02.signed" 1 1 "$CNAME_EXTRA_RECORDS"
 expect_error "cname_multiple.zone"   1 1 "$CNAME_MULTIPLE"
 expect_error "dname_children.zone"   1 1 "$DNAME_CHILDREN"
 
-expect_error "missing_ns.zone" 0 1 "$MISSING_NS_DEL_POINT"
-expect_error "missing_glue_01.zone" 0 2 "$GLUE_RECORD"
-expect_error "missing_glue_02.zone" 0 1 "$GLUE_RECORD"
-expect_error "missing_glue_03.zone" 0 1 "$GLUE_RECORD"
+expect_error "missing_ns.zone" 0 1 "$NS_APEX"
+expect_error "missing_glue_01.zone" 0 2 "$NS_GLUE"
+expect_error "missing_glue_02.zone" 0 1 "$NS_GLUE"
+expect_error "missing_glue_03.zone" 0 1 "$NS_GLUE"
 expect_error "different_signer_name.signed" 0 1 "$RRSIG_RDATA_DNSKEY_OWNER \(record type NSEC\)"
-expect_error "different_signer_name.signed" 0 1 "$UNVERIFIABLE \(record type NSEC\)"
+expect_error "different_signer_name.signed" 0 1 "$RRSIG_UNVERIFIABLE \(record type NSEC\)"
 expect_error "no_rrsig.signed" 0 1 "$RRSIG_NO_RRSIG \(record type A\)"
 expect_error "no_rrsig.signed" 0 1 "$RRSIG_NO_RRSIG \(record type NSEC\)"
 expect_error "no_rrsig_with_delegation.signed" 0 1 "$RRSIG_NO_RRSIG \(record type NSEC\)"
@@ -112,17 +114,17 @@ expect_error "nsec3_param_invalid.signed" 0 1 "$NSEC3PARAM_FLAGS"
 expect_error "rrsig_signed.signed" 0 1 "$RRSIG_SIGNED"
 expect_error "rrsig_ttl.signed" 0 1 "$RRSIG_TTL \(record type A\)"
 expect_error "rrsig_rdata_ttl.signed" 0 1 "$RRSIG_RDATA_TTL \(record type A\)"
-expect_error "duplicate.signature" 0 7 "$EXPIRED"
+expect_error "duplicate.signature" 0 7 "$RRSIG_EXPIRED"
 expect_error "missing.signed" 0 1 "$NSEC_NONE"
-expect_error "dnskey_param_error.signed" 0 1 "$INVALIDKEY \(invalid protocol\)"
+expect_error "dnskey_param_error.signed" 0 1 "$DNSKEY_PROTO"
 expect_error "invalid_ds.signed" 0 2 "$DS_ALG \(keytag 60485\)"
-expect_error "cdnskey.invalid" 0 1 "$CDS \(not match\)"
-expect_error "cdnskey.invalid.param" 0 1 "$CDS \(not match\)"
-expect_error "cdnskey.nocds" 0 1 "$CDS \(no CDS\)"
-expect_error "cdnskey.nocdnskey" 0 1 "$CDS \(no CDNSKEY\)"
-expect_error "cdnskey.nodnskey" 0 1 "$CDS \(no corresponding DNSKEY\)"
-expect_error "cdnskey.two" 0 1 "$CDS \(ambiguous CDS record\)"
-expect_error "cdnskey.two" 0 1 "$CDS \(ambiguous CDNSKEY record\)"
+expect_error "cdnskey.invalid" 0 1 "$CDS_NOT_MATCH"
+expect_error "cdnskey.invalid.param" 0 1 "$CDS_NOT_MATCH"
+expect_error "cdnskey.nocds" 0 1 "$CDS_NONE"
+expect_error "cdnskey.nocdnskey" 0 1 "$CDNSKEY_NONE"
+expect_error "cdnskey.nodnskey" 0 1 "$CDNSKEY_NOT_MATCH"
+expect_error "cdnskey.two" 0 1 "$CDS_MULTIPLE"
+expect_error "cdnskey.two" 0 1 "$CDNSKEY_MULTIPLE"
 
 test_correct "no_error_delegaton_bitmap.signed"
 test_correct "no_error_nsec3_delegation.signed"