[PATCH IP6TABLES]: don't allow to specify protocol of IPv6 extension header

[PATCH IP6TABLES]: don't allow to specify protocol of IPv6 extension header

[Yasuyuki KOZAKAI]

[-- Attachment #1: Type: Text/Plain, Size: 256 bytes --]


Hi,

Sometimes I hear that people do 'ip6tables -p ah ...' which never matches
any packet. IPv6 extension headers except of ESP are skipped and invalid
as argument of '-p'. Then I propose that ip6tables exits with error in such
case.

-- Yasuyuki Kozakai

[-- Attachment #2: check-exthdr.patch --]
[-- Type: Text/Plain, Size: 1485 bytes --]

[PATCH] don't allow to specify protocol of IPv6 extension header

---
commit 9bbbc3c90face808eb05da8aae69c9e6f7b508b3
tree 5506d3520a60553607f074ed21969ca0c2fbb8aa
parent 5a2223d5d7d2d81ab036effd7460a9dea45d247e
author Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> Wed, 29 Mar 2006 12:50:32 +0900
committer Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> Wed, 29 Mar 2006 12:50:32 +0900

 ip6tables.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/ip6tables.c b/ip6tables.c
index dcf7d36..a6471ee 100644
--- a/ip6tables.c
+++ b/ip6tables.c
@@ -849,6 +849,17 @@ parse_protocol(const char *s)
 	return (u_int16_t)proto;
 }
 
+/* proto means upper-layer protocol or ESP ? */
+static int is_exthdr(u_int16_t proto)
+{
+	return (proto == IPPROTO_HOPOPTS ||
+		proto == IPPROTO_ROUTING ||
+		proto == IPPROTO_FRAGMENT ||
+		proto == IPPROTO_ESP ||
+		proto == IPPROTO_AH ||
+		proto == IPPROTO_DSTOPTS);
+}
+
 void parse_interface(const char *arg, char *vianame, unsigned char *mask)
 {
 	int vialen = strlen(arg);
@@ -1926,6 +1937,13 @@ int do_command6(int argc, char *argv[], 
 			    && (fw.ipv6.invflags & IP6T_INV_PROTO))
 				exit_error(PARAMETER_PROBLEM,
 					   "rule would never match protocol");
+			
+			if (fw.ipv6.proto != IPPROTO_ESP &&
+			    is_exthdr(fw.ipv6.proto))
+				exit_error(PARAMETER_PROBLEM,
+					   "invalid protocol: %s. "
+					   "use exension match instead.",
+					   protocol);
 			break;
 
 		case 's':

Re: [PATCH IP6TABLES]: don't allow to specify protocol of IPv6 extension header

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
> Hi,
> 
> Sometimes I hear that people do 'ip6tables -p ah ...' which never matches
> any packet. IPv6 extension headers except of ESP are skipped and invalid
> as argument of '-p'. Then I propose that ip6tables exits with error in such
> case.

How about a warning for some time first? If people use iptables-restore
this could break their entire ruleset ..

Re: [PATCH IP6TABLES]: don't allow to specify protocol of IPv6 extension header

[Yasuyuki KOZAKAI]

[-- Attachment #1: Type: Text/Plain, Size: 440 bytes --]

From: Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp>
Date: Wed, 29 Mar 2006 15:59:12 +0900 (JST)

> Sometimes I hear that people do 'ip6tables -p ah ...' which never matches
> any packet. IPv6 extension headers except of ESP are skipped and invalid
> as argument of '-p'. Then I propose that ip6tables exits with error in such
> case.

Sorry, above patch includes wrong comment. Please apply attached patch
instead.

-- Yasuyuki Kozakai

[-- Attachment #2: check-exthdr-fixed.patch --]
[-- Type: Text/Plain, Size: 1479 bytes --]

[PATCH] don't allow to specify protocol of IPv6 extension header

---
commit 9bbbc3c90face808eb05da8aae69c9e6f7b508b3
tree 5506d3520a60553607f074ed21969ca0c2fbb8aa
parent 5a2223d5d7d2d81ab036effd7460a9dea45d247e
author Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> Wed, 29 Mar 2006 12:50:32 +0900
committer Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> Wed, 29 Mar 2006 12:50:32 +0900

 ip6tables.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/ip6tables.c b/ip6tables.c
index dcf7d36..a6471ee 100644
--- a/ip6tables.c
+++ b/ip6tables.c
@@ -849,6 +849,17 @@ parse_protocol(const char *s)
 	return (u_int16_t)proto;
 }
 
+/* proto means IPv6 extension header ? */
+static int is_exthdr(u_int16_t proto)
+{
+	return (proto == IPPROTO_HOPOPTS ||
+		proto == IPPROTO_ROUTING ||
+		proto == IPPROTO_FRAGMENT ||
+		proto == IPPROTO_ESP ||
+		proto == IPPROTO_AH ||
+		proto == IPPROTO_DSTOPTS);
+}
+
 void parse_interface(const char *arg, char *vianame, unsigned char *mask)
 {
 	int vialen = strlen(arg);
@@ -1926,6 +1937,13 @@ int do_command6(int argc, char *argv[], 
 			    && (fw.ipv6.invflags & IP6T_INV_PROTO))
 				exit_error(PARAMETER_PROBLEM,
 					   "rule would never match protocol");
+			
+			if (fw.ipv6.proto != IPPROTO_ESP &&
+			    is_exthdr(fw.ipv6.proto))
+				exit_error(PARAMETER_PROBLEM,
+					   "invalid protocol: %s. "
+					   "use exension match instead.",
+					   protocol);
 			break;
 
 		case 's':

Re: [PATCH IP6TABLES]: don't allow to specify protocol of IPv6 extension header

[Yasuyuki KOZAKAI]

[-- Attachment #1: Type: Text/Plain, Size: 584 bytes --]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 29 Mar 2006 10:11:19 +0200

> Yasuyuki KOZAKAI wrote:
> > Hi,
> > 
> > Sometimes I hear that people do 'ip6tables -p ah ...' which never matches
> > any packet. IPv6 extension headers except of ESP are skipped and invalid
> > as argument of '-p'. Then I propose that ip6tables exits with error in such
> > case.
> 
> How about a warning for some time first? If people use iptables-restore
> this could break their entire ruleset ..

OK, then please ignore previous 'fixed patch' and apply this. Sorry for mess.

-- Yasuyuki Kozakai


[-- Attachment #2: check-exthdr-fixed-2.patch --]
[-- Type: Text/Plain, Size: 1456 bytes --]

[PATCH] don't allow to specify protocol of IPv6 extension header

---
commit 9bbbc3c90face808eb05da8aae69c9e6f7b508b3
tree 5506d3520a60553607f074ed21969ca0c2fbb8aa
parent 5a2223d5d7d2d81ab036effd7460a9dea45d247e
author Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> Wed, 29 Mar 2006 12:50:32 +0900
committer Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> Wed, 29 Mar 2006 12:50:32 +0900

 ip6tables.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/ip6tables.c b/ip6tables.c
index dcf7d36..a6471ee 100644
--- a/ip6tables.c
+++ b/ip6tables.c
@@ -849,6 +849,17 @@ parse_protocol(const char *s)
 	return (u_int16_t)proto;
 }
 
+/* proto means IPv6 extension header ? */
+static int is_exthdr(u_int16_t proto)
+{
+	return (proto == IPPROTO_HOPOPTS ||
+		proto == IPPROTO_ROUTING ||
+		proto == IPPROTO_FRAGMENT ||
+		proto == IPPROTO_ESP ||
+		proto == IPPROTO_AH ||
+		proto == IPPROTO_DSTOPTS);
+}
+
 void parse_interface(const char *arg, char *vianame, unsigned char *mask)
 {
 	int vialen = strlen(arg);
@@ -1926,6 +1937,11 @@ int do_command6(int argc, char *argv[], 
 			    && (fw.ipv6.invflags & IP6T_INV_PROTO))
 				exit_error(PARAMETER_PROBLEM,
 					   "rule would never match protocol");
+			
+			if (fw.ipv6.proto != IPPROTO_ESP &&
+			    is_exthdr(fw.ipv6.proto))
+				printf("Warning: never matched protocol: %s. "
+				       "use exension match instead.", protocol);
 			break;
 
 		case 's':

Re: [PATCH IP6TABLES]: don't allow to specify protocol of IPv6 extension header

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Wed, 29 Mar 2006 10:11:19 +0200
> 
> 
>>Yasuyuki KOZAKAI wrote:
>>
>>>Hi,
>>>
>>>Sometimes I hear that people do 'ip6tables -p ah ...' which never matches
>>>any packet. IPv6 extension headers except of ESP are skipped and invalid
>>>as argument of '-p'. Then I propose that ip6tables exits with error in such
>>>case.
>>
>>How about a warning for some time first? If people use iptables-restore
>>this could break their entire ruleset ..
> 
> 
> OK, then please ignore previous 'fixed patch' and apply this. Sorry for mess.

Done, thanks. Is your SVN access not working?

Re: [PATCH IP6TABLES]: don't allow to specify protocol of IPv6 extension header

[Yasuyuki KOZAKAI]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 29 Mar 2006 11:09:52 +0200

> Done, thanks. Is your SVN access not working?

works fine. Is it better to commit myself ? If so, I'll do it from next
time. But I'll keep sending patches to things in svn tree to
netfilter-devel so that folks can aware of them and ack/comments/objects.

-- Yasuyuki Kozakai

Re: [PATCH IP6TABLES]: don't allow to specify protocol of IPv6 extension header

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Wed, 29 Mar 2006 11:09:52 +0200
> 
> 
>>Done, thanks. Is your SVN access not working?
> 
> 
> works fine. Is it better to commit myself ? If so, I'll do it from next
> time. But I'll keep sending patches to things in svn tree to
> netfilter-devel so that folks can aware of them and ack/comments/objects.

I think thats the best way, I usually do it like that myself except
for trivial patches.