From 1780a91a0a47225035289a83954e7e6cd3f5727b Mon Sep 17 00:00:00 2001
From: Daniel Salzman <daniel.salzman@nic.cz>
Date: Mon, 6 Sep 2021 16:11:11 +0200
Subject: [PATCH] acl: apply deny rule without tsig to queries with tsig
 present

---
 src/knot/updates/acl.c | 29 +++++++++++++++++++----------
 tests/knot/test_acl.c  | 27 ++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/src/knot/updates/acl.c b/src/knot/updates/acl.c
index 21dd74b189..4335e90573 100644
--- a/src/knot/updates/acl.c
+++ b/src/knot/updates/acl.c
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2020 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+/*  Copyright (C) 2021 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
@@ -156,7 +156,7 @@ static bool update_match(conf_t *conf, conf_val_t *acl, knot_dname_t *key_name,
 
 static bool check_addr_key(conf_t *conf, conf_val_t *addr_val, conf_val_t *key_val,
                            bool remote, const struct sockaddr_storage *addr,
-                           const knot_tsig_key_t *tsig)
+                           const knot_tsig_key_t *tsig, bool deny)
 {
 	/* Check if the address matches the acl address list or remote addresses. */
 	if (addr_val->code != KNOT_ENOENT) {
@@ -201,9 +201,17 @@ static bool check_addr_key(conf_t *conf, conf_val_t *addr_val, conf_val_t *key_v
 			conf_val_next(key_val);
 		}
 	}
