Re: netfilter/iptables and network interface names

Re: netfilter/iptables and network interface names

[Jan Engelhardt]

On Thursday 2010-02-04 13:58, Thomas Woerner wrote:

> Hello,
>
> I just fell over network interface definitions in kernel and
> netfilter/iptables.
>
> Interfaces names in the kernel have few limits: Only spaces and slashes are not
> allowed, but everything else, even Ctrl characters are.
>
> In iptables, there are other limits: Colon, exclamation mark and asterisk are
> not allowed. The plus is treated special for matching. But what will be matched
> if the interface starts with or contains a plus?
>
> iptables allows to use interfaces containing spaces and slashes. iptables-save
> is not escaping interface names. The interface "+" will be treated as no
> interface limit at all and will be dropped, therefore all interfaces match.
>
> I think there should be some discussion about interface names and limits in
> kernel and netfilter/iptables.
>
> What do you think?

I think we should cc netdev for a start.

Re: netfilter/iptables and network interface names

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2010-02-04 13:58, Thomas Woerner wrote:
> 
>> Hello,
>>
>> I just fell over network interface definitions in kernel and
>> netfilter/iptables.
>>
>> Interfaces names in the kernel have few limits: Only spaces and slashes are not
>> allowed, but everything else, even Ctrl characters are.
>>
>> In iptables, there are other limits: Colon, exclamation mark and asterisk are
>> not allowed. The plus is treated special for matching. But what will be matched
>> if the interface starts with or contains a plus?
>>
>> iptables allows to use interfaces containing spaces and slashes. iptables-save
>> is not escaping interface names. The interface "+" will be treated as no
>> interface limit at all and will be dropped, therefore all interfaces match.
>>
>> I think there should be some discussion about interface names and limits in
>> kernel and netfilter/iptables.
>>
>> What do you think?
> 
> I think we should cc netdev for a start.

I don't think there is a reason for this limitation in iptables,
so why not simply remove it?

Re: netfilter/iptables and network interface names

[Jan Engelhardt]

On Thursday 2010-02-04 15:31, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> On Thursday 2010-02-04 13:58, Thomas Woerner wrote:
>>
>>> I think there should be some discussion about interface names and
>>> limits in kernel and netfilter/iptables.
>>>
>>> What do you think?

When was the last time you needed a control character in your
interface name?

>> I think we should cc netdev for a start.
>
>I don't think there is a reason for this limitation in iptables,
>so why not simply remove it?

Like this?

parent 350661a6eb089f3e54e67e022db9e16ea280499f (v1.4.6-7-g350661a)
commit 02020c8fb965bbedae9eb43d9a9b964c46388cc3
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Thu Feb 4 15:46:03 2010 +0100

Lift restrictions on interface names

