Merge branch 'jb48869' into 'master'

[firewall] Fix tests and special cases. JB#48869

See merge request mer-core/connman!275

connman/doc/connman-firewall.config.5.in

@@ -1,9 +1,9 @@
 .\" connman-firewall.config(5) manual page
 .\"
 .\" Copyright (C) 2018-2019  Jolla Ltd.
-.\" Copyright (C) 2019  Open Mobile Platform LLC.
+.\" Copyright (C) 2019-2020  Open Mobile Platform LLC.
 .\"
-.TH "connman-firewall.config" "5" "2019-12-16" ""
+.TH "connman-firewall.config" "5" "2020-06-01" ""
 .SH NAME
 firewall.conf \- ConnMan firewall configuration file
 .SH DESCRIPTION
@@ -134,8 +134,27 @@ files (only allowed in General section) overrules all previous POLICY keys set
 for the CHAIN with given PROTOCOL.
 .SH RULE FORMAT
 .P
-Rules follow iptables rule format, for reference see:
-.URL "https://www.frozentux.net/iptables-tutorial/iptables-tutorial.html" "https://www.frozentux.net/iptables-tutorial/iptables-tutorial.html"
+Rules follow iptables rule format in general, with some notable exceptions, for
+reference see
+.UR https://www.frozentux.net/iptables-tutorial/iptables-tutorial.html
+the Iptables Tutorial
+.UE
+.SS
+The most notable exceptions are:
+.IP 1. 3
+-p protocol does NOT imply -m protocol. -p protocol only implies the protocol
+in the IP header. So e.g. to add a http port opening, you MUST use -m tcp:
+
+  IPv4.INPUT.RULES = -p tcp -m tcp --dport 80 -j ACCEPT
+
+.IP 2. 3
+The order is stricter - if a match relies on a specific protocol, -p must be
+specified before -m on the commandline.
+
+.IP 3. 3
+No command abbrievations are allowed - it is not possible to use --dest as
+short for --destination.
+
 .P
 Rules are separated with semicolons (;). All rules for a key must be on one 
 line.
@@ -153,40 +172,122 @@ IPv4.INPUT.RULES = #-p udp -m udp --dport 23 -j ACCEPT; -p udp -m udp --dport 24
 Will discard the first --dport 23 rule and use the second --dport 24 rule
 .SS
 Each rule:
-.TP
-Has to have one target (-j|--jump TARGET) or goto (-g|--goto TARGET) which is the bare minimum of the rule.
-.TP
-Can have 0...1 protocol matches (-p|--protocol protocol).
-.TP
-Can have 0...2 match speficiers (-m|--match match).
-.TP
-Can have 0...2 port switches with a protocol modifier (-m|--match protocol) OR
-.TP
-Can have 0...1 port switches with multiport modifier (-m|--match multiport)
-.TP
-Can have 0...2 destination specifiers (same direction cannot be used twice)
-.TP
-Can have 0...2 interface switches in [General] section (same direction cannot be used twice)
+.IP - 2
+Must have one target (-j|--jump TARGET) or goto (-g|--goto) which is the bare
+minimum of the rule
+
+E.g., to allow all traffic:
+ -j ACCEPT
+
+.IP - 2
+Can have 1 protocol selector (-p|--protocol protocol)
+
+E.g., to block all ICMP traffic:
+ -p icmp -j DROP
+
+.IP - 2
+Can have an address specifier for each of traffic directions: --source/-s,
+--destination/-d
+
+E.g. to block all traffic to Google's public DNS servers:
+ -d 8.8.8.8 -d DROP
+
+.IP - 2
+Can have an interface switch for each directions in [General] section:
+--in-interface/-i, --out-interface/-o
+
+.IP - 2
+Can have match speficiers (-m|--match match), restrictions apply per match.
+
+E.g., to allow one attempt per second:
+ -m limit --limit 1/s --limit-burst 1 -j ACCEPT
+
+.IP - 2
+Any match that requires a specific set of options must include them after
+the -m match for the rule to be approved.
+
 .SS
 Targets:
 .P
 The targets (-j TARGET) are the same as with default iptables: ACCEPT, DROP, REJECT, LOG and QUEUE.
 .SS
 Protocols:
 .P
