Skip to content

try harder to burn commandline passwords (and some things about vendor strings)

I started out trying to fix a few things that were raised on the mailing list.

  1. our --vendor--* version options are a bit clumsy to use
  2. version strings containing ~ don't work
  3. long version names are truncated in man pages.
  4. there's a strange warning about passwords when you use -UAdministrator without a password.

But it turned out that our secret burning code, which triggered the strange warning needed more attention.

cmdline_burn() in lib/cmdline/cmdline.c is used executables to redact secrets from command line strings in /proc (thence, ps, top, etc). It is used by Python as well as C, to reduce logical duplication, but Python only actually redacts if the setproctitle Python module is available. It only tries to do this if cmdline_burn() says it redacted something, but cmdline_burn() was lying in the case of -Uxxx which contains no secret (unlike -Uxxx%yyy).

Looking at cmdline_burn() revealed various other problems. It redacted --password=secret-007, but not --password secret-007, but it did redact --password abc123 by the accident of the password not being the same length as --password. It also would not redact many secret arguments used in samba-tool, like --adminpass in domain provision. On the other hand, it might redact arguments to --user-allowed-to-authenticate-to which are not secret.

Now it will still not properly redact --password --password secret-007, because as it steps through the arguments it will redact the second --password and not process it, leaving the secret-007 in the clear. We hope however that normal argument handling will reject this construction so the process will be short lived.

There are like 4 different bug numbers, which I kind of regret, yet there are more than that many bugs.

Checklist

  • Commits have Signed-off-by: with name/author being identical to the commit author
  • (optional) This MR is just one part towards a larger feature.
  • (optional, if backport required) Bugzilla bug filed and BUG: tag added
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated
  • CI timeout is 3h or higher (see Settings/CICD/General pipelines/ Timeout)

Reviewer's checklist:

  • There is a test suite reasonably covering new functionality or modifications
  • Function naming, parameters, return values, types, etc., are consistent and according to README.Coding.md
  • This feature/change has adequate documentation added
  • No obvious mistakes in the code

Merge request reports