rpms/samba
6
0
Fork 0
Code Issues Pull Requests Releases Activity
14fb12ea63
samba/SOURCES/samba-4.19-redhat.patch
eabdullin 14fb12ea63 Import from CS git
2025-11-05 09:54:55 +00:00

5905 lines
195 KiB
Diff
Raw Blame History

From 3c29fc78029e1274f931e171c9e04c19ad0182c1 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Thu, 17 Aug 2023 01:05:54 +0300
Subject: [PATCH 01/63] gp: Support more global trust directories
In addition to the SUSE global trust directory, add support for RHEL and
Debian-based distributions (including Ubuntu).
To determine the correct directory to use, we iterate over the variants
and stop at the first which is a directory.
In case none is found, fallback to the first option which will produce a
warning as it did previously.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit a1b285e485c0b5a8747499bdbbb9f3f4fc025b2f)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 312c8ddf467..1b90ab46e90 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -45,10 +45,12 @@ cert_wrap = b"""
-----BEGIN CERTIFICATE-----
%s
-----END CERTIFICATE-----"""
-global_trust_dir = '/etc/pki/trust/anchors'
endpoint_re = '(https|HTTPS)://(?P<server>[a-zA-Z0-9.-]+)/ADPolicyProvider' + \
'_CEP_(?P<auth>[a-zA-Z]+)/service.svc/CEP'
+global_trust_dirs = ['/etc/pki/trust/anchors', # SUSE
+ '/etc/pki/ca-trust/source/anchors', # RHEL/Fedora
+ '/usr/local/share/ca-certificates'] # Debian/Ubuntu
def octet_string_to_objectGUID(data):
"""Convert an octet string to an objectGUID."""
@@ -249,12 +251,20 @@ def getca(ca, url, trust_dir):
return root_certs
+def find_global_trust_dir():
+ """Return the global trust dir using known paths from various Linux distros."""
+ for trust_dir in global_trust_dirs:
+ if os.path.isdir(trust_dir):
+ return trust_dir
+ return global_trust_dirs[0]
+
def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
"""Install the root certificate chain."""
data = dict({'files': [], 'templates': []}, **ca)
url = 'http://%s/CertSrv/mscep/mscep.dll/pkiclient.exe?' % ca['hostname']
root_certs = getca(ca, url, trust_dir)
data['files'].extend(root_certs)
+ global_trust_dir = find_global_trust_dir()
for src in root_certs:
# Symlink the certs to global trust dir
dst = os.path.join(global_trust_dir, os.path.basename(src))
--
2.51.0
From 063606e8ec83a58972df47eb561ab267f8937ba4 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Thu, 17 Aug 2023 01:09:28 +0300
Subject: [PATCH 02/63] gp: Support update-ca-trust helper
This is used on RHEL/Fedora instead of update-ca-certificates. They
behave similarly so it's enough to change the command name.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit fa80d1d86439749c44e60cf9075e84dc9ed3c268)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 1b90ab46e90..cefdafa21b2 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -258,6 +258,10 @@ def find_global_trust_dir():
return trust_dir
return global_trust_dirs[0]
+def update_ca_command():
+ """Return the command to update the CA trust store."""
+ return which('update-ca-certificates') or which('update-ca-trust')
+
def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
"""Install the root certificate chain."""
data = dict({'files': [], 'templates': []}, **ca)
@@ -283,7 +287,7 @@ def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
# already exists. Ignore the FileExistsError. Preserve the
# existing symlink in the unapply data.
data['files'].append(dst)
- update = which('update-ca-certificates')
+ update = update_ca_command()
if update is not None:
Popen([update]).wait()
# Setup Certificate Auto Enrollment
--
2.51.0
From 3b548bf280ca59ef12a7af10a9131813067a850a Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Fri, 11 Aug 2023 18:46:42 +0300
Subject: [PATCH 03/63] gp: Change root cert extension suffix
On Ubuntu, certificates must end in '.crt' in order to be considered by
the `update-ca-certificates` helper.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit bce3a89204545dcab5fb39a712590f6e166f997b)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index cefdafa21b2..c562722906b 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -241,7 +241,8 @@ def getca(ca, url, trust_dir):
certs = load_der_pkcs7_certificates(r.content)
for i in range(0, len(certs)):
cert = certs[i].public_bytes(Encoding.PEM)
- dest = '%s.%d' % (root_cert, i)
+ filename, extension = root_cert.rsplit('.', 1)
+ dest = '%s.%d.%s' % (filename, i, extension)
with open(dest, 'wb') as w:
w.write(cert)
root_certs.append(dest)
--
2.51.0
From 7592ed5032836dc43f657f66607a0a4661edcdb4 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Fri, 18 Aug 2023 17:06:43 +0300
Subject: [PATCH 04/63] gp: Test with binary content for certificate data
This fails all GPO-related tests that call `gpupdate --rsop`.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit 1ef722cf66f9ec99f52939f1cfca031c5fe1ad70)
---
python/samba/tests/gpo.py | 8 ++++----
selftest/knownfail.d/gpo | 13 +++++++++++++
2 files changed, 17 insertions(+), 4 deletions(-)
create mode 100644 selftest/knownfail.d/gpo
diff --git a/python/samba/tests/gpo.py b/python/samba/tests/gpo.py
index e4b75cc62a4..963f873f755 100644
--- a/python/samba/tests/gpo.py
+++ b/python/samba/tests/gpo.py
@@ -6783,14 +6783,14 @@ class GPOTests(tests.TestCase):
ldb.add({'dn': certa_dn,
'objectClass': 'certificationAuthority',
'authorityRevocationList': ['XXX'],
- 'cACertificate': 'XXX',
+ 'cACertificate': b'0\x82\x03u0\x82\x02]\xa0\x03\x02\x01\x02\x02\x10I',
'certificateRevocationList': ['XXX'],
})
# Write the dummy pKIEnrollmentService
enroll_dn = 'CN=%s,CN=Enrollment Services,%s' % (ca_cn, confdn)
ldb.add({'dn': enroll_dn,
'objectClass': 'pKIEnrollmentService',
- 'cACertificate': 'XXXX',
+ 'cACertificate': b'0\x82\x03u0\x82\x02]\xa0\x03\x02\x01\x02\x02\x10I',
'certificateTemplates': ['Machine'],
'dNSHostName': hostname,
})
@@ -7201,14 +7201,14 @@ class GPOTests(tests.TestCase):
ldb.add({'dn': certa_dn,
'objectClass': 'certificationAuthority',
'authorityRevocationList': ['XXX'],
- 'cACertificate': 'XXX',
+ 'cACertificate': b'0\x82\x03u0\x82\x02]\xa0\x03\x02\x01\x02\x02\x10I',
'certificateRevocationList': ['XXX'],
})
# Write the dummy pKIEnrollmentService
enroll_dn = 'CN=%s,CN=Enrollment Services,%s' % (ca_cn, confdn)
ldb.add({'dn': enroll_dn,
'objectClass': 'pKIEnrollmentService',
- 'cACertificate': 'XXXX',
+ 'cACertificate': b'0\x82\x03u0\x82\x02]\xa0\x03\x02\x01\x02\x02\x10I',
'certificateTemplates': ['Machine'],
'dNSHostName': hostname,
})
diff --git a/selftest/knownfail.d/gpo b/selftest/knownfail.d/gpo
new file mode 100644
index 00000000000..0aad59607c2
--- /dev/null
+++ b/selftest/knownfail.d/gpo
@@ -0,0 +1,13 @@
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_user_centrify_crontab_ext
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_user_scripts_ext
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_rsop
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_access
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_files
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_issue
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_motd
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_openssh
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_startup_scripts
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_sudoers
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_symlink
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_advanced_gp_cert_auto_enroll_ext
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_cert_auto_enroll_ext
--
2.51.0
From 7f7b235bda9e85c5ea330e52e734d1113a884571 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Wed, 16 Aug 2023 12:20:11 +0300
Subject: [PATCH 05/63] gp: Convert CA certificates to base64
I don't know whether this applies universally, but in our case the
contents of `es['cACertificate'][0]` are binary, so cleanly converting
to a string fails with the following:
'utf-8' codec can't decode byte 0x82 in position 1: invalid start byte
We found a fix to be encoding the certificate to base64 when
constructing the CA list.
Section 4.4.5.2 of MS-CAESO also suggests that the content of
`cACertificate` is binary (OCTET string).
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit 157335ee93eb866f9b6a47486a5668d6e76aced5)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 5 ++---
selftest/knownfail.d/gpo | 13 -------------
2 files changed, 2 insertions(+), 16 deletions(-)
delete mode 100644 selftest/knownfail.d/gpo
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index c562722906b..c8b5368c16a 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -158,7 +158,7 @@ def fetch_certification_authorities(ldb):
for es in res:
data = { 'name': get_string(es['cn'][0]),
'hostname': get_string(es['dNSHostName'][0]),
- 'cACertificate': get_string(es['cACertificate'][0])
+ 'cACertificate': get_string(base64.b64encode(es['cACertificate'][0]))
}
result.append(data)
return result
@@ -176,8 +176,7 @@ def fetch_template_attrs(ldb, name, attrs=None):
return {'msPKI-Minimal-Key-Size': ['2048']}
def format_root_cert(cert):
- cert = base64.b64encode(cert.encode())
- return cert_wrap % re.sub(b"(.{64})", b"\\1\n", cert, 0, re.DOTALL)
+ return cert_wrap % re.sub(b"(.{64})", b"\\1\n", cert.encode(), 0, re.DOTALL)
def find_cepces_submit():
certmonger_dirs = [os.environ.get("PATH"), '/usr/lib/certmonger',
diff --git a/selftest/knownfail.d/gpo b/selftest/knownfail.d/gpo
deleted file mode 100644
index 0aad59607c2..00000000000
--- a/selftest/knownfail.d/gpo
+++ /dev/null
@@ -1,13 +0,0 @@
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_user_centrify_crontab_ext
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_user_scripts_ext
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_rsop
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_access
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_files
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_issue
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_motd
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_openssh
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_startup_scripts
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_sudoers
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_vgp_symlink
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_advanced_gp_cert_auto_enroll_ext
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_cert_auto_enroll_ext
--
2.51.0
From 49cc74015a603e80048a38fe635cd1ac28938ee4 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Fri, 18 Aug 2023 17:16:23 +0300
Subject: [PATCH 06/63] gp: Test adding new cert templates enforces changes
Ensure that cepces-submit reporting additional templates and re-applying
will enforce the updated policy.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit 2d6943a864405f324c467e8c3464c31ac08457b0)
---
python/samba/tests/bin/cepces-submit | 3 +-
python/samba/tests/gpo.py | 48 ++++++++++++++++++++++++++++
selftest/knownfail.d/gpo | 2 ++
3 files changed, 52 insertions(+), 1 deletion(-)
create mode 100644 selftest/knownfail.d/gpo
diff --git a/python/samba/tests/bin/cepces-submit b/python/samba/tests/bin/cepces-submit
index 668682a9f58..de63164692b 100755
--- a/python/samba/tests/bin/cepces-submit
+++ b/python/samba/tests/bin/cepces-submit
@@ -14,4 +14,5 @@ if __name__ == "__main__":
assert opts.auth == 'Kerberos'
if 'CERTMONGER_OPERATION' in os.environ and \
os.environ['CERTMONGER_OPERATION'] == 'GET-SUPPORTED-TEMPLATES':
- print('Machine') # Report a Machine template
+ templates = os.environ.get('CEPCES_SUBMIT_SUPPORTED_TEMPLATES', 'Machine').split(',')
+ print('\n'.join(templates)) # Report the requested templates
diff --git a/python/samba/tests/gpo.py b/python/samba/tests/gpo.py
index 963f873f755..e75c411bde7 100644
--- a/python/samba/tests/gpo.py
+++ b/python/samba/tests/gpo.py
@@ -6812,6 +6812,23 @@ class GPOTests(tests.TestCase):
self.assertTrue(os.path.exists(machine_crt),
'Machine key was not generated')
+ # Subsequent apply should react to new certificate templates
+ os.environ['CEPCES_SUBMIT_SUPPORTED_TEMPLATES'] = 'Machine,Workstation'
+ self.addCleanup(os.environ.pop, 'CEPCES_SUBMIT_SUPPORTED_TEMPLATES')
+ ext.process_group_policy([], gpos, dname, dname)
+ self.assertTrue(os.path.exists(ca_crt),
+ 'Root CA certificate was not requested')
+ self.assertTrue(os.path.exists(machine_crt),
+ 'Machine certificate was not requested')
+ self.assertTrue(os.path.exists(machine_crt),
+ 'Machine key was not generated')
+ workstation_crt = os.path.join(dname, '%s.Workstation.crt' % ca_cn)
+ self.assertTrue(os.path.exists(workstation_crt),
+ 'Workstation certificate was not requested')
+ workstation_key = os.path.join(dname, '%s.Workstation.key' % ca_cn)
+ self.assertTrue(os.path.exists(workstation_crt),
+ 'Workstation key was not generated')
+
# Verify RSOP does not fail
ext.rsop([g for g in gpos if g.name == guid][0])
@@ -6829,11 +6846,17 @@ class GPOTests(tests.TestCase):
'Machine certificate was not removed')
self.assertFalse(os.path.exists(machine_crt),
'Machine key was not removed')
+ self.assertFalse(os.path.exists(workstation_crt),
+ 'Workstation certificate was not removed')
+ self.assertFalse(os.path.exists(workstation_crt),
+ 'Workstation key was not removed')
out, _ = Popen(['getcert', 'list-cas'], stdout=PIPE).communicate()
self.assertNotIn(get_bytes(ca_cn), out, 'CA was not removed')
out, _ = Popen(['getcert', 'list'], stdout=PIPE).communicate()
self.assertNotIn(b'Machine', out,
'Machine certificate not removed')
+ self.assertNotIn(b'Workstation', out,
+ 'Workstation certificate not removed')
# Remove the dummy CA, pKIEnrollmentService, and pKICertificateTemplate
ldb.delete(certa_dn)
@@ -7233,6 +7256,25 @@ class GPOTests(tests.TestCase):
self.assertTrue(os.path.exists(machine_crt),
'Machine key was not generated')
+ # Subsequent apply should react to new certificate templates
+ os.environ['CEPCES_SUBMIT_SUPPORTED_TEMPLATES'] = 'Machine,Workstation'
+ self.addCleanup(os.environ.pop, 'CEPCES_SUBMIT_SUPPORTED_TEMPLATES')
+ ext.process_group_policy([], gpos, dname, dname)
+ for ca in ca_list:
+ self.assertTrue(os.path.exists(ca_crt),
+ 'Root CA certificate was not requested')
+ self.assertTrue(os.path.exists(machine_crt),
+ 'Machine certificate was not requested')
+ self.assertTrue(os.path.exists(machine_crt),
+ 'Machine key was not generated')
+
+ workstation_crt = os.path.join(dname, '%s.Workstation.crt' % ca)
+ self.assertTrue(os.path.exists(workstation_crt),
+ 'Workstation certificate was not requested')
+ workstation_key = os.path.join(dname, '%s.Workstation.key' % ca)
+ self.assertTrue(os.path.exists(workstation_crt),
+ 'Workstation key was not generated')
+
# Verify RSOP does not fail
ext.rsop([g for g in gpos if g.name == guid][0])
@@ -7250,12 +7292,18 @@ class GPOTests(tests.TestCase):
'Machine certificate was not removed')
self.assertFalse(os.path.exists(machine_crt),
'Machine key was not removed')
+ self.assertFalse(os.path.exists(workstation_crt),
+ 'Workstation certificate was not removed')
+ self.assertFalse(os.path.exists(workstation_crt),
+ 'Workstation key was not removed')
out, _ = Popen(['getcert', 'list-cas'], stdout=PIPE).communicate()
for ca in ca_list:
self.assertNotIn(get_bytes(ca), out, 'CA was not removed')
out, _ = Popen(['getcert', 'list'], stdout=PIPE).communicate()
self.assertNotIn(b'Machine', out,
'Machine certificate not removed')
+ self.assertNotIn(b'Workstation', out,
+ 'Workstation certificate not removed')
# Remove the dummy CA, pKIEnrollmentService, and pKICertificateTemplate
ldb.delete(certa_dn)
diff --git a/selftest/knownfail.d/gpo b/selftest/knownfail.d/gpo
new file mode 100644
index 00000000000..4edc1dce730
--- /dev/null
+++ b/selftest/knownfail.d/gpo
@@ -0,0 +1,2 @@
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_advanced_gp_cert_auto_enroll_ext
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_cert_auto_enroll_ext
--
2.51.0
From 4c0906bd79f030e591701234bc54bc749a42d686 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Wed, 16 Aug 2023 12:37:17 +0300
Subject: [PATCH 07/63] gp: Template changes should invalidate cache
If certificate templates are added or removed, the autoenroll extension
should react to this and reapply the policy. Previously this wasn't
taken into account.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit 2a6ae997f2464b12b72b5314fa80d9784fb0f6c1)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 15 ++++++++++-----
selftest/knownfail.d/gpo | 2 --
2 files changed, 10 insertions(+), 7 deletions(-)
delete mode 100644 selftest/knownfail.d/gpo
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index c8b5368c16a..8233713e8ad 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -262,6 +262,11 @@ def update_ca_command():
"""Return the command to update the CA trust store."""
return which('update-ca-certificates') or which('update-ca-trust')
+def changed(new_data, old_data):
+ """Return True if any key present in both dicts has changed."""
+ return any((new_data[k] != old_data[k] if k in old_data else False) \
+ for k in new_data.keys())
+
def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
"""Install the root certificate chain."""
data = dict({'files': [], 'templates': []}, **ca)
@@ -351,12 +356,12 @@ class gp_cert_auto_enroll_ext(gp_pol_ext, gp_applier):
# If the policy has changed, unapply, then apply new policy
old_val = self.cache_get_attribute_value(guid, attribute)
old_data = json.loads(old_val) if old_val is not None else {}
- if all([(ca[k] == old_data[k] if k in old_data else False) \
- for k in ca.keys()]) or \
- self.cache_get_apply_state() == GPOSTATE.ENFORCE:
+ templates = ['%s.%s' % (ca['name'], t.decode()) for t in get_supported_templates(ca['hostname'])]
+ new_data = { 'templates': templates, **ca }
+ if changed(new_data, old_data) or self.cache_get_apply_state() == GPOSTATE.ENFORCE:
self.unapply(guid, attribute, old_val)
- # If policy is already applied, skip application
- if old_val is not None and \
+ # If policy is already applied and unchanged, skip application
+ if old_val is not None and not changed(new_data, old_data) and \
self.cache_get_apply_state() != GPOSTATE.ENFORCE:
return
diff --git a/selftest/knownfail.d/gpo b/selftest/knownfail.d/gpo
deleted file mode 100644
index 4edc1dce730..00000000000
--- a/selftest/knownfail.d/gpo
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_advanced_gp_cert_auto_enroll_ext
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_cert_auto_enroll_ext
--
2.51.0
From e61f30dc2518d5a1c239f090baea4a309307f3f8 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Fri, 18 Aug 2023 17:26:59 +0300
Subject: [PATCH 08/63] gp: Test disabled enrollment unapplies policy
For this we need to stage a Registry.pol file with certificate
autoenrollment enabled, but with checkboxes unticked.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit ee814f7707a8ddef2657212cd6d31799501b7bb3)
---
python/samba/tests/gpo.py | 54 +++++++++++++++++++++++++++++++++++++++
selftest/knownfail.d/gpo | 1 +
2 files changed, 55 insertions(+)
create mode 100644 selftest/knownfail.d/gpo
diff --git a/python/samba/tests/gpo.py b/python/samba/tests/gpo.py
index e75c411bde7..580f3568de8 100644
--- a/python/samba/tests/gpo.py
+++ b/python/samba/tests/gpo.py
@@ -281,6 +281,28 @@ b"""
</PolFile>
"""
+auto_enroll_unchecked_reg_pol = \
+b"""
+<?xml version="1.0" encoding="utf-8"?>
+<PolFile num_entries="3" signature="PReg" version="1">
+ <Entry type="4" type_name="REG_DWORD">
+ <Key>Software\Policies\Microsoft\Cryptography\AutoEnrollment</Key>
+ <ValueName>AEPolicy</ValueName>
+ <Value>0</Value>
+ </Entry>
+ <Entry type="4" type_name="REG_DWORD">
+ <Key>Software\Policies\Microsoft\Cryptography\AutoEnrollment</Key>
+ <ValueName>OfflineExpirationPercent</ValueName>
+ <Value>10</Value>
+ </Entry>
+ <Entry type="1" type_name="REG_SZ">
+ <Key>Software\Policies\Microsoft\Cryptography\AutoEnrollment</Key>
+ <ValueName>OfflineExpirationStoreNames</ValueName>
+ <Value>MY</Value>
+ </Entry>
+</PolFile>
+"""
+
advanced_enroll_reg_pol = \
b"""
<?xml version="1.0" encoding="utf-8"?>
@@ -6836,6 +6858,38 @@ class GPOTests(tests.TestCase):
ret = rsop(self.lp)
self.assertEqual(ret, 0, 'gpupdate --rsop failed!')
+ # Remove policy by staging pol file with auto-enroll unchecked
+ parser.load_xml(etree.fromstring(auto_enroll_unchecked_reg_pol.strip()))
+ ret = stage_file(reg_pol, ndr_pack(parser.pol_file))
+ self.assertTrue(ret, 'Could not create the target %s' % reg_pol)
+ ext.process_group_policy([], gpos, dname, dname)
+ self.assertFalse(os.path.exists(ca_crt),
+ 'Root CA certificate was not removed')
+ self.assertFalse(os.path.exists(machine_crt),
+ 'Machine certificate was not removed')
+ self.assertFalse(os.path.exists(machine_crt),
+ 'Machine key was not removed')
+ self.assertFalse(os.path.exists(workstation_crt),
+ 'Workstation certificate was not removed')
+ self.assertFalse(os.path.exists(workstation_crt),
+ 'Workstation key was not removed')
+
+ # Reapply policy by staging the enabled pol file
+ parser.load_xml(etree.fromstring(auto_enroll_reg_pol.strip()))
+ ret = stage_file(reg_pol, ndr_pack(parser.pol_file))
+ self.assertTrue(ret, 'Could not create the target %s' % reg_pol)
+ ext.process_group_policy([], gpos, dname, dname)
+ self.assertTrue(os.path.exists(ca_crt),
+ 'Root CA certificate was not requested')
+ self.assertTrue(os.path.exists(machine_crt),
+ 'Machine certificate was not requested')
+ self.assertTrue(os.path.exists(machine_crt),
+ 'Machine key was not generated')
+ self.assertTrue(os.path.exists(workstation_crt),
+ 'Workstation certificate was not requested')
+ self.assertTrue(os.path.exists(workstation_crt),
+ 'Workstation key was not generated')
+
# Remove policy
gp_db = store.get_gplog(machine_creds.get_username())
del_gpos = get_deleted_gpos_list(gp_db, [])
diff --git a/selftest/knownfail.d/gpo b/selftest/knownfail.d/gpo
new file mode 100644
index 00000000000..83bc9f0ac1f
--- /dev/null
+++ b/selftest/knownfail.d/gpo
@@ -0,0 +1 @@
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_cert_auto_enroll_ext
--
2.51.0
From 7757b9b48546d71e19798d1260da97780caa99c3 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Wed, 16 Aug 2023 12:33:59 +0300
Subject: [PATCH 09/63] gp: Send list of keys instead of dict to remove
`cache_get_all_attribute_values` returns a dict whereas we need to pass
a list of keys to `remove`. These will be interpolated in the gpdb search.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Mon Aug 28 03:01:22 UTC 2023 on atb-devel-224
(cherry picked from commit 7dc181757c76b881ceaf1915ebb0bfbcf5aca83a)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 2 +-
selftest/knownfail.d/gpo | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
delete mode 100644 selftest/knownfail.d/gpo
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 8233713e8ad..64c35782ae8 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -415,7 +415,7 @@ class gp_cert_auto_enroll_ext(gp_pol_ext, gp_applier):
# remove any existing policy
ca_attrs = \
self.cache_get_all_attribute_values(gpo.name)
- self.clean(gpo.name, remove=ca_attrs)
+ self.clean(gpo.name, remove=list(ca_attrs.keys()))
def __read_cep_data(self, guid, ldb, end_point_information,
trust_dir, private_dir):
diff --git a/selftest/knownfail.d/gpo b/selftest/knownfail.d/gpo
deleted file mode 100644
index 83bc9f0ac1f..00000000000
--- a/selftest/knownfail.d/gpo
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_cert_auto_enroll_ext
--
2.51.0
From 4e9b2e6409c5764ec0e66cc6c90b08e70f702e7c Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Tue, 9 Jan 2024 08:50:01 +0100
Subject: [PATCH 10/63] python:gp: Print a nice message if cepces-submit can't
be found
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15552
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>
(cherry picked from commit 8eb42425a8eb1b30ca0e94dfc01d8175ae5cde4b)
Autobuild-User(v4-19-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-19-test): Mon Jan 15 11:11:31 UTC 2024 on atb-devel-224
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 24 ++++++++++++----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 64c35782ae8..08d1a7348cd 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -185,17 +185,19 @@ def find_cepces_submit():
def get_supported_templates(server):
cepces_submit = find_cepces_submit()
- if os.path.exists(cepces_submit):
- env = os.environ
- env['CERTMONGER_OPERATION'] = 'GET-SUPPORTED-TEMPLATES'
- p = Popen([cepces_submit, '--server=%s' % server, '--auth=Kerberos'],
- env=env, stdout=PIPE, stderr=PIPE)
- out, err = p.communicate()
- if p.returncode != 0:
- data = { 'Error': err.decode() }
- log.error('Failed to fetch the list of supported templates.', data)
- return out.strip().split()
- return []
+ if not cepces_submit or not os.path.exists(cepces_submit):
+ log.error('Failed to find cepces-submit')
+ return []
+
+ env = os.environ
+ env['CERTMONGER_OPERATION'] = 'GET-SUPPORTED-TEMPLATES'
+ p = Popen([cepces_submit, '--server=%s' % server, '--auth=Kerberos'],
+ env=env, stdout=PIPE, stderr=PIPE)
+ out, err = p.communicate()
+ if p.returncode != 0:
+ data = {'Error': err.decode()}
+ log.error('Failed to fetch the list of supported templates.', data)
+ return out.strip().split()
def getca(ca, url, trust_dir):
--
2.51.0
From fb3aefff51c02cf8ba3f8dfeb7d3f971e8d4902a Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Mon, 8 Jan 2024 18:05:08 +0200
Subject: [PATCH 11/63] gpo: Test certificate policy without NDES
As of 8231eaf856b, the NDES feature is no longer required on Windows, as
cert auto-enroll can use the certificate from the LDAP request.
However, 157335ee93e changed the implementation to convert the LDAP
certificate to base64 due to it failing to cleanly convert to a string.
Because of insufficient test coverage I missed handling the part where
NDES is disabled or not reachable and the LDAP certificate was imported.
The call to load_der_x509_certificate now fails with an error because it
expects binary data, yet it receives a base64 encoded string.
This adds a test to confirm the issue.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15557
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(cherry picked from commit 0d1ff69936f18ea729fc11fbbb1569a833302572)
---
python/samba/tests/gpo.py | 126 ++++++++++++++++++++++++++++++++++++--
selftest/knownfail.d/gpo | 1 +
2 files changed, 121 insertions(+), 6 deletions(-)
create mode 100644 selftest/knownfail.d/gpo
diff --git a/python/samba/tests/gpo.py b/python/samba/tests/gpo.py
index 580f3568de8..a78af17dba4 100644
--- a/python/samba/tests/gpo.py
+++ b/python/samba/tests/gpo.py
@@ -102,17 +102,21 @@ def dummy_certificate():
# Dummy requests structure for Certificate Auto Enrollment
class dummy_requests(object):
- @staticmethod
- def get(url=None, params=None):
+ class exceptions(object):
+ ConnectionError = Exception
+
+ def __init__(self, want_exception=False):
+ self.want_exception = want_exception
+
+ def get(self, url=None, params=None):
+ if self.want_exception:
+ raise self.exceptions.ConnectionError
+
dummy = requests.Response()
dummy._content = dummy_certificate()
dummy.headers = {'Content-Type': 'application/x-x509-ca-cert'}
return dummy
- class exceptions(object):
- ConnectionError = Exception
-cae.requests = dummy_requests
-
realm = os.environ.get('REALM')
policies = realm + '/POLICIES'
realm = realm.lower()
@@ -6764,6 +6768,114 @@ class GPOTests(tests.TestCase):
# Unstage the Registry.pol file
unstage_file(reg_pol)
+ def test_gp_cert_auto_enroll_ext_without_ndes(self):
+ local_path = self.lp.cache_path('gpo_cache')
+ guid = '{31B2F340-016D-11D2-945F-00C04FB984F9}'
+ reg_pol = os.path.join(local_path, policies, guid,
+ 'MACHINE/REGISTRY.POL')
+ cache_dir = self.lp.get('cache directory')
+ store = GPOStorage(os.path.join(cache_dir, 'gpo.tdb'))
+
+ machine_creds = Credentials()
+ machine_creds.guess(self.lp)
+ machine_creds.set_machine_account()
+
+ # Initialize the group policy extension
+ cae.requests = dummy_requests(want_exception=True)
+ ext = cae.gp_cert_auto_enroll_ext(self.lp, machine_creds,
+ machine_creds.get_username(), store)
+
+ gpos = get_gpo_list(self.server, machine_creds, self.lp,
+ machine_creds.get_username())
+
+ # Stage the Registry.pol file with test data
+ parser = GPPolParser()
+ parser.load_xml(etree.fromstring(auto_enroll_reg_pol.strip()))
+ ret = stage_file(reg_pol, ndr_pack(parser.pol_file))
+ self.assertTrue(ret, 'Could not create the target %s' % reg_pol)
+
+ # Write the dummy CA entry, Enrollment Services, and Templates Entries
+ admin_creds = Credentials()
+ admin_creds.set_username(os.environ.get('DC_USERNAME'))
+ admin_creds.set_password(os.environ.get('DC_PASSWORD'))
+ admin_creds.set_realm(os.environ.get('REALM'))
+ hostname = get_dc_hostname(machine_creds, self.lp)
+ url = 'ldap://%s' % hostname
+ ldb = Ldb(url=url, session_info=system_session(),
+ lp=self.lp, credentials=admin_creds)
+ # Write the dummy CA
+ confdn = 'CN=Public Key Services,CN=Services,CN=Configuration,%s' % base_dn
+ ca_cn = '%s-CA' % hostname.replace('.', '-')
+ certa_dn = 'CN=%s,CN=Certification Authorities,%s' % (ca_cn, confdn)
+ ldb.add({'dn': certa_dn,
+ 'objectClass': 'certificationAuthority',
+ 'authorityRevocationList': ['XXX'],
+ 'cACertificate': dummy_certificate(),
+ 'certificateRevocationList': ['XXX'],
+ })
+ # Write the dummy pKIEnrollmentService
+ enroll_dn = 'CN=%s,CN=Enrollment Services,%s' % (ca_cn, confdn)
+ ldb.add({'dn': enroll_dn,
+ 'objectClass': 'pKIEnrollmentService',
+ 'cACertificate': dummy_certificate(),
+ 'certificateTemplates': ['Machine'],
+ 'dNSHostName': hostname,
+ })
+ # Write the dummy pKICertificateTemplate
+ template_dn = 'CN=Machine,CN=Certificate Templates,%s' % confdn
+ ldb.add({'dn': template_dn,
+ 'objectClass': 'pKICertificateTemplate',
+ })
+
+ with TemporaryDirectory() as dname:
+ try:
+ ext.process_group_policy([], gpos, dname, dname)
+ except Exception as e:
+ self.fail(str(e))
+
+ ca_crt = os.path.join(dname, '%s.crt' % ca_cn)
+ self.assertTrue(os.path.exists(ca_crt),
+ 'Root CA certificate was not requested')
+ machine_crt = os.path.join(dname, '%s.Machine.crt' % ca_cn)
+ self.assertTrue(os.path.exists(machine_crt),
+ 'Machine certificate was not requested')
+ machine_key = os.path.join(dname, '%s.Machine.key' % ca_cn)
+ self.assertTrue(os.path.exists(machine_key),
+ 'Machine key was not generated')
+
+ # Verify RSOP does not fail
+ ext.rsop([g for g in gpos if g.name == guid][0])
+
+ # Check that a call to gpupdate --rsop also succeeds
+ ret = rsop(self.lp)
+ self.assertEqual(ret, 0, 'gpupdate --rsop failed!')
+
+ # Remove policy
+ gp_db = store.get_gplog(machine_creds.get_username())
+ del_gpos = get_deleted_gpos_list(gp_db, [])
+ ext.process_group_policy(del_gpos, [], dname)
+ self.assertFalse(os.path.exists(ca_crt),
+ 'Root CA certificate was not removed')
+ self.assertFalse(os.path.exists(machine_crt),
+ 'Machine certificate was not removed')
+ self.assertFalse(os.path.exists(machine_key),
+ 'Machine key was not removed')
+ out, _ = Popen(['getcert', 'list-cas'], stdout=PIPE).communicate()
+ self.assertNotIn(get_bytes(ca_cn), out, 'CA was not removed')
+ out, _ = Popen(['getcert', 'list'], stdout=PIPE).communicate()
+ self.assertNotIn(b'Machine', out,
+ 'Machine certificate not removed')
+ self.assertNotIn(b'Workstation', out,
+ 'Workstation certificate not removed')
+
+ # Remove the dummy CA, pKIEnrollmentService, and pKICertificateTemplate
+ ldb.delete(certa_dn)
+ ldb.delete(enroll_dn)
+ ldb.delete(template_dn)
+
+ # Unstage the Registry.pol file
+ unstage_file(reg_pol)
+
def test_gp_cert_auto_enroll_ext(self):
local_path = self.lp.cache_path('gpo_cache')
guid = '{31B2F340-016D-11D2-945F-00C04FB984F9}'
@@ -6777,6 +6889,7 @@ class GPOTests(tests.TestCase):
machine_creds.set_machine_account()
# Initialize the group policy extension
+ cae.requests = dummy_requests()
ext = cae.gp_cert_auto_enroll_ext(self.lp, machine_creds,
machine_creds.get_username(), store)
@@ -7241,6 +7354,7 @@ class GPOTests(tests.TestCase):
machine_creds.set_machine_account()
# Initialize the group policy extension
+ cae.requests = dummy_requests()
ext = cae.gp_cert_auto_enroll_ext(self.lp, machine_creds,
machine_creds.get_username(), store)
diff --git a/selftest/knownfail.d/gpo b/selftest/knownfail.d/gpo
new file mode 100644
index 00000000000..f1e590bc7d8
--- /dev/null
+++ b/selftest/knownfail.d/gpo
@@ -0,0 +1 @@
+^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_cert_auto_enroll_ext_without_ndes
--
2.51.0
From 1a9af36177c7491687c75df151474bb10285f00e Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Thu, 18 Jan 2024 20:23:24 +0200
Subject: [PATCH 12/63] gpo: Decode base64 root cert before importing
The reasoning behind this is described in the previous commit message,
but essentially this should either be wrapped in certificate blocks and
imported as PEM, or converted back to binary and imported as DER.
I've opted for the latter since it's how it used to work before it
regressed in 157335ee93e.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15557
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(cherry picked from commit 3f3ddfa699a33c2c8a59f7fb9ee044bb2a6e0e06)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 5 +++--
selftest/knownfail.d/gpo | 1 -
2 files changed, 3 insertions(+), 3 deletions(-)
delete mode 100644 selftest/knownfail.d/gpo
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 08d1a7348cd..cd5e54f1110 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -217,10 +217,11 @@ def getca(ca, url, trust_dir):
' installed or not configured.')
if 'cACertificate' in ca:
log.warn('Installing the server certificate only.')
+ der_certificate = base64.b64decode(ca['cACertificate'])
try:
- cert = load_der_x509_certificate(ca['cACertificate'])
+ cert = load_der_x509_certificate(der_certificate)
except TypeError:
- cert = load_der_x509_certificate(ca['cACertificate'],
+ cert = load_der_x509_certificate(der_certificate,
default_backend())
cert_data = cert.public_bytes(Encoding.PEM)
with open(root_cert, 'wb') as w:
diff --git a/selftest/knownfail.d/gpo b/selftest/knownfail.d/gpo
deleted file mode 100644
index f1e590bc7d8..00000000000
--- a/selftest/knownfail.d/gpo
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.gpo.samba.tests.gpo.GPOTests.test_gp_cert_auto_enroll_ext_without_ndes
--
2.51.0
From f5fc88f9ae255f4dc135580f0fa4a02f5addc390 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Fri, 19 Jan 2024 11:36:19 +0200
Subject: [PATCH 13/63] gpo: Do not get templates list on first run
This is a visual fix and has no impact on functionality apart from
cleaner log messages.
The point of this is to get the list of supported templates in order to
compute a diff between the current applied templates and the updated
list, so we are able to unapply and reapply the policy in case there are
differences.
However this code path is executed on first applies as well, at which
point the root CA is not yet set up. This causes the
`get_supported_templates` call to fail, which is not a hard failure but
still pollutes the logs. In this case it's safe to avoid executing the
command as the policy will be applied regardless.
Signed-off-by: Gabriel Nagy <gabriel.nagy@canonical.com>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Mon Jan 22 16:48:57 UTC 2024 on atb-devel-224
(cherry picked from commit 8579340fc540633c13c017d896034904a8dbd55c)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index cd5e54f1110..559c903e1a2 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -359,7 +359,8 @@ class gp_cert_auto_enroll_ext(gp_pol_ext, gp_applier):
# If the policy has changed, unapply, then apply new policy
old_val = self.cache_get_attribute_value(guid, attribute)
old_data = json.loads(old_val) if old_val is not None else {}
- templates = ['%s.%s' % (ca['name'], t.decode()) for t in get_supported_templates(ca['hostname'])]
+ templates = ['%s.%s' % (ca['name'], t.decode()) for t in get_supported_templates(ca['hostname'])] \
+ if old_val is not None else []
new_data = { 'templates': templates, **ca }
if changed(new_data, old_data) or self.cache_get_apply_state() == GPOSTATE.ENFORCE:
self.unapply(guid, attribute, old_val)
--
2.51.0
From e8a6219181f2af87813b53fd09684650c1aa6f90 Mon Sep 17 00:00:00 2001
From: David Mulder <dmulder@samba.org>
Date: Fri, 5 Jan 2024 08:47:07 -0700
Subject: [PATCH 14/63] gp: Skip site GP list if no site is found
[MS-GPOL] 3.2.5.1.4 Site Search says if the site
search returns ERROR_NO_SITENAME, the GP site
search should be skipped.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15548
Signed-off-by: David Mulder <dmulder@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Tue Jan 23 11:20:35 UTC 2024 on atb-devel-224
(cherry picked from commit f05b61b4991e7f51bd184d76a79f8b50114a0ff3)
---
python/samba/gp/gpclass.py | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/python/samba/gp/gpclass.py b/python/samba/gp/gpclass.py
index 617ef79350c..babd8f90748 100644
--- a/python/samba/gp/gpclass.py
+++ b/python/samba/gp/gpclass.py
@@ -866,19 +866,25 @@ def get_gpo_list(dc_hostname, creds, lp, username):
# (S)ite
if gpo_list_machine:
- site_dn = site_dn_for_machine(samdb, dc_hostname, lp, creds, username)
-
try:
- log.debug("get_gpo_list: query SITE: [%s] for GPOs" % site_dn)
- gp_link = get_gpo_link(samdb, site_dn)
- except ldb.LdbError as e:
- (enum, estr) = e.args
- log.debug(estr)
- else:
- add_gplink_to_gpo_list(samdb, gpo_list, forced_gpo_list,
- site_dn, gp_link,
- gpo.GP_LINK_SITE,
- add_only_forced_gpos, token)
+ site_dn = site_dn_for_machine(samdb, dc_hostname, lp, creds, username)
+
+ try:
+ log.debug("get_gpo_list: query SITE: [%s] for GPOs" % site_dn)
+ gp_link = get_gpo_link(samdb, site_dn)
+ except ldb.LdbError as e:
+ (enum, estr) = e.args
+ log.debug(estr)
+ else:
+ add_gplink_to_gpo_list(samdb, gpo_list, forced_gpo_list,
+ site_dn, gp_link,
+ gpo.GP_LINK_SITE,
+ add_only_forced_gpos, token)
+ except ldb.LdbError:
+ # [MS-GPOL] 3.2.5.1.4 Site Search: If the method returns
+ # ERROR_NO_SITENAME, the remainder of this message MUST be skipped
+ # and the protocol sequence MUST continue at GPO Search
+ pass
# (L)ocal
gpo_list.insert(0, gpo.GROUP_POLICY_OBJECT("Local Policy",
--
2.51.0
From d0d1a890d6f2466691fa4ee663232ee0bd1c3776 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 22 Jan 2024 14:14:30 +0100
Subject: [PATCH 15/63] python:gp: Avoid path check for cepces-submit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
find_cepces_submit() uses which(), which returns None if not found.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15559
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Pavel Filipenský <pfilipensky@samba.org>
(cherry picked from commit 6a9630eff624643fd725219775784e68d967d04c)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 559c903e1a2..7325d5132cf 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -185,7 +185,7 @@ def find_cepces_submit():
def get_supported_templates(server):
cepces_submit = find_cepces_submit()
- if not cepces_submit or not os.path.exists(cepces_submit):
+ if not cepces_submit:
log.error('Failed to find cepces-submit')
return []
@@ -301,7 +301,7 @@ def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
# Setup Certificate Auto Enrollment
getcert = which('getcert')
cepces_submit = find_cepces_submit()
- if getcert is not None and os.path.exists(cepces_submit):
+ if getcert is not None and cepces_submit is not None:
p = Popen([getcert, 'add-ca', '-c', ca['name'], '-e',
'%s --server=%s --auth=%s' % (cepces_submit,
ca['hostname'], auth)],
--
2.51.0
From 7f6c9a4945635c6eb8ada2255bd0febbf0f4e540 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 22 Jan 2024 14:07:47 +0100
Subject: [PATCH 16/63] python:gp: Improve logging for certificate enrollment
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15559
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Pavel Filipenský <pfilipensky@samba.org>
(cherry picked from commit 6d5507e05050690cd4c56f3f97f5fb7de0338b87)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 7325d5132cf..a25a9678587 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -274,6 +274,9 @@ def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
"""Install the root certificate chain."""
data = dict({'files': [], 'templates': []}, **ca)
url = 'http://%s/CertSrv/mscep/mscep.dll/pkiclient.exe?' % ca['hostname']
+
+ log.info("Try to get root or server certificates")
+
root_certs = getca(ca, url, trust_dir)
data['files'].extend(root_certs)
global_trust_dir = find_global_trust_dir()
@@ -283,6 +286,7 @@ def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
try:
os.symlink(src, dst)
data['files'].append(dst)
+ log.info("Created symlink: %s -> %s" % (src, dst))
except PermissionError:
log.warn('Failed to symlink root certificate to the'
' admin trust anchors')
@@ -295,9 +299,14 @@ def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
# already exists. Ignore the FileExistsError. Preserve the
# existing symlink in the unapply data.
data['files'].append(dst)
+
update = update_ca_command()
+ log.info("Running %s" % (update))
if update is not None:
- Popen([update]).wait()
+ ret = Popen([update]).wait()
+ if ret != 0:
+ log.error('Failed to run %s' % (update))
+
# Setup Certificate Auto Enrollment
getcert = which('getcert')
cepces_submit = find_cepces_submit()
--
2.51.0
From 5321d5b5bd24d7659743576f2e12a7dc0a93a828 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 22 Jan 2024 15:04:36 +0100
Subject: [PATCH 17/63] python:gp: Do not print an error, if CA already exists
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We will get an exit status for duplicate in future:
https://www.pagure.io/certmonger/issue/269
We can't really fix that right now, as older version of certmonger
don't support the `-v` option.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15559
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Pavel Filipenský <pfilipensky@samba.org>
(cherry picked from commit 728757cd1ff0465967fcbda100254c9312e87c93)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index a25a9678587..0b23cd688db 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -318,8 +318,12 @@ def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
out, err = p.communicate()
log.debug(out.decode())
if p.returncode != 0:
- data = { 'Error': err.decode(), 'CA': ca['name'] }
- log.error('Failed to add Certificate Authority', data)
+ if p.returncode == 2:
+ log.info('The CA [%s] already exists' % ca['name'])
+ else:
+ data = {'Error': err.decode(), 'CA': ca['name']}
+ log.error('Failed to add Certificate Authority', data)
+
supported_templates = get_supported_templates(ca['hostname'])
for template in supported_templates:
attrs = fetch_template_attrs(ldb, template)
--
2.51.0
From 6a7a8a4090b8cdb8e71f4ad590260ceeda253ce2 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 22 Jan 2024 15:05:02 +0100
Subject: [PATCH 18/63] python:gp: Do not print an error if template already
exists
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We will get an exit status for duplicate in future:
https://www.pagure.io/certmonger/issue/269
We can't really fix that right now, as older version of certmonger
don't support the `-v` option.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15559
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Pavel Filipenský <pfilipensky@samba.org>
(cherry picked from commit 98dc44286ea102ef7701ccdea26bbde32b523a7e)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index 0b23cd688db..db681cb6f69 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -338,8 +338,12 @@ def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
out, err = p.communicate()
log.debug(out.decode())
if p.returncode != 0:
- data = { 'Error': err.decode(), 'Certificate': nickname }
- log.error('Failed to request certificate', data)
+ if p.returncode == 2:
+ log.info('The template [%s] already exists' % (nickname))
+ else:
+ data = {'Error': err.decode(), 'Certificate': nickname}
+ log.error('Failed to request certificate', data)
+
data['files'].extend([keyfile, certfile])
data['templates'].append(nickname)
if update is not None:
--
2.51.0
From 43dc3d5d833bc1db885eb45402decd3225a7c946 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 22 Jan 2024 15:05:24 +0100
Subject: [PATCH 19/63] python:gp: Log an error if update fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15559
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Pavel Filipenský <pfilipensky@samba.org>
(cherry picked from commit 367756b85a9ac8daaac2326392bcd1373feed3b7)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index db681cb6f69..c8ad2039dc6 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -347,7 +347,9 @@ def cert_enroll(ca, ldb, trust_dir, private_dir, auth='Kerberos'):
data['files'].extend([keyfile, certfile])
data['templates'].append(nickname)
if update is not None:
- Popen([update]).wait()
+ ret = Popen([update]).wait()
+ if ret != 0:
+ log.error('Failed to run %s' % (update))
else:
log.warn('certmonger and cepces must be installed for ' +
'certificate auto enrollment to work')
--
2.51.0
From d8276d6a098d10f405b8f24c4dfb82af4496607c Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 22 Jan 2024 15:46:24 +0100
Subject: [PATCH 20/63] python:gp: Improve working of log messages to avoid
confusion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We should not use the word "Failed". We are totally fine if we can't
connect to NDES in the meantime. This logs:
Try to get root or server certificates.
Unable to install root certificates (requires NDES).
Installing the server certificate only.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15559
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>
Reviewed-by: Pavel Filipenský <pfilipensky@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Mon Jan 29 10:37:29 UTC 2024 on atb-devel-224
(cherry picked from commit 1f823424418e814d9dc0785658e2a7d92643dab2)
---
python/samba/gp/gp_cert_auto_enroll_ext.py | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/python/samba/gp/gp_cert_auto_enroll_ext.py b/python/samba/gp/gp_cert_auto_enroll_ext.py
index c8ad2039dc6..2b7f7d22c2b 100644
--- a/python/samba/gp/gp_cert_auto_enroll_ext.py
+++ b/python/samba/gp/gp_cert_auto_enroll_ext.py
@@ -209,12 +209,10 @@ def getca(ca, url, trust_dir):
r = requests.get(url=url, params={'operation': 'GetCACert',
'message': 'CAIdentifier'})
except requests.exceptions.ConnectionError:
- log.warn('Failed to establish a new connection')
+ log.warn('Could not connect to Network Device Enrollment Service.')
r = None
if r is None or r.content == b'' or r.headers['Content-Type'] == 'text/html':
- log.warn('Failed to fetch the root certificate chain.')
- log.warn('The Network Device Enrollment Service is either not' +
- ' installed or not configured.')
+ log.warn('Unable to fetch root certificates (requires NDES).')
if 'cACertificate' in ca:
log.warn('Installing the server certificate only.')
der_certificate = base64.b64decode(ca['cACertificate'])
--
2.51.0
From 585357bf0d8889747a2769c2451ee34766087d95 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 29 Jan 2024 17:46:30 +0100
Subject: [PATCH 21/63] python:gp: Fix logging with gp
This allows enable INFO level logging with: `samba-gpupdate -d3`
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15558
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Tue Jan 30 07:18:05 UTC 2024 on atb-devel-224
(cherry picked from commit 145194071b10c4c1857f28fe79c57fd63ffab889)
---
python/samba/gp/util/logging.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/python/samba/gp/util/logging.py b/python/samba/gp/util/logging.py
index a74a8707d50..c3de32825db 100644
--- a/python/samba/gp/util/logging.py
+++ b/python/samba/gp/util/logging.py
@@ -24,9 +24,10 @@ import gettext
import random
import sys
-logger = logging.getLogger()
+logger = logging.getLogger("gp")
+
+
def logger_init(name, log_level):
- logger = logging.getLogger(name)
logger.addHandler(logging.StreamHandler(sys.stdout))
logger.setLevel(logging.CRITICAL)
if log_level == 1:
--
2.51.0
From 14ceb0b5f2f954bbabdaf78b8185fc515e3c8294 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org>
Date: Wed, 13 Mar 2024 13:55:41 +0100
Subject: [PATCH 22/63] docs-xml: Add parameter all_groupmem to idmap_ad
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15605
Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(cherry picked from commit a485d9de2f2d6a9815dcac6addb988a8987e111c)
---
docs-xml/manpages/idmap_ad.8.xml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/docs-xml/manpages/idmap_ad.8.xml b/docs-xml/manpages/idmap_ad.8.xml
index b364bbfa231..de6d36afe95 100644
--- a/docs-xml/manpages/idmap_ad.8.xml
+++ b/docs-xml/manpages/idmap_ad.8.xml
@@ -100,6 +100,16 @@
</listitem>
</varlistentry>
<varlistentry>
+ <term>all_groupmem = yes/no</term>
+ <listitem><para>
+ If set to <parameter>yes</parameter> winbind will retrieve all
+ group members for getgrnam(3), getgrgid(3) and getgrent(3) calls,
+ including those with missing uidNumber.
+ </para>
+ <para>Default: no</para>
+ </listitem>
+ </varlistentry>
+ <varlistentry>
<term>deny ous</term>
<listitem><para>This parameter is a list of OUs from
which objects will not be mapped via the ad idmap
--
2.51.0
From ac4184c8c3220263cb6f1a46a012533ed1c4e047 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org>
Date: Tue, 12 Mar 2024 13:20:24 +0100
Subject: [PATCH 23/63] s3:winbindd: Improve performance of lookup_groupmem()
in idmap_ad
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The LDAP query of lookup_groupmem() returns all group members from AD
even those with missing uidNumber. Such group members are useless in
UNIX environment for idmap_ad backend since there is no uid mapping.
'test_user' is member of group "Domanin Users" with 200K members,
only 20K members have set uidNumber.
Without this fix:
$ time id test_user
real 1m5.946s
user 0m0.019s
sys 0m0.012s
With this fix:
$ time id test_user
real 0m3.544s
user 0m0.004s
sys 0m0.007s
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15605
Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(cherry picked from commit 5d475d26a3d545f04791a04e85a06b8b192e3fcf)
---
source3/winbindd/winbindd_ads.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
index d7a665abbc6..e625aa6473f 100644
--- a/source3/winbindd/winbindd_ads.c
+++ b/source3/winbindd/winbindd_ads.c
@@ -1037,7 +1037,7 @@ static NTSTATUS lookup_useraliases(struct winbindd_domain *domain,
}
static NTSTATUS add_primary_group_members(
- ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, uint32_t rid,
+ ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, uint32_t rid, const char *domname,
char ***all_members, size_t *num_all_members)
{
char *filter;
@@ -1049,10 +1049,13 @@ static NTSTATUS add_primary_group_members(
char **members;
size_t num_members;
ads_control args;
+ bool all_groupmem = idmap_config_bool(domname, "all_groupmem", false);
filter = talloc_asprintf(
- mem_ctx, "(&(objectCategory=user)(primaryGroupID=%u))",
- (unsigned)rid);
+ mem_ctx,
+ "(&(objectCategory=user)(primaryGroupID=%u)%s)",
+ (unsigned)rid,
+ all_groupmem ? "" : "(uidNumber=*)(!(uidNumber=0))");
if (filter == NULL) {
goto done;
}
@@ -1204,7 +1207,7 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain,
DEBUG(10, ("ads lookup_groupmem: got %d sids via extended dn call\n", (int)num_members));
- status = add_primary_group_members(ads, mem_ctx, rid,
+ status = add_primary_group_members(ads, mem_ctx, rid, domain->name,
&members, &num_members);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10, ("%s: add_primary_group_members failed: %s\n",
--
2.51.0
From d0e2002efcc37055b35c351a6b936e6ab89fad32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org>
Date: Mon, 25 Mar 2024 22:38:18 +0100
Subject: [PATCH 24/63] selftest: Add "winbind expand groups = 1" to
setup_ad_member_idmap_ad
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15605
Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(backported from commit 2dab3a331b5511b4f2253f2b3b4513db7e52ea9a)
---
selftest/target/Samba3.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 44ac4a5901a..606c65f8ab1 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1412,6 +1412,7 @@ sub setup_ad_member_idmap_ad
idmap config $dcvars->{TRUST_DOMAIN} : backend = ad
idmap config $dcvars->{TRUST_DOMAIN} : range = 2000000-2999999
gensec_gssapi:requested_life_time = 5
+ winbind expand groups = 1
";
my $ret = $self->provision(
--
2.51.0
From 9625b6aed981aa4e70fe11d9d1acdb54db7591a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org>
Date: Thu, 14 Mar 2024 15:24:21 +0100
Subject: [PATCH 25/63] tests: Add a test for "all_groups=no" to
test_idmap_ad.sh
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15605
Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Autobuild-User(master): Pavel Filipensky <pfilipensky@samba.org>
Autobuild-Date(master): Tue Apr 2 13:25:39 UTC 2024 on atb-devel-224
(cherry picked from commit f8b72aa1f72881989990fabc9f4888968bb81967)
---
nsswitch/tests/test_idmap_ad.sh | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/nsswitch/tests/test_idmap_ad.sh b/nsswitch/tests/test_idmap_ad.sh
index 7ae112ada71..1d4bd395ba9 100755
--- a/nsswitch/tests/test_idmap_ad.sh
+++ b/nsswitch/tests/test_idmap_ad.sh
@@ -94,6 +94,14 @@ gidNumber: 2000001
unixHomeDirectory: /home/forbidden
loginShell: /bin/tcsh
gecos: User in forbidden OU
+
+dn: CN=no_posix_id,CN=Users,$BASE_DN
+changetype: add
+objectClass: user
+samaccountName: no_posix_id
+unixHomeDirectory: /home/no_posix_id
+loginShell: /bin/sh
+gecos: User without uidNumber and gidNumber
EOF
#
@@ -171,6 +179,17 @@ then
failed=$(($failed + 1))
fi
+#
+# Test 6: Make sure that with the default "all_groups=no"
+# the group "domain users" will not show user "no_posix_id"
+# but will show "SAMBA2008R2/administrator"
+#
+
+dom_users="$DOMAIN/domain users" # Extra step to make sure that all is one word
+out="$($wbinfo --group-info "$dom_users")"
+testit_grep_count "no_posix_id1" "no_posix_id" 0 echo "$out" || failed=$(expr $failed + 1)
+testit_grep "no_posix_id2" "SAMBA2008R2/administrator" echo "$out" || failed=$(expr $failed + 1)
+
#
# Trusted domain test 1: Test uid of Administrator, should be 2500000
#
@@ -241,6 +260,9 @@ gidNumber: 2000002
dn: cn=forbidden,ou=sub,$BASE_DN
changetype: delete
+dn: CN=no_posix_id,CN=Users,$BASE_DN
+changetype: delete
+
dn: ou=sub,$BASE_DN
changetype: delete
EOF
--
2.51.0
From e5890e63c35a4a5af29ae16e6dd734c4a3a304cc Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Tue, 28 May 2024 13:51:53 +0200
Subject: [PATCH 26/63] s3:libads: Allow get_kdc_ip_string() to lookup the KDCs
IP
Remove the requirement to provide an IP address. We should look up the
IP of the KDC and use it for the specified realm/workgroup.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15653
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit 28aa0b815baf4668e3df01d52597c40fd430e2fb)
---
source3/libads/kerberos.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/source3/libads/kerberos.c b/source3/libads/kerberos.c
index 50f4a6de3c6..ddf97c11973 100644
--- a/source3/libads/kerberos.c
+++ b/source3/libads/kerberos.c
@@ -437,23 +437,23 @@ static char *get_kdc_ip_string(char *mem_ctx,
char *kdc_str = NULL;
char *canon_sockaddr = NULL;
- SMB_ASSERT(pss != NULL);
-
- canon_sockaddr = print_canonical_sockaddr_with_port(frame, pss);
- if (canon_sockaddr == NULL) {
- goto out;
- }
+ if (pss != NULL) {
+ canon_sockaddr = print_canonical_sockaddr_with_port(frame, pss);
+ if (canon_sockaddr == NULL) {
+ goto out;
+ }
- kdc_str = talloc_asprintf(frame,
- "\t\tkdc = %s\n",
- canon_sockaddr);
- if (kdc_str == NULL) {
- goto out;
- }
+ kdc_str = talloc_asprintf(frame,
+ "\t\tkdc = %s\n",
+ canon_sockaddr);
+ if (kdc_str == NULL) {
+ goto out;
+ }
- ok = sockaddr_storage_to_samba_sockaddr(&sa, pss);
- if (!ok) {
- goto out;
+ ok = sockaddr_storage_to_samba_sockaddr(&sa, pss);
+ if (!ok) {
+ goto out;
+ }
}
/*
--
2.51.0
From 96a1ecd8db249fa03db60259cf76fdef9c1bd749 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Tue, 28 May 2024 13:53:51 +0200
Subject: [PATCH 27/63] s3:libads: Do not fail if we don't get an IP passed
down
The IP should be optional and we should look it up if not provided.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15653
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit 9dcc52d2a57314ec9ddaae82b3c49da051d1f1d2)
---
source3/libads/kerberos.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/libads/kerberos.c b/source3/libads/kerberos.c
index ddf97c11973..f74d8eb567c 100644
--- a/source3/libads/kerberos.c
+++ b/source3/libads/kerberos.c
@@ -704,7 +704,7 @@ bool create_local_private_krb5_conf_for_domain(const char *realm,
return false;
}
- if (domain == NULL || pss == NULL) {
+ if (domain == NULL) {
return false;
}
--
2.51.0
From 4934642b7a7d92c6d81ba25ef6e4b66e3805f708 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Tue, 28 May 2024 13:54:24 +0200
Subject: [PATCH 28/63] s3:winbind: Fix idmap_ad creating an invalid local
krb5.conf
In case of a trusted domain, we are providing the realm of the primary
trust but specify the KDC IP of the trusted domain. This leads to
Kerberos ticket requests to the trusted domain KDC which doesn't know
about the machine account. However we need a ticket from our primary
trust KDC.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15653
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(backported from commit 8989aa47b7493e6b7978c2efc4a40c781e9a2aee)
---
source3/winbindd/idmap_ad.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/source3/winbindd/idmap_ad.c b/source3/winbindd/idmap_ad.c
index 5c9fe07db95..b8002825161 100644
--- a/source3/winbindd/idmap_ad.c
+++ b/source3/winbindd/idmap_ad.c
@@ -320,7 +320,10 @@ static NTSTATUS idmap_ad_get_tldap_ctx(TALLOC_CTX *mem_ctx,
struct tldap_context **pld)
{
struct netr_DsRGetDCNameInfo *dcinfo;
- struct sockaddr_storage dcaddr;
+ struct sockaddr_storage dcaddr = {
+ .ss_family = AF_UNSPEC,
+ };
+ struct sockaddr_storage *pdcaddr = NULL;
struct cli_credentials *creds;
struct loadparm_context *lp_ctx;
struct tldap_context *ld;
@@ -362,9 +365,13 @@ static NTSTATUS idmap_ad_get_tldap_ctx(TALLOC_CTX *mem_ctx,
* create_local_private_krb5_conf_for_domain() can deal with
* sitename==NULL
*/
+ if (strequal(domname, lp_realm()) || strequal(domname, lp_workgroup()))
+ {
+ pdcaddr = &dcaddr;
+ }
ok = create_local_private_krb5_conf_for_domain(
- lp_realm(), lp_workgroup(), sitename, &dcaddr);
+ lp_realm(), lp_workgroup(), sitename, pdcaddr);
TALLOC_FREE(sitename);
if (!ok) {
DBG_DEBUG("Could not create private krb5.conf\n");
--
2.51.0
From cccc902c64c93db317bf4707d0af5e56b2887286 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@samba.org>
Date: Mon, 22 Jul 2024 12:26:55 +0200
Subject: [PATCH 29/63] s3:notifyd: Use a watcher per db record
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This fixes a O(n²) performance regression in notifyd. The problem was
that we had a watcher per notify instance. This changes the code to have
a watcher per notify db entry.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14430
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Tue Oct 1 14:22:43 UTC 2024 on atb-devel-224
(cherry picked from commit af011b987a4ad0d3753d83cc0b8d97ad64ba874a)
---
source3/smbd/notifyd/notifyd.c | 214 ++++++++++++++++++-------
source3/smbd/notifyd/notifyd_db.c | 5 +-
source3/smbd/notifyd/notifyd_entry.c | 51 ++++--
source3/smbd/notifyd/notifyd_private.h | 46 ++++--
4 files changed, 228 insertions(+), 88 deletions(-)
diff --git a/source3/smbd/notifyd/notifyd.c b/source3/smbd/notifyd/notifyd.c
index ca303bd4d51..b368b8390fa 100644
--- a/source3/smbd/notifyd/notifyd.c
+++ b/source3/smbd/notifyd/notifyd.c
@@ -337,6 +337,7 @@ static bool notifyd_apply_rec_change(
struct messaging_context *msg_ctx)
{
struct db_record *rec = NULL;
+ struct notifyd_watcher watcher = {};
struct notifyd_instance *instances = NULL;
size_t num_instances;
size_t i;
@@ -344,6 +345,7 @@ static bool notifyd_apply_rec_change(
TDB_DATA value;
NTSTATUS status;
bool ok = false;
+ bool new_watcher = false;
if (pathlen == 0) {
DBG_WARNING("pathlen==0\n");
@@ -374,8 +376,12 @@ static bool notifyd_apply_rec_change(
value = dbwrap_record_get_value(rec);
if (value.dsize != 0) {
- if (!notifyd_parse_entry(value.dptr, value.dsize, NULL,
- &num_instances)) {
+ ok = notifyd_parse_entry(value.dptr,
+ value.dsize,
+ &watcher,
+ NULL,
+ &num_instances);
+ if (!ok) {
goto fail;
}
}
@@ -390,8 +396,22 @@ static bool notifyd_apply_rec_change(
goto fail;
}
- if (value.dsize != 0) {
- memcpy(instances, value.dptr, value.dsize);
+ if (num_instances > 0) {
+ struct notifyd_instance *tmp = NULL;
+ size_t num_tmp = 0;
+
+ ok = notifyd_parse_entry(value.dptr,
+ value.dsize,
+ NULL,
+ &tmp,
+ &num_tmp);
+ if (!ok) {
+ goto fail;
+ }
+
+ memcpy(instances,
+ tmp,
+ sizeof(struct notifyd_instance) * num_tmp);
}
for (i=0; i<num_instances; i++) {
@@ -414,41 +434,106 @@ static bool notifyd_apply_rec_change(
*instance = (struct notifyd_instance) {
.client = *client,
.instance = *chg,
- .internal_filter = chg->filter,
- .internal_subdir_filter = chg->subdir_filter
};
num_instances += 1;
}
- if ((instance->instance.filter != 0) ||
- (instance->instance.subdir_filter != 0)) {
- int ret;
+ /*
+ * Calculate an intersection of the instances filters for the watcher.
+ */
+ if (instance->instance.filter > 0) {
+ uint32_t filter = instance->instance.filter;
+
+ if ((watcher.filter & filter) != filter) {
+ watcher.filter |= filter;
+
+ new_watcher = true;
+ }
+ }
+
+ /*
+ * Calculate an intersection of the instances subdir_filters for the
+ * watcher.
+ */
+ if (instance->instance.subdir_filter > 0) {
+ uint32_t subdir_filter = instance->instance.subdir_filter;
- TALLOC_FREE(instance->sys_watch);
+ if ((watcher.subdir_filter & subdir_filter) != subdir_filter) {
+ watcher.subdir_filter |= subdir_filter;
- ret = sys_notify_watch(entries, sys_notify_ctx, path,
- &instance->internal_filter,
- &instance->internal_subdir_filter,
- notifyd_sys_callback, msg_ctx,
- &instance->sys_watch);
- if (ret != 0) {
- DBG_WARNING("sys_notify_watch for [%s] returned %s\n",
- path, strerror(errno));
+ new_watcher = true;
}
}
if ((instance->instance.filter == 0) &&
(instance->instance.subdir_filter == 0)) {
+ uint32_t tmp_filter = 0;
+ uint32_t tmp_subdir_filter = 0;
+
/* This is a delete request */
- TALLOC_FREE(instance->sys_watch);
*instance = instances[num_instances-1];
num_instances -= 1;
+
+ for (i = 0; i < num_instances; i++) {
+ struct notifyd_instance *tmp = &instances[i];
+
+ tmp_filter |= tmp->instance.filter;
+ tmp_subdir_filter |= tmp->instance.subdir_filter;
+ }
+
+ /*
+ * If the filter has changed, register a new watcher with the
+ * changed filter.
+ */
+ if (watcher.filter != tmp_filter ||
+ watcher.subdir_filter != tmp_subdir_filter)
+ {
+ watcher.filter = tmp_filter;
+ watcher.subdir_filter = tmp_subdir_filter;
+
+ new_watcher = true;
+ }
+ }
+
+ if (new_watcher) {
+ /*
+ * In case we removed all notify instances, we want to remove
+ * the watcher. We won't register a new one, if no filters are
+ * set anymore.
+ */
+
+ TALLOC_FREE(watcher.sys_watch);
+
+ watcher.sys_filter = watcher.filter;
+ watcher.sys_subdir_filter = watcher.subdir_filter;
+
+ /*
+ * Only register a watcher if we have filter.
+ */
+ if (watcher.filter != 0 || watcher.subdir_filter != 0) {
+ int ret = sys_notify_watch(entries,
+ sys_notify_ctx,
+ path,
+ &watcher.sys_filter,
+ &watcher.sys_subdir_filter,
+ notifyd_sys_callback,
+ msg_ctx,
+ &watcher.sys_watch);
+ if (ret != 0) {
+ DBG_WARNING("sys_notify_watch for [%s] "
+ "returned %s\n",
+ path,
+ strerror(errno));
+ }
+ }
}
DBG_DEBUG("%s has %zu instances\n", path, num_instances);
if (num_instances == 0) {
+ TALLOC_FREE(watcher.sys_watch);
+
status = dbwrap_record_delete(rec);
if (!NT_STATUS_IS_OK(status)) {
DBG_WARNING("dbwrap_record_delete returned %s\n",
@@ -456,13 +541,21 @@ static bool notifyd_apply_rec_change(
goto fail;
}
} else {
- value = make_tdb_data(
- (uint8_t *)instances,
- sizeof(struct notifyd_instance) * num_instances);
+ struct TDB_DATA iov[2] = {
+ {
+ .dptr = (uint8_t *)&watcher,
+ .dsize = sizeof(struct notifyd_watcher),
+ },
+ {
+ .dptr = (uint8_t *)instances,
+ .dsize = sizeof(struct notifyd_instance) *
+ num_instances,
+ },
+ };
- status = dbwrap_record_store(rec, value, 0);
+ status = dbwrap_record_storev(rec, iov, ARRAY_SIZE(iov), 0);
if (!NT_STATUS_IS_OK(status)) {
- DBG_WARNING("dbwrap_record_store returned %s\n",
+ DBG_WARNING("dbwrap_record_storev returned %s\n",
nt_errstr(status));
goto fail;
}
@@ -706,12 +799,18 @@ static void notifyd_trigger_parser(TDB_DATA key, TDB_DATA data,
.when = tstate->msg->when };
struct iovec iov[2];
size_t path_len = key.dsize;
+ struct notifyd_watcher watcher = {};
struct notifyd_instance *instances = NULL;
size_t num_instances = 0;
size_t i;
+ bool ok;
- if (!notifyd_parse_entry(data.dptr, data.dsize, &instances,
- &num_instances)) {
+ ok = notifyd_parse_entry(data.dptr,
+ data.dsize,
+ &watcher,
+ &instances,
+ &num_instances);
+ if (!ok) {
DBG_DEBUG("Could not parse notifyd_entry\n");
return;
}
@@ -734,9 +833,11 @@ static void notifyd_trigger_parser(TDB_DATA key, TDB_DATA data,
if (tstate->covered_by_sys_notify) {
if (tstate->recursive) {
- i_filter = instance->internal_subdir_filter;
+ i_filter = watcher.sys_subdir_filter &
+ instance->instance.subdir_filter;
} else {
- i_filter = instance->internal_filter;
+ i_filter = watcher.sys_filter &
+ instance->instance.filter;
}
} else {
if (tstate->recursive) {
@@ -1142,46 +1243,39 @@ static int notifyd_add_proxy_syswatches(struct db_record *rec,
struct db_context *db = dbwrap_record_get_db(rec);
TDB_DATA key = dbwrap_record_get_key(rec);
TDB_DATA value = dbwrap_record_get_value(rec);
- struct notifyd_instance *instances = NULL;
- size_t num_instances = 0;
- size_t i;
+ struct notifyd_watcher watcher = {};
char path[key.dsize+1];
bool ok;
+ int ret;
memcpy(path, key.dptr, key.dsize);
path[key.dsize] = '\0';
- ok = notifyd_parse_entry(value.dptr, value.dsize, &instances,
- &num_instances);
+ /* This is a remote database, we just need the watcher. */
+ ok = notifyd_parse_entry(value.dptr, value.dsize, &watcher, NULL, NULL);
if (!ok) {
DBG_WARNING("Could not parse notifyd entry for %s\n", path);
return 0;
}
- for (i=0; i<num_instances; i++) {
- struct notifyd_instance *instance = &instances[i];
- uint32_t filter = instance->instance.filter;
- uint32_t subdir_filter = instance->instance.subdir_filter;
- int ret;
+ watcher.sys_watch = NULL;
+ watcher.sys_filter = watcher.filter;
+ watcher.sys_subdir_filter = watcher.subdir_filter;
- /*
- * This is a remote database. Pointers that we were
- * given don't make sense locally. Initialize to NULL
- * in case sys_notify_watch fails.
- */
- instances[i].sys_watch = NULL;
-
- ret = state->sys_notify_watch(
- db, state->sys_notify_ctx, path,
- &filter, &subdir_filter,
- notifyd_sys_callback, state->msg_ctx,
- &instance->sys_watch);
- if (ret != 0) {
- DBG_WARNING("inotify_watch returned %s\n",
- strerror(errno));
- }
+ ret = state->sys_notify_watch(db,
+ state->sys_notify_ctx,
+ path,
+ &watcher.filter,
+ &watcher.subdir_filter,
+ notifyd_sys_callback,
+ state->msg_ctx,
+ &watcher.sys_watch);
+ if (ret != 0) {
+ DBG_WARNING("inotify_watch returned %s\n", strerror(errno));
}
+ memcpy(value.dptr, &watcher, sizeof(struct notifyd_watcher));
+
return 0;
}
@@ -1189,21 +1283,17 @@ static int notifyd_db_del_syswatches(struct db_record *rec, void *private_data)
{
TDB_DATA key = dbwrap_record_get_key(rec);
TDB_DATA value = dbwrap_record_get_value(rec);
- struct notifyd_instance *instances = NULL;
- size_t num_instances = 0;
- size_t i;
+ struct notifyd_watcher watcher = {};
bool ok;
- ok = notifyd_parse_entry(value.dptr, value.dsize, &instances,
- &num_instances);
+ ok = notifyd_parse_entry(value.dptr, value.dsize, &watcher, NULL, NULL);
if (!ok) {
DBG_WARNING("Could not parse notifyd entry for %.*s\n",
(int)key.dsize, (char *)key.dptr);
return 0;
}
- for (i=0; i<num_instances; i++) {
- TALLOC_FREE(instances[i].sys_watch);
- }
+ TALLOC_FREE(watcher.sys_watch);
+
return 0;
}
diff --git a/source3/smbd/notifyd/notifyd_db.c b/source3/smbd/notifyd/notifyd_db.c
index 18228619e9a..7dc3cd58081 100644
--- a/source3/smbd/notifyd/notifyd_db.c
+++ b/source3/smbd/notifyd/notifyd_db.c
@@ -40,7 +40,10 @@ static bool notifyd_parse_db_parser(TDB_DATA key, TDB_DATA value,
memcpy(path, key.dptr, key.dsize);
path[key.dsize] = 0;
- ok = notifyd_parse_entry(value.dptr, value.dsize, &instances,
+ ok = notifyd_parse_entry(value.dptr,
+ value.dsize,
+ NULL,
+ &instances,
&num_instances);
if (!ok) {
DBG_DEBUG("Could not parse entry for path %s\n", path);
diff --git a/source3/smbd/notifyd/notifyd_entry.c b/source3/smbd/notifyd/notifyd_entry.c
index 539010de03a..f3b0e908136 100644
--- a/source3/smbd/notifyd/notifyd_entry.c
+++ b/source3/smbd/notifyd/notifyd_entry.c
@@ -21,22 +21,51 @@
* Parse an entry in the notifyd_context->entries database
*/
-bool notifyd_parse_entry(
- uint8_t *buf,
- size_t buflen,
- struct notifyd_instance **instances,
- size_t *num_instances)
+/**
+ * @brief Parse a notifyd database entry.
+ *
+ * The memory we pass down needs to be aligned. If it isn't aligned we can run
+ * into obscure errors as we just point into the data buffer.
+ *
+ * @param data The data to parse
+ * @param data_len The length of the data to parse
+ * @param watcher A pointer to store the watcher data or NULL.
+ * @param instances A pointer to store the array of notify instances or NULL.
+ * @param pnum_instances The number of elements in the array. If you just want
+ * the number of elements pass NULL for the watcher and instances pointers.
+ *
+ * @return true on success, false if an error occurred.
+ */
+bool notifyd_parse_entry(uint8_t *data,
+ size_t data_len,
+ struct notifyd_watcher *watcher,
+ struct notifyd_instance **instances,
+ size_t *pnum_instances)
{
- if ((buflen % sizeof(struct notifyd_instance)) != 0) {
- DBG_WARNING("invalid buffer size: %zu\n", buflen);
+ size_t ilen;
+
+ if (data_len < sizeof(struct notifyd_watcher)) {
return false;
}
- if (instances != NULL) {
- *instances = (struct notifyd_instance *)buf;
+ if (watcher != NULL) {
+ *watcher = *((struct notifyd_watcher *)(uintptr_t)data);
}
- if (num_instances != NULL) {
- *num_instances = buflen / sizeof(struct notifyd_instance);
+
+ ilen = data_len - sizeof(struct notifyd_watcher);
+ if ((ilen % sizeof(struct notifyd_instance)) != 0) {
+ return false;
+ }
+
+ if (pnum_instances != NULL) {
+ *pnum_instances = ilen / sizeof(struct notifyd_instance);
}
+ if (instances != NULL) {
+ /* The (uintptr_t) cast removes a warning from -Wcast-align. */
+ *instances =
+ (struct notifyd_instance *)(uintptr_t)
+ (data + sizeof(struct notifyd_watcher));
+ }
+
return true;
}
diff --git a/source3/smbd/notifyd/notifyd_private.h b/source3/smbd/notifyd/notifyd_private.h
index 36c08f47c54..db8e6e1c005 100644
--- a/source3/smbd/notifyd/notifyd_private.h
+++ b/source3/smbd/notifyd/notifyd_private.h
@@ -20,30 +20,48 @@
#include "lib/util/server_id.h"
#include "notifyd.h"
+
/*
- * notifyd's representation of a notify instance
+ * Representation of a watcher for a path
+ *
+ * This will be stored in the db.
*/
-struct notifyd_instance {
- struct server_id client;
- struct notify_instance instance;
-
- void *sys_watch; /* inotify/fam/etc handle */
+struct notifyd_watcher {
+ /*
+ * This is an intersections of the filter the watcher is listening for.
+ */
+ uint32_t filter;
+ uint32_t subdir_filter;
/*
- * Filters after sys_watch took responsibility of some bits
+ * Those are inout variables passed to the sys_watcher. The sys_watcher
+ * will remove the bits it can't handle.
*/
- uint32_t internal_filter;
- uint32_t internal_subdir_filter;
+ uint32_t sys_filter;
+ uint32_t sys_subdir_filter;
+
+ /* The handle for inotify/fam etc. */
+ void *sys_watch;
+};
+
+/*
+ * Representation of a notifyd instance
+ *
+ * This will be stored in the db.
+ */
+struct notifyd_instance {
+ struct server_id client;
+ struct notify_instance instance;
};
/*
* Parse an entry in the notifyd_context->entries database
*/
-bool notifyd_parse_entry(
- uint8_t *buf,
- size_t buflen,
- struct notifyd_instance **instances,
- size_t *num_instances);
+bool notifyd_parse_entry(uint8_t *data,
+ size_t data_len,
+ struct notifyd_watcher *watcher,
+ struct notifyd_instance **instances,
+ size_t *num_instances);
#endif
--
2.51.0
From b04cb93ee52aac0ce7213d0581d69e852df52d4a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 5 Feb 2024 15:03:48 +0100
Subject: [PATCH 30/63] smbd: simplify handling of failing fstat() after
unlinking file
close_remove_share_mode() already called vfs_stat_fsp(), so we can skip the
fstat() triggered in fd_close() by fsp->fsp_flags.fstat_before_close being true.
This avoids getting an EACCESS error when doing an fstat() on the removed file
which seems to happen with some FUSE filesystems.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15527
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
(cherry picked from commit 6e6324cff29089a636823786183222a73fe7cb28)
---
source3/smbd/close.c | 1 +
source3/smbd/open.c | 15 +--------------
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index af5e78daa10..e16cb2d3485 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -603,6 +603,7 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp,
*/
fsp->fsp_flags.delete_on_close = false;
+ fsp->fsp_flags.fstat_before_close = false;
lck_state.reset_delete_on_close = true;
done:
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 3581c4b9173..93c12e00eb0 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -997,20 +997,7 @@ NTSTATUS fd_close(files_struct *fsp)
if (fsp->fsp_flags.fstat_before_close) {
status = vfs_stat_fsp(fsp);
if (!NT_STATUS_IS_OK(status)) {
- /*
- * If this is a stream and delete-on-close was set, the
- * backing object (an xattr from streams_xattr) might
- * already be deleted so fstat() fails with
- * NT_STATUS_NOT_FOUND. So if fsp refers to a stream we
- * ignore the error and only bail for normal files where
- * an fstat() should still work. NB. We cannot use
- * fsp_is_alternate_stream(fsp) for this as the base_fsp
- * has already been closed at this point and so the value
- * fsp_is_alternate_stream() checks for is already NULL.
- */
- if (fsp->fsp_name->stream_name == NULL) {
- return status;
- }
+ return status;
}
}
--
2.51.0
From 29f0c0fb2f1cb0cfc4c615d31e82048b46a2cb0d Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power@suse.com>
Date: Tue, 20 Feb 2024 09:26:29 +0000
Subject: [PATCH 31/63] s3/smbd: If we fail to close file_handle ensure we
should reset the fd
if fsp_flags.fstat_before_close == true then close_file_smb will call
vfs_stat which can fail. If it does fail then the fd associated
with the file handle will still be set (and we will hit an assert
is the file handle destructor) when calling file_free.
We need to set fd to -1 to avoid that. To achieve that we capture and
return the vfs_stat_fsp failure status while still processing the rest
of the fd_close logic.
[2024/02/20 09:23:48.454671, 0, pid=9744] ../../source3/smbd/smb2_close.c:226(smbd_smb2_close)
smbd_smb2_close: close_file[]: NT_STATUS_ACCESS_DENIED
[2024/02/20 09:23:48.454757, 0, pid=9744] ../../source3/smbd/fd_handle.c:40(fd_handle_destructor)
PANIC: assert failed at ../../source3/smbd/fd_handle.c(40): (fh->fd == -1) || (fh->fd == AT_FDCWD)
[2024/02/20 09:23:48.454781, 0, pid=9744] ../../lib/util/fault.c:178(smb_panic_log)
===============================================================
[2024/02/20 09:23:48.454804, 0, pid=9744] ../../lib/util/fault.c:185(smb_panic_log)
INTERNAL ERROR: assert failed: (fh->fd == -1) || (fh->fd == AT_FDCWD) in smbd (smbd[192.168.10) (client [192.168.100.15]) pid 9744 (4.21.0pre1-DEVELOPERBUILD)
[2024/02/20 09:23:48.454844, 0, pid=9744] ../../lib/util/fault.c:190(smb_panic_log)
If you are running a recent Samba version, and if you think this problem is not yet fixed in the latest versions, please consider reporting this bug, see https://wiki.samba.org/index.php/Bug_Reporting
[2024/02/20 09:23:48.454869, 0, pid=9744] ../../lib/util/fault.c:191(smb_panic_log)
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15527
Signed-off-by: Noel Power <noel.power@suse.com>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Noel Power <npower@samba.org>
Autobuild-Date(master): Wed Mar 13 10:34:45 UTC 2024 on atb-devel-224
(cherry picked from commit 6ee3f809a54d7b833ff798e68a93ada00a215d4d)
---
source3/smbd/open.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 93c12e00eb0..74be444fef5 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -987,7 +987,7 @@ NTSTATUS fd_openat(const struct files_struct *dirfsp,
NTSTATUS fd_close(files_struct *fsp)
{
- NTSTATUS status;
+ NTSTATUS stat_status = NT_STATUS_OK;
int ret;
if (fsp == fsp->conn->cwd_fsp) {
@@ -995,10 +995,12 @@ NTSTATUS fd_close(files_struct *fsp)
}
if (fsp->fsp_flags.fstat_before_close) {
- status = vfs_stat_fsp(fsp);
- if (!NT_STATUS_IS_OK(status)) {
- return status;
- }
+ /*
+ * capture status, if failure
+ * continue close processing
+ * and return status
+ */
+ stat_status = vfs_stat_fsp(fsp);
}
if (fsp->dptr) {
@@ -1020,7 +1022,7 @@ NTSTATUS fd_close(files_struct *fsp)
if (ret == -1) {
return map_nt_error_from_unix(errno);
}
- return NT_STATUS_OK;
+ return stat_status;
}
/****************************************************************************
--
2.51.0
From ed138c4d679e8291de18162e1cac65cc9da33b4d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Wed, 15 Jan 2025 10:21:19 -0800
Subject: [PATCH 32/63] auth: Add missing talloc_free() in error code path.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15782
Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
Autobuild-User(master): Günther Deschner <gd@samba.org>
Autobuild-Date(master): Thu Jan 16 14:32:39 UTC 2025 on atb-devel-224
(cherry picked from commit c514ce8dcadcbbf0d86f3038d2be0f9253a76b75)
---
auth/kerberos/kerberos_pac.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/auth/kerberos/kerberos_pac.c b/auth/kerberos/kerberos_pac.c
index b914075d85c..196654b36bd 100644
--- a/auth/kerberos/kerberos_pac.c
+++ b/auth/kerberos/kerberos_pac.c
@@ -351,6 +351,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (ret) {
DEBUG(5, ("PAC Decode: Failed to verify the service "
"signature: %s\n", error_message(ret)));
+ talloc_free(tmp_ctx);
return NT_STATUS_ACCESS_DENIED;
}
--
2.51.0
From f8a7d7a3e8c3be3c7742c874239766b34c25ef3e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Thu, 16 Jan 2025 16:12:31 -0800
Subject: [PATCH 33/63] auth: Cleanup exit code paths in kerberos_decode_pac().
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
One more memory leak missed and now fixed. tmp_ctx
must be freed once the pac data is talloc_move'd.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15782
Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz>
Reviewed-by: Christian Ambach <ambi@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
Autobuild-User(master): Günther Deschner <gd@samba.org>
Autobuild-Date(master): Fri Jan 17 12:01:47 UTC 2025 on atb-devel-224
(cherry picked from commit f9eb0b248da0689c82656f3e482161c45749afb6)
---
auth/kerberos/kerberos_pac.c | 88 ++++++++++++++++++------------------
1 file changed, 43 insertions(+), 45 deletions(-)
diff --git a/auth/kerberos/kerberos_pac.c b/auth/kerberos/kerberos_pac.c
index 196654b36bd..abb096bde1b 100644
--- a/auth/kerberos/kerberos_pac.c
+++ b/auth/kerberos/kerberos_pac.c
@@ -128,7 +128,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
time_t tgs_authtime,
struct PAC_DATA **pac_data_out)
{
- NTSTATUS status;
+ NTSTATUS status = NT_STATUS_NO_MEMORY;
enum ndr_err_code ndr_err;
krb5_error_code ret;
DATA_BLOB modified_pac_blob;
@@ -164,8 +164,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
kdc_sig_wipe = talloc(tmp_ctx, struct PAC_SIGNATURE_DATA);
srv_sig_wipe = talloc(tmp_ctx, struct PAC_SIGNATURE_DATA);
if (!pac_data_raw || !pac_data || !kdc_sig_wipe || !srv_sig_wipe) {
- talloc_free(tmp_ctx);
- return NT_STATUS_NO_MEMORY;
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
}
ndr_err = ndr_pull_struct_blob(&pac_data_blob, pac_data, pac_data,
@@ -174,15 +174,14 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't parse the PAC: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
if (pac_data->num_buffers < 4) {
/* we need logon_ingo, service_key and kdc_key */
DEBUG(0,("less than 4 PAC buffers\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
ndr_err = ndr_pull_struct_blob(
@@ -192,15 +191,14 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't parse the PAC: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
if (pac_data_raw->num_buffers < 4) {
/* we need logon_ingo, service_key and kdc_key */
DEBUG(0,("less than 4 PAC buffers\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
if (pac_data->num_buffers != pac_data_raw->num_buffers) {
@@ -208,8 +206,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
DEBUG(0, ("misparse! PAC_DATA has %d buffers while "
"PAC_DATA_RAW has %d\n", pac_data->num_buffers,
pac_data_raw->num_buffers));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
for (i=0; i < pac_data->num_buffers; i++) {
@@ -220,8 +218,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
DEBUG(0, ("misparse! PAC_DATA buffer %d has type "
"%d while PAC_DATA_RAW has %d\n", i,
data_buf->type, raw_buf->type));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
switch (data_buf->type) {
case PAC_TYPE_LOGON_INFO:
@@ -254,26 +252,26 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (!logon_info) {
DEBUG(0,("PAC no logon_info\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
if (!logon_name) {
DEBUG(0,("PAC no logon_name\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
if (!srv_sig_ptr || !srv_sig_blob) {
DEBUG(0,("PAC no srv_key\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
if (!kdc_sig_ptr || !kdc_sig_blob) {
DEBUG(0,("PAC no kdc_key\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
/* Find and zero out the signatures,
@@ -288,8 +286,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't parse the KDC signature: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
ndr_err = ndr_pull_struct_blob(
@@ -299,8 +296,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't parse the SRV signature: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
/* Now zero the decoded structure */
@@ -317,8 +313,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't repack the KDC signature: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
ndr_err = ndr_push_struct_blob(
srv_sig_blob, pac_data_raw, srv_sig_wipe,
@@ -327,8 +322,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't repack the SRV signature: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
/* push out the whole structure, but now with zero'ed signatures */
@@ -339,8 +333,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't repack the RAW PAC: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
if (service_keyblock) {
@@ -351,8 +344,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (ret) {
DEBUG(5, ("PAC Decode: Failed to verify the service "
"signature: %s\n", error_message(ret)));
- talloc_free(tmp_ctx);
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
if (krbtgt_keyblock) {
@@ -362,8 +355,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (ret) {
DEBUG(1, ("PAC Decode: Failed to verify the KDC signature: %s\n",
smb_get_krb5_error_message(context, ret, tmp_ctx)));
- talloc_free(tmp_ctx);
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
}
}
@@ -379,8 +372,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
nt_time_string(tmp_ctx, logon_name->logon_time)));
DEBUG(2, ("PAC Decode: Ticket: %s\n",
nt_time_string(tmp_ctx, tgs_authtime_nttime)));
- talloc_free(tmp_ctx);
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
}
@@ -392,8 +385,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (ret) {
DEBUG(2, ("Could not unparse name from ticket to match with name from PAC: [%s]:%s\n",
logon_name->account_name, error_message(ret)));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
bool_ret = strcmp(client_principal_string, logon_name->account_name) == 0;
@@ -404,8 +397,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
logon_name->account_name,
client_principal_string));
SAFE_FREE(client_principal_string);
- talloc_free(tmp_ctx);
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
SAFE_FREE(client_principal_string);
@@ -426,10 +419,15 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
}
if (pac_data_out) {
- *pac_data_out = talloc_steal(mem_ctx, pac_data);
+ *pac_data_out = talloc_move(mem_ctx, &pac_data);
}
- return NT_STATUS_OK;
+ status = NT_STATUS_OK;
+
+ out:
+
+ TALLOC_FREE(tmp_ctx);
+ return status;
}
NTSTATUS kerberos_pac_logon_info(TALLOC_CTX *mem_ctx,
--
2.51.0
From 9fd06d5c331f5babaf417cc7339d12854a79fe4b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 15 Feb 2024 17:29:46 +0100
Subject: [PATCH 34/63] s3:libsmb/dsgetdcname: use
NETLOGON_NT_VERSION_AVOID_NT4EMUL
In 2024 we always want an active directory response...
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15620
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit 2b66663c75cdb3bc1b6bc5b1736dd9d35b094b42)
---
source3/libsmb/dsgetdcname.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
index 280ccd585b0..6fcaa26810c 100644
--- a/source3/libsmb/dsgetdcname.c
+++ b/source3/libsmb/dsgetdcname.c
@@ -930,6 +930,11 @@ static NTSTATUS process_dc_netbios(TALLOC_CTX *mem_ctx,
name_type = NBT_NAME_PDC;
}
+ /*
+ * It's 2024 we always want an AD style response!
+ */
+ nt_version |= NETLOGON_NT_VERSION_AVOID_NT4EMUL;
+
nt_version |= map_ds_flags_to_nt_version(flags);
snprintf(my_acct_name,
--
2.51.0
From 58e28d056f2df0906ee77ccfb9b56e8a764b38b4 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 7 May 2024 14:53:24 +0000
Subject: [PATCH 35/63] s3:libsmb: allow store_cldap_reply() to work with a
ipv6 response
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15642
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Fri May 10 01:35:18 UTC 2024 on atb-devel-224
(cherry picked from commit 712ffbffc03c7dcd551c1e22815ebe7c0b9b45d2)
---
source3/libsmb/dsgetdcname.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
index 6fcaa26810c..da173e7bbb0 100644
--- a/source3/libsmb/dsgetdcname.c
+++ b/source3/libsmb/dsgetdcname.c
@@ -196,7 +196,29 @@ static NTSTATUS store_cldap_reply(TALLOC_CTX *mem_ctx,
/* FIXME */
r->sockaddr_size = 0x10; /* the w32 winsock addr size */
r->sockaddr.sockaddr_family = 2; /* AF_INET */
- r->sockaddr.pdc_ip = talloc_strdup(mem_ctx, addr);
+ if (is_ipaddress_v4(addr)) {
+ r->sockaddr.pdc_ip = talloc_strdup(mem_ctx, addr);
+ if (r->sockaddr.pdc_ip == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+ } else {
+ /*
+ * ndr_push_NETLOGON_SAM_LOGON_RESPONSE_EX will
+ * fail with an ipv6 address.
+ *
+ * This matches windows behaviour in the CLDAP
+ * response when NETLOGON_NT_VERSION_5EX_WITH_IP
+ * is used.
+ *
+ * Windows returns the ipv4 address of the ipv6
+ * server interface and falls back to 127.0.0.1
+ * if there's no ipv4 address.
+ */
+ r->sockaddr.pdc_ip = talloc_strdup(mem_ctx, "127.0.0.1");
+ if (r->sockaddr.pdc_ip == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+ }
ndr_err = ndr_push_struct_blob(&blob, mem_ctx, r,
(ndr_push_flags_fn_t)ndr_push_NETLOGON_SAM_LOGON_RESPONSE_EX);
--
2.51.0
From e4d5269b2359c670acdf0cba81248f148ae68c17 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 11 Oct 2024 13:32:22 +0000
Subject: [PATCH 36/63] s3:libsmb: let discover_dc_netbios() return
DOMAIN_CONTROLLER_NOT_FOUND
We may get NT_STATUS_NOT_FOUND when the name can't be resolved
and NT_STATUS_INVALID_ADDRESS if the system doesn't have ipv4
addresses...
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(cherry picked from commit e47ce1d10b13d8ef165c70984e6e490f4c2a64c2)
---
source3/libsmb/dsgetdcname.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
index da173e7bbb0..8278959dd7d 100644
--- a/source3/libsmb/dsgetdcname.c
+++ b/source3/libsmb/dsgetdcname.c
@@ -483,7 +483,19 @@ static NTSTATUS discover_dc_netbios(TALLOC_CTX *mem_ctx,
&count,
resolve_order);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10,("discover_dc_netbios: failed to find DC\n"));
+ NTSTATUS raw_status = status;
+
+ if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+ status = NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND;
+ }
+ if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_ADDRESS)) {
+ status = NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND;
+ }
+
+ DBG_DEBUG("failed to find DC for %s: %s => %s\n",
+ domain_name,
+ nt_errstr(raw_status),
+ nt_errstr(status));
return status;
}
--
2.51.0
From d90d2b0e985913247f43192cb94eec0efb3e9046 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gd@samba.org>
Date: Wed, 2 Jul 2025 21:59:48 +0200
Subject: [PATCH 37/63] s3-winbindd: Fix internal winbind dsgetdcname calls
w.r.t. domain name
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
when winbind calls to dsgetdcname internally, make sure to
prefer the DNS domain name if we have it. Makes DNS lookups much more
likely to succeed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15876
Guenther
Signed-off-by: Guenther Deschner <gd@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Mon Jul 7 10:44:37 UTC 2025 on atb-devel-224
(cherry picked from commit 2560c9b3224816ffd371a62103f65b3aca301ad5)
---
source3/winbindd/wb_queryuser.c | 17 +++++++++++++----
source3/winbindd/wb_sids2xids.c | 17 +++++++++++++----
source3/winbindd/wb_xids2sids.c | 12 +++++++++---
source3/winbindd/winbindd_dual.c | 6 +++++-
source3/winbindd/winbindd_proto.h | 1 +
source3/winbindd/winbindd_util.c | 19 +++++++++++++++++++
6 files changed, 60 insertions(+), 12 deletions(-)
diff --git a/source3/winbindd/wb_queryuser.c b/source3/winbindd/wb_queryuser.c
index c2758f1b76a..db8e946ba71 100644
--- a/source3/winbindd/wb_queryuser.c
+++ b/source3/winbindd/wb_queryuser.c
@@ -289,10 +289,19 @@ static void wb_queryuser_done(struct tevent_req *subreq)
if (NT_STATUS_EQUAL(result, NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND) &&
!state->tried_dclookup) {
- D_DEBUG("GetNssInfo got DOMAIN_CONTROLLER_NOT_FOUND, calling wb_dsgetdcname_send()\n");
- subreq = wb_dsgetdcname_send(
- state, state->ev, state->info->domain_name, NULL, NULL,
- DS_RETURN_DNS_NAME);
+ const char *domain_name = find_dns_domain_name(
+ state->info->domain_name);
+
+ D_DEBUG("GetNssInfo got DOMAIN_CONTROLLER_NOT_FOUND, calling "
+ "wb_dsgetdcname_send(%s)\n",
+ domain_name);
+
+ subreq = wb_dsgetdcname_send(state,
+ state->ev,
+ domain_name,
+ NULL,
+ NULL,
+ DS_RETURN_DNS_NAME);
if (tevent_req_nomem(subreq, req)) {
return;
}
diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c
index f0f6c23fc20..03e5e7e0258 100644
--- a/source3/winbindd/wb_sids2xids.c
+++ b/source3/winbindd/wb_sids2xids.c
@@ -612,13 +612,22 @@ static void wb_sids2xids_done(struct tevent_req *subreq)
!state->tried_dclookup) {
struct lsa_DomainInfo *d;
+ const char *domain_name = NULL;
- D_DEBUG("Domain controller not found. Calling wb_dsgetdcname_send() to get it.\n");
d = &state->idmap_doms.domains[state->dom_index];
- subreq = wb_dsgetdcname_send(
- state, state->ev, d->name.string, NULL, NULL,
- DS_RETURN_DNS_NAME);
+ domain_name = find_dns_domain_name(d->name.string);
+
+ D_DEBUG("Domain controller not found. Calling "
+ "wb_dsgetdcname_send(%s) to get it.\n",
+ domain_name);
+
+ subreq = wb_dsgetdcname_send(state,
+ state->ev,
+ domain_name,
+ NULL,
+ NULL,
+ DS_RETURN_DNS_NAME);
if (tevent_req_nomem(subreq, req)) {
return;
}
diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c
index 86bd7f9deab..6fcf524d94f 100644
--- a/source3/winbindd/wb_xids2sids.c
+++ b/source3/winbindd/wb_xids2sids.c
@@ -143,9 +143,15 @@ static void wb_xids2sids_dom_done(struct tevent_req *subreq)
if (NT_STATUS_EQUAL(result, NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND) &&
!state->tried_dclookup) {
- subreq = wb_dsgetdcname_send(
- state, state->ev, state->dom_map->name, NULL, NULL,
- DS_RETURN_DNS_NAME);
+ const char *domain_name = find_dns_domain_name(
+ state->dom_map->name);
+
+ subreq = wb_dsgetdcname_send(state,
+ state->ev,
+ domain_name,
+ NULL,
+ NULL,
+ DS_RETURN_DNS_NAME);
if (tevent_req_nomem(subreq, req)) {
return;
}
diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c
index 36562ab10b8..02a10e41537 100644
--- a/source3/winbindd/winbindd_dual.c
+++ b/source3/winbindd/winbindd_dual.c
@@ -532,6 +532,7 @@ static void wb_domain_request_trigger(struct tevent_req *req,
struct wb_domain_request_state *state = tevent_req_data(
req, struct wb_domain_request_state);
struct winbindd_domain *domain = state->domain;
+ const char *domain_name = NULL;
struct tevent_req *subreq = NULL;
size_t shortest_queue_length;
@@ -604,8 +605,11 @@ static void wb_domain_request_trigger(struct tevent_req *req,
* which is indicated by DS_RETURN_DNS_NAME.
* For NT4 domains we still get the netbios name.
*/
+
+ domain_name = find_dns_domain_name(state->domain->name);
+
subreq = wb_dsgetdcname_send(state, state->ev,
- state->domain->name,
+ domain_name,
NULL, /* domain_guid */
NULL, /* site_name */
DS_RETURN_DNS_NAME); /* flags */
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index 9b10f2c061a..4f7dc8a15d6 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -567,6 +567,7 @@ bool parse_sidlist(TALLOC_CTX *mem_ctx, const char *sidstr,
struct dom_sid **sids, uint32_t *num_sids);
bool parse_xidlist(TALLOC_CTX *mem_ctx, const char *xidstr,
struct unixid **pxids, uint32_t *pnum_xids);
+const char *find_dns_domain_name(const char *domain_name);
/* The following definitions come from winbindd/winbindd_wins.c */
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index fe93528787d..eca4116d0c8 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -2181,3 +2181,22 @@ fail:
TALLOC_FREE(xids);
return false;
}
+
+/**
+ * Helper to extract the DNS Domain Name from a struct winbindd_domain
+ */
+const char *find_dns_domain_name(const char *domain_name)
+{
+ struct winbindd_domain *wbdom = NULL;
+
+ wbdom = find_domain_from_name(domain_name);
+ if (wbdom == NULL) {
+ return domain_name;
+ }
+
+ if (wbdom->active_directory && wbdom->alt_name != NULL) {
+ return wbdom->alt_name;
+ }
+
+ return wbdom->name;
+}
--
2.51.0
From 7da6072ce95bca445368f6d0453247c8f92fcdf2 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 9 May 2025 09:38:41 +0200
Subject: [PATCH 38/63] s3:winbindd: avoid using any netlogon call to get a dc
name
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15876
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(backported from commit f86a4bf6848ade2db7229d182576db3320c3ece7)
---
source3/winbindd/winbindd_cm.c | 145 ---------------------------
source3/winbindd/winbindd_dual_srv.c | 105 +------------------
2 files changed, 5 insertions(+), 245 deletions(-)
diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index 2ebfb0f6dd8..195259daa43 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -475,135 +475,6 @@ static bool cm_is_ipc_credentials(struct cli_credentials *creds)
return ret;
}
-static bool get_dc_name_via_netlogon(struct winbindd_domain *domain,
- fstring dcname,
- struct sockaddr_storage *dc_ss,
- uint32_t request_flags)
-{
- struct winbindd_domain *our_domain = NULL;
- struct rpc_pipe_client *netlogon_pipe = NULL;
- NTSTATUS result;
- WERROR werr;
- TALLOC_CTX *mem_ctx;
- unsigned int orig_timeout;
- const char *tmp = NULL;
- const char *p;
- struct dcerpc_binding_handle *b;
-
- /* Hmmmm. We can only open one connection to the NETLOGON pipe at the
- * moment.... */
-
- if (IS_DC) {
- return False;
- }
-
- if (domain->primary) {
- return False;
- }
-
- our_domain = find_our_domain();
-
- if ((mem_ctx = talloc_init("get_dc_name_via_netlogon")) == NULL) {
- return False;
- }
-
- result = cm_connect_netlogon(our_domain, &netlogon_pipe);
- if (!NT_STATUS_IS_OK(result)) {
- talloc_destroy(mem_ctx);
- return False;
- }
-
- b = netlogon_pipe->binding_handle;
-
- /* This call can take a long time - allow the server to time out.
- 35 seconds should do it. */
-
- orig_timeout = rpccli_set_timeout(netlogon_pipe, 35000);
-
- if (our_domain->active_directory) {
- struct netr_DsRGetDCNameInfo *domain_info = NULL;
-
- /*
- * TODO request flags are not respected in the server
- * (and in some cases, like REQUIRE_PDC, causes an error)
- */
- result = dcerpc_netr_DsRGetDCName(b,
- mem_ctx,
- our_domain->dcname,
- domain->name,
- NULL,
- NULL,
- request_flags|DS_RETURN_DNS_NAME,
- &domain_info,
- &werr);
- if (NT_STATUS_IS_OK(result) && W_ERROR_IS_OK(werr)) {
- tmp = talloc_strdup(
- mem_ctx, domain_info->dc_unc);
- if (tmp == NULL) {
- DEBUG(0, ("talloc_strdup failed\n"));
- talloc_destroy(mem_ctx);
- return false;
- }
- if (domain->alt_name == NULL) {
- domain->alt_name = talloc_strdup(domain,
- domain_info->domain_name);
- if (domain->alt_name == NULL) {
- DEBUG(0, ("talloc_strdup failed\n"));
- talloc_destroy(mem_ctx);
- return false;
- }
- }
- if (domain->forest_name == NULL) {
- domain->forest_name = talloc_strdup(domain,
- domain_info->forest_name);
- if (domain->forest_name == NULL) {
- DEBUG(0, ("talloc_strdup failed\n"));
- talloc_destroy(mem_ctx);
- return false;
- }
- }
- }
- } else {
- result = dcerpc_netr_GetAnyDCName(b, mem_ctx,
- our_domain->dcname,
- domain->name,
- &tmp,
- &werr);
- }
-
- /* And restore our original timeout. */
- rpccli_set_timeout(netlogon_pipe, orig_timeout);
-
- if (!NT_STATUS_IS_OK(result)) {
- DEBUG(10,("dcerpc_netr_GetAnyDCName failed: %s\n",
- nt_errstr(result)));
- talloc_destroy(mem_ctx);
- return false;
- }
-
- if (!W_ERROR_IS_OK(werr)) {
- DEBUG(10,("dcerpc_netr_GetAnyDCName failed: %s\n",
- win_errstr(werr)));
- talloc_destroy(mem_ctx);
- return false;
- }
-
- /* dcerpc_netr_GetAnyDCName gives us a name with \\ */
- p = strip_hostname(tmp);
-
- fstrcpy(dcname, p);
-
- talloc_destroy(mem_ctx);
-
- DEBUG(10,("dcerpc_netr_GetAnyDCName returned %s\n", dcname));
-
- if (!resolve_name(dcname, dc_ss, 0x20, true)) {
- return False;
- }
-
- return True;
-}
-
/**
* Helper function to assemble trust password and account name
*/
@@ -1279,24 +1150,8 @@ static bool get_dcs(TALLOC_CTX *mem_ctx, struct winbindd_domain *domain,
struct samba_sockaddr *sa_list = NULL;
size_t salist_size = 0;
size_t i;
- bool is_our_domain;
enum security_types sec = (enum security_types)lp_security();
- is_our_domain = strequal(domain->name, lp_workgroup());
-
- /* If not our domain, get the preferred DC, by asking our primary DC */
- if ( !is_our_domain
- && get_dc_name_via_netlogon(domain, dcname, &ss, request_flags)
- && add_one_dc_unique(mem_ctx, domain->name, dcname, &ss, dcs,
- num_dcs) )
- {
- char addr[INET6_ADDRSTRLEN];
- print_sockaddr(addr, sizeof(addr), &ss);
- DEBUG(10, ("Retrieved DC %s at %s via netlogon\n",
- dcname, addr));
- return True;
- }
-
if ((sec == SEC_ADS) && (domain->alt_name != NULL)) {
char *sitename = NULL;
diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c
index f0fd18a8fa6..47c68257b12 100644
--- a/source3/winbindd/winbindd_dual_srv.c
+++ b/source3/winbindd/winbindd_dual_srv.c
@@ -662,106 +662,11 @@ NTSTATUS _wbint_QueryUserRidList(struct pipes_struct *p,
NTSTATUS _wbint_DsGetDcName(struct pipes_struct *p, struct wbint_DsGetDcName *r)
{
- struct winbindd_domain *domain = wb_child_domain();
- struct rpc_pipe_client *netlogon_pipe;
- struct netr_DsRGetDCNameInfo *dc_info;
- NTSTATUS status;
- WERROR werr;
- unsigned int orig_timeout;
- struct dcerpc_binding_handle *b;
- bool retry = false;
- bool try_dsrgetdcname = false;
-
- if (domain == NULL) {
- return dsgetdcname(p->mem_ctx, global_messaging_context(),
- r->in.domain_name, r->in.domain_guid,
- r->in.site_name ? r->in.site_name : "",
- r->in.flags,
- r->out.dc_info);
- }
-
- if (domain->active_directory) {
- try_dsrgetdcname = true;
- }
-
-reconnect:
- status = cm_connect_netlogon(domain, &netlogon_pipe);
-
- reset_cm_connection_on_error(domain, NULL, status);
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("Can't contact the NETLOGON pipe\n"));
- return status;
- }
-
- b = netlogon_pipe->binding_handle;
-
- /* This call can take a long time - allow the server to time out.
- 35 seconds should do it. */
-
- orig_timeout = rpccli_set_timeout(netlogon_pipe, 35000);
-
- if (try_dsrgetdcname) {
- status = dcerpc_netr_DsRGetDCName(b,
- p->mem_ctx, domain->dcname,
- r->in.domain_name, NULL, r->in.domain_guid,
- r->in.flags, r->out.dc_info, &werr);
- if (NT_STATUS_IS_OK(status) && W_ERROR_IS_OK(werr)) {
- goto done;
- }
- if (!retry &&
- reset_cm_connection_on_error(domain, NULL, status))
- {
- retry = true;
- goto reconnect;
- }
- try_dsrgetdcname = false;
- retry = false;
- }
-
- /*
- * Fallback to less capable methods
- */
-
- dc_info = talloc_zero(r->out.dc_info, struct netr_DsRGetDCNameInfo);
- if (dc_info == NULL) {
- status = NT_STATUS_NO_MEMORY;
- goto done;
- }
-
- if (r->in.flags & DS_PDC_REQUIRED) {
- status = dcerpc_netr_GetDcName(b,
- p->mem_ctx, domain->dcname,
- r->in.domain_name, &dc_info->dc_unc, &werr);
- } else {
- status = dcerpc_netr_GetAnyDCName(b,
- p->mem_ctx, domain->dcname,
- r->in.domain_name, &dc_info->dc_unc, &werr);
- }
-
- if (!retry && reset_cm_connection_on_error(domain, b, status)) {
- retry = true;
- goto reconnect;
- }
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("dcerpc_netr_Get[Any]DCName failed: %s\n",
- nt_errstr(status)));
- goto done;
- }
- if (!W_ERROR_IS_OK(werr)) {
- DEBUG(10, ("dcerpc_netr_Get[Any]DCName failed: %s\n",
- win_errstr(werr)));
- status = werror_to_ntstatus(werr);
- goto done;
- }
-
- *r->out.dc_info = dc_info;
- status = NT_STATUS_OK;
-
-done:
- /* And restore our original timeout. */
- rpccli_set_timeout(netlogon_pipe, orig_timeout);
-
- return status;
+ return dsgetdcname(p->mem_ctx, global_messaging_context(),
+ r->in.domain_name, r->in.domain_guid,
+ r->in.site_name ? r->in.site_name : "",
+ r->in.flags,
+ r->out.dc_info);
}
NTSTATUS _wbint_LookupRids(struct pipes_struct *p, struct wbint_LookupRids *r)
--
2.51.0
From ad54ceadacfbcf0d9c96ad773e50db96003e2c08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org>
Date: Wed, 23 Jul 2025 15:09:21 +0200
Subject: [PATCH 39/63] s3:winbindd: Resolve dc name using CLDAP also for
ROLE_IPA_DC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
server role ROLE_IPA_DC (introduced in e2d5b4d) needs special handling
in dcip_check_name(). We should resolve the DC name using:
- CLDAP in dcip_check_name_ads()
instead of:
- NETBIOS in nbt_getdc() that fails if Windows is not providing netbios.
The impacted environment has:
domain->alt_name = example.com
domain->active_directory = 1
security = USER
server role = ROLE_IPA_DC
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15891
Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Signed-off-by: Andreas Schneider <asn@samba.org>
Pair-programmed-with: Andreas Schneider <asn@samba.org>
Reviewed-by: Alexander Bokovoy <ab@samba.org>
(cherry picked from commit 4921c3304e5e0480e5bb80a757b3f04b3b92c3b1)
(cherry picked from commit fe8eafc289dfbb6f2b6c706f2a8a68186807d4f8)
---
source3/winbindd/winbindd_cm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index 195259daa43..86dbf68f033 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -1075,7 +1075,9 @@ static bool dcip_check_name(TALLOC_CTX *mem_ctx,
if ((lp_security() == SEC_ADS) && (domain->alt_name != NULL)) {
is_ad_domain = true;
- } else if (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC) {
+ } else if (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC ||
+ lp_server_role() == ROLE_IPA_DC)
+ {
is_ad_domain = domain->active_directory;
}
--
2.51.0
From b73efffbb02903427af2c2cc57171d4848ca11f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org>
Date: Mon, 4 Aug 2025 08:35:29 +0200
Subject: [PATCH 40/63] docs-xml: Make smb.conf 'server role' value consistent
with ROLE_IPA_DC in libparam
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15891
Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Alexander Bokovoy <ab@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(cherry picked from commit d88268102ade07fab345e04109818d97d8843a14)
(cherry picked from commit d14fa6eb96a9f296d386ff4864e4f016440f2ac8)
---
docs-xml/smbdotconf/security/serverrole.xml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs-xml/smbdotconf/security/serverrole.xml b/docs-xml/smbdotconf/security/serverrole.xml
index 4ea4e4751ee..40244e125ce 100644
--- a/docs-xml/smbdotconf/security/serverrole.xml
+++ b/docs-xml/smbdotconf/security/serverrole.xml
@@ -78,7 +78,7 @@
url="http://wiki.samba.org/index.php/Samba4/HOWTO">Samba4
HOWTO</ulink></para>
- <para><anchor id="IPA-DC"/><emphasis>SERVER ROLE = IPA DOMAIN CONTROLLER</emphasis></para>
+ <para><anchor id="IPA-DC"/><emphasis>SERVER ROLE = IPA PRIMARY DOMAIN CONTROLLER</emphasis></para>
<para>This mode of operation runs Samba in a hybrid mode for IPA
domain controller, providing forest trust to Active Directory.
--
2.51.0
From 832a4e31630fd441f8ab4325439f90d561cb8fa4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org>
Date: Mon, 4 Aug 2025 23:26:02 +0200
Subject: [PATCH 41/63] s3:netlogon: IPA DC is the PDC as well - allow
ROLE_IPA_DC in _netr_DsRGetForestTrustInformation()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15891
Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Alexander Bokovoy <ab@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(cherry picked from commit 1dbafcc4e4ff8f39af5ca737b30e9821413dd1f2)
(cherry picked from commit 00adb3104e745babb2c330fa9c9e324805395edb)
---
source3/rpc_server/netlogon/srv_netlog_nt.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c
index c5a4b0ef30c..7957d3ab34d 100644
--- a/source3/rpc_server/netlogon/srv_netlog_nt.c
+++ b/source3/rpc_server/netlogon/srv_netlog_nt.c
@@ -2613,7 +2613,10 @@ WERROR _netr_DsRGetForestTrustInformation(struct pipes_struct *p,
return WERR_INVALID_FLAGS;
}
- if ((r->in.flags & DS_GFTI_UPDATE_TDO) && (lp_server_role() != ROLE_DOMAIN_PDC)) {
+ if ((r->in.flags & DS_GFTI_UPDATE_TDO) &&
+ (lp_server_role() != ROLE_DOMAIN_PDC) &&
+ (lp_server_role() != ROLE_IPA_DC))
+ {
p->fault_state = DCERPC_FAULT_OP_RNG_ERROR;
return WERR_NERR_NOTPRIMARY;
}
--
2.51.0
From 8d5638581dfc539c8524d7a507e8cc8977e827a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org>
Date: Mon, 4 Aug 2025 23:28:24 +0200
Subject: [PATCH 42/63] s3:utils: Allow ROLE_IPA_DC to allow to use Kerberos in
gensec
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15891
Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Alexander Bokovoy <ab@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Tue Aug 5 14:51:51 UTC 2025 on atb-devel-224
(cherry picked from commit a4dff82e45308db3ccabac2a55c03d52f04d7b4d)
Autobuild-User(v4-22-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-22-test): Mon Aug 11 07:53:47 UTC 2025 on atb-devel-224
(cherry picked from commit 3364797676624aa9367076a69b2daf73870429ba)
---
source3/utils/ntlm_auth.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c
index cff3c53845f..2968ca47734 100644
--- a/source3/utils/ntlm_auth.c
+++ b/source3/utils/ntlm_auth.c
@@ -1341,7 +1341,11 @@ static NTSTATUS ntlm_auth_prepare_gensec_server(TALLOC_CTX *mem_ctx,
cli_credentials_set_conf(server_credentials, lp_ctx);
- if (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC || lp_security() == SEC_ADS || USE_KERBEROS_KEYTAB) {
+ if (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC ||
+ lp_server_role() == ROLE_IPA_DC ||
+ lp_security() == SEC_ADS ||
+ USE_KERBEROS_KEYTAB)
+ {
cli_credentials_set_kerberos_state(server_credentials,
CRED_USE_KERBEROS_DESIRED,
CRED_SPECIFIED);
--
2.51.0
From 3ef02a381cdc83549506e159ebc457730c06c547 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 22 Jul 2025 19:22:31 +0200
Subject: [PATCH 43/63] libads: fix get_kdc_ip_string()
Correctly handle the interaction between optionally passed in DC via
pss and DC lookup.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15876
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit 23f100f67c0586a940e91e9e1e6f42b804401322)
---
source3/libads/kerberos.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/source3/libads/kerberos.c b/source3/libads/kerberos.c
index f74d8eb567c..f324321c87b 100644
--- a/source3/libads/kerberos.c
+++ b/source3/libads/kerberos.c
@@ -523,10 +523,12 @@ static char *get_kdc_ip_string(char *mem_ctx,
DBG_DEBUG("%zu additional KDCs to test\n", num_dcs);
if (num_dcs == 0) {
/*
- * We do not have additional KDCs, but we have the one passed
- * in via `pss`. So just use that one and leave.
+ * We do not have additional KDCs, but if we have one passed
+ * in via `pss` just use that one, otherwise fail
*/
- result = talloc_move(mem_ctx, &kdc_str);
+ if (pss != NULL) {
+ result = talloc_move(mem_ctx, &kdc_str);
+ }
goto out;
}
@@ -567,6 +569,9 @@ static char *get_kdc_ip_string(char *mem_ctx,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10,("get_kdc_ip_string: cldap_multi_netlogon failed: "
"%s\n", nt_errstr(status)));
+ if (pss != NULL) {
+ result = talloc_move(mem_ctx, &kdc_str);
+ }
goto out;
}
--
2.51.0
From b0dbc167f85deabff2af5b18bc201e8db0d3b97d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 22 Jul 2025 19:16:14 +0200
Subject: [PATCH 44/63] winbindd: use find_domain_from_name_noinit() in
find_dns_domain_name()
Avoid triggering a connection to a DC of a trusted domain.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15876
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit 9ad2e59a464bb472da2071c61a254547b6497625)
---
source3/winbindd/winbindd_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index eca4116d0c8..3a7a9114988 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -2189,7 +2189,7 @@ const char *find_dns_domain_name(const char *domain_name)
{
struct winbindd_domain *wbdom = NULL;
- wbdom = find_domain_from_name(domain_name);
+ wbdom = find_domain_from_name_noinit(domain_name);
if (wbdom == NULL) {
return domain_name;
}
--
2.51.0
From 1961f54ce07f7dc3cfcae5c00b96b39109f08b3a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 19 Dec 2023 11:11:55 +0100
Subject: [PATCH 45/63] vfs_default: allow disabling /proc/fds and
RESOLVE_NO_SYMLINK at compile time
This will be used in CI to have a gitlab runner without all modern Linux
features we make use of as part of path processing:
- O_PATH
- openat2() with RESOLVE_NO_SYMLINKS
- somehow safely reopen an O_PATH file handle
That gives what a classix UNIX like AIX or Solaris offers feature wise.
Other OSes support other combinations of those features, but we leave the
exersize of possibly adding more runners supporting those combinations to the
reader.
The following list shows which features are available and used by Samba on a few
OSes:
| O_PATH | RESOLVE_NO_SYMLINKS | Safe reopen | CI covered
--------|----------------|---------------------|----------------------------
| Supported Used | Supported Used | Supported Used |
============================================================================
Linux | + + | + + | + + | +
FreeBSD | + + | + [1] - | + [2] - | -
AIX | - - | - - | - - | +
[1] via open() flag O_RESOLVE_BENEATH
[2] via open() flag O_EMPTY_PATH
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 5c2f96442a25a1725809a28b3719afbc0bd01830)
---
source3/modules/vfs_default.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 1d4b9b1a840..8d78831492f 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -52,6 +52,9 @@ static int vfswrap_connect(vfs_handle_struct *handle, const char *service, const
bool bval;
handle->conn->have_proc_fds = sys_have_proc_fds();
+#ifdef DISABLE_PROC_FDS
+ handle->conn->have_proc_fds = false;
+#endif
/*
* assume the kernel will support openat2(),
@@ -70,6 +73,9 @@ static int vfswrap_connect(vfs_handle_struct *handle, const char *service, const
handle->conn->open_how_resolve |=
VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS;
}
+#ifdef DISABLE_VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS
+ handle->conn->open_how_resolve &= ~VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS;
+#endif
return 0; /* Return >= 0 for success */
}
--
2.51.0
From 26de62a2a968dd5b73af296251b26112cdd533e5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 19 Dec 2023 11:12:49 +0100
Subject: [PATCH 46/63] CI: disable /proc/fds and RESOLVE_NO_SYMLINK in
samba-no-opath-build runner
This is a more sensible combination of missing Linux specific features:
- O_PATH
- openat2() with RESOLVE_NO_SYMLINKS
- somehow safely reopen an O_PATH file handle
Currently only O_PATH is disabled for these jobs, but that doesn't really match
and know OS.
The following list shows which features are available and used by Samba on a few
OSes:
| O_PATH | RESOLVE_NO_SYMLINKS | Safe reopen | CI covered
--------|----------------|---------------------|----------------------------
| Supported Used | Supported Used | Supported Used |
============================================================================
Linux | + + | + + | + + | +
FreeBSD | + + | + [1] - | + [2] - | -
AIX | - - | - - | - - | +
So by also disabling RESOLVE_NO_SYMLINKS and Safe Reopen, we cover classic UNIX
systems like AIX.
[1] via open() flag O_RESOLVE_BENEATH
[2] via open() flag O_EMPTY_PATH
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 62cbe145c7e500c4759ed2005c78bd5056c87f43)
---
script/autobuild.py | 2 +-
selftest/skip.opath-required | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/script/autobuild.py b/script/autobuild.py
index e074c39d3c0..85043032d73 100755
--- a/script/autobuild.py
+++ b/script/autobuild.py
@@ -296,7 +296,7 @@ tasks = {
"samba-no-opath-build": {
"git-clone-required": True,
"sequence": [
- ("configure", "ADDITIONAL_CFLAGS='-DDISABLE_OPATH=1' ./configure.developer --without-ad-dc " + samba_configure_params),
+ ("configure", "ADDITIONAL_CFLAGS='-DDISABLE_OPATH=1 -DDISABLE_VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS=1 -DDISABLE_PROC_FDS=1' ./configure.developer --without-ad-dc " + samba_configure_params),
("make", "make -j"),
("check-clean-tree", CLEAN_SOURCE_TREE_CMD),
("chmod-R-a-w", "chmod -R a-w ."),
diff --git a/selftest/skip.opath-required b/selftest/skip.opath-required
index c3a13f5ec6e..67764a0b027 100644
--- a/selftest/skip.opath-required
+++ b/selftest/skip.opath-required
@@ -14,3 +14,9 @@
# available this works fine. So for now restrict testing posix
# extensions to environments where we have O_PATH around
^samba.tests.smb1posix
+
+# These don't work without /proc/fd support
+^samba3.blackbox.test_symlink_traversal.*\(fileserver\)
+^samba3.blackbox.shadow_copy_torture.*\(fileserver\)
+^samba3.blackbox.virus_scanner.*\(fileserver:local\)
+^samba3.blackbox.shadow_copy2.*\(fileserver.*\)
--
2.51.0
From 2c27aae5a4c8d7368dc142fb2be36919296d2a02 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 2 Jan 2024 12:49:14 +0100
Subject: [PATCH 47/63] smbd: pass symlink target path to
safe_symlink_target_path()
Moves processing the symlink error response to the caller
filename_convert_dirfsp(). Prepares for using this in
non_widelink_open(), where it will replace symlink_target_below_conn()
with the same functionality.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(back-ported from commit 0515dded4ddb49e5570ae7df51126af1a2d643de)
---
source3/include/proto.h | 5 +++
source3/smbd/filename.c | 72 +++++++++++++++++++----------------------
2 files changed, 38 insertions(+), 39 deletions(-)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index 8eed81d8f2e..13240033bf1 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -719,6 +719,11 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx,
const SMB_STRUCT_STAT *psbuf,
NTTIME twrp,
uint32_t flags);
+NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx,
+ const char *connectpath,
+ const char *target,
+ size_t unparsed,
+ char **_relative);
NTSTATUS filename_convert_dirfsp(
TALLOC_CTX *ctx,
connection_struct *conn,
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 8693dcf1153..45fb90381e2 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -942,44 +942,34 @@ static char *symlink_target_path(
return ret;
}
-static NTSTATUS safe_symlink_target_path(
- TALLOC_CTX *mem_ctx,
- const char *connectpath,
- const char *name_in,
- const char *substitute,
- size_t unparsed,
- char **_name_out)
+NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx,
+ const char *connectpath,
+ const char *target,
+ size_t unparsed,
+ char **_relative)
{
- char *target = NULL;
char *abs_target = NULL;
char *abs_target_canon = NULL;
const char *relative = NULL;
- char *name_out = NULL;
- NTSTATUS status = NT_STATUS_NO_MEMORY;
bool in_share;
+ NTSTATUS status = NT_STATUS_NO_MEMORY;
- target = symlink_target_path(mem_ctx, name_in, substitute, unparsed);
- if (target == NULL) {
- goto fail;
- }
-
- DBG_DEBUG("name_in: %s, substitute: %s, unparsed: %zu, target=%s\n",
- name_in,
- substitute,
- unparsed,
- target);
+ DBG_DEBUG("connectpath [%s] target [%s] unparsed [%zu]\n",
+ connectpath, target, unparsed);
if (target[0] == '/') {
- abs_target = target;
+ abs_target = talloc_strdup(mem_ctx, target);
} else {
- abs_target = talloc_asprintf(
- target, "%s/%s", connectpath, target);
- if (abs_target == NULL) {
- goto fail;
- }
+ abs_target = talloc_asprintf(mem_ctx,
+ "%s/%s",
+ connectpath,
+ target);
+ }
+ if (abs_target == NULL) {
+ goto fail;
}
- abs_target_canon = canonicalize_absolute_path(target, abs_target);
+ abs_target_canon = canonicalize_absolute_path(abs_target, abs_target);
if (abs_target_canon == NULL) {
goto fail;
}
@@ -994,15 +984,14 @@ static NTSTATUS safe_symlink_target_path(
goto fail;
}
- name_out = talloc_strdup(mem_ctx, relative);
- if (name_out == NULL) {
+ *_relative = talloc_strdup(mem_ctx, relative);
+ if (*_relative == NULL) {
goto fail;
}
status = NT_STATUS_OK;
- *_name_out = name_out;
fail:
- TALLOC_FREE(target);
+ TALLOC_FREE(abs_target);
return status;
}
@@ -1438,6 +1427,7 @@ NTSTATUS filename_convert_dirfsp(
size_t unparsed = 0;
NTSTATUS status;
char *target = NULL;
+ char *safe_target = NULL;
size_t symlink_redirects = 0;
next:
@@ -1476,17 +1466,21 @@ next:
* resolve all symlinks locally.
*/
- status = safe_symlink_target_path(
- mem_ctx,
- conn->connectpath,
- name_in,
- substitute,
- unparsed,
- &target);
+ target = symlink_target_path(mem_ctx, name_in, substitute, unparsed);
+ if (target == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ status = safe_symlink_target_path(mem_ctx,
+ conn->connectpath,
+ target,
+ unparsed,
+ &safe_target);
+ TALLOC_FREE(target);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
- name_in = target;
+ name_in = safe_target;
symlink_redirects += 1;
--
2.51.0
From 99d7e841d4e18f760c137530bbed0dea6115311a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 2 Jan 2024 13:25:25 +0100
Subject: [PATCH 48/63] smbd: add a directory argument to
safe_symlink_target_path()
Existing caller passes NULL, no change in behaviour. Prepares for
replacing symlink_target_below_conn() in open.c.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit fc80c72d658a41fe4d93b24b793b52c91b350175)
---
source3/include/proto.h | 1 +
source3/smbd/filename.c | 15 ++++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index 13240033bf1..15c5839caf8 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -721,6 +721,7 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx,
uint32_t flags);
NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx,
const char *connectpath,
+ const char *dir,
const char *target,
size_t unparsed,
char **_relative);
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 45fb90381e2..55a49e0ba93 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -944,6 +944,7 @@ static char *symlink_target_path(
NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx,
const char *connectpath,
+ const char *dir,
const char *target,
size_t unparsed,
char **_relative)
@@ -959,10 +960,21 @@ NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx,
if (target[0] == '/') {
abs_target = talloc_strdup(mem_ctx, target);
- } else {
+ } else if (dir == NULL) {
+ abs_target = talloc_asprintf(mem_ctx,
+ "%s/%s",
+ connectpath,
+ target);
+ } else if (dir[0] == '/') {
abs_target = talloc_asprintf(mem_ctx,
"%s/%s",
+ dir,
+ target);
+ } else {
+ abs_target = talloc_asprintf(mem_ctx,
+ "%s/%s/%s",
connectpath,
+ dir,
target);
}
if (abs_target == NULL) {
@@ -1473,6 +1485,7 @@ next:
status = safe_symlink_target_path(mem_ctx,
conn->connectpath,
+ NULL,
target,
unparsed,
&safe_target);
--
2.51.0
From 5041a6fa5cdfd21bf697249d900ea5c107d355a2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 2 Jan 2024 14:34:26 +0100
Subject: [PATCH 49/63] smbd: use safe_symlink_target_path() in
symlink_target_below_conn()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 1965fc77b3852a0593e13897af08f5304a1ce3a2)
---
selftest/skip.opath-required | 2 -
source3/smbd/open.c | 73 +++++++-----------------------------
2 files changed, 14 insertions(+), 61 deletions(-)
diff --git a/selftest/skip.opath-required b/selftest/skip.opath-required
index 67764a0b027..9c6ba481cdf 100644
--- a/selftest/skip.opath-required
+++ b/selftest/skip.opath-required
@@ -16,7 +16,5 @@
^samba.tests.smb1posix
# These don't work without /proc/fd support
-^samba3.blackbox.test_symlink_traversal.*\(fileserver\)
^samba3.blackbox.shadow_copy_torture.*\(fileserver\)
^samba3.blackbox.virus_scanner.*\(fileserver:local\)
-^samba3.blackbox.shadow_copy2.*\(fileserver.*\)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 74be444fef5..6582bd60245 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -571,7 +571,6 @@ out:
static NTSTATUS symlink_target_below_conn(
TALLOC_CTX *mem_ctx,
const char *connection_path,
- size_t connection_path_len,
struct files_struct *fsp,
struct files_struct *dirfsp,
struct smb_filename *symlink_name,
@@ -579,9 +578,7 @@ static NTSTATUS symlink_target_below_conn(
{
char *target = NULL;
char *absolute = NULL;
- const char *relative = NULL;
NTSTATUS status;
- bool ok;
if (fsp_get_pathref_fd(fsp) != -1) {
/*
@@ -594,69 +591,28 @@ static NTSTATUS symlink_target_below_conn(
talloc_tos(), dirfsp, symlink_name, &target);
}
+ status = safe_symlink_target_path(talloc_tos(),
+ connection_path,
+ dirfsp->fsp_name->base_name,
+ target,
+ 0,
+ &absolute);
if (!NT_STATUS_IS_OK(status)) {
- DBG_DEBUG("readlink_talloc failed: %s\n", nt_errstr(status));
+ DBG_DEBUG("safe_symlink_target_path() failed: %s\n",
+ nt_errstr(status));
return status;
}
- if (target[0] != '/') {
- char *tmp = talloc_asprintf(
- talloc_tos(),
- "%s/%s/%s",
- connection_path,
- dirfsp->fsp_name->base_name,
- target);
-
- TALLOC_FREE(target);
-
- if (tmp == NULL) {
- return NT_STATUS_NO_MEMORY;
- }
- target = tmp;
- }
-
- DBG_DEBUG("redirecting to %s\n", target);
-
- absolute = canonicalize_absolute_path(talloc_tos(), target);
- TALLOC_FREE(target);
-
- if (absolute == NULL) {
- return NT_STATUS_NO_MEMORY;
- }
-
- /*
- * We're doing the "below connection_path" here because it's
- * cheap. It might be that we get a symlink out of the share,
- * pointing to yet another symlink getting us back into the
- * share. If we need that, we would have to remove the check
- * here.
- */
- ok = subdir_of(
- connection_path,
- connection_path_len,
- absolute,
- &relative);
- if (!ok) {
- DBG_NOTICE("Bad access attempt: %s is a symlink "
- "outside the share path\n"
- "conn_rootdir =%s\n"
- "resolved_name=%s\n",
- symlink_name->base_name,
- connection_path,
- absolute);
- TALLOC_FREE(absolute);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
- }
-
- if (relative[0] == '\0') {
+ if (absolute[0] == '\0') {
/*
* special case symlink to share root: "." is our
* share root filename
*/
- absolute[0] = '.';
- absolute[1] = '\0';
- } else {
- memmove(absolute, relative, strlen(relative)+1);
+ TALLOC_FREE(absolute);
+ absolute = talloc_strdup(talloc_tos(), ".");
+ if (absolute == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
}
*_target = absolute;
@@ -834,7 +790,6 @@ again:
status = symlink_target_below_conn(
talloc_tos(),
connpath,
- connpath_len,
fsp,
discard_const_p(files_struct, dirfsp),
smb_fname_rel,
--
2.51.0
From f2fc99f0c7d441115a486413f345c0226a00b38b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 18 Dec 2023 12:35:58 +0100
Subject: [PATCH 50/63] smbd: use dirfsp and atname in open_directory()
On systems without /proc/fd support this avoid the expensive chdir()
logic in non_widelink_open(). open_file_ntcreate() already passes
dirfsp and atname to reopen_from_fsp(), it was just missed in the
conversion.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549
Reviewed-by: Volker Lendecke <vl@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Mon Jan 22 12:00:56 UTC 2024 on atb-devel-224
(cherry picked from commit 2713023250f15cf9971d88620cab9dd4afd0dc73)
Autobuild-User(v4-19-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-19-test): Mon Jan 29 11:59:41 UTC 2024 on atb-devel-224
---
source3/smbd/open.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 6582bd60245..b9849f82396 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -4865,8 +4865,8 @@ static NTSTATUS open_directory(connection_struct *conn,
if (access_mask & need_fd_access) {
status = reopen_from_fsp(
- fsp->conn->cwd_fsp,
- fsp->fsp_name,
+ parent_dir_fname->fsp,
+ smb_fname_atname,
fsp,
O_RDONLY | O_DIRECTORY,
0,
--
2.51.0
From 7d102268ebbebf6fc723a43485a82f72069d00ee Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Fri, 16 Dec 2022 16:35:00 +0100
Subject: [PATCH 51/63] smbd: Return open_symlink_err from
filename_convert_dirfsp_nosymlink()
Don't lose information returned from openat_pathref_fsp_nosymlink()
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(backported from commit c81d1d3fe4e3aeb2761dc539e8fb87d2ad862e5f)
Related to: https://bugzilla.samba.org/show_bug.cgi?id=15549
---
source3/smbd/filename.c | 77 +++++++++++++++++------------------------
1 file changed, 32 insertions(+), 45 deletions(-)
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 55a49e0ba93..9fd85af992a 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -944,37 +944,35 @@ static char *symlink_target_path(
NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx,
const char *connectpath,
- const char *dir,
- const char *target,
+ const char *name_in,
+ const char *substitute,
size_t unparsed,
char **_relative)
{
+ char *target = NULL;
char *abs_target = NULL;
char *abs_target_canon = NULL;
const char *relative = NULL;
bool in_share;
NTSTATUS status = NT_STATUS_NO_MEMORY;
+ target = symlink_target_path(mem_ctx,
+ name_in,
+ substitute,
+ unparsed);
+ if (target == NULL) {
+ goto fail;
+ }
+
DBG_DEBUG("connectpath [%s] target [%s] unparsed [%zu]\n",
connectpath, target, unparsed);
if (target[0] == '/') {
- abs_target = talloc_strdup(mem_ctx, target);
- } else if (dir == NULL) {
- abs_target = talloc_asprintf(mem_ctx,
- "%s/%s",
- connectpath,
- target);
- } else if (dir[0] == '/') {
- abs_target = talloc_asprintf(mem_ctx,
- "%s/%s",
- dir,
- target);
+ abs_target = target;
} else {
- abs_target = talloc_asprintf(mem_ctx,
- "%s/%s/%s",
+ abs_target = talloc_asprintf(target,
+ "%s/%s",
connectpath,
- dir,
target);
}
if (abs_target == NULL) {
@@ -1019,8 +1017,7 @@ static NTSTATUS filename_convert_dirfsp_nosymlink(
NTTIME twrp,
struct files_struct **_dirfsp,
struct smb_filename **_smb_fname,
- char **_substitute,
- size_t *_unparsed)
+ struct open_symlink_err **_symlink_err)
{
struct smb_filename *smb_dirname = NULL;
struct smb_filename *smb_fname_rel = NULL;
@@ -1142,11 +1139,8 @@ static NTSTATUS filename_convert_dirfsp_nosymlink(
SMB_ASSERT(name_in_len >= dirname_len);
- *_substitute = talloc_move(
- mem_ctx,
- &symlink_err->reparse->substitute_name);
- *_unparsed = symlink_err->unparsed +
- (name_in_len - dirname_len);
+ symlink_err->unparsed += (name_in_len - dirname_len);
+ *_symlink_err = symlink_err;
goto fail;
}
@@ -1435,10 +1429,9 @@ NTSTATUS filename_convert_dirfsp(
struct files_struct **_dirfsp,
struct smb_filename **_smb_fname)
{
- char *substitute = NULL;
- size_t unparsed = 0;
+ struct open_symlink_err *symlink_err = NULL;
NTSTATUS status;
- char *target = NULL;
+ char *substitute = NULL;
char *safe_target = NULL;
size_t symlink_redirects = 0;
@@ -1447,16 +1440,14 @@ next:
return NT_STATUS_OBJECT_PATH_NOT_FOUND;
}
- status = filename_convert_dirfsp_nosymlink(
- mem_ctx,
- conn,
- name_in,
- ucf_flags,
- twrp,
- _dirfsp,
- _smb_fname,
- &substitute,
- &unparsed);
+ status = filename_convert_dirfsp_nosymlink(mem_ctx,
+ conn,
+ name_in,
+ ucf_flags,
+ twrp,
+ _dirfsp,
+ _smb_fname,
+ &symlink_err);
if (!NT_STATUS_EQUAL(status, NT_STATUS_STOPPED_ON_SYMLINK)) {
return status;
@@ -1477,19 +1468,15 @@ next:
* UCF_POSIX_PATHNAMES set to cause the client to
* resolve all symlinks locally.
*/
-
- target = symlink_target_path(mem_ctx, name_in, substitute, unparsed);
- if (target == NULL) {
- return NT_STATUS_NO_MEMORY;
- }
+ substitute = symlink_err->reparse->substitute_name;
status = safe_symlink_target_path(mem_ctx,
conn->connectpath,
- NULL,
- target,
- unparsed,
+ name_in,
+ substitute,
+ symlink_err->unparsed,
&safe_target);
- TALLOC_FREE(target);
+ TALLOC_FREE(symlink_err);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
--
2.51.0
From edaabc3d53fddd9e2fa6168c8bf01ebfbf229657 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 25 Apr 2024 15:24:57 +0200
Subject: [PATCH 52/63] s3/lib: add next helper variable in server_id_watch_*
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit d76edcd48437715c7541b5b1e6a56245c25f460b)
---
source3/lib/server_id_watch.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/source3/lib/server_id_watch.c b/source3/lib/server_id_watch.c
index f0189e0e896..50b35f27b3e 100644
--- a/source3/lib/server_id_watch.c
+++ b/source3/lib/server_id_watch.c
@@ -27,6 +27,7 @@
struct server_id_watch_state {
struct tevent_context *ev;
struct server_id pid;
+ struct timeval start;
};
static void server_id_watch_waited(struct tevent_req *subreq);
@@ -37,6 +38,7 @@ struct tevent_req *server_id_watch_send(TALLOC_CTX *mem_ctx,
{
struct tevent_req *req, *subreq;
struct server_id_watch_state *state;
+ struct timeval next;
req = tevent_req_create(mem_ctx, &state, struct server_id_watch_state);
if (req == NULL) {
@@ -44,14 +46,15 @@ struct tevent_req *server_id_watch_send(TALLOC_CTX *mem_ctx,
}
state->ev = ev;
state->pid = pid;
+ state->start = tevent_timeval_current();
if (!serverid_exists(&state->pid)) {
tevent_req_done(req);
return tevent_req_post(req, ev);
}
- subreq = tevent_wakeup_send(
- state, ev, tevent_timeval_current_ofs(0, 500000));
+ next = tevent_timeval_add(&state->start, 0, 500000);
+ subreq = tevent_wakeup_send(state, ev, next);
if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
}
@@ -66,6 +69,8 @@ static void server_id_watch_waited(struct tevent_req *subreq)
subreq, struct tevent_req);
struct server_id_watch_state *state = tevent_req_data(
req, struct server_id_watch_state);
+ struct timeval now;
+ struct timeval next;
bool ok;
ok = tevent_wakeup_recv(subreq);
@@ -80,8 +85,9 @@ static void server_id_watch_waited(struct tevent_req *subreq)
return;
}
- subreq = tevent_wakeup_send(
- state, state->ev, tevent_timeval_current_ofs(0, 500000));
+ now = tevent_timeval_current();
+ next = tevent_timeval_add(&now, 0, 500000);
+ subreq = tevent_wakeup_send(state, state->ev, next);
if (tevent_req_nomem(subreq, req)) {
return;
}
--
2.51.0
From c25f1811c2ccaa2d5cc8005597fb9979aa1102ee Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 4 Apr 2024 12:31:05 +0200
Subject: [PATCH 53/63] s3/lib: add option "serverid watch:debug = yes" to
print kernel stack of hanging process
We only do if sys_have_proc_fds() returns true, so it's most likely
linux...
Enabled by default with log level 10...
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit 5c57e840527432c4b1a7ec94894939022a9e9622)
---
source3/lib/server_id_watch.c | 62 +++++++++++++++++++++++++++++++++--
1 file changed, 59 insertions(+), 3 deletions(-)
diff --git a/source3/lib/server_id_watch.c b/source3/lib/server_id_watch.c
index 50b35f27b3e..c372ec8c431 100644
--- a/source3/lib/server_id_watch.c
+++ b/source3/lib/server_id_watch.c
@@ -17,17 +17,18 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include "replace.h"
-#include <tevent.h>
-#include <talloc.h>
+#include "includes.h"
#include "serverid.h"
#include "server_id_watch.h"
+#include "lib/util/server_id.h"
#include "lib/util/tevent_unix.h"
struct server_id_watch_state {
struct tevent_context *ev;
struct server_id pid;
struct timeval start;
+ struct timeval warn;
+ bool debug;
};
static void server_id_watch_waited(struct tevent_req *subreq);
@@ -47,6 +48,12 @@ struct tevent_req *server_id_watch_send(TALLOC_CTX *mem_ctx,
state->ev = ev;
state->pid = pid;
state->start = tevent_timeval_current();
+ state->warn = tevent_timeval_add(&state->start, 10, 0);
+
+ state->debug = lp_parm_bool(GLOBAL_SECTION_SNUM,
+ "serverid watch",
+ "debug",
+ CHECK_DEBUGLVL(DBGLVL_DEBUG));
if (!serverid_exists(&state->pid)) {
tevent_req_done(req);
@@ -86,6 +93,55 @@ static void server_id_watch_waited(struct tevent_req *subreq)
}
now = tevent_timeval_current();
+
+ if (!state->debug) {
+ goto next;
+ }
+
+ if (timeval_compare(&state->warn, &now) == -1) {
+ double duration = timeval_elapsed2(&state->start, &now);
+ char proc_path[64] = { 0, };
+ char *kstack = NULL;
+ struct server_id_buf buf;
+ const char *pid = server_id_str_buf(state->pid, &buf);
+ int ret;
+
+ state->warn = tevent_timeval_add(&now, 10, 0);
+
+ if (!procid_is_local(&state->pid) || !sys_have_proc_fds()) {
+ DBG_ERR("Process %s hanging for %f seconds?\n",
+ pid, duration);
+ goto next;
+ }
+
+ ret = snprintf(proc_path,
+ ARRAY_SIZE(proc_path),
+ "/proc/%" PRIu64 "/stack",
+ state->pid.pid);
+ if (ret < 0) {
+ DBG_ERR("Process %s hanging for %f seconds?\n"
+ "snprintf failed\n",
+ pid, duration);
+ goto next;
+ }
+
+ become_root();
+ kstack = file_load(proc_path, NULL, 0, state);
+ unbecome_root();
+ if (kstack == NULL) {
+ DBG_ERR("Process %s hanging for %f seconds?\n"
+ "file_load [%s] failed\n",
+ pid, duration, proc_path);
+ goto next;
+ }
+
+ DBG_ERR("Process %s hanging for %f seconds?\n"
+ "%s:\n%s",
+ pid, duration, proc_path, kstack);
+ TALLOC_FREE(kstack);
+ }
+
+next:
next = tevent_timeval_add(&now, 0, 500000);
subreq = tevent_wakeup_send(state, state->ev, next);
if (tevent_req_nomem(subreq, req)) {
--
2.51.0
From 23dbf8f0317810d65e716a3c9b947c7a6549cb46 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 25 Apr 2024 15:17:08 +0200
Subject: [PATCH 54/63] s3/lib: add option "serverid watch:debug script"
This takes just PID and NODE:PID on a cluster.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit 7add7dbf1aee13b4d9ab70d1a5312c8ff30d9e00)
---
source3/lib/server_id_watch.c | 52 +++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/source3/lib/server_id_watch.c b/source3/lib/server_id_watch.c
index c372ec8c431..8ddf9c6b1c8 100644
--- a/source3/lib/server_id_watch.c
+++ b/source3/lib/server_id_watch.c
@@ -100,6 +100,7 @@ static void server_id_watch_waited(struct tevent_req *subreq)
if (timeval_compare(&state->warn, &now) == -1) {
double duration = timeval_elapsed2(&state->start, &now);
+ const char *cmd = NULL;
char proc_path[64] = { 0, };
char *kstack = NULL;
struct server_id_buf buf;
@@ -108,6 +109,57 @@ static void server_id_watch_waited(struct tevent_req *subreq)
state->warn = tevent_timeval_add(&now, 10, 0);
+ cmd = lp_parm_const_string(GLOBAL_SECTION_SNUM,
+ "serverid watch",
+ "debug script",
+ NULL);
+ if (cmd != NULL) {
+ char *cmdstr = NULL;
+ char *output = NULL;
+ int fd;
+
+ /*
+ * Note in a cluster setup pid will be
+ * a NOTE:PID like '1:3978365'
+ *
+ * Without clustering it is just '3978365'
+ */
+ cmdstr = talloc_asprintf(state, "%s %s", cmd, pid);
+ if (cmdstr == NULL) {
+ DBG_ERR("Process %s hanging for %f seconds?\n"
+ "talloc_asprintf failed\n",
+ pid, duration);
+ goto next;
+ }
+
+ become_root();
+ ret = smbrun(cmdstr, &fd, NULL);
+ unbecome_root();
+ if (ret != 0) {
+ DBG_ERR("Process %s hanging for %f seconds?\n"
+ "smbrun('%s') failed\n",
+ pid, duration, cmdstr);
+ TALLOC_FREE(cmdstr);
+ goto next;
+ }
+
+ output = fd_load(fd, NULL, 0, state);
+ close(fd);
+ if (output == NULL) {
+ DBG_ERR("Process %s hanging for %f seconds?\n"
+ "fd_load() of smbrun('%s') failed\n",
+ pid, duration, cmdstr);
+ TALLOC_FREE(cmdstr);
+ goto next;
+ }
+ DBG_ERR("Process %s hanging for %f seconds?\n"
+ "%s returned:\n%s",
+ pid, duration, cmdstr, output);
+ TALLOC_FREE(cmdstr);
+ TALLOC_FREE(output);
+ goto next;
+ }
+
if (!procid_is_local(&state->pid) || !sys_have_proc_fds()) {
DBG_ERR("Process %s hanging for %f seconds?\n",
pid, duration);
--
2.51.0
From 59975168627e4bfbd2e75a611cb8cb13019a7df3 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 5 Apr 2024 12:15:28 +0200
Subject: [PATCH 55/63] smbd: log share_mode_watch_recv() errors as errors
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit b45e78871aadca6ae33475bee890736838f44219)
---
source3/smbd/open.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index b9849f82396..da129119c7f 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2969,8 +2969,9 @@ static void defer_open_done(struct tevent_req *req)
status = share_mode_watch_recv(req, NULL, NULL);
TALLOC_FREE(req);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(5, ("dbwrap_watched_watch_recv returned %s\n",
- nt_errstr(status)));
+ DBG_ERR("share_mode_watch_recv() returned %s, "
+ "rescheduling mid %" PRIu64 "\n",
+ nt_errstr(status), state->mid);
/*
* Even if it failed, retry anyway. TODO: We need a way to
* tell a re-scheduled open about that error.
--
2.51.0
From e619b72fe1b9c36963c452c1d102009b28e8e289 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 4 Apr 2024 19:18:19 +0200
Subject: [PATCH 56/63] smbd: add option "smbd lease break:debug hung procs"
By enabling this a process sending a lease break message to another process
holding a lease will start watching that process and if that process didn't
process the lease break within 10 seconds (cf server_id_watch_waited()), we log
a kernel stack backtrace of that process.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit d8613d7ee23c4e990285a387eb9ac2eeefff9749)
---
source3/smbd/open.c | 110 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 102 insertions(+), 8 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index da129119c7f..4cc5190f690 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -38,6 +38,7 @@
#include "serverid.h"
#include "messages.h"
#include "source3/lib/dbwrap/dbwrap_watch.h"
+#include "source3/lib/server_id_watch.h"
#include "locking/leases_db.h"
#include "librpc/gen_ndr/ndr_leases_db.h"
#include "lib/util/time_basic.h"
@@ -2472,6 +2473,10 @@ static int map_lease_type_to_oplock(uint32_t lease_type)
return result;
}
+struct blocker_debug_state {
+ size_t num_blockers;
+};
+
struct delay_for_oplock_state {
struct files_struct *fsp;
const struct smb2_lease *lease;
@@ -2483,8 +2488,22 @@ struct delay_for_oplock_state {
bool have_other_lease;
uint32_t total_lease_types;
bool delay;
+ struct blocker_debug_state *blocker_debug_state;
};
+static int blocker_debug_state_destructor(struct blocker_debug_state *state)
+{
+ if (state->num_blockers == 0) {
+ return 0;
+ }
+
+ DBG_DEBUG("blocker_debug_state [%p] num_blockers [%zu]\n",
+ state, state->num_blockers);
+ return 0;
+}
+
+static void delay_for_oplock_fn_watch_done(struct tevent_req *subreq);
+
static bool delay_for_oplock_fn(
struct share_mode_entry *e,
bool *modified,
@@ -2497,6 +2516,8 @@ static bool delay_for_oplock_fn(
uint32_t e_lease_type = SMB2_LEASE_NONE;
uint32_t break_to;
bool lease_is_breaking = false;
+ struct tevent_req *subreq = NULL;
+ struct server_id_buf idbuf = {};
if (e_is_lease) {
NTSTATUS status;
@@ -2636,9 +2657,56 @@ static bool delay_for_oplock_fn(
state->delay = true;
}
+ if (!state->delay) {
+ return false;
+ }
+
+ if (state->blocker_debug_state == NULL) {
+ return false;
+ }
+
+ subreq = server_id_watch_send(state->blocker_debug_state,
+ fsp->conn->sconn->ev_ctx,
+ e->pid);
+ if (subreq == NULL) {
+ DBG_ERR("server_id_watch_send(%s) returned NULL\n",
+ server_id_str_buf(e->pid, &idbuf));
+ return false;
+ }
+
+ tevent_req_set_callback(subreq,
+ delay_for_oplock_fn_watch_done,
+ state->blocker_debug_state);
+
+ state->blocker_debug_state->num_blockers++;
+
+ DBG_DEBUG("Starting to watch pid [%s] state [%p] num_blockers [%zu]\n",
+ server_id_str_buf(e->pid, &idbuf),
+ state->blocker_debug_state,
+ state->blocker_debug_state->num_blockers);
+
return false;
};
+static void delay_for_oplock_fn_watch_done(struct tevent_req *subreq)
+{
+ struct blocker_debug_state *blocker_debug_state = tevent_req_callback_data(
+ subreq, struct blocker_debug_state);
+ struct server_id pid = {};
+ struct server_id_buf idbuf = {};
+ int ret;
+
+ ret = server_id_watch_recv(subreq, &pid);
+ if (ret != 0) {
+ DBG_ERR("server_id_watch_recv failed %s\n", strerror(ret));
+ return;
+ }
+
+ DBG_DEBUG("state [%p] server_id_watch_recv() returned pid [%s] exited\n",
+ blocker_debug_state,
+ server_id_str_buf(pid, &idbuf));
+}
+
static NTSTATUS delay_for_oplock(files_struct *fsp,
int oplock_request,
const struct smb2_lease *lease,
@@ -2647,7 +2715,8 @@ static NTSTATUS delay_for_oplock(files_struct *fsp,
uint32_t create_disposition,
bool first_open_attempt,
int *poplock_type,
- uint32_t *pgranted)
+ uint32_t *pgranted,
+ struct blocker_debug_state **blocker_debug_state)
{
struct delay_for_oplock_state state = {
.fsp = fsp,
@@ -2693,6 +2762,22 @@ static NTSTATUS delay_for_oplock(files_struct *fsp,
goto grant;
}
+ if (lp_parm_bool(GLOBAL_SECTION_SNUM,
+ "smbd lease break",
+ "debug hung procs",
+ false))
+ {
+ state.blocker_debug_state = talloc_zero(fsp,
+ struct blocker_debug_state);
+ if (state.blocker_debug_state == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+ talloc_steal(talloc_tos(), state.blocker_debug_state);
+
+ talloc_set_destructor(state.blocker_debug_state,
+ blocker_debug_state_destructor);
+ }
+
state.delay_mask = have_sharing_violation ?
SMB2_LEASE_HANDLE : SMB2_LEASE_WRITE;
@@ -2714,6 +2799,7 @@ static NTSTATUS delay_for_oplock(files_struct *fsp,
}
if (state.delay) {
+ *blocker_debug_state = state.blocker_debug_state;
return NT_STATUS_RETRY;
}
@@ -2827,7 +2913,8 @@ static NTSTATUS handle_share_mode_lease(
const struct smb2_lease *lease,
bool first_open_attempt,
int *poplock_type,
- uint32_t *pgranted)
+ uint32_t *pgranted,
+ struct blocker_debug_state **blocker_debug_state)
{
bool sharing_violation = false;
NTSTATUS status;
@@ -2868,7 +2955,8 @@ static NTSTATUS handle_share_mode_lease(
create_disposition,
first_open_attempt,
poplock_type,
- pgranted);
+ pgranted,
+ blocker_debug_state);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
@@ -2903,7 +2991,8 @@ static void defer_open_done(struct tevent_req *req);
static void defer_open(struct share_mode_lock *lck,
struct timeval timeout,
struct smb_request *req,
- struct file_id id)
+ struct file_id id,
+ struct blocker_debug_state **blocker_debug_state)
{
struct deferred_open_record *open_rec = NULL;
struct timeval abs_timeout;
@@ -2947,6 +3036,8 @@ static void defer_open(struct share_mode_lock *lck,
}
tevent_req_set_callback(watch_req, defer_open_done, watch_state);
+ talloc_move(watch_req, blocker_debug_state);
+
ok = tevent_req_set_endtime(watch_req, req->sconn->ev_ctx, abs_timeout);
if (!ok) {
exit_server("tevent_req_set_endtime failed");
@@ -3229,7 +3320,8 @@ static bool open_match_attributes(connection_struct *conn,
static void schedule_defer_open(struct share_mode_lock *lck,
struct file_id id,
- struct smb_request *req)
+ struct smb_request *req,
+ struct blocker_debug_state **blocker_debug_state)
{
/* This is a relative time, added to the absolute
request_time value to get the absolute timeout time.
@@ -3253,7 +3345,7 @@ static void schedule_defer_open(struct share_mode_lock *lck,
return;
}
- defer_open(lck, timeout, req, id);
+ defer_open(lck, timeout, req, id, blocker_debug_state);
}
/****************************************************************************
@@ -3315,6 +3407,7 @@ static NTSTATUS check_and_store_share_mode(
int oplock_type = NO_OPLOCK;
uint32_t granted_lease = 0;
const struct smb2_lease_key *lease_key = NULL;
+ struct blocker_debug_state *blocker_debug_state = NULL;
bool delete_on_close;
bool ok;
@@ -3337,9 +3430,10 @@ static NTSTATUS check_and_store_share_mode(
lease,
first_open_attempt,
&oplock_type,
- &granted_lease);
+ &granted_lease,
+ &blocker_debug_state);
if (NT_STATUS_EQUAL(status, NT_STATUS_RETRY)) {
- schedule_defer_open(lck, fsp->file_id, req);
+ schedule_defer_open(lck, fsp->file_id, req, &blocker_debug_state);
return NT_STATUS_SHARING_VIOLATION;
}
if (!NT_STATUS_IS_OK(status)) {
--
2.51.0
From e6a0d821ba28839728371ca94bb364dd6865b5dd Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Wed, 20 Mar 2024 14:27:27 +0100
Subject: [PATCH 57/63] smbd: move trace_state variable behind tv variable
Next commit adds timestamp variables to trace_state that want to be initialized
with the current time, so moving behind tv we can then just reuse tv for that.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit 679e12aee2f0c283a6f9b9c6008c549a6ca9633e)
---
source3/smbd/smb2_process.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c
index fbbe4ef3992..188eaa14839 100644
--- a/source3/smbd/smb2_process.c
+++ b/source3/smbd/smb2_process.c
@@ -1783,10 +1783,6 @@ void smbd_process(struct tevent_context *ev_ctx,
int sock_fd,
bool interactive)
{
- struct smbd_tevent_trace_state trace_state = {
- .ev = ev_ctx,
- .frame = talloc_stackframe(),
- };
const struct loadparm_substitution *lp_sub =
loadparm_s3_global_substitution();
struct smbXsrv_client *client = NULL;
@@ -1797,6 +1793,10 @@ void smbd_process(struct tevent_context *ev_ctx,
int ret;
NTSTATUS status;
struct timeval tv = timeval_current();
+ struct smbd_tevent_trace_state trace_state = {
+ .ev = ev_ctx,
+ .frame = talloc_stackframe(),
+ };
NTTIME now = timeval_to_nttime(&tv);
char *chroot_dir = NULL;
int rc;
--
2.51.0
From 15276d7645255ddddf2a3bf6b7a429e3d40ec9b7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Wed, 20 Mar 2024 14:28:43 +0100
Subject: [PATCH 58/63] smbd: add option "smbd:debug events" for tevent
handling duration threshold warnings
Can be used to enable printing an error message if tevent event handlers ran
longer then three seconds. Also logs a message with a loglevel of 3 if there
were no events at hall.
Enabled by default with 'log level = 10' or
'smbd profiling level = on'...
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit 90d776cb18395ed804f0ab4fd13ef571fc0ad827)
---
source3/smbd/smb2_process.c | 64 +++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c
index 188eaa14839..dbe91132f7f 100644
--- a/source3/smbd/smb2_process.c
+++ b/source3/smbd/smb2_process.c
@@ -1692,8 +1692,36 @@ struct smbd_tevent_trace_state {
struct tevent_context *ev;
TALLOC_CTX *frame;
SMBPROFILE_BASIC_ASYNC_STATE(profile_idle);
+ struct timeval before_wait_tv;
+ struct timeval after_wait_tv;
};
+static inline void smbd_tevent_trace_callback_before_wait(
+ struct smbd_tevent_trace_state *state)
+{
+ struct timeval now = timeval_current();
+ struct timeval diff;
+
+ diff = tevent_timeval_until(&state->after_wait_tv, &now);
+ if (diff.tv_sec > 3) {
+ DBG_ERR("Handling event took %ld seconds!\n", (long)diff.tv_sec);
+ }
+ state->before_wait_tv = now;
+}
+
+static inline void smbd_tevent_trace_callback_after_wait(
+ struct smbd_tevent_trace_state *state)
+{
+ struct timeval now = timeval_current();
+ struct timeval diff;
+
+ diff = tevent_timeval_until(&state->before_wait_tv, &now);
+ if (diff.tv_sec > 30) {
+ DBG_NOTICE("No event for %ld seconds!\n", (long)diff.tv_sec);
+ }
+ state->after_wait_tv = now;
+}
+
static inline void smbd_tevent_trace_callback_before_loop_once(
struct smbd_tevent_trace_state *state)
{
@@ -1729,6 +1757,30 @@ static void smbd_tevent_trace_callback(enum tevent_trace_point point,
errno = 0;
}
+static void smbd_tevent_trace_callback_debug(enum tevent_trace_point point,
+ void *private_data)
+{
+ struct smbd_tevent_trace_state *state =
+ (struct smbd_tevent_trace_state *)private_data;
+
+ switch (point) {
+ case TEVENT_TRACE_BEFORE_WAIT:
+ smbd_tevent_trace_callback_before_wait(state);
+ break;
+ case TEVENT_TRACE_AFTER_WAIT:
+ smbd_tevent_trace_callback_after_wait(state);
+ break;
+ case TEVENT_TRACE_BEFORE_LOOP_ONCE:
+ smbd_tevent_trace_callback_before_loop_once(state);
+ break;
+ case TEVENT_TRACE_AFTER_LOOP_ONCE:
+ smbd_tevent_trace_callback_after_loop_once(state);
+ break;
+ }
+
+ errno = 0;
+}
+
static void smbd_tevent_trace_callback_profile(enum tevent_trace_point point,
void *private_data)
{
@@ -1737,6 +1789,7 @@ static void smbd_tevent_trace_callback_profile(enum tevent_trace_point point,
switch (point) {
case TEVENT_TRACE_BEFORE_WAIT:
+ smbd_tevent_trace_callback_before_wait(state);
if (!smbprofile_dump_pending()) {
/*
* If there's no dump pending
@@ -1749,6 +1802,7 @@ static void smbd_tevent_trace_callback_profile(enum tevent_trace_point point,
SMBPROFILE_BASIC_ASYNC_START(idle, profile_p, state->profile_idle);
break;
case TEVENT_TRACE_AFTER_WAIT:
+ smbd_tevent_trace_callback_after_wait(state);
SMBPROFILE_BASIC_ASYNC_END(state->profile_idle);
if (!smbprofile_dump_pending()) {
/*
@@ -1796,7 +1850,13 @@ void smbd_process(struct tevent_context *ev_ctx,
struct smbd_tevent_trace_state trace_state = {
.ev = ev_ctx,
.frame = talloc_stackframe(),
+ .before_wait_tv = tv,
+ .after_wait_tv = tv,
};
+ bool debug = lp_parm_bool(GLOBAL_SECTION_SNUM,
+ "smbd",
+ "debug events",
+ CHECK_DEBUGLVL(DBGLVL_DEBUG));
NTTIME now = timeval_to_nttime(&tv);
char *chroot_dir = NULL;
int rc;
@@ -2041,6 +2101,10 @@ void smbd_process(struct tevent_context *ev_ctx,
tevent_set_trace_callback(ev_ctx,
smbd_tevent_trace_callback_profile,
&trace_state);
+ } else if (debug) {
+ tevent_set_trace_callback(ev_ctx,
+ smbd_tevent_trace_callback_debug,
+ &trace_state);
} else {
tevent_set_trace_callback(ev_ctx,
smbd_tevent_trace_callback,
--
2.51.0
From 4631b9d60a874db10dbdd52406d0094a7dbd1356 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Mon, 26 Aug 2024 14:11:02 +0200
Subject: [PATCH 59/63] vfs_error_inject: add 'error_inject:durable_reconnect =
st_ex_nlink'
This allows to simulate durable reconnect failures because the stat
information of the file changed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit 692ed832dfff61ad1c9b646b5c8d6f85f25efb99)
---
source3/modules/vfs_error_inject.c | 76 ++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/source3/modules/vfs_error_inject.c b/source3/modules/vfs_error_inject.c
index 529504fd8d5..dcf0de0a2d9 100644
--- a/source3/modules/vfs_error_inject.c
+++ b/source3/modules/vfs_error_inject.c
@@ -19,6 +19,7 @@
#include "includes.h"
#include "smbd/smbd.h"
+#include "librpc/gen_ndr/ndr_open_files.h"
#undef DBGC_CLASS
#define DBGC_CLASS DBGC_VFS
@@ -204,11 +205,86 @@ static int vfs_error_inject_unlinkat(struct vfs_handle_struct *handle,
return -1;
}
+static NTSTATUS vfs_error_inject_durable_reconnect(struct vfs_handle_struct *handle,
+ struct smb_request *smb1req,
+ struct smbXsrv_open *op,
+ const DATA_BLOB old_cookie,
+ TALLOC_CTX *mem_ctx,
+ struct files_struct **fsp,
+ DATA_BLOB *new_cookie)
+{
+ const char *vfs_func = "durable_reconnect";
+ const char *err_str = NULL;
+ NTSTATUS status;
+ enum ndr_err_code ndr_err;
+ struct vfs_default_durable_cookie cookie;
+ DATA_BLOB modified_cookie = data_blob_null;
+
+ err_str = lp_parm_const_string(SNUM(handle->conn),
+ "error_inject",
+ vfs_func,
+ NULL);
+ if (err_str == NULL) {
+ return SMB_VFS_NEXT_DURABLE_RECONNECT(handle,
+ smb1req,
+ op,
+ old_cookie,
+ mem_ctx,
+ fsp,
+ new_cookie);
+ }
+
+ ndr_err = ndr_pull_struct_blob(&old_cookie, talloc_tos(), &cookie,
+ (ndr_pull_flags_fn_t)ndr_pull_vfs_default_durable_cookie);
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ status = ndr_map_error2ntstatus(ndr_err);
+ return status;
+ }
+
+ if (strcmp(cookie.magic, VFS_DEFAULT_DURABLE_COOKIE_MAGIC) != 0) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ if (cookie.version != VFS_DEFAULT_DURABLE_COOKIE_VERSION) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ if (strequal(err_str, "st_ex_nlink")) {
+ cookie.stat_info.st_ex_nlink += 1;
+ } else {
+ DBG_ERR("Unknown error inject %s requested "
+ "for vfs function %s\n", err_str, vfs_func);
+ return SMB_VFS_NEXT_DURABLE_RECONNECT(handle,
+ smb1req,
+ op,
+ old_cookie,
+ mem_ctx,
+ fsp,
+ new_cookie);
+ }
+
+ ndr_err = ndr_push_struct_blob(&modified_cookie, talloc_tos(), &cookie,
+ (ndr_push_flags_fn_t)ndr_push_vfs_default_durable_cookie);
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ status = ndr_map_error2ntstatus(ndr_err);
+ return status;
+ }
+
+ return SMB_VFS_NEXT_DURABLE_RECONNECT(handle,
+ smb1req,
+ op,
+ modified_cookie,
+ mem_ctx,
+ fsp,
+ new_cookie);
+}
+
static struct vfs_fn_pointers vfs_error_inject_fns = {
.chdir_fn = vfs_error_inject_chdir,
.pwrite_fn = vfs_error_inject_pwrite,
.openat_fn = vfs_error_inject_openat,
.unlinkat_fn = vfs_error_inject_unlinkat,
+ .durable_reconnect_fn = vfs_error_inject_durable_reconnect,
};
static_decl_vfs;
--
2.51.0
From c8e88652163cc56b1f9fb0926a140c81e6b7ec94 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Mon, 26 Aug 2024 14:42:02 +0200
Subject: [PATCH 60/63] s4:torture/smb2: add
smb2.durable-v2-regressions.durable_v2_reconnect_bug15624
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit ef4ef04e7f83b1029446ff8b5fc5fdf4ab33edbd)
---
selftest/skip | 1 +
source4/torture/smb2/durable_v2_open.c | 118 +++++++++++++++++++++++++
source4/torture/smb2/smb2.c | 2 +
3 files changed, 121 insertions(+)
diff --git a/selftest/skip b/selftest/skip
index e808367c00d..056c54ea287 100644
--- a/selftest/skip
+++ b/selftest/skip
@@ -149,3 +149,4 @@ bench # don't run benchmarks in our selftest
^samba.tests.reparsepoints.*
^samba.tests.smb2symlink.*
^samba3.blackbox.open-eintr.*
+smb2.durable-v2-regressions # Only used in blackbox tests
diff --git a/source4/torture/smb2/durable_v2_open.c b/source4/torture/smb2/durable_v2_open.c
index 9b9af11124c..7447dd287a4 100644
--- a/source4/torture/smb2/durable_v2_open.c
+++ b/source4/torture/smb2/durable_v2_open.c
@@ -2355,6 +2355,112 @@ done:
return ret;
}
+/**
+ * basic test for doing a durable open
+ * tcp disconnect, reconnect, do a durable reopen (succeeds)
+ */
+static bool test_durable_v2_reconnect_bug15624(struct torture_context *tctx,
+ struct smb2_tree *tree,
+ struct smb2_tree *tree2)
+{
+ NTSTATUS status;
+ TALLOC_CTX *mem_ctx = talloc_new(tctx);
+ char fname[256];
+ struct smb2_handle _h;
+ struct smb2_handle *h = NULL;
+ struct smb2_create io;
+ struct GUID create_guid = GUID_random();
+ struct smbcli_options options;
+ uint64_t previous_session_id;
+ uint8_t b = 0;
+ bool ret = true;
+ bool ok;
+
+ if (!torture_setting_bool(tctx, "bug15624", false)) {
+ torture_comment(tctx,
+ "share requires:\n"
+ "'vfs objects = error_inject'\n"
+ "'error_inject:durable_reconnect=st_ex_nlink'\n"
+ "test requires:\n"
+ "'--option=torture:bug15624=yes'\n");
+ torture_skip(tctx, "'--option=torture:bug15624=yes' missing");
+ }
+
+ options = tree->session->transport->options;
+ previous_session_id = smb2cli_session_current_id(tree->session->smbXcli);
+
+ /* Choose a random name in case the state is left a little funky. */
+ snprintf(fname,
+ sizeof(fname),
+ "durable_v2_reconnect_bug15624_%s.dat",
+ generate_random_str(tctx, 8));
+
+ smb2_util_unlink(tree, fname);
+
+ smb2_oplock_create_share(&io, fname,
+ smb2_util_share_access(""),
+ smb2_util_oplock_level("b"));
+ io.in.durable_open = false;
+ io.in.durable_open_v2 = true;
+ io.in.persistent_open = false;
+ io.in.create_guid = create_guid;
+ io.in.timeout = 0;
+
+ status = smb2_create(tree, mem_ctx, &io);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ _h = io.out.file.handle;
+ h = &_h;
+ CHECK_VAL(io.out.oplock_level, smb2_util_oplock_level("b"));
+ CHECK_VAL(io.out.durable_open_v2, true);
+
+ status = smb2_util_write(tree, *h, &b, 0, 1);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ /* disconnect, leaving the durable open */
+ TALLOC_FREE(tree);
+ h = NULL;
+
+ ok = torture_smb2_connection_ext(tctx, previous_session_id,
+ &options, &tree);
+ torture_assert_goto(tctx, ok, ret, done, "couldn't reconnect, bailing\n");
+
+ ZERO_STRUCT(io);
+ io.in.fname = fname;
+ io.in.durable_open_v2 = false;
+ io.in.durable_handle_v2 = &_h;
+ io.in.create_guid = create_guid;
+
+ /*
+ * This assumes 'error_inject:durable_reconnect = st_ex_nlink'
+ * will cause the durable reconnect to fail...
+ * in order to have a regression test for the dead lock.
+ */
+ status = smb2_create(tree, mem_ctx, &io);
+ CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_NOT_FOUND);
+
+ /*
+ * With the regression this will fail with
+ * a timeout...
+ */
+ status = smb2_util_unlink(tree2, fname);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+done:
+ if (h != NULL) {
+ smb2_util_close(tree, *h);
+ }
+ TALLOC_FREE(tree);
+
+ smb2_util_unlink(tree2, fname);
+
+ TALLOC_FREE(tree2);
+
+ talloc_free(mem_ctx);
+
+ return ret;
+}
+
struct torture_suite *torture_smb2_durable_v2_delay_init(TALLOC_CTX *ctx)
{
struct torture_suite *suite =
@@ -2369,3 +2475,15 @@ struct torture_suite *torture_smb2_durable_v2_delay_init(TALLOC_CTX *ctx)
return suite;
}
+
+struct torture_suite *torture_smb2_durable_v2_regressions_init(TALLOC_CTX *ctx)
+{
+ struct torture_suite *suite =
+ torture_suite_create(ctx, "durable-v2-regressions");
+
+ torture_suite_add_2smb2_test(suite,
+ "durable_v2_reconnect_bug15624",
+ test_durable_v2_reconnect_bug15624);
+
+ return suite;
+}
diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c
index 5b6477e47bc..9cf7f5da78b 100644
--- a/source4/torture/smb2/smb2.c
+++ b/source4/torture/smb2/smb2.c
@@ -170,6 +170,8 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx)
torture_smb2_durable_v2_open_init(suite));
torture_suite_add_suite(suite,
torture_smb2_durable_v2_delay_init(suite));
+ torture_suite_add_suite(suite,
+ torture_smb2_durable_v2_regressions_init(suite));
torture_suite_add_suite(suite, torture_smb2_dir_init(suite));
torture_suite_add_suite(suite, torture_smb2_lease_init(suite));
torture_suite_add_suite(suite, torture_smb2_compound_init(suite));
--
2.51.0
From 56a3aaf95c44052b19b61115686c71d5b7dbab4a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Mon, 26 Aug 2024 14:42:12 +0200
Subject: [PATCH 61/63] s3:tests: let test_durable_handle_reconnect.sh run
smb2.durable-v2-regressions.durable_v2_reconnect_bug15624
This demonstrates the dead lock after a durable reconnect failed
because the stat info changed, the file can't be accessed anymore
as we leak the incomplete share mode entry in a still running
process.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit 14875448ca06a3a28800343a3a326f1a66bccec0)
---
.../samba3.blackbox.durable_v2_delay | 1 +
.../tests/test_durable_handle_reconnect.sh | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
create mode 100644 selftest/knownfail.d/samba3.blackbox.durable_v2_delay
diff --git a/selftest/knownfail.d/samba3.blackbox.durable_v2_delay b/selftest/knownfail.d/samba3.blackbox.durable_v2_delay
new file mode 100644
index 00000000000..88e29960797
--- /dev/null
+++ b/selftest/knownfail.d/samba3.blackbox.durable_v2_delay
@@ -0,0 +1 @@
+^samba3.blackbox.durable_v2_delay.durable-v2-regressions.durable_v2_reconnect_bug15624
diff --git a/source3/script/tests/test_durable_handle_reconnect.sh b/source3/script/tests/test_durable_handle_reconnect.sh
index 0ab32974824..fd5c156956f 100755
--- a/source3/script/tests/test_durable_handle_reconnect.sh
+++ b/source3/script/tests/test_durable_handle_reconnect.sh
@@ -33,4 +33,22 @@ testit "durable_v2_delay.durable_v2_reconnect_delay_msec" $VALGRIND \
rm $delay_inject_conf
+error_inject_conf=$(dirname $SMB_CONF_PATH)/error_inject.conf
+
+cat > $error_inject_conf << _EOF
+ kernel share modes = no
+ kernel oplocks = no
+ posix locking = no
+ error_inject:durable_reconnect = st_ex_nlink
+_EOF
+
+testit "durable-v2-regressions.durable_v2_reconnect_bug15624" \
+ $VALGRIND $BINDIR/smbtorture //$SERVER_IP/error_inject \
+ -U$USERNAME%$PASSWORD \
+ --option=torture:bug15624=yes \
+ smb2.durable-v2-regressions.durable_v2_reconnect_bug15624 ||
+ failed=$(expr $failed + 1)
+
+rm $error_inject_conf
+
testok $0 $failed
--
2.51.0
From d8f01885145ecfce15f2507fdcc625442db1738c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 9 Apr 2024 14:52:44 +0200
Subject: [PATCH 62/63] smbd: consolidate DH reconnect failure code
No change in behaviour, except that we now
also call fd_close() if vfs_default_durable_cookie()
failed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
(cherry picked from commit a91457f97c98fcec1ed062514c364271af1df669)
---
source3/smbd/durable.c | 142 ++++++++++++++---------------------------
1 file changed, 47 insertions(+), 95 deletions(-)
diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c
index b21c223b2e4..50075ddd3f7 100644
--- a/source3/smbd/durable.c
+++ b/source3/smbd/durable.c
@@ -624,22 +624,22 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
ok = share_mode_forall_entries(lck, durable_reconnect_fn, &e);
if (!ok) {
DBG_WARNING("share_mode_forall_entries failed\n");
- TALLOC_FREE(lck);
- return NT_STATUS_INTERNAL_DB_ERROR;
+ status = NT_STATUS_INTERNAL_DB_ERROR;
+ goto fail;
}
if (e.pid.pid == 0) {
DBG_WARNING("Did not find a unique valid share mode entry\n");
- TALLOC_FREE(lck);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ goto fail;
}
if (!server_id_is_disconnected(&e.pid)) {
DEBUG(5, ("vfs_default_durable_reconnect: denying durable "
"reconnect for handle that was not marked "
"disconnected (e.g. smbd or cluster node died)\n"));
- TALLOC_FREE(lck);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ goto fail;
}
if (e.share_file_id != op->global->open_persistent_id) {
@@ -648,8 +648,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
"(e.g. another client had opened the file)\n",
e.share_file_id,
op->global->open_persistent_id);
- TALLOC_FREE(lck);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ goto fail;
}
if ((e.access_mask & (FILE_WRITE_DATA|FILE_APPEND_DATA)) &&
@@ -658,8 +658,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
DEBUG(5, ("vfs_default_durable_reconnect: denying durable "
"share[%s] is not writeable anymore\n",
lp_servicename(talloc_tos(), lp_sub, SNUM(conn))));
- TALLOC_FREE(lck);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ goto fail;
}
/*
@@ -670,8 +670,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("vfs_default_durable_reconnect: failed to create "
"new fsp: %s\n", nt_errstr(status)));
- TALLOC_FREE(lck);
- return status;
+ goto fail;
}
fh_set_private_options(fsp->fh, e.private_options);
@@ -714,9 +713,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
*/
if (!GUID_equal(fsp_client_guid(fsp),
&e.client_guid)) {
- TALLOC_FREE(lck);
- file_free(smb1req, fsp);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ goto fail;
}
status = leases_db_get(
@@ -730,9 +728,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
&lease_version, /* lease_version */
&epoch); /* epoch */
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(lck);
- file_free(smb1req, fsp);
- return status;
+ goto fail;
}
fsp->lease = find_fsp_lease(
@@ -742,9 +738,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
lease_version,
epoch);
if (fsp->lease == NULL) {
- TALLOC_FREE(lck);
- file_free(smb1req, fsp);
- return NT_STATUS_NO_MEMORY;
+ status = NT_STATUS_NO_MEMORY;
+ goto fail;
}
}
@@ -760,12 +755,10 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
status = fsp_set_smb_fname(fsp, smb_fname);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(lck);
- file_free(smb1req, fsp);
DEBUG(0, ("vfs_default_durable_reconnect: "
"fsp_set_smb_fname failed: %s\n",
nt_errstr(status)));
- return status;
+ goto fail;
}
op->compat = fsp;
@@ -780,11 +773,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
fh_get_gen_id(fsp->fh));
if (!ok) {
DBG_DEBUG("Could not set new share_mode_entry values\n");
- TALLOC_FREE(lck);
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return NT_STATUS_INTERNAL_ERROR;
+ status = NT_STATUS_INTERNAL_ERROR;
+ goto fail;
}
ok = brl_reconnect_disconnected(fsp);
@@ -793,11 +783,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
DEBUG(1, ("vfs_default_durable_reconnect: "
"failed to reopen brlocks: %s\n",
nt_errstr(status)));
- TALLOC_FREE(lck);
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return status;
+ goto fail;
}
/*
@@ -813,13 +799,9 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
status = fd_openat(conn->cwd_fsp, fsp->fsp_name, fsp, &how);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(lck);
DEBUG(1, ("vfs_default_durable_reconnect: failed to open "
"file: %s\n", nt_errstr(status)));
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return status;
+ goto fail;
}
/*
@@ -833,48 +815,22 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
ret = SMB_VFS_FSTAT(fsp, &fsp->fsp_name->st);
if (ret == -1) {
- NTSTATUS close_status;
status = map_nt_error_from_unix_common(errno);
DEBUG(1, ("Unable to fstat stream: %s => %s\n",
smb_fname_str_dbg(smb_fname),
nt_errstr(status)));
- close_status = fd_close(fsp);
- if (!NT_STATUS_IS_OK(close_status)) {
- DBG_ERR("fd_close failed (%s) - leaking file "
- "descriptor\n", nt_errstr(close_status));
- }
- TALLOC_FREE(lck);
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return status;
+ goto fail;
}
if (!S_ISREG(fsp->fsp_name->st.st_ex_mode)) {
- NTSTATUS close_status = fd_close(fsp);
- if (!NT_STATUS_IS_OK(close_status)) {
- DBG_ERR("fd_close failed (%s) - leaking file "
- "descriptor\n", nt_errstr(close_status));
- }
- TALLOC_FREE(lck);
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ goto fail;
}
file_id = vfs_file_id_from_sbuf(conn, &fsp->fsp_name->st);
if (!file_id_equal(&cookie.id, &file_id)) {
- NTSTATUS close_status = fd_close(fsp);
- if (!NT_STATUS_IS_OK(close_status)) {
- DBG_ERR("fd_close failed (%s) - leaking file "
- "descriptor\n", nt_errstr(close_status));
- }
- TALLOC_FREE(lck);
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ goto fail;
}
(void)fdos_mode(fsp);
@@ -883,42 +839,21 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
&fsp->fsp_name->st,
fsp_str_dbg(fsp));
if (!ok) {
- NTSTATUS close_status = fd_close(fsp);
- if (!NT_STATUS_IS_OK(close_status)) {
- DBG_ERR("fd_close failed (%s) - leaking file "
- "descriptor\n", nt_errstr(close_status));
- }
- TALLOC_FREE(lck);
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+ goto fail;
}
status = set_file_oplock(fsp);
if (!NT_STATUS_IS_OK(status)) {
- NTSTATUS close_status = fd_close(fsp);
- if (!NT_STATUS_IS_OK(close_status)) {
- DBG_ERR("fd_close failed (%s) - leaking file "
- "descriptor\n", nt_errstr(close_status));
- }
- TALLOC_FREE(lck);
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return status;
+ goto fail;
}
status = vfs_default_durable_cookie(fsp, mem_ctx, &new_cookie_blob);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(lck);
DEBUG(1, ("vfs_default_durable_reconnect: "
"vfs_default_durable_cookie - %s\n",
nt_errstr(status)));
- op->compat = NULL;
- fsp->op = NULL;
- file_free(smb1req, fsp);
- return status;
+ goto fail;
}
smb1req->chain_fsp = fsp;
@@ -935,4 +870,21 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
*new_cookie = new_cookie_blob;
return NT_STATUS_OK;
+
+fail:
+ if (fsp != NULL && fsp_get_pathref_fd(fsp) != -1) {
+ NTSTATUS close_status;
+ close_status = fd_close(fsp);
+ if (!NT_STATUS_IS_OK(close_status)) {
+ DBG_ERR("fd_close failed (%s), leaking fd\n",
+ nt_errstr(close_status));
+ }
+ }
+ TALLOC_FREE(lck);
+ if (fsp != NULL) {
+ op->compat = NULL;
+ fsp->op = NULL;
+ file_free(smb1req, fsp);
+ }
+ return status;
}
--
2.51.0
From b248ddd3dd7193ba44c9ad86488dd180a25e3774 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 9 Apr 2024 14:53:32 +0200
Subject: [PATCH 63/63] smbd: remove just created sharemode entry in the error
codepaths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Without this we leave stale sharemode entries around that can lead to all sorts
of havoc.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
Autobuild-User(master): Günther Deschner <gd@samba.org>
Autobuild-Date(master): Thu Sep 19 19:36:19 UTC 2024 on atb-devel-224
(cherry picked from commit 2ff3b9bc0d254a63a913ff9084de3d794fee27d0)
---
selftest/knownfail.d/samba3.blackbox.durable_v2_delay | 1 -
source3/smbd/durable.c | 8 ++++++++
2 files changed, 8 insertions(+), 1 deletion(-)
delete mode 100644 selftest/knownfail.d/samba3.blackbox.durable_v2_delay
diff --git a/selftest/knownfail.d/samba3.blackbox.durable_v2_delay b/selftest/knownfail.d/samba3.blackbox.durable_v2_delay
deleted file mode 100644
index 88e29960797..00000000000
--- a/selftest/knownfail.d/samba3.blackbox.durable_v2_delay
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.blackbox.durable_v2_delay.durable-v2-regressions.durable_v2_reconnect_bug15624
diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c
index 50075ddd3f7..98d0d403e30 100644
--- a/source3/smbd/durable.c
+++ b/source3/smbd/durable.c
@@ -538,6 +538,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
enum ndr_err_code ndr_err;
struct vfs_default_durable_cookie cookie;
DATA_BLOB new_cookie_blob = data_blob_null;
+ bool have_share_mode_entry = false;
*result = NULL;
*new_cookie = data_blob_null;
@@ -776,6 +777,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
status = NT_STATUS_INTERNAL_ERROR;
goto fail;
}
+ have_share_mode_entry = true;
ok = brl_reconnect_disconnected(fsp);
if (!ok) {
@@ -872,6 +874,12 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
return NT_STATUS_OK;
fail:
+ if (fsp != NULL && have_share_mode_entry) {
+ /*
+ * Something is screwed up, delete the sharemode entry.
+ */
+ del_share_mode(lck, fsp);
+ }
if (fsp != NULL && fsp_get_pathref_fd(fsp) != -1) {
NTSTATUS close_status;
close_status = fd_close(fsp);
--
2.51.0
Reference in New Issue View Git Blame Copy Permalink