parser: add port range support on network policy

Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>

parser/apparmor.d.pod

@@ -175,7 +175,7 @@ B<NETWORK PEER EXPR> = 'peer' '=' '(' ( I<NETWORK IP COND> | I<NETWORK PORT COND
 
 B<NETWORK IP COND> = 'ip' '=' ( 'none' | I<NETWORK IPV4> | I<NETWORK IPV6> )
 
-B<NETWORK PORT COND> = 'port' '=' ( I<NETWORK PORT> )
+B<NETWORK PORT COND> = 'port' '=' ( I<NETWORK PORT> | I<NETWORK PORT> '-' I<NETWORK PORT> )
 
 B<NETWORK IPV4> = IPv4, represented by four 8-bit decimal numbers separated by '.'
 
@@ -996,13 +996,14 @@ can be represented by:
  network ip=0.0.0.0;	#allow INADDR_ANY
 
 The network rules support the specification of local and remote IP
-addresses and ports.
+addresses, ports, and port ranges.
 
  network ip=127.0.0.1 port=8080,
  network peer=(ip=10.139.15.23 port=8081),
  network ip=fd74:1820:b03a:b361::cf32 peer=(ip=fd74:1820:b03a:b361::a0f9),
  network port=8080 peer=(port=8081),
  network ip=127.0.0.1 port=8080 peer=(ip=10.139.15.23 port=8081),
+ network ip=127.0.0.1 port=8080-8084,
 
 =head2 Mount Rules
 

parser/network.cc

@@ -352,10 +352,41 @@ bool parse_port_number(const char *port_entry, uint16_t *port) {
 	return false;
 }
 
+bool parse_range(const char *range, uint16_t *from, uint16_t *to)
+{
+	char *range_tmp = strdup(range);
+	char *dash = strchr(range_tmp, '-');
+	bool ret = false;
+	if (dash == NULL)
+		goto out;
+	*dash = '\0';
+
+	if (parse_port_number(range_tmp, from)) {
+		if (parse_port_number(dash + 1, to)) {
+			if (*from > *to) {
+				goto out;
+			}
+			ret = true;
+			goto out;
+		}
+	}
+
+out:
+	free(range_tmp);
+	return ret;
+}
+
 bool network_rule::parse_port(ip_conds &entry)
 {
 	entry.is_port = true;
-	return parse_port_number(entry.sport, &entry.port);
+	if (parse_range(entry.sport, &entry.from_port, &entry.to_port))
+		return true;
+	if (parse_port_number(entry.sport, &entry.from_port)) {
+		/* if range is not used, from and to have the same value */
+		entry.to_port = entry.from_port;
+		return true;
+	}
+	return false;
 }
 
 bool network_rule::parse_address(ip_conds &entry)
@@ -650,23 +681,23 @@ std::list<std::ostringstream> copy_streams_list(std::list<std::ostringstream> &s
 	return streams_copy;
 }
 
-bool network_rule::gen_ip_conds(Profile &prof, std::list<std::ostringstream> &streams, ip_conds &entry, bool is_peer, bool is_cmd)
+bool network_rule::gen_ip_conds(Profile &prof, std::list<std::ostringstream> &streams, ip_conds &entry, bool is_peer, uint16_t port, bool is_port, bool is_cmd)
 {
 	std::string buf;
 	perm32_t cond_perms;
 	std::list<std::ostringstream> ip_streams;
 
 	for (auto &oss : streams) {
-		if (entry.is_port && !(entry.is_ip && entry.is_none)) {
+		if (is_port && !(entry.is_ip && entry.is_none)) {
 			/* encode port type (privileged - 1, remote - 2, unprivileged - 0) */
-			if (!is_peer && perms & AA_NET_BIND && entry.port < IPPORT_RESERVED)
+			if (!is_peer && perms & AA_NET_BIND && port < IPPORT_RESERVED)
 				oss << "\\x01";
 			else if (is_peer)
 				oss << "\\x02";
 			else
 				oss << "\\x00";
 
-			oss << gen_port_cond(entry.port);
+			oss << gen_port_cond(port);
 		} else {
 			/* port type + port number */
 			oss << "...";
@@ -764,68 +795,83 @@ bool network_rule::gen_net_rule(Profile &prof, u16 family, unsigned int type_mas
 	}
 
 	if (perms & AA_PEER_NET_PERMS) {
+		for (int peer_port = peer.from_port; peer_port <= peer.to_port; peer_port++) {
+			std::list<std::ostringstream> streams;
+			std::ostringstream cmd_buffer;
+
+			cmd_buffer << buffer.str();
+			streams.push_back(std::move(cmd_buffer));
+
+			if (!gen_ip_conds(prof, streams, peer, true, peer_port, peer.is_port, false))
+				return false;
+
+			for (auto &oss : streams) {
+				oss << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_ADDR;
+			}
+
+			for (int local_port = local.from_port; local_port <= local.to_port; local_port++) {
+				std::list<std::ostringstream> localstreams;
+
+				for (auto &oss : streams) {
+					/* we need to copy streams because each local_port should be an unique entry */
+					std::ostringstream local_buffer;
+					local_buffer << oss.str();
+					localstreams.push_back(std::move(local_buffer));
+				}
+
+				if (!gen_ip_conds(prof, localstreams, local, false, local_port, local.is_port, true))
+					return false;
+			}
+		}
+	}
+
+
+	for (int local_port = local.from_port; local_port <= local.to_port; local_port++) {
 		std::list<std::ostringstream> streams;
-		std::ostringstream cmd_buffer;
+		std::ostringstream common_buffer;
 
-		cmd_buffer << buffer.str();
+		common_buffer << buffer.str();
-		streams.push_back(std::move(cmd_buffer));
+		streams.push_back(std::move(common_buffer));
-
+		if (!gen_ip_conds(prof, streams, local, false, local_port, local.is_port, false))
-		if (!gen_ip_conds(prof, streams, peer, true, false))
 			return false;
 
-		for (auto &oss : streams) {
+		if (perms & AA_NET_LISTEN) {
-			oss << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_ADDR;
+			std::list<std::ostringstream> cmd_streams;
+			cmd_streams = copy_streams_list(streams);
+
+			for (auto &cmd_buffer : streams) {
+				std::ostringstream listen_buffer;
+				listen_buffer << cmd_buffer.str();
+				listen_buffer << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_LISTEN;
+				/* length of queue allowed - not used for now */
+				listen_buffer << "..";
+				buf = listen_buffer.str();
+				if (!prof.policy.rules->add_rule(buf.c_str(), priority,
+								 rule_mode, map_perms(perms),
+								 dedup_perms_rule_t::audit == AUDIT_FORCE ? map_perms(perms) : 0,
+								 parseopts))
+					return false;
+			}
 		}
+		if (perms & AA_NET_OPT) {
+			std::list<std::ostringstream> cmd_streams;
+			cmd_streams = copy_streams_list(streams);
 
-		if (!gen_ip_conds(prof, streams, local, false, true))
+			for (auto &cmd_buffer : streams) {
-			return false;
+				std::ostringstream opt_buffer;
-	}
+				opt_buffer << cmd_buffer.str();
-
+				opt_buffer << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_OPT;
-	std::list<std::ostringstream> streams;
+				/* level - not used for now */
-	std::ostringstream common_buffer;
+				opt_buffer << "..";
-
+				/* socket mapping - not used for now */
-	common_buffer << buffer.str();
+				opt_buffer << "..";
-	streams.push_back(std::move(common_buffer));
+				buf = opt_buffer.str();
-
+				if (!prof.policy.rules->add_rule(buf.c_str(), priority,
-	if (!gen_ip_conds(prof, streams, local, false, false))
+								 rule_mode, map_perms(perms),
-		return false;
+								 dedup_perms_rule_t::audit == AUDIT_FORCE ? map_perms(perms) : 0,
-
+								 parseopts))
-	if (perms & AA_NET_LISTEN) {
+					return false;
-		std::list<std::ostringstream> cmd_streams;
+			}
-		cmd_streams = copy_streams_list(streams);
-
-		for (auto &cmd_buffer : streams) {
-			std::ostringstream listen_buffer;
-			listen_buffer << cmd_buffer.str();
-			listen_buffer << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_LISTEN;
-			/* length of queue allowed - not used for now */
-			listen_buffer << "..";
-			buf = listen_buffer.str();
-			if (!prof.policy.rules->add_rule(buf.c_str(), priority,
-						rule_mode, map_perms(perms),
-						dedup_perms_rule_t::audit == AUDIT_FORCE ? map_perms(perms) : 0,
-						 parseopts))
-				return false;
-		}
-	}
-	if (perms & AA_NET_OPT) {
-		std::list<std::ostringstream> cmd_streams;
-		cmd_streams = copy_streams_list(streams);
-
-		for (auto &cmd_buffer : streams) {
-			std::ostringstream opt_buffer;
-			opt_buffer << cmd_buffer.str();
-			opt_buffer << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_OPT;
-			/* level - not used for now */
-			opt_buffer << "..";
-			/* socket mapping - not used for now */
-			opt_buffer << "..";
-			buf = opt_buffer.str();
-			if (!prof.policy.rules->add_rule(buf.c_str(), priority,
-						rule_mode, map_perms(perms),
-						dedup_perms_rule_t::audit == AUDIT_FORCE ? map_perms(perms) : 0,
-						parseopts))
-				return false;
 		}
 	}
 

parser/network.h

@@ -130,7 +130,9 @@ public:
 	bool is_ip = false;
 	bool is_port = false;
 
-	uint16_t port;
+	uint16_t from_port = 0;
+	uint16_t to_port = 0;
+
 	struct ip_address ip;
 
 	bool is_none = false;
@@ -187,7 +189,7 @@ public:
 		}
 	};
 
-	bool gen_ip_conds(Profile &prof, std::list<std::ostringstream> &streams, ip_conds &entry, bool is_peer, bool is_cmd);
+	bool gen_ip_conds(Profile &prof, std::list<std::ostringstream> &streams, ip_conds &entry, bool is_peer, uint16_t port, bool is_port, bool is_cmd);
 	bool gen_net_rule(Profile &prof, u16 family, unsigned int type_mask, unsigned int protocol);
 	void set_netperm(unsigned int family, unsigned int type, unsigned int protocol);
 	void update_compat_net(void);

parser/tst/simple_tests/network/network_ok_17.sd

@@ -0,0 +1,12 @@
+#
+#=DESCRIPTION network port range conditional test
+#=EXRESULT PASS
+#
+/usr/bin/foo {
+  network peer=(port=22-443),
+  network port=22-443,
+  network port=22-443 peer=(port=1-100),
+  network ip=127.0.0.1 port=3456-3457,
+  network ip=127.0.0.1 port=3456-3457 peer=(ip=127.0.0.2 port=8765-8770),
+
+}

tests/regression/apparmor/net_inet.sh

@@ -81,7 +81,7 @@ while lsof -i:$bind_port >/dev/null; do
 	let bind_port=$bind_port+1
 done
 
-let remote_port=$bind_port+1
+let remote_port=$bind_port+50
 while lsof -i:$remote_port >/dev/null; do
 	let remote_port=$remote_port+1
 done
@@ -100,6 +100,23 @@ setsockopt_rules="network;(setopt,getopt);ip=0.0.0.0;port=0" # INADDR_ANY
 rcv_rules="network;ip=$bind_ipv4;peer=(ip=none)"
 snd_rules="network;ip=$remote_ipv4;peer=(ip=none)"
 
+# port range tests
+let invalid1=$bind_port-1
+let end_range=$bind_port+10
+let invalid2=$bind_port+11
+
+for test_port in $(seq $bind_port $end_range); do
+    generate_profile="genprofile network;ip=$bind_ipv4;port=$bind_port-$end_range $setsockopt_rules $sender:px -- image=$sender network $setsockopt_rules $snd_rules"
+    do_tests "ipv4 udp port range $test_port generic perms" pass pass $bind_ipv4 $test_port $remote_ipv4 $remote_port udp "$generate_profile"
+done
+
+generate_profile="genprofile network;ip=$bind_ipv4;port=$bind_port-$end_range $setsockopt_rules $sender:px -- image=$sender network $setsockopt_rules $snd_rules"
+do_tests "ipv4 udp port range $invalid1 generic perms" fail fail $bind_ipv4 $invalid1 $remote_ipv4 $remote_port udp "$generate_profile"
+
+generate_profile="genprofile network;ip=$bind_ipv4;port=$bind_port-$end_range $setsockopt_rules $sender:px -- image=$sender network $setsockopt_rules $snd_rules"
+do_tests "ipv4 udp port range $invalid2 generic perms" fail fail $bind_ipv4 $invalid2 $remote_ipv4 $remote_port udp "$generate_profile"
+# end of port range tests
+
 generate_profile="genprofile network;ip=$bind_ipv4;port=$bind_port;peer=(ip=$remote_ipv4,port=$remote_port) $setsockopt_rules $rcv_rules $sender:px -- image=$sender network;ip=$remote_ipv4;port=$remote_port;peer=(ip=$bind_ipv4,port=$bind_port) $setsockopt_rules $snd_rules"
 do_tests "ipv4 udp generic perms" pass pass $bind_ipv4 $bind_port $remote_ipv4 $remote_port udp "$generate_profile"
 

utils/test/test-parser-simple-tests.py

@@ -427,6 +427,8 @@ syntax_failure = (
     'vars/vars_simple_assignment_12.sd',  # Redefining existing variable @{BAR} ('\' not handled)
     'bare_include_tests/ok_2.sd',  # two #include<...> in one line
 
+    # network port range
+    'network/network_ok_17.sd',
 )