-Protocols (-p protocol) are the same as with iptables: tcp, udp, udplite, icmp, icmpv6, ipv6-icmp, esp, ah, sctp, mh, dccp and the special keyword all. These can be given in numeric format as well.
+Protocols (-p protocol) are the same as with iptables. One can use any protocol
+name that is resolvable through /etc/protocols, or use numerical values
+directly. As an exception, protocol names "icmpv6", "ipv6-mh", "mh" and "all"
+are also allowed.
 .SS
 Disabled switches:
 .P
-Following switches are disabled and if a rule contains any of them the rule will be ignored:
-.IP
-All chain modifiers, since rules are added to managed chains, following modifiers are disabled: --append, -A, --delete, -D, --delete-chain, -X, --flush, -F, --insert, -I, --new-chain, -N, --policy, -P, --rename-chain, -E, --replace, -R, --zero, -Z
-.IP
-Destination speficiers for DNAT are disabled : --to-destination, --from-destination
-.IP
-Some matches (with -m) are disabled (cause crash or commit errors). IPv4: comment, state, recent, sctp, dccp, mh, hashlimit, icmpv6, ipv6-icmp. IPv6: comment, state, recent, ttl, sctp, dccp, mh, hashlimit, frag, icmp
-.P
-Interface specifiers (--in-interface, -i, --out-interface, -o) are not allowed in tethering or service type sections: 
+Following switches are disabled and if a rule contains any of them the rule will
+be ignored:
+.IP - 2
+All chain modifiers are disabled: --append, -A, --delete, -D, --delete-chain,
+-X, --flush, -F, --insert, -I, --new-chain, -N, --policy, -P, --rename-chain,
+-E, --replace, -R, --zero, -Z
+.IP - 2
+Destination speficiers for DNAT are disabled: --to-destination,
+--from-destination
+.IP - 2
+Fragment: -f, --fragment
+.IP - 2
+IP family options: --ipv4, -4, --ipv6, -6
+.IP - 2
+Interface specifiers are not allowed in tethering or service type sections:
+--in-interface, -i, --out-interface, -o
+.IP - 2
+Any match that is not supported by syntax parser will make the rule invalid.
+See the next section for a list of supported matches. Note that ipv6-specific
+matches are not supported in ipv4 and vice-versa.
+
+.SH MATCH SPECIFIC OPTIONS
+.P
+Currently supported matches are:
+ - ah
+ - conntrack
+ - dccp
+ - ecn
+ - esp
+ - helper
+ - icmp
+ - icmp6, icmpv6, ipv6-icmp
+ - iprange
+ - limit
+ - mark
+ - mh
+ - multiport
+ - owner
+ - pkttype
+ - rpfilter
+ - sctp
+ - tcp
+ - ttl
+ - udp
+.P
+For match-specific options, please see "iptables -m $match --help".
+.P
+Port matches (--dport and/or --sport) are supported with -m protocol that
+supports ports. These are:
+ - tcp
+ - udp
+ - dccp
+ - sctp
+.P
+Only one --dport and only one --sport is allowed.
+.P
+Multiport match requires a protocol that has ports. These are:
+ - tcp
+ - udp
+ - udplite
+ - dccp
+ - sctp
+.P
+Multiport match supports only one option, either --sports, or --dports.
+However, it is possible to use multiple -m multiport specifiers to match both
+directions, e.g.:
+  -p tcp -m multiport --dports 80 -m multiport --sports 1024:65535 -j ACCEPT
+
 .SH CONFIGURATION: GENERAL SECTION
 .P
 General section contains the main static firewall rules. In this section both

connman/doc/firewall-config-format.txt

@@ -104,9 +104,20 @@ for the CHAIN with given PROTOCOL.
 Rule formatting and exceptions
 ==============================
 
-Rules follow iptables rule format, for reference see:
+Rules follow iptables rule format in general, with some notable exceptions, for
+reference see:
 https://www.frozentux.net/iptables-tutorial/iptables-tutorial.html
 
