nwfilter: fix for directionality of ICMP traffic

Changes from V1 to V2 of this patch
- I had reversed the logic thinking that icmp type 0 is a echo
request,but it's reply -- needed to reverse the logic
- Found that ebtables takes the --ip-tos argument only as a hex number

This patch enables the skipping of some of the ICMP traffic rules on the
iptables level under certain circumstances so that the following filter
properly enables unidirectional pings:

<filter name='testcase'>
    <uuid>d6b1a2af-def6-2898-9f8d-4a74e3c39558</uuid>
    <!-- allow incoming ICMP Echo Request -->
    <rule action='accept' direction='in' priority='500'>
        <icmp type='8'/>
    </rule>
    <!-- allow outgoing ICMP Echo Reply -->
    <rule action='accept' direction='out' priority='500'>
        <icmp type='0'/>
    </rule>
    <!-- drop all other ICMP traffic -->
    <rule action='drop' direction='inout' priority='600'>
        <icmp/>
    </rule>
</filter>

diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 23c50dfd12cc1bd8c48db4dc8d6088542cd7c32d..f17f8e31a2ef98c1f648700d039358ba8f944a1b 100644 (file)
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -1022,6 +1022,12 @@ err_exit:
  * @ifname : The name of the interface to apply the rule to
  * @vars : A map containing the variables to resolve
  * @res : The data structure to store the result(s) into
+ * @match : optional string for state match
+ * @accept_target : where to jump to on accepted traffic, i.e., "RETURN"
+ *    "ACCEPT"
+ * @isIPv6 : Whether this is an IPv6 rule
+ * @maySkipICMP : whether this rule may under certain circumstances skip
+ *           the ICMP rule from being created
  *
  * Convert a single rule into its representation for later instantiation
  *
@@ -1039,7 +1045,8 @@ _iptablesCreateRuleInstance(int directionIn,
                             virNWFilterRuleInstPtr res,
                             const char *match,
                             const char *accept_target,
-                            bool isIPv6)
+                            bool isIPv6,
+                            bool maySkipICMP)
 {
     char chain[MAX_CHAINNAME_LENGTH];
     char number[20];
@@ -1265,6 +1272,10 @@ _iptablesCreateRuleInstance(int directionIn,
 
         if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPType)) {
             const char *parm;
+
+            if (maySkipICMP)
+                goto exit_no_error;
+
             if (rule->prtclType == VIR_NWFILTER_RULE_PROTOCOL_ICMP)
                 parm = "--icmp-type";
             else
@@ -1386,6 +1397,10 @@ err_exit:
 
     return -1;
 
+exit_no_error:
+    virBufferFreeAndReset(&buf);
+
+    return 0;
 }
 
 
@@ -1401,15 +1416,19 @@ iptablesCreateRuleInstance(virNWFilterDefPtr nwfilter,
     int directionIn = 0;
     char chainPrefix[2];
     int needState = 1;
+    bool maySkipICMP, inout = false;
 
     if ((rule->tt == VIR_NWFILTER_RULE_DIRECTION_IN) ||
         (rule->tt == VIR_NWFILTER_RULE_DIRECTION_INOUT)) {
         directionIn = 1;
         needState = 0;
+        inout = (rule->tt == VIR_NWFILTER_RULE_DIRECTION_INOUT);
     }
 
     chainPrefix[0] = 'F';
 
+    maySkipICMP = directionIn || inout;
+
     chainPrefix[1] = CHAINPREFIX_HOST_IN_TEMP;
     rc = _iptablesCreateRuleInstance(directionIn,
                                      chainPrefix,
@@ -1421,10 +1440,14 @@ iptablesCreateRuleInstance(virNWFilterDefPtr nwfilter,
                                      needState ? MATCH_STATE_OUT
                                                : NULL,
                                      "RETURN",
-                                     isIPv6);
+                                     isIPv6,
+                                     maySkipICMP);
     if (rc)
         return rc;
 
+
+    maySkipICMP = !directionIn || inout;
+
     chainPrefix[1] = CHAINPREFIX_HOST_OUT_TEMP;
     rc = _iptablesCreateRuleInstance(!directionIn,
                                      chainPrefix,
@@ -1436,10 +1459,13 @@ iptablesCreateRuleInstance(virNWFilterDefPtr nwfilter,
                                      needState ? MATCH_STATE_IN
                                                : NULL,
                                      "ACCEPT",
-                                     isIPv6);
+                                     isIPv6,
+                                     maySkipICMP);
     if (rc)
         return rc;
 
+    maySkipICMP = directionIn;
+
     chainPrefix[0] = 'H';
     chainPrefix[1] = CHAINPREFIX_HOST_IN_TEMP;
     rc = _iptablesCreateRuleInstance(directionIn,
@@ -1451,9 +1477,8 @@ iptablesCreateRuleInstance(virNWFilterDefPtr nwfilter,
                                      res,
                                      NULL,
                                      "ACCEPT",
-                                     isIPv6);
-    if (rc)
-        return rc;
+                                     isIPv6,
+                                     maySkipICMP);
 
     return rc;
 }
@@ -1750,9 +1775,9 @@ ebtablesCreateRuleInstance(char chainPrefix,
         }
 
         if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.ipHdr.dataDSCP)) {
-            if (printDataType(vars,
-                              number, sizeof(number),
-                              &rule->p.ipHdrFilter.ipHdr.dataDSCP))
+            if (printDataTypeAsHex(vars,
+                                   number, sizeof(number),
+                                   &rule->p.ipHdrFilter.ipHdr.dataDSCP))
                 goto err_exit;
 
             virBufferVSprintf(&buf,