Fix samba-gpupdate
resolves: RHEL-22157 resolves: RHEL-20897
This commit is contained in:
		
							parent
							
								
									ce5f8ce6ca
								
							
						
					
					
						commit
						f5909fc924
					
				| @ -1,638 +0,0 @@ | ||||
| From 549b5fe579fc15d63b71b1cc8a0ebf4e4869171b 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 1/9] 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.43.0 | ||||
| 
 | ||||
| 
 | ||||
| From c624a1e9b1d09fe2bb3f9778cb616230e57168a8 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 2/9] 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.43.0 | ||||
| 
 | ||||
| 
 | ||||
| From 086406ca457cc17e15001fb44802276ada068679 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 3/9] 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.43.0 | ||||
| 
 | ||||
| 
 | ||||
| From c57c32020cc9017191b8c8657ebabe00d552a6e3 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 4/9] 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.43.0 | ||||
| 
 | ||||
| 
 | ||||
| From c53b2994fd13f4c74cee891e725a4558cdb06b2d 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 5/9] 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.43.0 | ||||
| 
 | ||||
| 
 | ||||
| From fd13702a9cd6475a14113de87ccad6588d2d443b 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 6/9] 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.43.0 | ||||
| 
 | ||||
| 
 | ||||
| From 4578c6664ab6eac476ee10afae4a1a95b3b63272 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 7/9] 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.43.0 | ||||
| 
 | ||||
| 
 | ||||
| From 2d641b736b42f7623955f251ad354439b954159d 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 8/9] 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.43.0 | ||||
| 
 | ||||
| 
 | ||||
| From e5588f8800899894388284468b9e25463d3c3e6c 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 9/9] 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.43.0 | ||||
| 
 | ||||
							
								
								
									
										0
									
								
								samba-4.19-redhat.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										0
									
								
								samba-4.19-redhat.patch
									
									
									
									
									
										Normal file
									
								
							
							
								
								
									
										12
									
								
								samba.spec
									
									
									
									
									
								
							
							
						
						
									
										12
									
								
								samba.spec
									
									
									
									
									
								
							| @ -147,7 +147,7 @@ | ||||
| %define samba_requires_eq()  %(LC_ALL="C" echo '%*' | xargs -r rpm -q --qf 'Requires: %%{name} = %%{epoch}:%%{version}\\n' | sed -e 's/ (none):/ /' -e 's/ 0:/ /' | grep -v "is not") | ||||
| 
 | ||||
| %global samba_version 4.19.4 | ||||
| %global baserelease 100 | ||||
| %global baserelease 101 | ||||
| # This should be rc1 or %%nil | ||||
| %global pre_release %nil | ||||
| 
 | ||||
| @ -238,7 +238,11 @@ Source18:       samba-winbind-systemd-sysusers.conf | ||||
| Source201:      README.downgrade | ||||
| Source202:      samba.abignore | ||||
| 
 | ||||
| Patch0:         samba-4.19-fix-gpupdate-rhel-fedora.patch | ||||
| # Backport bug fixes to https://gitlab.com/samba-redhat/samba/-/tree/v4-19-redhat | ||||
| # This will give us CI and makes it easy to generate patchsets. | ||||
| # | ||||
| # Generate the patchset using: git format-patch -l1 --stdout -N > samba-4.19-redhat.patch | ||||
| Patch0:        samba-4.19-redhat.patch | ||||
| 
 | ||||
| Requires(pre): %{name}-common = %{samba_depver} | ||||
| Requires: %{name}-common = %{samba_depver} | ||||
| @ -4470,6 +4474,10 @@ fi | ||||
| %endif | ||||
| 
 | ||||
| %changelog | ||||
| * Tue Jan 30 2024 Andreas Schneider <asn@redhat.com> - 4.19.4-101 | ||||
| - resolves: RHEL-22157 - Don't require NDES for samba-gpupdate | ||||
| - resolves: RHEL-20897 - Fix site name fetching for samba-gpupdate | ||||
| 
 | ||||
| * Wed Jan 10 2024 Andreas Schneider <asn@redhat.com> - 4.19.4-100 | ||||
| - related: RHEL-16476 - Update to version 4.19.4 | ||||
| - resolves: RHEL-2109 - Add support for certificate auto enrollment | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user