+The most notable exceptions are:
+1. -p protocol does NOT imply -m protocol. -p protocol only implies the
+   protocol in the IP header. So e.g. to add a http port opening, you MUST
+   use -m tcp:
+   "IPv4.INPUT.RULES = -p tcp -m tcp --dport 80 -j ACCEPT"
+2. The order is stricter - if a match relies on a specific protocol, -p must
+   be specified before -m on the commandline.
+3. No command abbrievations are allowed - it is not possible to use --dest as
+   short for --destination.
+
 Rules are separated with semicolons (;). All rules for a key must be on one
 line.
 
@@ -116,23 +127,24 @@ there are missing/invalid values the rule is ignored. Negations in rules are
 supported as with iptables command.
 
 Each rule:
- - Has to have one target (-j|--jump TARGET) or goto (-g|--goto) which is the
+ - Must have one target (-j|--jump TARGET) or goto (-g|--goto) which is the
    bare minimum of the rule
- - Can have 0...1 protocol matches (-p|--protocol protocol)
- - Can have 0...2 match speficiers (-m|--match match),
-   - E.g., to allow one attempt per second to telnet:
-     -p udp -m udp --dport 23 -m limit --limit 1/s --limit-burst 1 -j ACCEPT
- - Can have
-   - 0...2 port switches with a protocol modifier (-m|--match protocol):
-     --destination-port, --dport, --source-port, --sport 
-   - 0...1 port switches with multiport modifier (-m|--match multiport):
-     --destination-ports, --dports, --source-ports, --sports, --port, --ports,
-     --destination-port, --dport, --source-port, --sport
- - Can have 0...2 destination specifiers (same direction cannot be used twice)
-   --source, --src, -s, --destination, --dst, -d
- - Can have 0...2 interface switches in [General] section (same direction cannot
-   be used twice):
-   --in-interface, -i, --out-interface, -o
+   - E.g., to allow all traffic:
+     -j ACCEPT
+ - Can have 1 protocol selector (-p|--protocol protocol)
+   - E.g., to block all ICMP traffic:
+     -p icmp -j DROP
+ - Can have an address specifier for each of traffic directions:
+   --source/-s, --destination/-d
+   - E.g. to block all traffic to Google's public DNS servers:
+     -d 8.8.8.8 -d DROP
+ - Can have an interface switch for each directions in [General] section:
+   --in-interface/-i, --out-interface/-o
+ - Can have match speficiers (-m|--match match), restrictions apply per match.
+   - E.g., to allow one attempt per second:
+     -m limit --limit 1/s --limit-burst 1 -j ACCEPT
+ - Any match that requires a specific set of options must include them after
+   the -m match for the rule to be approved.
 
 Rules can be commented out with hash tag (#) as first character. Commented rules
 are simply ignored. For example:
@@ -145,28 +157,78 @@ Will discard the first --dport 23 rule and use the second --dport 24 rule.
 The targets (-j TARGET) are the same as with default iptables: ACCEPT, DROP,
 REJECT, LOG and QUEUE.
 
-Protocols (-p protocol) are the same as with iptables: tcp, udp, udplite, icmp,
-icmpv6, ipv6-icmp, esp, ah, sctp, mh, dccp and the special keyword all. These
-can be given in numeric format as well.
+Protocols (-p protocol) are the same as with iptables. One can use any protocol
+name that is resolvable through /etc/protocols, or use numerical values
+directly. As an exception, protocol names "icmpv6", "ipv6-mh", "mh" and "all"
+are also allowed.
 
 Following switches are disabled and if a rule contains any of them the rule will
 be ignored:
 
- - All chain modifiers, since rules are added to managed chains, all chain
-   modifiers are disabled: --append, -A, --delete, -D, --delete-chain, -X, 
-   --flush, -F, --insert, -I, --new-chain, -N, --policy, -P, --rename-chain, -E,
-   --replace, -R, --zero, -Z
- - Destination speficiers for DNAT are disabled : --to-destination,
+ - All chain modifiers are disabled: --append, -A, --delete, -D, --delete-chain,
+   -X, --flush, -F, --insert, -I, --new-chain, -N, --policy, -P, --rename-chain,
+   -E, --replace, -R, --zero, -Z
+ - Destination speficiers for DNAT are disabled: --to-destination,
    --from-destination
  - Fragment: -f, --fragment
  - IP family options: --ipv4, -4, --ipv6, -6
- - Some matches (with -m) are disabled (cause crash or commit errors):
-   - IPv4: comment, state, recent, sctp, dccp, mh, hashlimit,
-     icmpv6, ipv6-icmp
-   - IPv6: comment, state, recent, ttl, sctp, dccp, mh,
-     hashlimit, frag, icmp
  - Interface specifiers are not allowed in tethering or service type sections:
    --in-interface, -i, --out-interface, -o
+ - Any match that is not supported by syntax parser will make the rule invalid.
+   See the next section for a list of supported matches. Note that ipv6-specific
+   matches are not supported in ipv4 and vice-versa.
+
+ - Any match that is not supported by syntax parser is disabled. See next
+   section for a list of supported matches. Note that ipv6-specific matches
+   are not supported in ipv4 and vice-versa.
+
+Match specific options
+======================
+
+Currently supported matches are:
+ - ah
+ - conntrack
+ - dccp
+ - ecn
+ - esp
+ - helper
+ - icmp
+ - icmp6, icmpv6, ipv6-icmp
+ - iprange
+ - limit
+ - mark
+ - mh
+ - multiport
+ - owner
+ - pkttype
+ - rpfilter
+ - sctp
+ - tcp
+ - ttl
+ - udp
+
+For match-specific options, please see "iptables -m $match --help".
+
+Port matches (--dport and/or --sport) are supported with -m protocol that
+supports ports. These are:
+ - tcp
+ - udp
+ - dccp
+ - sctp
+
+Only one --dport and only one --sport is allowed.
+
+Multiport match requires a protocol that has ports. These are:
+ - tcp
+ - udp
+ - udplite
+ - dccp
+ - sctp
+
+Multiport match supports only one option, either --sports, or --dports.
+However, it is possible to use multiple -m multiport specifiers to match both
+directions, e.g.:
+  -p tcp -m multiport --dports 80 -m multiport --sports 1024:65535 -j ACCEPT
 
 General section [General]
 =========================

connman/src/iptables-validate.c

@@ -286,7 +286,7 @@ static bool is_valid_port_or_service_range(const char *protocol,
 
 	/* Range can have only two set */
 	if (g_strv_length(tokens) == 2) {
-		for (i = 0; i < 2 && tokens[i]; i++) {
+		for (i = 0; i < 2; i++) {
 			if (!is_valid_port_or_service(protocol, tokens[i],
 						&ports[i])) {
 				DBG("invalid port/service %s in %s", tokens[i],
@@ -659,6 +659,7 @@ static bool handle_ports(struct validator_data *data, gchar **args)
 	struct protoent *p;
 	char *protoname = NULL;
 	gchar **tokens = NULL;
+	int token_count;
 
 	 /* In iptables ports are separated with commas, ranges with colon. */
 	const char delimeter[] = ",";
@@ -674,7 +675,11 @@ static bool handle_ports(struct validator_data *data, gchar **args)
 	if (!tokens)
 		return false;
 
-	for (i = 0; tokens[i]; i++) {
+	token_count = g_strv_length(tokens);
+	if (token_count < 1)
+		ret = false;
+
+	for (i = 0; i < token_count; i++) {
 		/*
 		 * If ':' exists it is a range. Check that only one ':' exists
 		 * and the port range is specified correctly
@@ -1571,9 +1576,7 @@ static void clean_match_options(struct validator_data *data, gchar *match)
 	int i;
 
 	for (i = 0; known_matches[i].match_name; i++) {
-		if (!g_strcmp0(known_matches[i].match_name, match) &&
-			known_matches[i].family_dep == data->family) {
-
+		if (!g_strcmp0(known_matches[i].match_name, match)) {
 			remove_unique_ids_from_invoked(data,
 					 known_matches[i].opts_enabled);
 		}