[svn-commits] kpfleming: trunk r370426 - in /trunk: main/acl.c tests/test_acl.c

SVN commits to the Digium repositories svn-commits at lists.digium.com
Tue Jul 24 11:47:36 CDT 2012


Author: kpfleming
Date: Tue Jul 24 11:47:33 2012
New Revision: 370426

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=370426
Log:
Allow permit/deny ACL lines to contain multiple items and negated entries.

Rules in ACLs (specified using 'permit' and 'deny') can now contain multiple
items (separated by commas), and items in the rule can be negated by prefixing
them with '!'. This simplifies Asterisk Realtime configurations, since it is no
longer necessray to control the order that the 'permit' and 'deny' columns are
returned from queries.

Review: https://reviewboard.asterisk.org/r/1592/
Initial patch contributed by Tilghman Lesher
Unit tests written by Kevin P. Fleming


Modified:
    trunk/main/acl.c
    trunk/tests/test_acl.c

Modified: trunk/main/acl.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/acl.c?view=diff&rev=370426&r1=370425&r2=370426
==============================================================================
--- trunk/main/acl.c (original)
+++ trunk/main/acl.c Tue Jul 24 11:47:33 2012
@@ -602,9 +602,11 @@
 	struct ast_ha *ha;
 	struct ast_ha *prev = NULL;
 	struct ast_ha *ret;
-	char *tmp = ast_strdupa(stuff);
+	char *tmp, *list = ast_strdupa(stuff);
 	char *address = NULL, *mask = NULL;
 	int addr_is_v4;
+	int allowing = strncasecmp(sense, "p", 1) ? AST_SENSE_DENY : AST_SENSE_ALLOW;
+	const char *parsed_addr, *parsed_mask;
 
 	ret = path;
 	while (path) {
@@ -612,105 +614,110 @@
 		path = path->next;
 	}
 
