[SCM] Samba Shared Repository - branch master updated
Douglas Bagnall
dbagnall at samba.org
Wed Jul 10 06:29:01 UTC 2024
The branch, master has been updated
via 86843685419 cmdline:burn: list commands to always burn; warn on unknown
via d2b119e34b4 cmdline: samba-tool test for bad option warning
from ef0068cd2cb vfs_ceph: Disable the module on unsupported Ceph versions
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 86843685419921e28c37f3c1b33011f14940e02f
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Jul 5 14:31:58 2024 +1200
cmdline:burn: list commands to always burn; warn on unknown
We burn arguments to all unknown options containing "pass" (e.g.
"--passionate=false") in case they are a password option, but is bad
in the case where the unknown option takes no argument but the next
option *is* a password (like "--overpass --password2 barney". In that
case "--password2" would be burnt and not "barney".
The burning behaviour doesn't change with this commit, but users will now
see an error message explaining that the option was unknown. This is not
so much aimed at end users -- for who an invalid option will hopefully
lead to --help like output -- but to developers who add a new "pass"
option.
This also slightly speeds up the processing of known password options,
which is a little bit important because we are in a race to replace the
command line in /proc before an attacker sees it.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15674
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jo Sutton <josutton at catalyst.net.nz>
Autobuild-User(master): Douglas Bagnall <dbagnall at samba.org>
Autobuild-Date(master): Wed Jul 10 06:28:08 UTC 2024 on atb-devel-224
commit d2b119e34b4e523a3bc6699e4d8a370bf8403d0b
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Jul 5 16:13:04 2024 +1200
cmdline: samba-tool test for bad option warning
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jo Sutton <josutton at catalyst.net.nz>
-----------------------------------------------------------------------
Summary of changes:
lib/cmdline/cmdline.c | 58 ++++++++++++++++++++++++++++++-----
python/samba/tests/samba_tool/help.py | 9 ++++++
2 files changed, 60 insertions(+), 7 deletions(-)
Changeset truncated at 500 lines:
diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index 0f453b6c8be..8efefc67a64 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -150,14 +150,36 @@ static bool strneq_cmdline_exact(const char *p, const char *option, size_t len)
}
/*
- * If an option looks like a password, and it isn't in the allow list, we
- * will burn it.
+ * Return true if the argument to the option should be redacted.
*
- * *ulen is set to zero if found is false (we don't need it in that case).
+ * The option name is presumed to contain the substring "pass". It is checked
+ * against a list of options that specify secrets. If it is there, the value
+ * should be redacted and we return early.
+ *
+ * Otherwise, it is checked against a list of known safe options. If it is
+ * there, we return false.
+ *
+ * If the option is not in either list, we assume it might be secret and
+ * redact the argument, but warn loadly about it. The hope is that developers
+ * will see what they're doing and add the option to the appropriate list.
+ *
+ * If true is returned, *ulen will be set to the apparent length of the
+ * option. It is set to zero if false is returned (we don't need it in that
+ * case).
*/
-static bool burn_possible_password(const char *p, size_t *ulen)
+static bool is_password_option(const char *p, size_t *ulen)
{
size_t i, len;
+ static const char *must_burn[] = {
+ "--password",
+ "--newpassword",
+ "--password2",
+ "--adminpass",
+ "--dnspass",
+ "--machinepass",
+ "--krbtgtpass",
+ "--fixed-password",
+ };
static const char *allowed[] = {
"--bad-password-count-reset",
"--badpassword-frequency",
@@ -179,9 +201,20 @@ static bool burn_possible_password(const char *p, size_t *ulen)
"--strip-passed-output",
"--with-smbpasswd-file",
};
+
char *equals = NULL;
*ulen = 0;
+ for (i = 0; i < ARRAY_SIZE(must_burn); i++) {
+ bool secret;
+ len = strlen(must_burn[i]);
+ secret = strneq_cmdline_exact(p, must_burn[i], len);
+ if (secret) {
+ *ulen = len;
+ return true;
+ }
+ }
+
for (i = 0; i < ARRAY_SIZE(allowed); i++) {
bool safe;
len = strlen(allowed[i]);
@@ -215,7 +248,18 @@ static bool burn_possible_password(const char *p, size_t *ulen)
}
*ulen = equals - p;
}
- DBG_NOTICE("burning command line argument %*s\n", (int)(*ulen), p);
+ /*
+ * This message will be seen with Python tools when an option
+ * is misspelt, but not with C tools, because in C burning
+ * happens after the command line is parsed, while in Python
+ * it happens before (on a copy of argv).
+ *
+ * In either case it will appear for a newly added option, and
+ * we hope developers will notice it before pushing.
+ */
+ DBG_ERR("\nNote for developers: if '%*s' is not misspelt, it should be "
+ "added to the appropriate list in is_password_option().\n\n",
+ (int)(*ulen), p);
return true;
}
@@ -266,11 +310,11 @@ bool samba_cmdline_burn(int argc, char *argv[])
} else if (strncmp(p, "--", 2) == 0 && strstr(p, "pass")) {
/*
* We have many secret options like --password,
- * --adminpass, --client-password, and we could easily
+ * --adminpass, --newpassword, and we could easily
* add more, so we will use an allowlist to let the
* safe ones through (of which there are also many).
*/
- found = burn_possible_password(p, &ulen);
+ found = is_password_option(p, &ulen);
}
if (found) {
diff --git a/python/samba/tests/samba_tool/help.py b/python/samba/tests/samba_tool/help.py
index fa7836d8432..16eb6b74c5d 100644
--- a/python/samba/tests/samba_tool/help.py
+++ b/python/samba/tests/samba_tool/help.py
@@ -79,3 +79,12 @@ class HelpTestCase(SambaToolCmdTest):
known_commands = new_commands
self.assertEqual(failed_commands, [])
+
+ def test_bad_password_option(self):
+ """Do we get a warning with an invalid --pass option?"""
+ (result, out, err) = self.run_command(["samba-tool",
+ "processes",
+ "--pass-the-salt-please",
+ "pleeease"])
+ self.assertIn("if '--pass-the-salt-please' is not misspelt", err)
+ self.assertIn("the appropriate list in is_password_option", err)
--
Samba Shared Repository
More information about the samba-cvs
mailing list