Follow-up on "Audit tca-portal"

refs #18374 (closed) and particularly jvoisin's audit and converting it into a checklist:

• The validate_args method of BaseCommand's docstring should be "Validate and update" instead of "Validate of update" I think.

• gnome_env_vars should have some comments explaining what it's doing/returning, because it looks a bit scary: should an attacker be able to influence it?

  • status: it received some love, more improvements will be in safely get gnome_env_vars (!819 - merged)

• I would anchor self.re in SetTimeCommand with ^ and $, even if fullmatch is used.

  • Is args[0] allowed to be empty? Currently, it is.
  • If not, is there a minimum length for it?
  • Would it make sense to use something like ^[0-9]{4}-[0-9][0-9]-[0-9][0-9]T[0-9][0-9][0-9]:[0-9][0-9]\+[0-9][0-9]:[0-9][0-9]$ for the validating regex, instead of the current [0-9+:T-]* one?
  • this was moved to a dedicated sub-task in Audit tca-portal (#18374 - closed)

• In the Handler class, I would use tuple instead of list, since the former can't be modified, while the latter can, to indicate that the commands are constant. In fact, I would make use of tuple as much as possible everywhere.

• Some commands are calling binaries using an absolute path, other are relying on PATH. Making use of absolute paths as much as possible is a good practice

• In the Handler class, the handle_line as a docstring saying Parse the line and do what's appropriate.: what is meant by this? It's unclear.

• In the run method of the Handler class, maybe add a comment or something like _, _ = process.communicate() to make it explicit that we don't care about stdout nor stderr of the called process.

• Maybe make use of https://docs.python.org/3/library/argparse.html#argparse.ArgumentParser.add_mutually_exclusive_group for --systemd-socket and --listen?

• The following code is a bit scary:

if os.path.exists(args.listen) and stat.S_ISSOCK(os.stat(args.listen).st_mode):
  os.remove(args.listen)

it looks like a TOCTOU issue waiting to happen (see CVE-2019-7307. I would recommend checking that args.listen is owned by root, and bail out if it isn't.

• check that PersistenceSetupCommand still works after it is not receiving gnome_env_vars anymore (bebcf652)