-	/* Check for key match or empty list without key provided. */
-	if (key_val->code != KNOT_EOK &&
-	    !(key_val->code == KNOT_ENOENT && tsig->name == NULL)) {
+	switch (key_val->code) {
+	case KNOT_EOK:
+		// Key match.
+		break;
+	case KNOT_ENOENT:
+		// Empty list without key provided or denied.
+		if (tsig->name == NULL || deny) {
+			break;
+		}
+		// FALLTHROUGH
+	default:
 		return false;
 	}
 
@@ -221,13 +229,15 @@ bool acl_allowed(conf_t *conf, conf_val_t *acl, acl_action_t action,
 	while (acl->code == KNOT_EOK) {
 		conf_val_t rmt_val = conf_id_get(conf, C_ACL, C_RMT, acl);
 		bool remote = (rmt_val.code == KNOT_EOK);
+		conf_val_t deny_val = conf_id_get(conf, C_ACL, C_DENY, acl);
+		bool deny = conf_bool(&deny_val);
 
 		/* Check if a remote matches given address and key. */
 		conf_val_t addr_val, key_val;
 		while (rmt_val.code == KNOT_EOK) {
 			addr_val = conf_id_get(conf, C_RMT, C_ADDR, &rmt_val);
 			key_val = conf_id_get(conf, C_RMT, C_KEY, &rmt_val);
-			if (check_addr_key(conf, &addr_val, &key_val, remote, addr, tsig)) {
+			if (check_addr_key(conf, &addr_val, &key_val, remote, addr, tsig, deny)) {
 				break;
 			}
 			conf_val_next(&rmt_val);
@@ -239,7 +249,7 @@ bool acl_allowed(conf_t *conf, conf_val_t *acl, acl_action_t action,
 		if (!remote) {
 			addr_val = conf_id_get(conf, C_ACL, C_ADDR, acl);
 			key_val = conf_id_get(conf, C_ACL, C_KEY, acl);
-			if (!check_addr_key(conf, &addr_val, &key_val, remote, addr, tsig)) {
+			if (!check_addr_key(conf, &addr_val, &key_val, remote, addr, tsig, deny)) {
 				goto next_acl;
 			}
 		}
@@ -272,14 +282,13 @@ bool acl_allowed(conf_t *conf, conf_val_t *acl, acl_action_t action,
 		}
 
 		/* Check if denied. */
-		conf_val_t val = conf_id_get(conf, C_ACL, C_DENY, acl);
-		if (conf_bool(&val)) {
+		if (deny) {
 			return false;
 		}
 
 		/* Fill the output with tsig secret if provided. */
 		if (tsig->name != NULL) {
-			val = conf_id_get(conf, C_KEY, C_SECRET, &key_val);
+			conf_val_t val = conf_id_get(conf, C_KEY, C_SECRET, &key_val);
 			tsig->secret.data = (uint8_t *)conf_bin(&val, &tsig->secret.size);
 		}
 
diff --git a/tests/knot/test_acl.c b/tests/knot/test_acl.c
index 43b020bcde..ae409211b4 100644
--- a/tests/knot/test_acl.c
+++ b/tests/knot/test_acl.c
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2020 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+/*  Copyright (C) 2021 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
@@ -25,6 +25,7 @@
 #include "contrib/sockaddr.h"
 
 #define ZONE	"example.zone"
+#define ZONE2	"example2.zone"
 #define KEY1	"key1_md5"
 #define KEY2	"key2_md5"
 #define KEY3	"key3_sha256"
@@ -69,6 +70,8 @@ static void test_acl_allowed(void)
 
 	knot_dname_t *zone_name = knot_dname_from_str_alloc(ZONE);
 	ok(zone_name != NULL, "create zone dname");
+	knot_dname_t *zone2_name = knot_dname_from_str_alloc(ZONE2);
+	ok(zone2_name != NULL, "create zone2 dname");
 	knot_dname_t *key1_name = knot_dname_from_str_alloc(KEY1);
 	ok(key1_name != NULL, "create "KEY1);
 	knot_dname_t *key2_name = knot_dname_from_str_alloc(KEY2);
@@ -121,6 +124,13 @@ static void test_acl_allowed(void)
 		"  - id: acl_range_addr\n"
 		"    address: [ 100.0.0.0-100.0.0.5, ::0-::5 ]\n"
 		"    action: [ transfer ]\n"
+		"  - id: acl_deny_no_action_no_key\n"
+		"    address: [ 240.0.0.4 ]\n"
+		"    deny: on\n"
+		"  - id: acl_notify_key\n"
+		"    address: [ 240.0.0.0/24 ]\n"
+		"    key: "KEY1"\n"
+		"    action: [ notify ]\n"
 		"  - id: acl_update_key\n"
 		"    key: "KEY1"\n"
 		"    update-owner: key\n"
@@ -138,6 +148,8 @@ static void test_acl_allowed(void)
 		"    acl: [ acl_key_addr, acl_deny, acl_no_action_deny ]\n"
 		"    acl: [ acl_multi_addr, acl_multi_key ]\n"
 		"    acl: [ acl_range_addr ]\n"
+		"  - domain: "ZONE2"\n"
+		"    acl: [ acl_deny_no_action_no_key, acl_notify_key ]\n"
 		"  - domain: "KEY1"\n"
 		"    acl: acl_update_key\n"
 		"  - domain: "KEY2"\n"
@@ -230,6 +242,18 @@ static void test_acl_allowed(void)
 	ret = acl_allowed(conf(), &acl, ACL_ACTION_TRANSFER, &addr, &key0, zone_name, NULL);
 	ok(ret == true, "IPv6 address from range, no key, action match");
 
+	acl = conf_zone_get(conf(), C_ACL, zone2_name);
+	ok(acl.code == KNOT_EOK, "Get zone2 ACL");
+	check_sockaddr_set(&addr, AF_INET, "240.0.0.4", 0);
+	ret = acl_allowed(conf(), &acl, ACL_ACTION_NOTIFY, &addr, &key1, zone2_name, NULL);
+	ok(ret == false, "Address, key, action, denied");
+
+	acl = conf_zone_get(conf(), C_ACL, zone2_name);
+	ok(acl.code == KNOT_EOK, "Get zone2 ACL");
+	check_sockaddr_set(&addr, AF_INET, "240.0.0.1", 0);
+	ret = acl_allowed(conf(), &acl, ACL_ACTION_NOTIFY, &addr, &key1, zone2_name, NULL);
+	ok(ret == true, "Address, key, action, match");
+
 	knot_rrset_t A;
 	knot_rrset_init(&A, key1_name, KNOT_RRTYPE_A, KNOT_CLASS_IN, 3600);
 	knot_rrset_add_rdata(&A, (uint8_t *)"\x00\x00\x00\x00", 4, NULL);
@@ -273,6 +297,7 @@ static void test_acl_allowed(void)
 
 	conf_free(conf());
 	knot_dname_free(zone_name, NULL);
+	knot_dname_free(zone2_name, NULL);
 	knot_dname_free(key1_name, NULL);
 	knot_dname_free(key2_name, NULL);
 	knot_dname_free(key3_name, NULL);
-- 
GitLab