| Message ID | alpine.LSU.2.01.1002041542150.13435@obet.zrqbmnf.qr |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
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 ".". -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; > } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)? :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Engelhardt wrote: > Lift restrictions on interface names > > The kernel has few restrictions. Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }