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"""

+

+<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"""

@@ -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