The kernel has few restrictions.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 xtables.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/xtables.c b/xtables.c
index 63c5db7..f3baf84 100644
--- a/xtables.c
+++ b/xtables.c
@@ -450,7 +450,7 @@ u_int16_t xtables_parse_port(const char *port, const char *proto)
 void xtables_parse_interface(const char *arg, char *vianame,
 			     unsigned char *mask)
 {
-	int vialen = strlen(arg);
+	unsigned int vialen = strlen(arg);
 	unsigned int i;
 
 	memset(mask, 0, IFNAMSIZ);
@@ -462,7 +462,7 @@ void xtables_parse_interface(const char *arg, char *vianame,
 			   " (%i)", arg, IFNAMSIZ-1);
 
 	strcpy(vianame, arg);
-	if ((vialen == 0) || (vialen == 1 && vianame[0] == '+'))
+	if (vialen == 0)
 		memset(mask, 0, IFNAMSIZ);
 	else if (vianame[vialen - 1] == '+') {
 		memset(mask, 0xFF, vialen - 1);
@@ -473,12 +473,11 @@ void xtables_parse_interface(const char *arg, char *vianame,
 		memset(mask, 0xFF, vialen + 1);
 		memset(mask + vialen + 1, 0, IFNAMSIZ - vialen - 1);
 		for (i = 0; vianame[i]; i++) {
-			if (vianame[i] == ':' ||
-			    vianame[i] == '!' ||
-			    vianame[i] == '*') {
+			if (vianame[i] == '/' ||
+			    vianame[i] == ' ') {
 				fprintf(stderr,
 					"Warning: weird character in interface"
-					" `%s' (No aliases, :, ! or *).\n",
+					" `%s' ('/' and ' ' are not allowed by the kernel).\n",
 					vianame);
 				break;
 			}
-- 
# Created with git-export-patch

Re: netfilter/iptables and network interface names

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2010-02-04 15:31, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> On Thursday 2010-02-04 13:58, Thomas Woerner wrote:
>>>
>>>> I think there should be some discussion about interface names and
>>>> limits in kernel and netfilter/iptables.
>>>>
>>>> What do you think?
> 
> When was the last time you needed a control character in your
> interface name?

Actually I did use ansi-colors in my interface names once for fun :)

Anyways, I don't see why iptables should impose arbitrary restrictions.

>>> I think we should cc netdev for a start.
>> I don't think there is a reason for this limitation in iptables,
>> so why not simply remove it?
> 
> Like this?
> 
> @@ -473,12 +473,11 @@ void xtables_parse_interface(const char *arg, char *vianame,
>  		memset(mask, 0xFF, vialen + 1);
>  		memset(mask + vialen + 1, 0, IFNAMSIZ - vialen - 1);
>  		for (i = 0; vianame[i]; i++) {
> -			if (vianame[i] == ':' ||
> -			    vianame[i] == '!' ||
> -			    vianame[i] == '*') {
> +			if (vianame[i] == '/' ||
> +			    vianame[i] == ' ') {
>  				fprintf(stderr,
>  					"Warning: weird character in interface"
> -					" `%s' (No aliases, :, ! or *).\n",
> +					" `%s' ('/' and ' ' are not allowed by the kernel).\n",
>  					vianame);

The kernel also forbids ".".

Re: netfilter/iptables and network interface names

[Jan Engelhardt]

On Thursday 2010-02-04 15:55, Patrick McHardy wrote:

>>>>> What do you think?
>> 
>> When was the last time you needed a control character in your
>> interface name?
>
>Actually I did use ansi-colors in my interface names once for fun :)

Madness!

>Anyways, I don't see why iptables should impose arbitrary restrictions.

Well iptables does allow \e!

>> Like this?
>> 
>> @@ -473,12 +473,11 @@ void xtables_parse_interface(const char *arg, char *vianame,
>>  		memset(mask, 0xFF, vialen + 1);
>>  		memset(mask + vialen + 1, 0, IFNAMSIZ - vialen - 1);
>>  		for (i = 0; vianame[i]; i++) {
>> -			if (vianame[i] == ':' ||
>> -			    vianame[i] == '!' ||
>> -			    vianame[i] == '*') {
>> +			if (vianame[i] == '/' ||
>> +			    vianame[i] == ' ') {
>>  				fprintf(stderr,
>>  					"Warning: weird character in interface"
>> -					" `%s' (No aliases, :, ! or *).\n",
>> +					" `%s' ('/' and ' ' are not allowed by the kernel).\n",
>>  					vianame);
>
>The kernel also forbids ".".

My first thought was the same, but:

a) Interestingly, it does not prohibit '.'

	ip tunnel add foo0.3 mode sit local 1.2.3.4 remote 5.6.7.8

b) The '.' is to be seen as valid as far as xtables.c goes, so
   as to match VLAN interfaces.

Re: netfilter/iptables and network interface names

[Patrick McHardy]

Jan Engelhardt wrote:
>> The kernel also forbids ".".
> 
> My first thought was the same, but:
> 
> a) Interestingly, it does not prohibit '.'
> 
> 	ip tunnel add foo0.3 mode sit local 1.2.3.4 remote 5.6.7.8
> 
> b) The '.' is to be seen as valid as far as xtables.c goes, so
>    as to match VLAN interfaces.

Ah of course, its only invalid at the beginning of the name.

Re: netfilter/iptables and network interface names

[Jan Engelhardt]

On Thursday 2010-02-04 17:49, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>>> The kernel also forbids ".".
>> 
>> My first thought was the same, but:
>> 
>> a) Interestingly, it does not prohibit '.'
>> 
>> 	ip tunnel add foo0.3 mode sit local 1.2.3.4 remote 5.6.7.8
>> 
>> b) The '.' is to be seen as valid as far as xtables.c goes, so
>>    as to match VLAN interfaces.
>
>Ah of course, its only invalid at the beginning of the name.
>
Nope:

19:44 borg:/home/jengelh # ip tunnel add .3 mode sit local 1.2.3.6 remote
5.6.7.7
19:44 borg:/home/jengelh # ip a
17: .3: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN 
    link/sit 1.2.3.6 peer 5.6.7.7

Re: netfilter/iptables and network interface names

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2010-02-04 17:49, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>>> The kernel also forbids ".".
>>> My first thought was the same, but:
>>>
>>> a) Interestingly, it does not prohibit '.'
>>>
>>> 	ip tunnel add foo0.3 mode sit local 1.2.3.4 remote 5.6.7.8
>>>
>>> b) The '.' is to be seen as valid as far as xtables.c goes, so
>>>    as to match VLAN interfaces.
>> Ah of course, its only invalid at the beginning of the name.
>>
> Nope:
> 
> 19:44 borg:/home/jengelh # ip tunnel add .3 mode sit local 1.2.3.6 remote
> 5.6.7.7
> 19:44 borg:/home/jengelh # ip a
> 17: .3: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN 
>     link/sit 1.2.3.6 peer 5.6.7.7

OK you're right, but this seems to be an oversight. The names
are checked during rename, but not on device registration.

Re: netfilter/iptables and network interface names

[Jan Engelhardt]

On Friday 2010-02-05 11:27, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> On Thursday 2010-02-04 17:49, Patrick McHardy wrote:
>>> Jan Engelhardt wrote:
>>>>> The kernel also forbids ".".
>> 19:44 borg:/home/jengelh # ip tunnel add .3 mode sit local 1.2.3.6 remote
>> 5.6.7.7
>> 19:44 borg:/home/jengelh # ip a
>> 17: .3: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN 
>>     link/sit 1.2.3.6 peer 5.6.7.7
>
>OK you're right, but this seems to be an oversight. The names
>are checked during rename, but not on device registration.
>

So, what about the patch(es)? :-)

Re: netfilter/iptables and network interface names

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2010-02-04 15:31, Patrick McHardy wrote:
>> I don't think there is a reason for this limitation in iptables,
>> so why not simply remove it?
> 
> Like this?
> 
> parent 350661a6eb089f3e54e67e022db9e16ea280499f (v1.4.6-7-g350661a)
> commit 02020c8fb965bbedae9eb43d9a9b964c46388cc3
> Author: Jan Engelhardt <jengelh@medozas.de>
> Date:   Thu Feb 4 15:46:03 2010 +0100
> 
> Lift restrictions on interface names
> 
> The kernel has few restrictions.
> 
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>

OK this looks fine. But I guess we still need to properly escape
names in iptables-save as Thomas has pointed out.

> ---
>  xtables.c |   11 +++++------
>  1 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/xtables.c b/xtables.c
> index 63c5db7..f3baf84 100644
> --- a/xtables.c
> +++ b/xtables.c
> @@ -450,7 +450,7 @@ u_int16_t xtables_parse_port(const char *port, const char *proto)
>  void xtables_parse_interface(const char *arg, char *vianame,
>  			     unsigned char *mask)
>  {
> -	int vialen = strlen(arg);
> +	unsigned int vialen = strlen(arg);
>  	unsigned int i;
>  
>  	memset(mask, 0, IFNAMSIZ);
> @@ -462,7 +462,7 @@ void xtables_parse_interface(const char *arg, char *vianame,
>  			   " (%i)", arg, IFNAMSIZ-1);
>  
>  	strcpy(vianame, arg);
> -	if ((vialen == 0) || (vialen == 1 && vianame[0] == '+'))
> +	if (vialen == 0)
>  		memset(mask, 0, IFNAMSIZ);
>  	else if (vianame[vialen - 1] == '+') {
>  		memset(mask, 0xFF, vialen - 1);
> @@ -473,12 +473,11 @@ void xtables_parse_interface(const char *arg, char *vianame,
>  		memset(mask, 0xFF, vialen + 1);
>  		memset(mask + vialen + 1, 0, IFNAMSIZ - vialen - 1);
>  		for (i = 0; vianame[i]; i++) {
> -			if (vianame[i] == ':' ||
> -			    vianame[i] == '!' ||
> -			    vianame[i] == '*') {
> +			if (vianame[i] == '/' ||
> +			    vianame[i] == ' ') {
>  				fprintf(stderr,
>  					"Warning: weird character in interface"
> -					" `%s' (No aliases, :, ! or *).\n",
> +					" `%s' ('/' and ' ' are not allowed by the kernel).\n",
>  					vianame);
>  				break;
>  			}

Re: netfilter/iptables and network interface names

[Patrick McHardy]

Jan Engelhardt wrote:
> Lift restrictions on interface names
> 
> The kernel has few restrictions.

Applied, thanks.