[SCM] Samba Shared Repository - branch master updated
Douglas Bagnall
dbagnall at samba.org
Wed Jul 3 02:36:02 UTC 2024
The branch, master has been updated
via 95420715881 pidl:Wireshark Fix the type of array of pointerse to hf_ values
via 4a050601322 pidl:Wireshark Rename tvb_new_subset()
via 39f05512626 pidl:Wireshark Get rid of Boolean "flags" with no bit set
via b8d902df037 pidl:Wireshark Fix array of pointers NULL termination
via 3c97ad41c3b pidl:Wireshark Use proto_tree_add_bitmask_with_flags
via 97be45f9ea3 cmdline:burn: add a note about short option combinations
via 63a83fb7bb3 cmdline:burn: explicitly burn --username
via f1fbba6dc60 cmdline:burn: use allowlist to ensure more passwords burn
via c4df89e9640 cmdline: test_cmdline tests more burning
via 6effed31899 cmdline:burn: do not burn options starting --user-*, --password-*
via f5233ddf974 cmdline:burn: localise some variables
via d3d8dffc021 cmdline:burn: always return true if burnt
via 53a11845252 cmdline:burn: handle arguments separated from their --options
via 2f6020cf3da cmdline:burn: do not retain false memories
via 05128a1f5f1 cmdline:tests: extend cmdline_burn tests
via f17a2b1b25f selftest: run the cmdline tests that we already have
via f3b240da5c2 cmdline:burn: '-U' does not imply secrets without '%'
via 7fb38aee129 docs-xml:manpages: allow for longer version strings
via 673c8e6ca59 build: --vendor-suffix instead of --vendor-patch-revision --vendor-name
via 0bc5b6f2930 buildtools: sanitise strange characters in vendor strings
from 056dd415dda ctdb-failover: omit "restrict" optimization keyword
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 95420715881337187bda2311ced667a704fab737
Author: John Thacker <johnthacker at gmail.com>
Date: Tue Jun 25 18:40:20 2024 -0400
pidl:Wireshark Fix the type of array of pointerse to hf_ values
Picked from Wireshark's fork:
commit e1d9a226a2b8f2824a0eb162a8dc972e6e6c2dd4
Author: Guy Harris <gharris at sonic.net>
Date: Thu Jun 18 18:14:46 2020 -0700
Fix the type of arrays of pointers to hf_ values for bitfield routines.
The static arrays are supposed to be arrays of const pointers to int,
not arrays of non-const pointers to const int.
Fixing that means some bugs (scribbling on what's *supposed* to be a
const array) will be caught (see packet-ieee80211-radiotap.c for
examples, the first of which inspired this change and the second of
which was discovered while testing compiles with this change), and
removes the need for some annoying casts.
Also make some of those arrays static while we're at it.
Update documentation and dissector-generator tools.
Change-Id: I789da5fc60aadc15797cefecfd9a9fbe9a130ccc
Reviewed-on: https://code.wireshark.org/review/37517
Petri-Dish: Guy Harris <gharris at sonic.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58 at gmail.com>
Signed-off-by: John Thacker <johnthacker at gmail.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
Autobuild-User(master): Douglas Bagnall <dbagnall at samba.org>
Autobuild-Date(master): Wed Jul 3 02:35:43 UTC 2024 on atb-devel-224
commit 4a050601322f51fb66c90fa8c457924838430158
Author: John Thacker <johnthacker at gmail.com>
Date: Tue Jun 25 18:38:43 2024 -0400
pidl:Wireshark Rename tvb_new_subset()
Picked from Wireshark's fork:
commit 7cd6906056922e4b8f68f1216d94eaa0809896fe
Author: Guy Harris <guy at alum.mit.edu>
Date: Mon Jan 9 22:18:49 2017 -0800
Rename tvb_new_subset() to tvb_new_subset_length_caplen().
This emphasizes that there is no such thing as *the* routine to
construct a subset tvbuff; you need to choose one of
tvb_new_subset_remaining() (if you want a new tvbuff that contains
everything past a certain point in an existing tvbuff),
tvb_new_subset_length() (if you want a subset that contains everything
past a certain point, for some number of bytes, in an existing tvbuff),
and tvb_new_subset_length_caplen() (for all other cases).
Many of the calls to tvb_new_subset_length_caplen() should really be
calling one of the other routines; that's the next step. (This also
makes it easier to find the calls that need fixing.)
Change-Id: Ieb3d676d8cda535451c119487d7cd3b559221f2b
Reviewed-on: https://code.wireshark.org/review/19597
Reviewed-by: Guy Harris <guy at alum.mit.edu>
Signed-off-by: John Thacker <johnthacker at gmail.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 39f0551262671a6217cd0844437b9776b048578a
Author: John Thacker <johnthacker at gmail.com>
Date: Tue Jun 25 18:37:28 2024 -0400
pidl:Wireshark Get rid of Boolean "flags" with no bit set
Picked from Wireshark's fork:
commit 5ae9af9e50c89d8700e5a166a9c345fc46b3a4ca
Author: Guy Harris <guy at alum.mit.edu>
Date: Sun Nov 6 11:02:51 2016 -0800
Get rid of Boolean "flags" that don't have any bit set.
And tweak the Pidl generator for Wireshark not to generate "flags" like
that.
(The generator also does field name and true/false strings' case
differently, so I didn't use it to regenerate the dissectors; that needs
to be looked at.)
Change-Id: Ie1657a782ebdb107e58792cedd29bbaa79b17bd4
Reviewed-on: https://code.wireshark.org/review/18695
Reviewed-by: Guy Harris <guy at alum.mit.edu>
Signed-off-by: John Thacker <johnthacker at gmail.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit b8d902df0379f5a230743ddf63f40bbe78d915ae
Author: John Thacker <johnthacker at gmail.com>
Date: Tue Jun 25 18:31:39 2024 -0400
pidl:Wireshark Fix array of pointers NULL termination
Picked from Wireshark's PIDL fork:
commit c07fd447c362099b04eedb445e8fa469643403f7
Author: Binh Trinh <beango at gmail.com>
Date: Fri Jun 17 21:46:11 2016 -0400
DCE/RPC: fix array of pointers with NULL
Change-Id: Ie89f8fd4ec744d427d41866206d5a6784c5b224f
Reviewed-on: https://code.wireshark.org/review/16004
Petri-Dish: Jaap Keuter <jaap.keuter at xs4all.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply at wireshark.org>
Reviewed-by: Michael Mann <mmann78 at netscape.net>
Signed-off-by: John Thacker <johnthacker at gmail.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 3c97ad41c3ba440e7bb6bf7f5132aeb9032f08e9
Author: John Thacker <johnthacker at gmail.com>
Date: Tue Jun 25 18:30:22 2024 -0400
pidl:Wireshark Use proto_tree_add_bitmask_with_flags
Picked from the Wireshark fork:
commit 9a5e6a6884b2369527638fecd49c4c58d8c10378
Author: Michael Mann <mmann78 at netscape.net>
Date: Wed Jun 15 15:35:51 2016 -0400
DCE/RPC proto_tree_add_boolean -> proto_tree_add_bitmask_with_flags
Change-Id: I8891ec90244ffd9609d8443df631a7c8e6453b7e
Reviewed-on: https://code.wireshark.org/review/15942
Petri-Dish: Michael Mann <mmann78 at netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply at wireshark.org>
Reviewed-by: Michael Mann <mmann78 at netscape.net>
Signed-off-by: John Thacker <johnthacker at gmail.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 97be45f9ea3410392cd37eab5cfafd3ad00cfe57
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Jul 3 11:50:43 2024 +1200
cmdline:burn: add a note about short option combinations
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>
commit 63a83fb7bb312731047f361f89766e0be492f83e
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Jul 3 11:23:36 2024 +1200
cmdline:burn: explicitly burn --username
This is the long form of -U in samba-tool.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jo Sutton <josutton at catalyst.net.nz>
commit f1fbba6dc609590854c0d7c5e72b58fabc356695
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Sat Jun 29 13:44:46 2024 +1200
cmdline:burn: use allowlist to ensure more passwords burn
We treat any option containing 'pass' with suspicion, unless we know it
is OK.
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>
commit c4df89e9640c1306aa390cdacaa974c870c3f5bb
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Sat Jun 29 13:43:03 2024 +1200
cmdline: test_cmdline tests more burning
We have more secret arguments, like --client-password, --adminpass,
so we are going to use an allowlist for options containing 'pass', but
we don't want to burn the likes of --group=passionfruit.
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>
commit 6effed31899a1be8194a851e5a4023276b8a5f38
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Sat Jun 29 11:30:19 2024 +1200
cmdline:burn: do not burn options starting --user-*, --password-*
We have options that start with --user or --password that we don't
want to burn. Some grepping says:
2 --user1
1 --user2
10 --user-allowed-to-authenticate-from
6 --user-allowed-to-authenticate-to
2 --user-allow-ntlm-auth
25 --user-authentication-policy
1 --user-config
4 --user-domgroups
5 --user-ext-name
2 --user-groups
6 --user-info
27 --username
1 --username2
2 --userou
1 --users
2 --user-sidinfo
6 --user-sids
14 --user-tgt-lifetime-mins
2 --password2
118 --password-file
2 --password-from-stdin
# from here, grepping for strings around POPT_ constants
5 "user"
2 "user1"
2 "user2"
1 "userd"
1 "user-domgroups"
1 "user-groups"
1 "user-info"
2 "username"
1 "user-sidinfo"
1 "user-sids"
1 passwordd
4 "password"
Not all of these use lib/cmdline, but I think most do, via Python
which defers to cmdline_burn().
Note that there are options we should burn that aren't on this list,
like --adminpass. That's another matter.
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>
commit f5233ddf974f9649d8a12b151b6843412eab489c
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Jun 27 16:33:16 2024 +1200
cmdline:burn: localise some variables
As this function increases in complexity, it helps to keep things close.
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>
commit d3d8dffc0212662456a6251baee5afd432160fa2
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Jun 27 16:03:30 2024 +1200
cmdline:burn: always return true if burnt
Before we have been trying to cram three cases into a boolean return
value:
* cmdline had secrets, we burnt them -> true
* cmdline had no secrets, all good -> false
* cmdline has NULL string, WTF! emergency! -> false
This return value is only used by Python which wants to know whether to
go to the trouble of replacing the command line. If samba_cmdline_burn()
returns false, no action is taken.
If samba_cmdline_burn() burns a password and then hits a NULL, it would
be better not to do nothing. It would be better to crash. And that is
what Python will end up doing, by some talloc returning NULL triggering
a MemoryError.
What about the case like {"--foo", NULL, "-Ua%b"} where the secret comes
after the NULL? That will still be ignored by Python, as it is by all C
tools, but we are hoping that can't happen anyway.
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>
commit 53a1184525279741e116350a9b53da15cb2f41d0
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Jun 27 15:40:16 2024 +1200
cmdline:burn: handle arguments separated from their --options
We weren't treating "--password secret" the same as "--password=secret",
which sometimes led to secrets not being redacted.
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>
commit 2f6020cf3dadf484251701040e09a10fba2f644e
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Jun 27 15:20:27 2024 +1200
cmdline:burn: do not retain false memories
If argv contains a secret option without an '=' (or in the case of
"-U", the username is separated by space), we will get to the
`if (strlen(p) == ulen) { continue; }` without resetting the found
and is_user variables. This *sometimes* has the right effect, because
the next string in argv ought to contain the secret.
But in a case like {"--password", "1234567890"}, where the secret
string is the same length as the option, we *again* take that branch
and the password is not redacted, though the argument after it will be
unless it is also of the same length.
If we always set the flags at the start we avoid this. This makes
things worse in the short term for secrets that are not the same
length as their options, but we'll get to that in another commit soon.
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>
commit 05128a1f5f17c55a8d8da42c6c52c4235adf36d4
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Jun 27 15:05:03 2024 +1200
cmdline:tests: extend cmdline_burn tests
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>
commit f17a2b1b25f2ffa5e3caeb8f81101e66b843cc29
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Jun 27 17:04:47 2024 +1200
selftest: run the cmdline tests that we already have
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>
commit f3b240da5c209a51fa43de23e8ecfea2f32bbfd5
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Jun 21 09:21:43 2024 +1200
cmdline:burn: '-U' does not imply secrets without '%'
We return true from this function when a secret has been erased,
and were accidentally treating as if it had secrets.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15671
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jo Sutton <josutton at catalyst.net.nz>
commit 7fb38aee129789cce28ddf54bd7234f8c5f57d97
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Jun 21 11:29:36 2024 +1200
docs-xml:manpages: allow for longer version strings
The default value (30) truncates "Samba 4.21.0pre1-DEVELOPERBUILD" to
"Samba 4.21.0pre1-DEVELOPE" in the bottom left corner of the man page.
("Samba 4.21.0pre1-DEVELOPE" is only 25 bytes long, not 30, but let's
not worry about that).
On narrow terminals (< ~75 columns) this makes it more likely that
the version string will run into the date string.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15672
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jo Sutton <josutton at catalyst.net.nz>
commit 673c8e6ca5994973e4887641c3599707a66a608c
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Jun 21 10:40:59 2024 +1200
build: --vendor-suffix instead of --vendor-patch-revision --vendor-name
In practice there isn't a use for two options, and neither quite
matched what people thought they were doing.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15673
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jo Sutton <josutton at catalyst.net.nz>
commit 0bc5b6f29307ce758774c1b2f48ce62315fdc7f9
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Jun 21 10:38:40 2024 +1200
buildtools: sanitise strange characters in vendor strings
There is no reason to think '-' and '+' are the only characters that
might sneak into a vendor string; Debian habitually use '~'.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15673
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jo Sutton <josutton at catalyst.net.nz>
-----------------------------------------------------------------------
Summary of changes:
buildtools/wafsamba/samba_abi.py | 6 +-
docs-xml/xslt/man.xsl | 3 +
lib/cmdline/cmdline.c | 173 ++++++++++++++++++++++++++++++-----
lib/cmdline/tests/test_cmdline.c | 54 +++++++++--
pidl/lib/Parse/Pidl/Wireshark/NDR.pm | 97 ++++++++++++--------
script/autobuild.py | 4 +-
selftest/tests.py | 2 +
wscript | 12 +--
8 files changed, 267 insertions(+), 84 deletions(-)
Changeset truncated at 500 lines:
diff --git a/buildtools/wafsamba/samba_abi.py b/buildtools/wafsamba/samba_abi.py
index 2d9505d255c..c82ba3424f9 100644
--- a/buildtools/wafsamba/samba_abi.py
+++ b/buildtools/wafsamba/samba_abi.py
@@ -286,7 +286,7 @@ def abi_build_vscript(task):
f.close()
def VSCRIPT_MAP_PRIVATE(bld, libname, orig_vscript, version, private_vscript):
- version = version.replace("-", "_").replace("+","_").upper()
+ version = re.sub(r'\W', '_', version).upper()
t = bld.SAMBA_GENERATOR(private_vscript,
rule=abi_build_vscript,
source=orig_vscript,
@@ -314,8 +314,8 @@ def ABI_VSCRIPT(bld, libname, abi_directory, version, vscript, abi_match=None, p
libname = os.path.basename(libname)
version = os.path.basename(version)
- libname = libname.replace("-", "_").replace("+","_").upper()
- version = version.replace("-", "_").replace("+","_").upper()
+ libname = re.sub(r'\W', '_', libname).upper()
+ version = re.sub(r'\W', '_', version).upper()
t = bld.SAMBA_GENERATOR(vscript,
rule=abi_build_vscript,
diff --git a/docs-xml/xslt/man.xsl b/docs-xml/xslt/man.xsl
index e252b56d5e5..a1870079ba6 100644
--- a/docs-xml/xslt/man.xsl
+++ b/docs-xml/xslt/man.xsl
@@ -11,6 +11,9 @@
<xsl:param name="use.id.as.filename" select="1"/>
<xsl:param name="man.endnotes.are.numbered" select="0"/>
+<!-- make room for long version numbers -->
+<xsl:param name="man.th.extra2.max.length">40</xsl:param>
+
<!--
Our ulink stylesheet omits @url part if content was specified
-->
diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index 93d59d4f9d3..0f453b6c8be 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -135,63 +135,186 @@ void samba_cmdline_set_machine_account_fn(
cli_credentials_set_machine_account_fn = fn;
}
+/*
+ * Are the strings p and option equal from the point of view of option
+ * parsing, meaning is the next character '\0' or '='.
+ */
+static bool strneq_cmdline_exact(const char *p, const char *option, size_t len)
+{
+ if (strncmp(p, option, len) == 0) {
+ if (p[len] == 0 || p[len] == '=') {
+ return true;
+ }
+ }
+ return false;
+}
+
+/*
+ * If an option looks like a password, and it isn't in the allow list, we
+ * will burn it.
+ *
+ * *ulen is set to zero if found is false (we don't need it in that case).
+ */
+static bool burn_possible_password(const char *p, size_t *ulen)
+{
+ size_t i, len;
+ static const char *allowed[] = {
+ "--bad-password-count-reset",
+ "--badpassword-frequency",
+ "--change-user-password",
+ "--force-initialized-passwords",
+ "--machine-pass", /* distinct from --machinepass */
+ "--managed-password-interval",
+ "--no-pass",
+ "--no-pass2",
+ "--no-passthrough",
+ "--no-password",
+ "--passcmd",
+ "--passwd",
+ "--passwd_path",
+ "--password-file",
+ "--password-from-stdin",
+ "--random-password",
+ "--smbpasswd-style",
+ "--strip-passed-output",
+ "--with-smbpasswd-file",
+ };
+ char *equals = NULL;
+ *ulen = 0;
+
+ for (i = 0; i < ARRAY_SIZE(allowed); i++) {
+ bool safe;
+ len = strlen(allowed[i]);
+ safe = strneq_cmdline_exact(p, allowed[i], len);
+ if (safe) {
+ return false;
+ }
+ }
+ /*
+ * We have found a suspicious option, and we need to work out where to
+ * burn it from. It could be
+ *
+ * --secret-password=cow -> password after '='
+ * --secret-password -> password is in next argument.
+ *
+ * but we also have the possibility of
+ *
+ * --cow=secret-password
+ *
+ * that is, the 'pass' in this option string is not in the option but
+ * the argument to it, which should not be burnt.
+ */
+ equals = strchr(p, '=');
+ if (equals == NULL) {
+ *ulen = strlen(p);
+ } else {
+ char *pass = (strstr(p, "pass"));
+ if (pass > equals) {
+ /* this is --foo=pass, not --pass=foo */
+ return false;
+ }
+ *ulen = equals - p;
+ }
+ DBG_NOTICE("burning command line argument %*s\n", (int)(*ulen), p);
+ return true;
+}
+
bool samba_cmdline_burn(int argc, char *argv[])
{
bool burnt = false;
- bool found = false;
- bool is_user = false;
- char *p = NULL;
int i;
- size_t ulen = 0;
for (i = 0; i < argc; i++) {
+ bool found = false;
+ bool is_user = false;
+ size_t ulen = 0;
+ char *p = NULL;
+
p = argv[i];
if (p == NULL) {
- return false;
+ return burnt;
}
- /*
- * Take care that this list must be in longest-match
- * first order
- */
if (strncmp(p, "-U", 2) == 0) {
+ /*
+ * Note: this won't catch combinations of
+ * short options like
+ * `samba-tool -NUAdministrator%...`, which is
+ * not possible in general outside of the
+ * actual parser (consider for example
+ * `-NHUroot%password`, which parses as
+ * `-N -H 'Uroot%password'`). We don't know
+ * here which short options might take
+ * arguments.
+ *
+ * This is an argument for embedding redaction
+ * inside the parser (e.g. by adding a flag to
+ * the option definitions), but we decided not
+ * to do that in order to share cmdline_burn().
+ */
ulen = 2;
found = true;
is_user = true;
- } else if (strncmp(p, "--user", 6) == 0) {
+ } else if (strneq_cmdline_exact(p, "--user", 6)) {
ulen = 6;
found = true;
is_user = true;
- } else if (strncmp(p, "--password2", 11) == 0) {
- ulen = 11;
- found = true;
- } else if (strncmp(p, "--password", 10) == 0) {
+ } else if (strneq_cmdline_exact(p, "--username", 10)) {
ulen = 10;
found = true;
- } else if (strncmp(p, "--newpassword", 13) == 0) {
- ulen = 13;
- found = true;
+ is_user = true;
+ } else if (strncmp(p, "--", 2) == 0 && strstr(p, "pass")) {
+ /*
+ * We have many secret options like --password,
+ * --adminpass, --client-password, 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);
}
if (found) {
- char *q = NULL;
-
if (strlen(p) == ulen) {
- continue;
+ /*
+ * The option string has no '=', so
+ * its argument will come in the NEXT
+ * argv member. If there is one, we
+ * can just step forward and take it,
+ * setting ulen to 0.
+ *
+ * {"--password=secret"} --> {"--password"}
+ * {"--password", "secret"} --> {"--password", ""}
+ * {"-Uadmin%secret"} --> {"-Uadmin"}
+ * {"-U", "admin%secret"} --> {"-U", "admin"}
+ */
+ i++;
+ if (i == argc) {
+ /*
+ * this looks like an invalid
+ * command line, but that's
+ * for the caller to decide.
+ */
+ return burnt;
+ }
+ p = argv[i];
+ if (p == NULL) {
+ return burnt;
+ }
+ ulen = 0;
}
if (is_user) {
- q = strchr_m(p, '%');
- if (q != NULL) {
- p = q;
+ char *q = strchr_m(p, '%');
+ if (q == NULL) {
+ /* -U without '%' has no secret */
+ continue;
}
+ p = q;
} else {
p += ulen;
}
memset_s(p, strlen(p), '\0', strlen(p));
- found = false;
- is_user = false;
burnt = true;
}
}
diff --git a/lib/cmdline/tests/test_cmdline.c b/lib/cmdline/tests/test_cmdline.c
index 16dd09c63fa..f9733546288 100644
--- a/lib/cmdline/tests/test_cmdline.c
+++ b/lib/cmdline/tests/test_cmdline.c
@@ -24,6 +24,7 @@
#include <cmocka.h>
#include <time.h>
#include <sys/time.h>
+#include "replace.h"
#include "lib/cmdline/cmdline.h"
@@ -61,20 +62,59 @@ static void torture_cmdline_sanity_check_bad(void **state)
static void torture_cmdline_burn(void **state)
{
+ /* arg1 would require -U' Administrator%secret' */
char arg1[] = "-U Administrator%secret";
- char arg2[] = "--user=Administrator%secret";
- char arg3[] = "--user=Administrator%super%secret";
- char arg4[] = "--password=super%secret";
+ char arg2[] = "--no-no-no-not-secret=not%secret";
+ char arg3[] = "--user=Administrator%secret";
+ char arg4[] = "--user=Administrator%super%secret";
+ char arg5[] = "--password=super%secret";
+ char arg6[] = "--no-no-no-not-secret=not%secret";
+ char arg7[] = "-U";
+ char arg8[] = "fish%chips";
+ char arg9[] = "--password";
+ char arg10[] = "fish%chips";
+ char arg11[] = "--password2";
+ char arg12[] = "fish%chips";
+ char arg13[] = "--username=Admonisher % secretest";
+ /*
+ * The next two are not used in samba (--client-password
+ * appears in a Heimdal script that won't use lib/cmdline even
+ * if built) and are burnt by virtue of not being in the allow
+ * list.
+ */
+ char arg14[] = "--client-password=bean stew";
+ char arg15[] = "--enpassant="; /* like --enpassant='', no effect on affect next arg */
+ char arg16[] = "bean";
+ char arg17[] = "--bean=password";
+ char arg18[] = "--name";
+ char arg19[] = "Compass Alompass";
- char *argv[] = { arg1, arg2, arg3, arg4, NULL };
- int argc = 4;
+ char *argv[] = { arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8,
+ arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17,
+ arg18, arg19, NULL };
+ int argc = ARRAY_SIZE(argv) - 1;
samba_cmdline_burn(argc, argv);
assert_string_equal(arg1, "-U Administrator");
- assert_string_equal(arg2, "--user=Administrator");
+ assert_string_equal(arg2, "--no-no-no-not-secret=not%secret");
assert_string_equal(arg3, "--user=Administrator");
- assert_string_equal(arg4, "--password");
+ assert_string_equal(arg4, "--user=Administrator");
+ assert_string_equal(arg5, "--password");
+ assert_string_equal(arg6, "--no-no-no-not-secret=not%secret");
+ assert_string_equal(arg7, "-U");
+ assert_string_equal(arg8, "fish");
+ assert_string_equal(arg9, "--password");
+ assert_string_equal(arg10, "");
+ assert_string_equal(arg11, "--password2");
+ assert_string_equal(arg12, "");
+ assert_string_equal(arg13, "--username=Admonisher ");
+ assert_string_equal(arg14, "--client-password");
+ assert_string_equal(arg15, "--enpassant");
+ assert_string_equal(arg16, "bean");
+ assert_string_equal(arg17, "--bean=password");
+ assert_string_equal(arg18, "--name");
+ assert_string_equal(arg19, "Compass Alompass");
}
int main(int argc, char *argv[])
diff --git a/pidl/lib/Parse/Pidl/Wireshark/NDR.pm b/pidl/lib/Parse/Pidl/Wireshark/NDR.pm
index 23e8a0f0820..05ef8b78554 100644
--- a/pidl/lib/Parse/Pidl/Wireshark/NDR.pm
+++ b/pidl/lib/Parse/Pidl/Wireshark/NDR.pm
@@ -201,6 +201,8 @@ sub Bitmap($$$$)
{
my ($self,$e,$name,$ifname) = @_;
my $dissectorname = "$ifname\_dissect\_bitmap\_".StripPrefixes($name, $self->{conformance}->{strip_prefixes});
+ my $element_count = 0;
+ my $total_ev = 0;
$self->register_ett("ett_$ifname\_$name");
@@ -211,9 +213,29 @@ sub Bitmap($$$$)
$self->pidl_code("$dissectorname(tvbuff_t *tvb _U_, int offset _U_, packet_info *pinfo _U_, proto_tree *parent_tree _U_, dcerpc_info* di _U_, guint8 *drep _U_, int hf_index _U_, guint32 param _U_)");
$self->pidl_code("{");
$self->indent;
- $self->pidl_code("proto_item *item = NULL;");
- $self->pidl_code("proto_tree *tree = NULL;");
- $self->pidl_code("");
+ foreach (@{$e->{ELEMENTS}}) {
+ next unless (/([^ ]*) (.*)/);
+ $element_count++;
+ }
+ if ($element_count > 0) {
+ $self->pidl_code("proto_item *item;");
+ $self->pidl_code("static int * const $ifname\_$name\_fields[] = {");
+ $self->indent;
+ foreach (@{$e->{ELEMENTS}}) {
+ next unless (/([^ ]*) (.*)/);
+ my ($en,$ev) = ($1,$2);
+ my $hf_bitname = "hf_$ifname\_$name\_$1";
+
+ $ev =~ s/[()\s]//g;
+ if (hex($ev) != 0) {
+ $total_ev += hex($ev);
+ $self->pidl_code("&$hf_bitname,");
+ }
+ }
+ $self->pidl_code("NULL");
+ $self->deindent;
+ $self->pidl_code("};");
+ }
$self->pidl_code("g$e->{BASE_TYPE} flags;");
if ($e->{ALIGN} > 1) {
@@ -222,18 +244,23 @@ sub Bitmap($$$$)
$self->pidl_code("");
- $self->pidl_code("if (parent_tree) {");
- $self->indent;
- $self->pidl_code("item = proto_tree_add_item(parent_tree, hf_index, tvb, offset, $e->{ALIGN}, DREP_ENC_INTEGER(drep));");
- $self->pidl_code("tree = proto_item_add_subtree(item,ett_$ifname\_$name);");
- $self->deindent;
- $self->pidl_code("}\n");
+ if ($element_count > 0) {
+ $self->pidl_code("item = proto_tree_add_bitmask_with_flags(parent_tree, tvb, offset, hf_index,");
+ $self->pidl_code("\t\t\tett_$ifname\_$name, $ifname\_$name\_fields, DREP_ENC_INTEGER(drep), BMT_NO_FALSE);");
+ $self->pidl_code("");
- $self->pidl_code("offset = dissect_ndr_$e->{BASE_TYPE}(tvb, offset, pinfo, tree, di, drep, -1, &flags);");
+ $self->pidl_code("offset = dissect_ndr_$e->{BASE_TYPE}(tvb, offset, pinfo, parent_tree, di, drep, -1, &flags);");
+ $self->pidl_code("");
+
+ $self->pidl_code("if (!flags)");
+ $self->pidl_code("\tproto_item_append_text(item, \": (No values set)\");\n");
+ } else {
+ $self->pidl_code("proto_tree_add_item(parent_tree, hf_index, tvb, offset, $e->{ALIGN}, DREP_ENC_INTEGER(drep));");
+ $self->pidl_code("");
- $self->pidl_code("proto_item_append_text(item, \": \");\n");
- $self->pidl_code("if (!flags)");
- $self->pidl_code("\tproto_item_append_text(item, \"(No values set)\");\n");
+ $self->pidl_code("offset = dissect_ndr_$e->{BASE_TYPE}(tvb, offset, pinfo, parent_tree, di, drep, -1, &flags);");
+ $self->pidl_code("");
+ }
foreach (@{$e->{ELEMENTS}}) {
next unless (/([^ ]*) (.*)/);
@@ -243,32 +270,30 @@ sub Bitmap($$$$)
$self->{hf_used}->{$hf_bitname} = 1;
- $self->register_hf_field($hf_bitname, field2name($en), $filtername, "FT_BOOLEAN", $e->{ALIGN} * 8, "TFS(&$name\_$en\_tfs)", $ev, "");
+ $ev =~ s/[()\s]//g;
+ if (hex($ev) != 0) {
+ $self->register_hf_field($hf_bitname, field2name($en), $filtername, "FT_BOOLEAN", $e->{ALIGN} * 8, "TFS(&$name\_$en\_tfs)", "( $ev )", "");
- $self->pidl_def("static const true_false_string $name\_$en\_tfs = {");
- if (defined($self->{conformance}->{tfs}->{$hf_bitname})) {
- $self->pidl_def(" $self->{conformance}->{tfs}->{$hf_bitname}->{TRUE_STRING},");
- $self->pidl_def(" $self->{conformance}->{tfs}->{$hf_bitname}->{FALSE_STRING},");
- $self->{conformance}->{tfs}->{$hf_bitname}->{USED} = 1;
- } else {
- $self->pidl_def(" \"$en is SET\",");
- $self->pidl_def(" \"$en is NOT SET\",");
+ $self->pidl_def("static const true_false_string $name\_$en\_tfs = {");
+ if (defined($self->{conformance}->{tfs}->{$hf_bitname})) {
+ $self->pidl_def(" $self->{conformance}->{tfs}->{$hf_bitname}->{TRUE_STRING},");
+ $self->pidl_def(" $self->{conformance}->{tfs}->{$hf_bitname}->{FALSE_STRING},");
+ $self->{conformance}->{tfs}->{$hf_bitname}->{USED} = 1;
+ } else {
+ $self->pidl_def(" \"$en is SET\",");
+ $self->pidl_def(" \"$en is NOT SET\",");
+ }
+ $self->pidl_def("};");
}
- $self->pidl_def("};");
-
- $self->pidl_code("proto_tree_add_boolean(tree, $hf_bitname, tvb, offset-$e->{ALIGN}, $e->{ALIGN}, flags);");
- $self->pidl_code("if (flags&$ev){");
- $self->pidl_code("\tproto_item_append_text(item, \"$en\");");
- $self->pidl_code("\tif (flags & (~$ev))");
- $self->pidl_code("\t\tproto_item_append_text(item, \", \");");
- $self->pidl_code("}");
- $self->pidl_code("flags&=(~$ev);");
- $self->pidl_code("");
}
- $self->pidl_code("if (flags) {");
- $self->pidl_code("\tproto_item_append_text(item, \"Unknown bitmap value 0x%x\", flags);");
- $self->pidl_code("}\n");
+ if ($element_count > 0) {
+ my $total_ev_hex = sprintf("0x%08x", $total_ev);
+ $self->pidl_code("if (flags & (~$total_ev_hex)) {");
+ $self->pidl_code("\tflags &= (~$total_ev_hex);");
+ $self->pidl_code("\tproto_item_append_text(item, \"Unknown bitmap value 0x%x\", flags);");
+ $self->pidl_code("}\n");
+ }
$self->pidl_code("return offset;");
$self->deindent;
$self->pidl_code("}\n");
@@ -396,7 +421,7 @@ sub ElementLevel($$$$$$$$)
# continue to dissect handmarshalled stuff with pidl
$self->pidl_code("di->call_data->flags &= ~DCERPC_IS_NDR64;");
- $self->pidl_code("subtvb = tvb_new_subset(tvb, offset, (const gint)size, -1);");
+ $self->pidl_code("subtvb = tvb_new_subset_length_caplen(tvb, offset, (const gint)size, -1);");
if ($param ne 0) {
$self->pidl_code("$myname\_(subtvb, 0, pinfo, tree, di, drep, $param);");
} else {
diff --git a/script/autobuild.py b/script/autobuild.py
index c25fb94b3d5..7d9dc008bcf 100755
--- a/script/autobuild.py
+++ b/script/autobuild.py
@@ -887,10 +887,10 @@ tasks = {
("tevent-make", "cd lib/tevent && make"),
("tevent-install", "cd lib/tevent && make install"),
- ("nondevel-configure", samba_libs_envvars + " ./configure --private-libraries='!ldb' --vendor-name=autobuild-TEST-STRING --vendor-patch-revision=5 ${PREFIX}"),
+ ("nondevel-configure", samba_libs_envvars + " ./configure --private-libraries='!ldb' --vendor-suffix=TEST-STRING~5.1.2 ${PREFIX}"),
("nondevel-make", "make -j"),
("nondevel-check", "./bin/smbd -b | grep WITH_NTVFS_FILESERVER && exit 1; exit 0"),
- ("nondevel-check", "./bin/smbd --version | grep -e '-autobuild-TEST-STRING-5' && exit 0; exit 1"),
+ ("nondevel-check", "./bin/smbd --version | grep -F 'TEST-STRING~5.1.2' && exit 0; exit 1"),
("nondevel-no-libtalloc", "find ./bin | grep -v 'libtalloc-report' | grep 'libtalloc' && exit 1; exit 0"),
("nondevel-no-libtdb", "find ./bin | grep -v 'libtdb-wrap' | grep 'libtdb' && exit 1; exit 0"),
("nondevel-no-libtevent", "find ./bin | grep -v 'libtevent-util' | grep 'libtevent' && exit 1; exit 0"),
diff --git a/selftest/tests.py b/selftest/tests.py
index d0ce5fc6b4f..d585e7cda37 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -586,3 +586,5 @@ plantestsuite("samba.unittests.run_conditional_ace", "none",
[os.path.join(bindir(), "test_run_conditional_ace")])
plantestsuite("samba.unittests.claim_conversion", "none",
[os.path.join(bindir(), "test_claim_conversion")])
--
Samba Shared Repository
More information about the samba-cvs
mailing list