V3: - renamed IPTABLES_MAX_COMMENT_SIZE to IPTABLES_MAX_COMMENT_LENGTH - building comment string in separate variable -> introducing 'prefix' virBuffer where the comment is written into (if needed) V2: following Eric's comments: - commands in the script are assigned using cmd='...' rather than cmd="..." - invoke commands using eval res=... rather than res=eval ... - rewrote function escaping ` and single quotes (which became more tricky) In this patch I am extending the rule instantiator to create the comment node where supported, which is the case for iptables and ip6tables. Since commands are written in the format cmd='iptables ...-m comment --comment \"\" ' certain characters ('`) in the comment need to be escaped to prevent comments from becoming commands themselves or cause other forms of (bash) substitutions. I have tested this with various input and in my tests the input made it straight into the comment. A test case for TCK will be provided separately that tests this. Signed-off-by: Stefan Berger --- src/nwfilter/nwfilter_ebiptables_driver.c | 87 ++++++++++++++++++++++++------ src/nwfilter/nwfilter_ebiptables_driver.h | 2 2 files changed, 74 insertions(+), 15 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -23,6 +23,7 @@ #include +#include #include #include @@ -52,10 +53,10 @@ #define CMD_SEPARATOR "\n" -#define CMD_DEF_PRE "cmd=\"" -#define CMD_DEF_POST "\"" +#define CMD_DEF_PRE "cmd='" +#define CMD_DEF_POST "'" #define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST -#define CMD_EXEC "res=`${cmd}`" CMD_SEPARATOR +#define CMD_EXEC "eval res=\\`\"${cmd}\"\\`" CMD_SEPARATOR #define CMD_STOPONERR(X) \ X ? "if [ $? -ne 0 ]; then" \ " echo \"Failure to execute command '${cmd}'.\";" \ @@ -106,6 +107,7 @@ static const char *m_physdev_out_str = " #define MATCH_PHYSDEV_IN m_physdev_in_str #define MATCH_PHYSDEV_OUT m_physdev_out_str +#define COMMENT_VARNAME "comment" static int ebtablesRemoveBasicRules(const char *ifname); static int ebiptablesDriverInit(void); @@ -292,6 +294,26 @@ printDataTypeAsHex(virNWFilterHashTableP static void +printCommentVar(virBufferPtr dest, const char *buf) +{ + size_t i, len = strlen(buf); + + virBufferAddLit(dest, COMMENT_VARNAME "='"); + + if (len > IPTABLES_MAX_COMMENT_LENGTH) + len = IPTABLES_MAX_COMMENT_LENGTH; + + for (i = 0; i < len; i++) { + if (buf[i] == '\'') + virBufferAddLit(dest, "'\\''"); + else + virBufferAddChar(dest, buf[i]); + } + virBufferAddLit(dest,"'" CMD_SEPARATOR); +} + + +static void ebiptablesRuleInstFree(ebiptablesRuleInstPtr inst) { if (!inst) @@ -846,7 +868,8 @@ iptablesHandleIpHdr(virBufferPtr buf, virNWFilterHashTablePtr vars, ipHdrDataDefPtr ipHdr, int directionIn, - bool *skipRule, bool *skipMatch) + bool *skipRule, bool *skipMatch, + virBufferPtr prefix) { char ipaddr[INET6_ADDRSTRLEN], number[20]; @@ -993,6 +1016,13 @@ iptablesHandleIpHdr(virBufferPtr buf, } } + if (HAS_ENTRY_ITEM(&ipHdr->dataComment)) { + printCommentVar(prefix, ipHdr->dataComment.u.string); + + virBufferAddLit(buf, + " -m comment --comment \"$" COMMENT_VARNAME "\""); + } + return 0; err_exit: @@ -1106,7 +1136,9 @@ _iptablesCreateRuleInstance(int directio { char chain[MAX_CHAINNAME_LENGTH]; char number[20]; + virBuffer prefix = VIR_BUFFER_INITIALIZER; virBuffer buf = VIR_BUFFER_INITIALIZER; + virBufferPtr final = NULL; const char *target; const char *iptables_cmd = (isIPv6) ? ip6tables_cmd_path : iptables_cmd_path; @@ -1148,7 +1180,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.tcpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; if (iptablesHandlePortData(&buf, @@ -1193,7 +1226,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.udpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; if (iptablesHandlePortData(&buf, @@ -1225,7 +1259,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.udpliteHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1252,7 +1287,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.espHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1279,7 +1315,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.ahHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1306,7 +1343,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.sctpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; if (iptablesHandlePortData(&buf, @@ -1341,7 +1379,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.icmpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPType)) { @@ -1400,7 +1439,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.igmpHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1427,7 +1467,8 @@ _iptablesCreateRuleInstance(int directio vars, &rule->p.allHdrFilter.ipHdr, directionIn, - &skipRule, &skipMatch)) + &skipRule, &skipMatch, + &prefix)) goto err_exit; break; @@ -1439,6 +1480,7 @@ _iptablesCreateRuleInstance(int directio if ((srcMacSkipped && bufUsed == virBufferUse(&buf)) || skipRule) { virBufferFreeAndReset(&buf); + virBufferFreeAndReset(&prefix); return 0; } @@ -1458,14 +1500,29 @@ _iptablesCreateRuleInstance(int directio CMD_EXEC, target); - if (virBufferError(&buf)) { + if (virBufferError(&buf) || virBufferError(&prefix)) { virBufferFreeAndReset(&buf); + virBufferFreeAndReset(&prefix); virReportOOMError(); return -1; } + if (virBufferUse(&prefix)) { + virBufferVSprintf(&prefix, "%s", virBufferContentAndReset(&buf)); + + final = &prefix; + + if (virBufferError(&prefix)) { + virBufferFreeAndReset(&prefix); + virReportOOMError(); + return -1; + } + } else + final = &buf; + + return ebiptablesAddRuleInst(res, - virBufferContentAndReset(&buf), + virBufferContentAndReset(final), nwfilter->chainsuffix, '\0', rule->priority, Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h @@ -45,4 +45,6 @@ extern virNWFilterTechDriver ebiptables_ # define EBIPTABLES_DRIVER_ID "ebiptables" +# define IPTABLES_MAX_COMMENT_LENGTH 256 + #endif