* [ANNOUNCE] iptables 1.4.16.1 release
@ 2012-10-07 23:17 Pablo Neira Ayuso
2012-10-07 23:51 ` Pablo Neira Ayuso
2012-10-08 0:14 ` Jan Engelhardt
0 siblings, 2 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2012-10-07 23:17 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, netfilter, netfilter-announce, lwn
[-- Attachment #1: Type: text/plain, Size: 775 bytes --]
On Mon, Oct 08, 2012 at 12:24:41AM +0200, Pablo Neira Ayuso wrote:
Hi!
The Netfilter project proudly presents:
iptables 1.4.16.1
This release fixes a major breakage introduced by:
commit cd2f9bdbb7f9b737e5d640aafeb78bcd8e3a7adf
Author: Jan Engelhardt <jengelh@inai.de>
Date: Tue Sep 4 05:24:47 2012 +0200
iptables: support for target aliases
This is really unfortunate, it seems this patch has been pushed
mainstream without sufficient testing. We are really sorry for the
inconvenience. Please, don't use 1.4.16, this bug reders it
completely useless.
See ChangeLog that comes attached to this email for more details.
You can download it from:
http://www.netfilter.org/projects/iptables/downloads.html
ftp://ftp.netfilter.org/pub/iptables/
Have fun!
[-- Attachment #2: changes-iptables-1.4.16.1.txt --]
[-- Type: text/plain, Size: 91 bytes --]
Pablo Neira Ayuso (2):
iptables: fix standard target
bump version to 1.4.16.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [ANNOUNCE] iptables 1.4.16.1 release 2012-10-07 23:17 [ANNOUNCE] iptables 1.4.16.1 release Pablo Neira Ayuso @ 2012-10-07 23:51 ` Pablo Neira Ayuso 2012-10-08 0:14 ` Jan Engelhardt 1 sibling, 0 replies; 6+ messages in thread From: Pablo Neira Ayuso @ 2012-10-07 23:51 UTC (permalink / raw) To: netfilter-devel [-- Attachment #1: Type: text/plain, Size: 1122 bytes --] On Mon, Oct 08, 2012 at 01:17:06AM +0200, Pablo Neira Ayuso wrote: > On Mon, Oct 08, 2012 at 12:24:41AM +0200, Pablo Neira Ayuso wrote: > Hi! > > The Netfilter project proudly presents: > > iptables 1.4.16.1 > > This release fixes a major breakage introduced by: > > commit cd2f9bdbb7f9b737e5d640aafeb78bcd8e3a7adf > Author: Jan Engelhardt <jengelh@inai.de> > Date: Tue Sep 4 05:24:47 2012 +0200 > > iptables: support for target aliases > > This is really unfortunate, it seems this patch has been pushed > mainstream without sufficient testing. We are really sorry for the > inconvenience. Please, don't use 1.4.16, this bug reders it > completely useless. > > See ChangeLog that comes attached to this email for more details. I got everything working back but the new target alias infrastructure while rushing to fix the breakage. The patch attached should get the target aliasing working. This is not serious though, the only target that can benefit from this is feature is NOTRACK, and it will be removed in the upcoming 3.7, ie. there is no kernel available that can benefit from this feature. [-- Attachment #2: x --] [-- Type: text/plain, Size: 1032 bytes --] diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c index faddb71..1683621 100644 --- a/iptables/ip6tables.c +++ b/iptables/ip6tables.c @@ -1286,7 +1286,7 @@ static void command_jump(struct iptables_command_state *cs) cs->target->t = xtables_calloc(1, size); cs->target->t->u.target_size = size; - if (cs->target->real_name != NULL) + if (strcmp(cs->target->real_name, "standard") == 0) strcpy(cs->target->t->u.user.name, cs->jumpto); else strcpy(cs->target->t->u.user.name, cs->target->real_name); diff --git a/iptables/iptables.c b/iptables/iptables.c index 96cea64..ae22ead 100644 --- a/iptables/iptables.c +++ b/iptables/iptables.c @@ -1295,7 +1295,7 @@ static void command_jump(struct iptables_command_state *cs) cs->target->t = xtables_calloc(1, size); cs->target->t->u.target_size = size; - if (cs->target->real_name != NULL) + if (strcmp(cs->target->real_name, "standard") == 0) strcpy(cs->target->t->u.user.name, cs->jumpto); else strcpy(cs->target->t->u.user.name, cs->target->real_name); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [ANNOUNCE] iptables 1.4.16.1 release 2012-10-07 23:17 [ANNOUNCE] iptables 1.4.16.1 release Pablo Neira Ayuso 2012-10-07 23:51 ` Pablo Neira Ayuso @ 2012-10-08 0:14 ` Jan Engelhardt 2012-10-08 0:32 ` [PATCH] iptables: restore NOTRACK functionality, target aliasing Jan Engelhardt 1 sibling, 1 reply; 6+ messages in thread From: Jan Engelhardt @ 2012-10-08 0:14 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: netfilter-devel, netdev, netfilter, netfilter-announce, lwn On Monday 2012-10-08 01:17, Pablo Neira Ayuso wrote: >The Netfilter project proudly presents: > > iptables 1.4.16.1 > >iptables -I INPUT -j ACCEPT >says: >iptables: No chain/target/match by that name. >This also breaks iptables-restore, of course. Jan, you'll have to explain >me how you have tested this. This was tested by adding rules with different targets that had both aliases defined and those without. ./iptables/xtables-multi main4 -t raw -N foo ./iptables/xtables-multi main4 -t raw -A foo -j NOTRACK with kernels that had xt_CT and no xt_CT at all ./iptables/xtables-multi main4 -N foo ./iptables/xtables-multi main4 -A foo -m state --state NEW with kernels that had xt_conntrack.3, and xt_conntrack.3 removed (leaving only xt_conntrack.2) ./iptables/xtables-multi main4 -t raw -N bar ./iptables/xtables-multi main4 -t raw -A bar -j MARK --set-xmark 1 ./iptables/xtables-multi main4 -t raw -A foo -j bar plus of course the "standard" (no pun intended) testsuite that we had so far: # ./iptables/xtables-multi restore6 tests/options-most.rules WARNING: --localtz is being replaced by --kerneltz, since "local" is ambiguous. Note the kernel timezone has caveats - see manpage for details. As you spotted, options-most.rules did not include -j <verdict>. While v1.4.16-1-g2aaa7ec fixes -j verdict, it breaks NOTRACK in all instances. To reuse a line, "you'll have to explain me how you have tested this." A patch to what I think should fly is posted as a reply hereto. Please give that a spin. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] iptables: restore NOTRACK functionality, target aliasing 2012-10-08 0:14 ` Jan Engelhardt @ 2012-10-08 0:32 ` Jan Engelhardt 2012-10-08 8:37 ` Pablo Neira Ayuso 0 siblings, 1 reply; 6+ messages in thread From: Jan Engelhardt @ 2012-10-08 0:32 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel, jengelh Commit v1.4.16-1-g2aaa7ec is testing for real_name (not) being NULL which was always false (true). real_name was never NULL, so cs->jumpto would always be used, which rendered -j NOTRACK unusable, since the chosen real name.revision is for example NOTRACK.1, which does not exist at the kernel side. # ./iptables/xtables-multi main4 -t raw -A foo -j NOTRACK dbg: Using NOTRACK.1 WARNING: The NOTRACK target is obsolete. Use CT instead. iptables: Protocol wrong type for socket. To reasonably support the extra-special verdict names, make it so that real_name remains NULL when an extension defined no alias, which we can then use to determine whether the user entered an alias name (which needs to be followed) or not. Signed-off-by: Jan Engelhardt <jengelh@inai.de> --- (This is a try at a resend since I have not gotten back this very email from majordomo as a list subscriber...) Patch downloadable via git://git.inai.de/iptables master as well iptables/ip6tables.c | 21 ++++++++++++++------- iptables/iptables.c | 23 +++++++++++++++-------- libxtables/xtables.c | 26 ++++++++++++++------------ 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c index faddb71..0f32b32 100644 --- a/iptables/ip6tables.c +++ b/iptables/ip6tables.c @@ -1286,15 +1286,19 @@ static void command_jump(struct iptables_command_state *cs) cs->target->t = xtables_calloc(1, size); cs->target->t->u.target_size = size; - if (cs->target->real_name != NULL) + if (cs->target->real_name == NULL) { + /* + * Spooky action at a distance: Verdicts like ACCEPT, DROP, + * etc. are translated to the real name in libiptc instead. + */ strcpy(cs->target->t->u.user.name, cs->jumpto); - else + } else { strcpy(cs->target->t->u.user.name, cs->target->real_name); - cs->target->t->u.user.revision = cs->target->revision; - if (cs->target->real_name != cs->target->name) fprintf(stderr, "WARNING: The %s target is obsolete. " "Use %s instead.\n", cs->jumpto, cs->target->real_name); + } + cs->target->t->u.user.revision = cs->target->revision; xs_init_target(cs->target); if (cs->target->x6_options != NULL) @@ -1322,11 +1326,14 @@ static void command_match(struct iptables_command_state *cs) size = XT_ALIGN(sizeof(struct xt_entry_match)) + m->size; m->m = xtables_calloc(1, size); m->m->u.match_size = size; - strcpy(m->m->u.user.name, m->real_name); - m->m->u.user.revision = m->revision; - if (m->real_name != m->name) + if (m->real_name == NULL) { + strcpy(m->m->u.user.name, m->name); + } else { + strcpy(m->m->u.user.name, m->real_name); fprintf(stderr, "WARNING: The %s match is obsolete. " "Use %s instead.\n", m->name, m->real_name); + } + m->m->u.user.revision = m->revision; xs_init_match(m); if (m == m->next) diff --git a/iptables/iptables.c b/iptables/iptables.c index 96cea64..cd34401 100644 --- a/iptables/iptables.c +++ b/iptables/iptables.c @@ -1295,16 +1295,20 @@ static void command_jump(struct iptables_command_state *cs) cs->target->t = xtables_calloc(1, size); cs->target->t->u.target_size = size; - if (cs->target->real_name != NULL) + if (cs->target->real_name == NULL) { + /* + * Spooky action at a distance: Verdicts like ACCEPT, DROP, + * etc. are translated to the real name in libiptc instead. + */ strcpy(cs->target->t->u.user.name, cs->jumpto); - else - strcpy(cs->target->t->u.user.name, cs->target->real_name); - cs->target->t->u.user.revision = cs->target->revision; - if (cs->target->real_name != cs->target->name) + } else { /* Alias support for userspace side */ + strcpy(cs->target->t->u.user.name, cs->target->real_name); fprintf(stderr, "WARNING: The %s target is obsolete. " "Use %s instead.\n", cs->jumpto, cs->target->real_name); + } + cs->target->t->u.user.revision = cs->target->revision; xs_init_target(cs->target); @@ -1333,11 +1337,14 @@ static void command_match(struct iptables_command_state *cs) size = XT_ALIGN(sizeof(struct xt_entry_match)) + m->size; m->m = xtables_calloc(1, size); m->m->u.match_size = size; - strcpy(m->m->u.user.name, m->real_name); - m->m->u.user.revision = m->revision; - if (m->real_name != m->name) + if (m->real_name == NULL) { + strcpy(m->m->u.user.name, m->name); + } else { + strcpy(m->m->u.user.name, m->real_name); fprintf(stderr, "WARNING: The %s match is obsolete. " "Use %s instead.\n", m->name, m->real_name); + } + m->m->u.user.revision = m->revision; xs_init_match(m); if (m == m->next) diff --git a/libxtables/xtables.c b/libxtables/xtables.c index 82c3643..4c91286 100644 --- a/libxtables/xtables.c +++ b/libxtables/xtables.c @@ -848,8 +848,6 @@ void xtables_register_match(struct xtables_match *me) exit(1); } - if (me->real_name == NULL) - me->real_name = me->name; if (me->x6_options != NULL) xtables_option_metavalidate(me->name, me->x6_options); if (me->extra_opts != NULL) @@ -905,9 +903,9 @@ xtables_mt_prefer(bool a_alias, unsigned int a_rev, unsigned int a_fam, static int xtables_match_prefer(const struct xtables_match *a, const struct xtables_match *b) { - return xtables_mt_prefer(a->name != a->real_name, + return xtables_mt_prefer(a->real_name != NULL, a->revision, a->family, - b->name != b->real_name, + b->real_name != NULL, b->revision, b->family); } @@ -919,15 +917,16 @@ static int xtables_target_prefer(const struct xtables_target *a, * xtables_register_*; the direct pointer comparison here is therefore * legitimate to detect an alias. */ - return xtables_mt_prefer(a->name != a->real_name, + return xtables_mt_prefer(a->real_name != NULL, a->revision, a->family, - b->name != b->real_name, + b->real_name != NULL, b->revision, b->family); } static void xtables_fully_register_pending_match(struct xtables_match *me) { struct xtables_match **i, *old; + const char *rn; int compare; old = xtables_find_match(me->name, XTF_DURING_LOAD, NULL); @@ -941,12 +940,14 @@ static void xtables_fully_register_pending_match(struct xtables_match *me) } /* Now we have two (or more) options, check compatibility. */ + rn = (old->real_name != NULL) ? old->real_name : old->name; if (compare > 0 && - compatible_match_revision(old->real_name, old->revision)) + compatible_match_revision(rn, old->revision)) return; /* See if new match can be used. */ - if (!compatible_match_revision(me->real_name, me->revision)) + rn = (me->real_name != NULL) ? me->real_name : me->name; + if (!compatible_match_revision(rn, me->revision)) return; /* Delete old one. */ @@ -1005,8 +1006,6 @@ void xtables_register_target(struct xtables_target *me) exit(1); } - if (me->real_name == NULL) - me->real_name = me->name; if (me->x6_options != NULL) xtables_option_metavalidate(me->name, me->x6_options); if (me->extra_opts != NULL) @@ -1024,6 +1023,7 @@ void xtables_register_target(struct xtables_target *me) static void xtables_fully_register_pending_target(struct xtables_target *me) { struct xtables_target *old; + const char *rn; int compare; old = xtables_find_target(me->name, XTF_DURING_LOAD); @@ -1039,12 +1039,14 @@ static void xtables_fully_register_pending_target(struct xtables_target *me) } /* Now we have two (or more) options, check compatibility. */ + rn = (old->real_name != NULL) ? old->real_name : old->name; if (compare > 0 && - compatible_target_revision(old->real_name, old->revision)) + compatible_target_revision(rn, old->revision)) return; /* See if new target can be used. */ - if (!compatible_target_revision(me->real_name, me->revision)) + rn = (me->real_name != NULL) ? me->real_name : me->name; + if (!compatible_target_revision(rn, me->revision)) return; /* Delete old one. */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] iptables: restore NOTRACK functionality, target aliasing 2012-10-08 0:32 ` [PATCH] iptables: restore NOTRACK functionality, target aliasing Jan Engelhardt @ 2012-10-08 8:37 ` Pablo Neira Ayuso 2012-10-08 12:02 ` Jan Engelhardt 0 siblings, 1 reply; 6+ messages in thread From: Pablo Neira Ayuso @ 2012-10-08 8:37 UTC (permalink / raw) To: Jan Engelhardt; +Cc: netfilter-devel On Mon, Oct 08, 2012 at 02:32:36AM +0200, Jan Engelhardt wrote: > Commit v1.4.16-1-g2aaa7ec is testing for real_name (not) being NULL > which was always false (true). real_name was never NULL, so cs->jumpto > would always be used, which rendered -j NOTRACK unusable, since the > chosen real name.revision is for example NOTRACK.1, which does not exist > at the kernel side. > > # ./iptables/xtables-multi main4 -t raw -A foo -j NOTRACK > dbg: Using NOTRACK.1 > WARNING: The NOTRACK target is obsolete. Use CT instead. > iptables: Protocol wrong type for socket. > > To reasonably support the extra-special verdict names, make it so that > real_name remains NULL when an extension defined no alias, which we can > then use to determine whether the user entered an alias name (which > needs to be followed) or not. I have applied this and made a new release. I kindly told you. I don't want late patches to hit iptables if I'm about to release it, ie. close to when the Linux kernel comes out. The reason was that chances to hit bugs and not noticing becomes higher. In other words, stick to conservative mode. Let this serve as proof of it. You disregarded my advice and now we have this shame, three releases in one day just because of rushing. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iptables: restore NOTRACK functionality, target aliasing 2012-10-08 8:37 ` Pablo Neira Ayuso @ 2012-10-08 12:02 ` Jan Engelhardt 0 siblings, 0 replies; 6+ messages in thread From: Jan Engelhardt @ 2012-10-08 12:02 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel On Monday 2012-10-08 10:37, Pablo Neira Ayuso wrote: >On Mon, Oct 08, 2012 at 02:32:36AM +0200, Jan Engelhardt wrote: >> Commit v1.4.16-1-g2aaa7ec is testing for real_name (not) being NULL >> which was always false (true). real_name was never NULL, so cs->jumpto >> would always be used, which rendered -j NOTRACK unusable, since the >> chosen real name.revision is for example NOTRACK.1, which does not exist >> at the kernel side. >> >> # ./iptables/xtables-multi main4 -t raw -A foo -j NOTRACK >> dbg: Using NOTRACK.1 >> WARNING: The NOTRACK target is obsolete. Use CT instead. >> iptables: Protocol wrong type for socket. >> >> To reasonably support the extra-special verdict names, make it so that >> real_name remains NULL when an extension defined no alias, which we can >> then use to determine whether the user entered an alias name (which >> needs to be followed) or not. > >I have applied this and made a new release. > >I kindly told you. I don't want late patches to hit iptables if I'm >about to release it, ie. close to when the Linux kernel comes out. >The reason was that chances to hit bugs and not noticing becomes >higher. In other words, stick to conservative mode. That is clear now, but was not when when the Alias Support patches went in. Sorry for that. As for the documentation updates ("doc:" labeled patches in http://marc.info/?l=netfilter-devel&m=134903982604088&w=2 ), you gave an OK, but then infuriated over them being merged, though documentation generally does not cause bugs. > >Let this serve as proof of it. > >You disregarded my advice and now we have this shame, three releases >in one day just because of rushing. I will heed your words, but I do also think that letting patches sit in git for longer may not necessarily buy more. For -j verdict, maybe. But in general, iptables firewalling & networking users do not use git so much, but they love tarballs, also because that gets spread out by distros in a glimpse. Here is an instance of iproute2 where an issue also only got noticed after the tar got out: http://archlinux.2023198.n4.nabble.com/ip-addr-show-doesn-t-show-my-ipv4-address-after-update-is-this-expected-behaviour-td4658792.html http://forums.funtoo.org/viewtopic.php?id=1526 (this subsequently also got posted to netdev) There is also that one NLM_F_MULTI issue in libnfnetlink, which slept for 2 months in git before becoming a tar. If, and only if, the "next" branch is empty (which was the case for libnfnetlink), I do not think synchronization to the kernel release schedule makes sense. But that is just my opinion.. Something completely different: There seems to be something happening in your git cherry-pick process causing the time to shift. Commit ed03bc396b861ae0302b5cc9b3826cf58ef6aa24 from git://git.inai.de/iptables has AuthorDate 2012-10-08 01:54:48+0200, but commit dd43527cb6bdf3d469100850ca10dcd2fb761304 in iptables has 2012-10-07 14:32:36+0000. I am not sure if your clock is off, since the CommitDate looks accurate. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-08 12:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-07 23:17 [ANNOUNCE] iptables 1.4.16.1 release Pablo Neira Ayuso 2012-10-07 23:51 ` Pablo Neira Ayuso 2012-10-08 0:14 ` Jan Engelhardt 2012-10-08 0:32 ` [PATCH] iptables: restore NOTRACK functionality, target aliasing Jan Engelhardt 2012-10-08 8:37 ` Pablo Neira Ayuso 2012-10-08 12:02 ` Jan Engelhardt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).