-	if (!(ha = ast_calloc(1, sizeof(*ha)))) {
-		if (error) {
-			*error = 1;
-		}
-		return ret;
-	}
-
-	address = strsep(&tmp, "/");
-	if (!address) {
-		address = tmp;
-	} else {
-		mask = tmp;
-	}
-
-	if (!ast_sockaddr_parse(&ha->addr, address, PARSE_PORT_FORBID)) {
-		ast_log(LOG_WARNING, "Invalid IP address: %s\n", address);
-		ast_free_ha(ha);
-		if (error) {
-			*error = 1;
-		}
-		return ret;
-	}
-
-	/* If someone specifies an IPv4-mapped IPv6 address,
-	 * we just convert this to an IPv4 ACL
-	 */
-	if (ast_sockaddr_ipv4_mapped(&ha->addr, &ha->addr)) {
-		ast_log(LOG_NOTICE, "IPv4-mapped ACL network address specified. "
-				"Converting to an IPv4 ACL network address.\n");
-	}
-
-	addr_is_v4 = ast_sockaddr_is_ipv4(&ha->addr);
-
-	if (!mask) {
-		parse_cidr_mask(&ha->netmask, addr_is_v4, addr_is_v4 ? "32" : "128");
-	} else if (strchr(mask, ':') || strchr(mask, '.')) {
-		int mask_is_v4;
-		/* Mask is of x.x.x.x or x:x:x:x:x:x:x:x variety */
-		if (!ast_sockaddr_parse(&ha->netmask, mask, PARSE_PORT_FORBID)) {
-			ast_log(LOG_WARNING, "Invalid netmask: %s\n", mask);
+	while ((tmp = strsep(&list, ","))) {
+		if (!(ha = ast_calloc(1, sizeof(*ha)))) {
+			if (error) {
+				*error = 1;
+			}
+			return ret;
+		}
+
+		address = strsep(&tmp, "/");
+		if (!address) {
+			address = tmp;
+		} else {
+			mask = tmp;
+		}
+
+		if (*address == '!') {
+			ha->sense = (allowing == AST_SENSE_DENY) ? AST_SENSE_ALLOW : AST_SENSE_DENY;
+			address++;
+		} else {
+			ha->sense = allowing;
+		}
+
+		if (!ast_sockaddr_parse(&ha->addr, address, PARSE_PORT_FORBID)) {
+			ast_log(LOG_WARNING, "Invalid IP address: %s\n", address);
 			ast_free_ha(ha);
 			if (error) {
 				*error = 1;
 			}
 			return ret;
 		}
-		/* If someone specifies an IPv4-mapped IPv6 netmask,
+
+		/* If someone specifies an IPv4-mapped IPv6 address,
 		 * we just convert this to an IPv4 ACL
 		 */
-		if (ast_sockaddr_ipv4_mapped(&ha->netmask, &ha->netmask)) {
-			ast_log(LOG_NOTICE, "IPv4-mapped ACL netmask specified. "
+		if (ast_sockaddr_ipv4_mapped(&ha->addr, &ha->addr)) {
+			ast_log(LOG_NOTICE, "IPv4-mapped ACL network address specified. "
+				"Converting to an IPv4 ACL network address.\n");
+		}
+
+		addr_is_v4 = ast_sockaddr_is_ipv4(&ha->addr);
+
+		if (!mask) {
+			parse_cidr_mask(&ha->netmask, addr_is_v4, addr_is_v4 ? "32" : "128");
+		} else if (strchr(mask, ':') || strchr(mask, '.')) {
+			int mask_is_v4;
+			/* Mask is of x.x.x.x or x:x:x:x:x:x:x:x variety */
+			if (!ast_sockaddr_parse(&ha->netmask, mask, PARSE_PORT_FORBID)) {
+				ast_log(LOG_WARNING, "Invalid netmask: %s\n", mask);
+				ast_free_ha(ha);
+				if (error) {
+					*error = 1;
+				}
+				return ret;
+			}
+			/* If someone specifies an IPv4-mapped IPv6 netmask,
+			 * we just convert this to an IPv4 ACL
+			 */
+			if (ast_sockaddr_ipv4_mapped(&ha->netmask, &ha->netmask)) {
+				ast_log(LOG_NOTICE, "IPv4-mapped ACL netmask specified. "
 					"Converting to an IPv4 ACL netmask.\n");
-		}
-		mask_is_v4 = ast_sockaddr_is_ipv4(&ha->netmask);
-		if (addr_is_v4 ^ mask_is_v4) {
-			ast_log(LOG_WARNING, "Address and mask are not using same address scheme.\n");
+			}
+			mask_is_v4 = ast_sockaddr_is_ipv4(&ha->netmask);
+			if (addr_is_v4 ^ mask_is_v4) {
+				ast_log(LOG_WARNING, "Address and mask are not using same address scheme.\n");
+				ast_free_ha(ha);
+				if (error) {
+					*error = 1;
+				}
+				return ret;
+			}
+		} else if (parse_cidr_mask(&ha->netmask, addr_is_v4, mask)) {
+			ast_log(LOG_WARNING, "Invalid CIDR netmask: %s\n", mask);
 			ast_free_ha(ha);
 			if (error) {
 				*error = 1;
 			}
 			return ret;
 		}
-	} else if (parse_cidr_mask(&ha->netmask, addr_is_v4, mask)) {
-		ast_log(LOG_WARNING, "Invalid CIDR netmask: %s\n", mask);
-		ast_free_ha(ha);
-		if (error) {
-			*error = 1;
-		}
-		return ret;
-	}
-
-	if (apply_netmask(&ha->addr, &ha->netmask, &ha->addr)) {
-		/* This shouldn't happen because ast_sockaddr_parse would
-		 * have failed much earlier on an unsupported address scheme
-		 */
-		char *failmask = ast_strdupa(ast_sockaddr_stringify(&ha->netmask));
-		char *failaddr = ast_strdupa(ast_sockaddr_stringify(&ha->addr));
-		ast_log(LOG_WARNING, "Unable to apply netmask %s to address %s\n", failmask, failaddr);
-		ast_free_ha(ha);
-		if (error) {
-			*error = 1;
-		}
-		return ret;
-	}
-
-	ha->sense = strncasecmp(sense, "p", 1) ? AST_SENSE_DENY : AST_SENSE_ALLOW;
-
-	ha->next = NULL;
-	if (prev) {
-		prev->next = ha;
-	} else {
-		ret = ha;
-	}
-
-	{
-		const char *addr = ast_strdupa(ast_sockaddr_stringify(&ha->addr));
-		const char *mask = ast_strdupa(ast_sockaddr_stringify(&ha->netmask));
-
-		ast_debug(3, "%s/%s sense %d appended to acl\n", addr, mask, ha->sense);
+
+		if (apply_netmask(&ha->addr, &ha->netmask, &ha->addr)) {
+			/* This shouldn't happen because ast_sockaddr_parse would
+			 * have failed much earlier on an unsupported address scheme
+			 */
+			char *failmask = ast_strdupa(ast_sockaddr_stringify(&ha->netmask));
+			char *failaddr = ast_strdupa(ast_sockaddr_stringify(&ha->addr));
+			ast_log(LOG_WARNING, "Unable to apply netmask %s to address %s\n", failmask, failaddr);
+			ast_free_ha(ha);
+			if (error) {
+				*error = 1;
+			}
+			return ret;
+		}
+
+		if (prev) {
+			prev->next = ha;
+		} else {
+			ret = ha;
+		}
+		prev = ha;
+
+		parsed_addr = ast_strdupa(ast_sockaddr_stringify(&ha->addr));
+		parsed_mask = ast_strdupa(ast_sockaddr_stringify(&ha->netmask));
+
+		ast_debug(3, "%s/%s sense %d appended to ACL\n", parsed_addr, parsed_mask, ha->sense);
 	}
 
 	return ret;

Modified: trunk/tests/test_acl.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_acl.c?view=diff&rev=370426&r1=370425&r2=370426
==============================================================================
--- trunk/tests/test_acl.c (original)
+++ trunk/tests/test_acl.c Tue Jul 24 11:47:33 2012
@@ -128,11 +128,13 @@
 	struct acl denyallv4 = { "0.0.0.0/0", "deny" };
 	struct acl permitallv6 = { "::/0", "permit" };
 	struct acl denyallv6 = { "::/0", "deny" };
+
 	struct acl acl1[] = {
 		{ "0.0.0.0/0.0.0.0", "deny" },
 		{ "10.0.0.0/255.0.0.0", "permit" },
 		{ "192.168.0.0/255.255.255.0", "permit" },
 	};
+
 	struct acl acl2[] = {
 		{ "10.0.0.0/8", "deny" },
 		{ "10.0.0.0/8", "permit" },
@@ -148,6 +150,23 @@
 	struct acl acl4[] = {
 		{ "::/0", "deny" },
 		{ "fe80::/64", "permit" },
+		{ "fe80::ffff:0:0:0/80", "deny" },
+		{ "fe80::ffff:0:ffff:0/112", "permit" },
+	};
+
+	struct acl acl5[] = {
+		{ "0.0.0.0/0.0.0.0", "deny" },
+		{ "10.0.0.0/255.0.0.0,192.168.0.0/255.255.255.0", "permit" },
+	};
+
+	struct acl acl6[] = {
+		{ "10.0.0.0/8", "deny" },
+		{ "10.0.0.0/8", "permit" },
+		{ "10.0.0.0/16,!10.0.0.0/24", "deny" },
+	};
+
+	struct acl acl7[] = {
+		{ "::/0,!fe80::/64", "deny" },
 		{ "fe80::ffff:0:0:0/80", "deny" },
 		{ "fe80::ffff:0:ffff:0/112", "permit" },
 	};
@@ -162,16 +181,19 @@
 		int acl2_result;
 		int acl3_result;
 		int acl4_result;
+		int acl5_result;
+		int acl6_result;
+		int acl7_result;
 	} acl_tests[] = {
-		{ "10.1.1.5", TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A },
-		{ "192.168.0.5", TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A },
-		{ "192.168.1.5", TACL_A, TACL_D, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A },
-		{ "10.0.0.1", TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A },
-		{ "10.0.10.10", TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A },
-		{ "172.16.0.1", TACL_A, TACL_D, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A },
-		{ "fe80::1234", TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A },
-		{ "fe80::ffff:1213:dead:beef", TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_D },
-		{ "fe80::ffff:0:ffff:ABCD", TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A },
+		{ "10.1.1.5",                  TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A },
+		{ "192.168.0.5",               TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A },
+		{ "192.168.1.5",               TACL_A, TACL_D, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A },
+		{ "10.0.0.1",                  TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A },
+		{ "10.0.10.10",                TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_D, TACL_A },
+		{ "172.16.0.1",                TACL_A, TACL_D, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A },
+		{ "fe80::1234",                TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A },
+		{ "fe80::ffff:1213:dead:beef", TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_D },
+		{ "fe80::ffff:0:ffff:ABCD",    TACL_A, TACL_A, TACL_A, TACL_D, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A, TACL_A },
 	};
 
 	struct ast_ha *permit_hav4 = NULL;
@@ -182,9 +204,27 @@
 	struct ast_ha *ha2 = NULL;
 	struct ast_ha *ha3 = NULL;
 	struct ast_ha *ha4 = NULL;
+	struct ast_ha *ha5 = NULL;
+	struct ast_ha *ha6 = NULL;
+	struct ast_ha *ha7 = NULL;
 	enum ast_test_result_state res = AST_TEST_PASS;
 	int err = 0;
 	int i;
+
+	int build_ha(const struct acl *acl, size_t len, struct ast_ha **ha, const char *acl_name) {
+		size_t i;
+
+		for (i = 0; i < len; ++i) {
+			if (!(*ha = ast_append_ha(acl[i].access, acl[i].host, *ha, &err))) {
+				ast_test_status_update(test, "Failed to add rule %s with access %s to %s\n",
+						       acl[i].host, acl[i].access, acl_name);
+				res = AST_TEST_FAIL;
+				return -1;
+			}
+		}
+
+		return 0;
+	}
 
 	switch (cmd) {
 	case TEST_INIT:
@@ -222,40 +262,32 @@
 		goto acl_cleanup;
 	}
 
-	for (i = 0; i < ARRAY_LEN(acl1); ++i) {
-		if (!(ha1 = ast_append_ha(acl1[i].access, acl1[i].host, ha1, &err))) {
-			ast_test_status_update(test, "Failed to add rule %s with access %s to ha1\n",
-					acl1[i].host, acl1[i].access);
-			res = AST_TEST_FAIL;
-			goto acl_cleanup;
-		}
-	}
-
-	for (i = 0; i < ARRAY_LEN(acl2); ++i) {
-		if (!(ha2 = ast_append_ha(acl2[i].access, acl2[i].host, ha2, &err))) {
-			ast_test_status_update(test, "Failed to add rule %s with access %s to ha2\n",
-					acl2[i].host, acl2[i].access);
-			res = AST_TEST_FAIL;
-			goto acl_cleanup;
-		}
-	}
-
-	for (i = 0; i < ARRAY_LEN(acl3); ++i) {
-		if (!(ha3 = ast_append_ha(acl3[i].access, acl3[i].host, ha3, &err))) {
-			ast_test_status_update(test, "Failed to add rule %s with access %s to ha3\n",
-					acl3[i].host, acl3[i].access);
-			res = AST_TEST_FAIL;
-			goto acl_cleanup;
-		}
-	}
-
-	for (i = 0; i < ARRAY_LEN(acl4); ++i) {
-		if (!(ha4 = ast_append_ha(acl4[i].access, acl4[i].host, ha4, &err))) {
-			ast_test_status_update(test, "Failed to add rule %s with access %s to ha4\n",
-					acl4[i].host, acl4[i].access);
-			res = AST_TEST_FAIL;
-			goto acl_cleanup;
-		}
+	if (build_ha(acl1, ARRAY_LEN(acl1), &ha1, "ha1") != 0) {
+		goto acl_cleanup;
+	}
+
+	if (build_ha(acl2, ARRAY_LEN(acl2), &ha2, "ha2") != 0) {
+		goto acl_cleanup;
+	}
+
+	if (build_ha(acl3, ARRAY_LEN(acl3), &ha3, "ha3") != 0) {
+		goto acl_cleanup;
+	}
+
+	if (build_ha(acl4, ARRAY_LEN(acl4), &ha4, "ha4") != 0) {
+		goto acl_cleanup;
+	}
+
+	if (build_ha(acl5, ARRAY_LEN(acl5), &ha5, "ha5") != 0) {
+		goto acl_cleanup;
+	}
+
+	if (build_ha(acl6, ARRAY_LEN(acl6), &ha6, "ha6") != 0) {
+		goto acl_cleanup;
+	}
+
+	if (build_ha(acl7, ARRAY_LEN(acl7), &ha7, "ha7") != 0) {
+		goto acl_cleanup;
 	}
 
 	for (i = 0; i < ARRAY_LEN(acl_tests); ++i) {
@@ -268,6 +300,9 @@
 		int acl2_res;
 		int acl3_res;
 		int acl4_res;
+		int acl5_res;
+		int acl6_res;
+		int acl7_res;
 
 		ast_sockaddr_parse(&addr, acl_tests[i].test_address, PARSE_PORT_FORBID);
 
@@ -279,6 +314,9 @@
 		acl2_res = ast_apply_ha(ha2, &addr);
 		acl3_res = ast_apply_ha(ha3, &addr);
 		acl4_res = ast_apply_ha(ha4, &addr);
+		acl5_res = ast_apply_ha(ha5, &addr);
+		acl6_res = ast_apply_ha(ha6, &addr);
+		acl7_res = ast_apply_ha(ha7, &addr);
 
 		if (permit_resv4 != acl_tests[i].v4_permitall_result) {
 			ast_test_status_update(test, "Access not as expected to %s on permitallv4. Expected %d but "
@@ -335,6 +373,27 @@
 			res = AST_TEST_FAIL;
 			goto acl_cleanup;
 		}
+
+		if (acl5_res != acl_tests[i].acl5_result) {
+			ast_test_status_update(test, "Access not as expected to %s on acl5. Expected %d but "
+					"got %d instead\n", acl_tests[i].test_address, acl_tests[i].acl5_result, acl5_res);
+			res = AST_TEST_FAIL;
+			goto acl_cleanup;
+		}
+
+		if (acl6_res != acl_tests[i].acl6_result) {
+			ast_test_status_update(test, "Access not as expected to %s on acl6. Expected %d but "
+					"got %d instead\n", acl_tests[i].test_address, acl_tests[i].acl6_result, acl6_res);
+			res = AST_TEST_FAIL;
+			goto acl_cleanup;
+		}
+
+		if (acl7_res != acl_tests[i].acl7_result) {
+			ast_test_status_update(test, "Access not as expected to %s on acl7. Expected %d but "
+					"got %d instead\n", acl_tests[i].test_address, acl_tests[i].acl7_result, acl7_res);
+			res = AST_TEST_FAIL;
+			goto acl_cleanup;
+		}
 	}
 
 acl_cleanup:
@@ -361,6 +420,15 @@
 	}
 	if (ha4) {
 		ast_free_ha(ha4);
+	}
+	if (ha5) {
+		ast_free_ha(ha5);
+	}
+	if (ha6) {
+		ast_free_ha(ha6);
+	}
+	if (ha7) {
+		ast_free_ha(ha7);
 	}
 	return res;
 }




More information about the svn-commits mailing list