[libvirt] [PATCH] nwfilter: Use -m conntrack rather than -m state
Since iptables version 1.4.16 '-m state --state NEW' is converted to '-m conntrack --ctstate NEW'. Therefore, when encountering this or later versions of iptables use '-m conntrack --ctstate'. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 50 +++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) 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 @@ -188,6 +188,9 @@ static const char ebiptables_script_set_ static const char *m_state_out_str = "-m state --state NEW,ESTABLISHED"; static const char *m_state_in_str = "-m state --state ESTABLISHED"; +static const char *m_state_out_str_new = "-m conntrack --ctstate NEW,ESTABLISHED"; +static const char *m_state_in_str_new = "-m conntrack --ctstate ESTABLISHED"; + static const char *m_physdev_in_str = "-m physdev " PHYSDEV_IN; static const char *m_physdev_out_str = "-m physdev " PHYSDEV_OUT; static const char *m_physdev_out_old_str = "-m physdev " PHYSDEV_OUT_OLD; @@ -4353,6 +4356,49 @@ ebiptablesDriverProbeCtdir(void) iptables_ctdir_corrected = CTDIR_STATUS_OLD; } +static void +ebiptablesDriverProbeStateMatch(void) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *cmdout = NULL, *version; + unsigned long thisversion; + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + + virBufferAsprintf(&buf, + "$IPT --version"); + + if (ebiptablesExecCLI(&buf, NULL, &cmdout) < 0) { + VIR_ERROR(_("Testing of iptables command failed: %s"), + cmdout); + return; + } + + /* + * we expect output in the format + * iptables v1.4.16 + */ + if (!(version = strchr(cmdout, 'v')) && + virParseVersionString(version + 1, &thisversion, true) < 0) { + VIR_ERROR(_("Could not determine iptables version from string %s"), + cmdout); + goto cleanup; + } + + /* + * since version 1.4.16 '-m state --state ...' will be converted to + * '-m conntrack --ctstate ...' + */ + if (thisversion > 1 * 1000000 + 4 * 1000 + 16) { + m_state_out_str = m_state_out_str_new; + m_state_in_str = m_state_in_str_new; + } + +cleanup: + VIR_FREE(cmdout); + return; +} + static int ebiptablesDriverInit(bool privileged) { @@ -4390,8 +4436,10 @@ ebiptablesDriverInit(bool privileged) return -ENOTSUP; } - if (iptables_cmd_path) + if (iptables_cmd_path) { ebiptablesDriverProbeCtdir(); + ebiptablesDriverProbeStateMatch(); + } ebiptables_driver.flags = TECHDRV_FLAG_INITIALIZED;
On 08/06/2013 09:52 AM, Stefan Berger wrote:
Since iptables version 1.4.16 '-m state --state NEW' is converted to '-m conntrack --ctstate NEW'. Therefore, when encountering this or later versions of iptables use '-m conntrack --ctstate'.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/nwfilter/nwfilter_ebiptables_driver.c | 50 +++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)
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 @@ -188,6 +188,9 @@ static const char ebiptables_script_set_
static const char *m_state_out_str = "-m state --state NEW,ESTABLISHED"; static const char *m_state_in_str = "-m state --state ESTABLISHED"; +static const char *m_state_out_str_new = "-m conntrack --ctstate NEW,ESTABLISHED"; +static const char *m_state_in_str_new = "-m conntrack --ctstate ESTABLISHED"; + static const char *m_physdev_in_str = "-m physdev " PHYSDEV_IN; static const char *m_physdev_out_str = "-m physdev " PHYSDEV_OUT; static const char *m_physdev_out_old_str = "-m physdev " PHYSDEV_OUT_OLD; @@ -4353,6 +4356,49 @@ ebiptablesDriverProbeCtdir(void) iptables_ctdir_corrected = CTDIR_STATUS_OLD; }
+static void +ebiptablesDriverProbeStateMatch(void) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *cmdout = NULL, *version; + unsigned long thisversion; + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + + virBufferAsprintf(&buf, + "$IPT --version"); + + if (ebiptablesExecCLI(&buf, NULL, &cmdout) < 0) { + VIR_ERROR(_("Testing of iptables command failed: %s"), + cmdout); + return; Probably should just goto cleanup since we'll need to free buf
+ } + + /* + * we expect output in the format + * iptables v1.4.16 + */ + if (!(version = strchr(cmdout, 'v')) && + virParseVersionString(version + 1, &thisversion, true) < 0) { + VIR_ERROR(_("Could not determine iptables version from string %s"), + cmdout); + goto cleanup; + } + + /* + * since version 1.4.16 '-m state --state ...' will be converted to + * '-m conntrack --ctstate ...' + */ + if (thisversion > 1 * 1000000 + 4 * 1000 + 16) { + m_state_out_str = m_state_out_str_new; + m_state_in_str = m_state_in_str_new; + } + +cleanup:
Need to free 'buf' too right? virBufferFreeAndReset(&buf); Otherwise, seems fine though John
+ VIR_FREE(cmdout); + return; +} + static int ebiptablesDriverInit(bool privileged) { @@ -4390,8 +4436,10 @@ ebiptablesDriverInit(bool privileged) return -ENOTSUP; }
- if (iptables_cmd_path) + if (iptables_cmd_path) { ebiptablesDriverProbeCtdir(); + ebiptablesDriverProbeStateMatch(); + }
ebiptables_driver.flags = TECHDRV_FLAG_INITIALIZED;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/06/2013 11:20 AM, John Ferlan wrote:
On 08/06/2013 09:52 AM, Stefan Berger wrote:
Since iptables version 1.4.16 '-m state --state NEW' is converted to '-m conntrack --ctstate NEW'. Therefore, when encountering this or later versions of iptables use '-m conntrack --ctstate'.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/nwfilter/nwfilter_ebiptables_driver.c | 50 +++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)
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 @@ -188,6 +188,9 @@ static const char ebiptables_script_set_
static const char *m_state_out_str = "-m state --state NEW,ESTABLISHED"; static const char *m_state_in_str = "-m state --state ESTABLISHED"; +static const char *m_state_out_str_new = "-m conntrack --ctstate NEW,ESTABLISHED"; +static const char *m_state_in_str_new = "-m conntrack --ctstate ESTABLISHED"; + static const char *m_physdev_in_str = "-m physdev " PHYSDEV_IN; static const char *m_physdev_out_str = "-m physdev " PHYSDEV_OUT; static const char *m_physdev_out_old_str = "-m physdev " PHYSDEV_OUT_OLD; @@ -4353,6 +4356,49 @@ ebiptablesDriverProbeCtdir(void) iptables_ctdir_corrected = CTDIR_STATUS_OLD; }
+static void +ebiptablesDriverProbeStateMatch(void) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *cmdout = NULL, *version; + unsigned long thisversion; + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + + virBufferAsprintf(&buf, + "$IPT --version"); + + if (ebiptablesExecCLI(&buf, NULL, &cmdout) < 0) { + VIR_ERROR(_("Testing of iptables command failed: %s"), + cmdout); + return; Probably should just goto cleanup since we'll need to free buf
ebiptablesExecCLI already takes care of freeing the buffer.
+ } + + /* + * we expect output in the format + * iptables v1.4.16 + */ + if (!(version = strchr(cmdout, 'v')) && + virParseVersionString(version + 1, &thisversion, true) < 0) { + VIR_ERROR(_("Could not determine iptables version from string %s"), + cmdout); + goto cleanup; + } + + /* + * since version 1.4.16 '-m state --state ...' will be converted to + * '-m conntrack --ctstate ...' + */ + if (thisversion > 1 * 1000000 + 4 * 1000 + 16) { + m_state_out_str = m_state_out_str_new; + m_state_in_str = m_state_in_str_new; + } + +cleanup: Need to free 'buf' too right?
Should not be needed due to the reason above. Stefan
On 08/06/2013 12:43 PM, Stefan Berger wrote:
On 08/06/2013 11:20 AM, John Ferlan wrote:
On 08/06/2013 09:52 AM, Stefan Berger wrote:
Since iptables version 1.4.16 '-m state --state NEW' is converted to '-m conntrack --ctstate NEW'. Therefore, when encountering this or later versions of iptables use '-m conntrack --ctstate'.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/nwfilter/nwfilter_ebiptables_driver.c | 50 +++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)
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 @@ -188,6 +188,9 @@ static const char ebiptables_script_set_ static const char *m_state_out_str = "-m state --state NEW,ESTABLISHED"; static const char *m_state_in_str = "-m state --state ESTABLISHED"; +static const char *m_state_out_str_new = "-m conntrack --ctstate NEW,ESTABLISHED"; +static const char *m_state_in_str_new = "-m conntrack --ctstate ESTABLISHED"; + static const char *m_physdev_in_str = "-m physdev " PHYSDEV_IN; static const char *m_physdev_out_str = "-m physdev " PHYSDEV_OUT; static const char *m_physdev_out_old_str = "-m physdev " PHYSDEV_OUT_OLD; @@ -4353,6 +4356,49 @@ ebiptablesDriverProbeCtdir(void) iptables_ctdir_corrected = CTDIR_STATUS_OLD; } +static void +ebiptablesDriverProbeStateMatch(void) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *cmdout = NULL, *version; + unsigned long thisversion; + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + + virBufferAsprintf(&buf, + "$IPT --version"); + + if (ebiptablesExecCLI(&buf, NULL, &cmdout) < 0) { + VIR_ERROR(_("Testing of iptables command failed: %s"), + cmdout); + return; Probably should just goto cleanup since we'll need to free buf
ebiptablesExecCLI already takes care of freeing the buffer.
+ } + + /* + * we expect output in the format + * iptables v1.4.16 + */ + if (!(version = strchr(cmdout, 'v')) && + virParseVersionString(version + 1, &thisversion, true) < 0) { + VIR_ERROR(_("Could not determine iptables version from string %s"), + cmdout); + goto cleanup; + } + + /* + * since version 1.4.16 '-m state --state ...' will be converted to + * '-m conntrack --ctstate ...' + */ + if (thisversion > 1 * 1000000 + 4 * 1000 + 16) { + m_state_out_str = m_state_out_str_new; + m_state_in_str = m_state_in_str_new; + } + +cleanup: Need to free 'buf' too right?
Should not be needed due to the reason above.
Ahh, I see... I must dig deeper :-) virCommandAddArgBuffer() does the magic free... Does the thisversion check need to be >= or is > proper? Reading the comment makes me believe it was added as of 1.4.16, thus a >= rather than >. Again - the changes seem reasonable to me. ACK John
On 08/06/2013 07:10 PM, John Ferlan wrote:
On 08/06/2013 12:43 PM, Stefan Berger wrote:
On 08/06/2013 11:20 AM, John Ferlan wrote:
On 08/06/2013 09:52 AM, Stefan Berger wrote:
Since iptables version 1.4.16 '-m state --state NEW' is converted to '-m conntrack --ctstate NEW'. Therefore, when encountering this or later versions of iptables use '-m conntrack --ctstate'.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/nwfilter/nwfilter_ebiptables_driver.c | 50 +++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)
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 @@ -188,6 +188,9 @@ static const char ebiptables_script_set_ static const char *m_state_out_str = "-m state --state NEW,ESTABLISHED"; static const char *m_state_in_str = "-m state --state ESTABLISHED"; +static const char *m_state_out_str_new = "-m conntrack --ctstate NEW,ESTABLISHED"; +static const char *m_state_in_str_new = "-m conntrack --ctstate ESTABLISHED"; + static const char *m_physdev_in_str = "-m physdev " PHYSDEV_IN; static const char *m_physdev_out_str = "-m physdev " PHYSDEV_OUT; static const char *m_physdev_out_old_str = "-m physdev " PHYSDEV_OUT_OLD; @@ -4353,6 +4356,49 @@ ebiptablesDriverProbeCtdir(void) iptables_ctdir_corrected = CTDIR_STATUS_OLD; } +static void +ebiptablesDriverProbeStateMatch(void) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *cmdout = NULL, *version; + unsigned long thisversion; + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + + virBufferAsprintf(&buf, + "$IPT --version"); + + if (ebiptablesExecCLI(&buf, NULL, &cmdout) < 0) { + VIR_ERROR(_("Testing of iptables command failed: %s"), + cmdout); + return; Probably should just goto cleanup since we'll need to free buf ebiptablesExecCLI already takes care of freeing the buffer.
+ } + + /* + * we expect output in the format + * iptables v1.4.16 + */ + if (!(version = strchr(cmdout, 'v')) && + virParseVersionString(version + 1, &thisversion, true) < 0) { + VIR_ERROR(_("Could not determine iptables version from string %s"), + cmdout); + goto cleanup; + } + + /* + * since version 1.4.16 '-m state --state ...' will be converted to + * '-m conntrack --ctstate ...' + */ + if (thisversion > 1 * 1000000 + 4 * 1000 + 16) { + m_state_out_str = m_state_out_str_new; + m_state_in_str = m_state_in_str_new; + } + +cleanup: Need to free 'buf' too right? Should not be needed due to the reason above.
Ahh, I see... I must dig deeper :-) virCommandAddArgBuffer() does the magic free...
Does the thisversion check need to be >= or is > proper? Reading the comment makes me believe it was added as of 1.4.16, thus a >= rather than >. Good catch... Sending v2 fixing also the && above...
Thanks. Stefan
On 08/06/2013 11:20 AM, John Ferlan wrote:
On 08/06/2013 09:52 AM, Stefan Berger wrote:
Since iptables version 1.4.16 '-m state --state NEW' is converted to '-m conntrack --ctstate NEW'. Therefore, when encountering this or later versions of iptables use '-m conntrack --ctstate'.
Forgot to mention that the reason for this patch is also to address the case where '-m state' was not supported by iptables at all anymore. I am not saying it will happen but one now gets a warning. Stefan
participants (2)
-
John Ferlan -
Stefan Berger