netdev - [PATCH iproute2-next v2 0/5] Change parsing in parse_one_of(), parse_on

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-next>] [day] [month] [year] [list]

Date: Wed, 22 Nov 2023 16:23:27 +0100
From: Petr Machata <petrm@...dia.com>
To: David Ahern <dsahern@...il.com>, Stephen Hemminger
	<stephen@...workplumber.org>, <netdev@...r.kernel.org>
CC: Patrisious Haddad <phaddad@...dia.com>, Petr Machata <petrm@...dia.com>
Subject: [PATCH iproute2-next v2 0/5] Change parsing in parse_one_of(), parse_on_off()

Library functions parse_one_of() and parse_on_off() were added about three
years ago to unify all the disparate reimplementations of the same basic
idea. It used the matches() function to determine whether a string under
consideration corresponds to one of the keywords. This reflected many,
though not all cases of on/off parsing at the time.

This decision has some odd consequences. In particular, "o" can be used as
a shorthand for "off", which is not obvious, because "o" is the prefix of
both. By sheer luck, the end result actually makes some sense: "on" means
on, anything else either means off or errors out. Similar issues are in
principle also possible for parse_one_of() uses, though currently this does
not come up.

Ideally parse_on_off() would accept the strings "on" and "off" and no
others.

Patch #1 is a cleanup. Patch #2 is shaping the code for the next patches.

Patch #3 converts parse_on_off() to strcmp(). See the commit message for
the rationale of why the change should be considered acceptable.

We'd ideally do parse_one_of() likewise. But the strings this function
parses tend to be longer, which means more opportunities for typos and more
of a reason to abbreviate things.

So instead, patch #4 adds a function parse_one_of_deprecated() for ip
macsec to use in one place, where these typos are to be expected, and
converts that site to the new function.

Then patch #5 changes the behavior of parse_one_of() to accept prefixes
like it has so far, but to warn that they are deprecated:

    # dcb ets set dev swp1 tc-tsa 0:s
    WARNING: 's' matches 'strict' by prefix.
    Matching by prefix is deprecated in this context, please use the full string.

The idea is that several releases down the line, we might consider
switching over to strcmp(), as presumably enough advance warning will have
been given.

v2:
- Ditch parse_on_off_deprecated() and just use strcmp outright.
- Only use parse_one_of_deprecated() for one call site. Change
  parse_one_of() to warn on prefix matches.

Petr Machata (5):
  lib: utils: Switch matches() to returning int again
  lib: utils: Generalize parse_one_of()
  lib: utils: Convert parse_on_off() to strcmp()
  lib: utils: Introduce parse_one_of_deprecated()
  lib: utils: Have parse_one_of() warn about prefix matches

 include/utils.h |  5 ++++-
 ip/ipmacsec.c   |  6 ++++--
 lib/utils.c     | 49 +++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 49 insertions(+), 11 deletions(-)

-- 
2.41.0