The branch, master has been updated
via 0fe263a56d0 pylibs: add string_is_guid() helper.
via 7b089e1206a samba-tool: with --json, error messages are in JSON
via 1f128fee27c samba-tool: instances remember whether --json was
requested
via 542ba5cbd5e samba-tool: add self.print_json_status() helper
via 742fc4d841c samba-tool: avoid mutable Command class values
via 29abab6a460 samba-tool domain level: avoid using assert
via 8650ba0a187 samba-tool domain claim: use secrets module for token
via 2908a6d67bc samba-tool user getpassword: Also return the time a
GMSA password is valid until
via 71f7c4a3c59 samba-tool: Allow ;format=UnixTime etc to operate on
virtual attributes
via dfe71c4235a python/samba/tests: Include more detail on invoication
in test of "samba-tool user show"
via 380c80b4d60 samba-tool user getpassword: Do not show preview of
gMSA password
via 801e3fd6dd1 s3:libads: Trace ldap search base/filter/scope
from 2b515b7dcc6 s4-kdc: Add "Fresh Public Key Identity" SID if PKINIT
freshness used
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 0fe263a56d049b62be71ced9d8a78bc0a749c195
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Thu Feb 15 21:20:24 2024 +0000
pylibs: add string_is_guid() helper.
In various places we use regular expressions to check for GUID-ness,
though typically we don't match GUIDs with uppercase hex digits when
we really should.
If we centralise the check, we have more chance of getting it right.
Pair-programmed-by: Andrew Bartlett <abart...@samba.org>
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Reviewed-by: Andrew Bartlett <abart...@samba.org>
Autobuild-User(master): Andrew Bartlett <abart...@samba.org>
Autobuild-Date(master): Thu Feb 29 02:38:07 UTC 2024 on atb-devel-224
commit 7b089e1206a8a8256ad108f5f0e03d3b33f8bf9f
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Wed Feb 28 16:14:24 2024 +1300
samba-tool: with --json, error messages are in JSON
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 1f128fee27c50aa305de3434443c4a52c408f9c6
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Wed Feb 28 16:13:15 2024 +1300
samba-tool: instances remember whether --json was requested
All our subcommands are going to learn --json eventually, and they
shouldn't all have to do this individually.
The next commit uses this to automatically format CommandErrors as JSON.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 542ba5cbd5e9a562cd81b5b2385b56d03555a87f
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Fri Feb 16 00:59:25 2024 +0000
samba-tool: add self.print_json_status() helper
This is a helper to return JSON for simple messages.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 742fc4d841c1b02cc733760e7841ca13a95f3ebc
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Fri Feb 23 16:19:02 2024 +1300
samba-tool: avoid mutable Command class values
These values are shared across all instances of the class,
which makes no difference in samba-tool itself, because there
is one instance per process. But in tests we can have many
Command classes at once (due to runcmd()), and if any of them
happened to append to takes_args or takes_options rather than
replacing it, well, the effect would be subtle.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 29abab6a460aa61699c4a1811c148552874c1236
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Wed Feb 14 05:09:30 2024 +0000
samba-tool domain level: avoid using assert
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 8650ba0a187d4c0a05fd4596570b940431338a27
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Fri Feb 2 14:23:38 2024 +1300
samba-tool domain claim: use secrets module for token
`binascii.hexlify(os.urandom(8)).decode()` was fine, but `os.urandom`
is OS specific and can theoretically block (says the documentation).
We will let Python's secrets module worry about such details.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 2908a6d67bca58c9de6991cbe312276408a34b7a
Author: Andrew Bartlett <abart...@samba.org>
Date: Fri Feb 9 11:44:33 2024 +1300
samba-tool user getpassword: Also return the time a GMSA password is valid
until
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
commit 71f7c4a3c59d170f3cf48c5230d3edf4d51d500c
Author: Andrew Bartlett <abart...@samba.org>
Date: Wed Feb 28 17:27:31 2024 +1300
samba-tool: Allow ;format=UnixTime etc to operate on virtual attributes
To convert a virtual attribute we must understand that it has
been put into "obj" under the name including the ;format= part
and so we must look it back up with that name when looking to
covert it from (say) NTTIME to a unix time.
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
commit dfe71c4235ad9943ba55d5d192b168554be749b3
Author: Andrew Bartlett <abart...@samba.org>
Date: Thu Feb 29 10:38:38 2024 +1300
python/samba/tests: Include more detail on invoication in test of
"samba-tool user show"
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
commit 380c80b4d609e87af63045ab7695c81159d89311
Author: Andrew Bartlett <abart...@samba.org>
Date: Fri Feb 2 16:10:06 2024 +1300
samba-tool user getpassword: Do not show preview of gMSA password
The AD server will send a preview of the next gMSA password, 5mins before
it is expected to be active.
This is useful in a keytab, which needs to be in place before a ticket
could possibly be issued, but is not helpful for authentication, as
the server also accepts passwords for 5mins after the change.
This avoids needing teach all users of this tool how to fall back to
the previous password for a 5min period every 30 days, by default.
Signed-off-by: Andrew Bartlett <abart...@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
commit 801e3fd6dd102e33f789cbe1e604b728aafc96bb
Author: Pavel Filipenský <pfilipen...@samba.org>
Date: Mon Feb 26 08:31:24 2024 +0100
s3:libads: Trace ldap search base/filter/scope
Signed-off-by: Pavel Filipenský <pfilipen...@samba.org>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
python/samba/__init__.py | 32 ++++++++++
python/samba/netcmd/__init__.py | 69 +++++++++++++++++++++-
python/samba/netcmd/domain/claim/claim_type.py | 6 +-
python/samba/netcmd/domain/level.py | 12 ++--
python/samba/netcmd/user/readpasswords/common.py | 38 ++++++++++--
python/samba/tests/samba_tool/user.py | 4 +-
.../tests/samba_tool/user_getpassword_gmsa.py | 24 +++++++-
source3/libads/ldap.c | 5 ++
8 files changed, 173 insertions(+), 17 deletions(-)
Changeset truncated at 500 lines:
diff --git a/python/samba/__init__.py b/python/samba/__init__.py
index 3e6ea7d18e1..3b253f7d79c 100644
--- a/python/samba/__init__.py
+++ b/python/samba/__init__.py
@@ -26,6 +26,7 @@ import os
import time
import ldb
import samba.param
+import re
from samba import _glue
from samba._ldb import Ldb as _Ldb
@@ -356,6 +357,37 @@ def arcfour_encrypt(key, data):
return arcfour_crypt_blob(data, key)
+GUID_RE = re.compile(
+ "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}")
+
+GUID_MIXCASE_RE = re.compile(
+ "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}",
+ flags = re.IGNORECASE)
+
+
+def string_is_guid(string, lower_case_only=False):
+ """Is the string an ordinary undecorated string GUID?
+
+ That is, like 12345678-abcd-1234-FEED-1234567890ab, and not like
+ variants which have surrounding curly brackets or lack hyphens.
+
+ If lower case_only is true, only lowercase hex characters are
+ accepted. This is tighter than we ever require, but matches what
+ we usually emit.
+ """
+ # Note: it is rightly tempting to use misc.GUID() here and catch
+ # the error, but misc.GUID is more forgiving than we want,
+ # allowing all kinds of weird variants.
+ if lower_case_only:
+ m = GUID_RE.fullmatch(string)
+ else:
+ m = GUID_MIXCASE_RE.fullmatch(string)
+
+ if m is None:
+ return False
+ return True
+
+
def enable_net_export_keytab():
"""This function modifies the samba.net.Net class to contain
an export_keytab() method."""
diff --git a/python/samba/netcmd/__init__.py b/python/samba/netcmd/__init__.py
index 7ddc1dc0828..01497055d68 100644
--- a/python/samba/netcmd/__init__.py
+++ b/python/samba/netcmd/__init__.py
@@ -82,8 +82,8 @@ class Command(object):
# synopsis must be defined in all subclasses in order to provide the
# command usage
synopsis = None
- takes_args = []
- takes_options = []
+ takes_args = ()
+ takes_options = ()
takes_optiongroups = {}
hidden = False
@@ -93,6 +93,7 @@ class Command(object):
raw_argv = None
raw_args = None
raw_kwargs = None
+ preferred_output_format = None
def _set_files(self, outf=None, errf=None):
if outf is not None:
@@ -108,6 +109,19 @@ class Command(object):
parser.print_usage()
def _print_error(self, msg, evalue=None, klass=None):
+ if self.preferred_output_format == 'json':
+ if evalue is None:
+ evalue = 1
+ else:
+ msg = f"{msg} - {evalue}"
+ if klass is not None:
+ kwargs = {'error class': klass}
+ else:
+ kwargs = {}
+
+ self.print_json_status(evalue, msg, **kwargs)
+ return
+
err = colour.c_DARK_RED("ERROR")
klass = '' if klass is None else f'({klass})'
@@ -155,6 +169,50 @@ class Command(object):
json.dump(data, self.outf, cls=JSONEncoder, indent=2, sort_keys=True)
self.outf.write("\n")
+ def print_json_status(self, error=None, message=None, **kwargs):
+ """For commands that really have nothing to say when they succeed
+ (`samba-tool foo delete --json`), we can still emit
+ '{"status": "OK"}\n'. And if they fail they can say:
+ '{"status": "error"}\n'.
+ This function hopes to keep things consistent.
+
+ If error is true-ish but not True, it is stringified and added
+ as a message. For example, if error is an LdbError with an
+ OBJECT_NOT_FOUND code, self.print_json_status(error) results
+ in this:
+
+ '{"status": "error", "message": "object not found"}\n'
+
+ unless an explicit message is added, in which case that is
+ used. A message can be provided on success, like this:
+
+ '{"status": "OK", "message": "thanks for asking!"}\n'
+
+ Extra keywords can be added too.
+
+ In summary, you might go:
+
+ try:
+ samdb.delete(dn)
+ except Exception as e:
+ print_json_status(e)
+ return
+ print_json_status()
+ """
+ data = {}
+ if error:
+ data['status'] = 'error'
+ if error is not True:
+ data['message'] = str(error)
+ else:
+ data['status'] = 'OK'
+
+ if message is not None:
+ data['message'] = message
+
+ data.update(kwargs)
+ self.print_json(data)
+
def show_command_error(self, e):
"""display a command error"""
if isinstance(e, CommandError):
@@ -256,6 +314,13 @@ class Command(object):
del kwargs[option.dest]
kwargs.update(optiongroups)
+ if kwargs.get('output_format') == 'json':
+ self.preferred_output_format = 'json'
+ else:
+ # we need to reset this for the tests that reuse the
+ # samba-tool object.
+ self.preferred_output_format = None
+
if self.use_colour:
self.apply_colour_choice(kwargs.pop('color', 'auto'))
diff --git a/python/samba/netcmd/domain/claim/claim_type.py
b/python/samba/netcmd/domain/claim/claim_type.py
index 754f8b11117..72e98d33125 100644
--- a/python/samba/netcmd/domain/claim/claim_type.py
+++ b/python/samba/netcmd/domain/claim/claim_type.py
@@ -20,8 +20,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
-import binascii
-import os
+import secrets
import samba.getopt as options
from samba.netcmd import Command, CommandError, Option, SuperCommand
@@ -97,8 +96,7 @@ class cmd_domain_claim_claim_type_create(Command):
# Generate the new Claim Type cn.
# Windows creates a random number here containing 16 hex digits.
- # We can achieve something similar using urandom(8)
- instance = binascii.hexlify(os.urandom(8)).decode()
+ instance = secrets.token_hex(8)
cn = f"ad://ext/{display_name}:{instance}"
# adminDescription should be present but still have a fallback.
diff --git a/python/samba/netcmd/domain/level.py
b/python/samba/netcmd/domain/level.py
index eefe360563d..f4d257ac324 100644
--- a/python/samba/netcmd/domain/level.py
+++ b/python/samba/netcmd/domain/level.py
@@ -81,22 +81,26 @@ class cmd_domain_level(Command):
try:
res_forest = samdb.search("CN=Partitions,%s" %
samdb.get_config_basedn(),
scope=ldb.SCOPE_BASE,
attrs=["msDS-Behavior-Version"])
- assert len(res_forest) == 1
+ if len(res_forest) != 1:
+ raise CommandError("Forest not found")
res_domain = samdb.search(domain_dn, scope=ldb.SCOPE_BASE,
attrs=["msDS-Behavior-Version",
"nTMixedDomain"])
- assert len(res_domain) == 1
+ if len(res_domain) != 1:
+ raise CommandError("domain not found")
res_domain_cross = samdb.search("CN=Partitions,%s" %
samdb.get_config_basedn(),
scope=ldb.SCOPE_SUBTREE,
expression="(&(objectClass=crossRef)(nCName=%s))" % domain_dn,
attrs=["msDS-Behavior-Version"])
- assert len(res_domain_cross) == 1
+ if len(res_domain_cross) != 1:
+ raise CommandError("no crossRef objects found")
res_dc_s = samdb.search("CN=Sites,%s" % samdb.get_config_basedn(),
scope=ldb.SCOPE_SUBTREE,
expression="(objectClass=nTDSDSA)",
attrs=["msDS-Behavior-Version"])
- assert len(res_dc_s) >= 1
+ if len(res_dc_s) == 0:
+ raise CommandError("no nTDSDSA objects found")
# default values, since "msDS-Behavior-Version" does not exist on
Windows 2000 AD
level_forest = DS_DOMAIN_FUNCTION_2000
diff --git a/python/samba/netcmd/user/readpasswords/common.py
b/python/samba/netcmd/user/readpasswords/common.py
index 6d44881823d..8af8be7341e 100644
--- a/python/samba/netcmd/user/readpasswords/common.py
+++ b/python/samba/netcmd/user/readpasswords/common.py
@@ -22,6 +22,7 @@
import base64
import builtins
import binascii
+import datetime
import errno
import io
import os
@@ -34,7 +35,8 @@ from samba.dcerpc import drsblobs, security, gmsa
from samba.ndr import ndr_unpack
from samba.netcmd import Command, CommandError
from samba.samdb import SamDB
-
+from samba.nt_time import timedelta_from_nt_time_delta, nt_time_from_datetime
+from samba.gkdi import MAX_CLOCK_SKEW
# python[3]-gpgme is abandoned since ubuntu 1804 and debian 9
# have to use python[3]-gpg instead
@@ -102,6 +104,8 @@ virtual_attributes = {
"unicodePwd": {
"flags": ldb.ATTR_FLAG_FORCE_BASE64_LDIF,
},
+ "virtualManagedPasswordQueryTime": {
+ },
}
@@ -369,10 +373,28 @@ class GetPasswordCommand(Command):
managed_password = obj["msDS-ManagedPassword"][0]
unpacked_managed_password = ndr_unpack(gmsa.MANAGEDPASSWORD_BLOB,
managed_password)
- calculated["Primary:CLEARTEXT"] = \
- unpacked_managed_password.passwords.current
calculated["OLDCLEARTEXT"] = \
unpacked_managed_password.passwords.previous
+ query_interval = unpacked_managed_password.passwords.query_interval
+ calculated["GMSA:query_interval"] = \
+ query_interval
+
+ query_time_datetime = \
+ timedelta_from_nt_time_delta(query_interval) +
datetime.datetime.now(tz=datetime.timezone.utc)
+
+ query_time_nttime = nt_time_from_datetime(query_time_datetime)
+
+ calculated["GMSA:query_time"] = query_time_nttime
+
+ # This password is useful for a keytab, but not for
+ # authentication, so don't show or provide it as the new password
+ # just yet
+ if calculated["GMSA:query_interval"] <= MAX_CLOCK_SKEW:
+ calculated["Primary:CLEARTEXT"] = \
+ unpacked_managed_password.passwords.previous
+ else:
+ calculated["Primary:CLEARTEXT"] = \
+ unpacked_managed_password.passwords.current
account_name = str(obj["sAMAccountName"][0])
if "userPrincipalName" in obj:
@@ -770,6 +792,10 @@ class GetPasswordCommand(Command):
v = get_wDigest(i, primary_wdigest, account_name, account_upn,
domain, dns_domain)
if v is None:
continue
+ elif a == "virtualManagedPasswordQueryTime":
+ if "GMSA:query_time" not in calculated:
+ continue
+ v = str(calculated["GMSA:query_time"])
else:
continue
obj[a] = ldb.MessageElement(v, ldb.FLAG_MOD_REPLACE,
vattr["raw_attr"])
@@ -840,10 +866,14 @@ class GetPasswordCommand(Command):
continue
if ra["vformat"] != fm:
continue
+
srcattr = get_src_attrname(ra["attr"])
+ if srcattr is not None:
+ an = "%s;format=%s" % (srcattr, fm)
+ else:
+ srcattr = an = get_src_attrname(ra["raw_attr"])
if srcattr is None:
continue
- an = "%s;format=%s" % (srcattr, fm)
if an in generated_formats:
continue
generated_formats[an] = fm
diff --git a/python/samba/tests/samba_tool/user.py
b/python/samba/tests/samba_tool/user.py
index 26c9748ffc3..a19592a1f8c 100644
--- a/python/samba/tests/samba_tool/user.py
+++ b/python/samba/tests/samba_tool/user.py
@@ -600,7 +600,9 @@ sAMAccountName: %s
"-H", "ldap://%s"; % os.environ["DC_SERVER"],
"-U%s%%%s" % (os.environ["DC_USERNAME"],
os.environ["DC_PASSWORD"]))
- self.assertCmdSuccess(result, out, err, "Error running show")
+ self.assertCmdSuccess(result, out, err,
+ "Error running show --attributes=%s"
+ % ",".join(attrs))
self.assertIn(";format=GeneralizedTime", out)
self.assertIn(";format=UnixTime", out)
diff --git a/python/samba/tests/samba_tool/user_getpassword_gmsa.py
b/python/samba/tests/samba_tool/user_getpassword_gmsa.py
index acd8907e992..bbb68c41b7e 100644
--- a/python/samba/tests/samba_tool/user_getpassword_gmsa.py
+++ b/python/samba/tests/samba_tool/user_getpassword_gmsa.py
@@ -26,6 +26,8 @@ import os
sys.path.insert(0, "bin/python")
os.environ["PYTHONUNBUFFERED"] = "1"
+import datetime, shlex
+
from ldb import SCOPE_BASE
from samba.credentials import MUST_USE_KERBEROS
@@ -33,7 +35,8 @@ from samba.dcerpc import security, samr
from samba.dsdb import UF_WORKSTATION_TRUST_ACCOUNT
from samba.netcmd.domain.models import User
from samba.ndr import ndr_pack, ndr_unpack
-from samba.tests import connect_samdb, delete_force
+from samba.nt_time import nt_time_from_datetime
+from samba.tests import connect_samdb, connect_samdb_env, delete_force
from samba.tests import BlackboxTestCase
@@ -88,7 +91,8 @@ class GMSAPasswordTest(BlackboxTestCase):
cls.user = User.get(cls.samdb, username=cls.username)
def getpassword(self, attrs):
- cmd = f"user getpassword --attributes={attrs} {self.username}"
+ shattrs = shlex.quote(attrs)
+ cmd = f"user getpassword --attributes={shattrs} {self.username}"
ldif = self.check_output(cmd).decode()
res = self.samdb.parse_ldif(ldif)
@@ -152,6 +156,22 @@ class GMSAPasswordTest(BlackboxTestCase):
self.assertEqual(self.user.object_sid, connecting_user_sid)
+ def test_querytime(self):
+ user_msg = self.getpassword("virtualManagedPasswordQueryTime")
+ querytime = int(user_msg["virtualManagedPasswordQueryTime"][0])
+
+ # Just assert the number makes sense
+ self.assertGreater(querytime,
nt_time_from_datetime(datetime.datetime.now(tz=datetime.timezone.utc)))
+ self.assertLess(querytime,
nt_time_from_datetime(datetime.datetime.now(tz=datetime.timezone.utc) +
datetime.timedelta(hours = 21)))
+
+ def test_querytime_unixtime(self):
+ user_msg =
self.getpassword("virtualManagedPasswordQueryTime;format=UnixTime")
+ querytime =
int(user_msg["virtualManagedPasswordQueryTime;format=UnixTime"][0])
+
+ # Just assert the number makes sense
+ self.assertGreater(querytime,
datetime.datetime.now(tz=datetime.timezone.utc).timestamp())
+ self.assertLess(querytime,
(datetime.datetime.now(tz=datetime.timezone.utc) + datetime.timedelta(hours =
21)).timestamp())
+
@classmethod
def _make_cmdline(cls, line):
"""Override to pass line as samba-tool subcommand instead.
diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
index 7f3c20746c8..ff67ad28a2a 100644
--- a/source3/libads/ldap.c
+++ b/source3/libads/ldap.c
@@ -154,6 +154,11 @@ static int ldap_search_with_timeout(LDAP *ld,
struct timeval *timeout_ptr = NULL;
int result;
+ DBG_DEBUG("ldap_search: base => [%s], filter => [%s], scope => [%d]\n",
+ base,
+ filter,
+ scope);
+
/* Setup timeout for the ldap_search_ext_s call - local and remote. */
gotalarm = 0;
--
Samba Shared Repository
