netdev - Re: [RFC iproute2] tos: interpret ToS in natural numeral system

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 17 Feb 2022 12:18:08 +0100
From:   Guillaume Nault <gnault@...hat.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "dsahern@...il.com" <dsahern@...il.com>,
        "stephen@...workplumber.org" <stephen@...workplumber.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [RFC iproute2] tos: interpret ToS in natural numeral system

On Wed, Feb 16, 2022 at 10:44:19PM +0000, David Laight wrote:
> From: Guillaume Nault
> > Sent: 16 February 2022 22:24
> > 
> > On Wed, Feb 16, 2022 at 11:42:05AM -0800, Jakub Kicinski wrote:
> > > Silently forcing a base numeral system is very painful for users.
> > > ip currently interprets tos 10 as 0x10. Imagine user's bash script
> > > does:
> > >
> > >   .. tos $((TOS * 2)) ..
> > >
> > > or any numerical operation on the ToS.
> > >
> > > This patch breaks existing scripts if they expect 10 to be 0x10.
> > 
> > I agree that we shouldn't have forced base 16 in the first place.
> > But after so many years I find it a bit dangerous to change that.
> 
> Aren't the TOS values made up of several multi-bit fields and
> very likely to be documented in hex?

In theory, they are. But as far as the kernel is concerned, they're
just plain integers with no significance (apart from some constraints
preventing the use of some values). Users are free to choose their own
values.

> I'm not sure $((TOS * 2)) (or even + 2) makes any sense at all.
> 
> What it more horrid that that base 0 treats numbers that start
> with a 0 as octal - has anyone really used octal since the 1970s
> (except for file permissions).

Right, but that'd be consistent with the rest of iproute2, so users
should be aware of this trap at this point (or most likely, they never
prefix their values with 0). Anyway, I think we agreed that it's now
too late to modify the base.

> I have written command line parsers that treat 0tnnn as decimal
> while defaulting to hex.
> That does make it easier to use shell arithmetic for field (like
> addresses) that you would never normally specify in decimal.
> 
> > 
> > What about just printing a warning when the value isn't prefixed with
> > '0x'? Something like (completely untested):
> > 
> > @@ -535,6 +535,12 @@ int rtnl_dsfield_a2n(__u32 *id, const char *arg)
> >  	if (!end || end == arg || *end || res > 255)
> >  		return -1;
> >  	*id = res;
> > +
> > +	if (strncmp("0x", arg, 2))
> > +		fprintf(stderr,
> > +			"Warning: dsfield and tos parameters are interpreted as hexadecimal values\n"
> > +			"Use 'dsfield 0x%02x' to avoid this message\n", res);
> 
> Ugg.

Not nice, I agree. But what else can we do without breaking backward
compatibility?
This is similar to the warning we have when creating a new vxlan device
without specifying the destination port:

  # ip link add type vxlan vni 200 remote 2001:db8::1
  vxlan: destination port not specified
  Will use Linux kernel default (non-standard value)
  Use 'dstport 4789' to get the IANA assigned value
  Use 'dstport 0' to get default and quiet this message

> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>