CVE-2020-25722 dsdb: Move krbtgt password setup after the point of checking if any...

This allows the add of an RODC, before setting the password, to avoid
this module, which helps isolate testing of security around the
msDS-SecondaryKrbTgtNumber attribute.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14703

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

diff --git a/selftest/knownfail.d/priv_attr b/selftest/knownfail.d/priv_attr
index 31b9cb23b4428e54091b1098da5eb536994b1823..ab6db192aae7d8f51e4890fcede13ce9097c0e75 100644 (file)
--- a/selftest/knownfail.d/priv_attr
+++ b/selftest/knownfail.d/priv_attr
@@ -14,14 +14,10 @@ samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_userAccoun
 samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_userAccountControl-a2d-user_add_admin-add_WP_user
 samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_userAccountControl-a2d-user_add_admin-add_default_computer
 samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_userAccountControl-a2d-user_add_admin-add_default_user
-samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_admin-add_WP_computer
-samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_admin-add_WP_user
-samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_admin-add_default_computer
-samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_admin-add_default_user
-samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_admin-add_WP_computer
-samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_admin-add_WP_user
-samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_admin-add_default_computer
-samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_admin-add_default_user
+samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_WP_computer
+samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_WP_user
+samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_default_computer
+samba4.priv_attrs.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-SecondaryKrbTgtNumber_add_CC_default_user
 samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_WP_computer
 samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_WP_user
 samba4.priv_attrs.strict.python\(.*\).__main__.PrivAttrsTests.test_priv_attr_msDS-AllowedToDelegateTo_add_CC_default_computer

diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
index 82d9e8ebd2ef86a25fefc4d7f0f9f60d8a9c46b6..bb437a3b98261a1b0c10606cd94308b037d2c4a1 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -2476,6 +2476,59 @@ static int setup_password_fields(struct setup_password_fields_io *io)
                return LDB_SUCCESS;
        }
 
+       if (io->u.is_krbtgt) {
+               size_t min = 196;
+               size_t max = 255;
+               size_t diff = max - min;
+               size_t len = max;
+               struct ldb_val *krbtgt_utf16 = NULL;
+
+               if (!io->ac->pwd_reset) {
+                       return dsdb_module_werror(io->ac->module,
+                                       LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS,
+                                       WERR_DS_ATT_ALREADY_EXISTS,
+                                       "Password change on krbtgt not permitted!");
+               }
+
+               if (io->n.cleartext_utf16 == NULL) {
+                       return dsdb_module_werror(io->ac->module,
+                                       LDB_ERR_UNWILLING_TO_PERFORM,
+                                       WERR_DS_INVALID_ATTRIBUTE_SYNTAX,
+                                       "Password reset on krbtgt requires UTF16!");
+               }
+
+               /*
+                * Instead of taking the callers value,
+                * we just generate a new random value here.
+                *
+                * Include null termination in the array.
+                */
+               if (diff > 0) {
+                       size_t tmp;
+
+                       generate_random_buffer((uint8_t *)&tmp, sizeof(tmp));
+
+                       tmp %= diff;
+
+                       len = min + tmp;
+               }
+
+               krbtgt_utf16 = talloc_zero(io->ac, struct ldb_val);
+               if (krbtgt_utf16 == NULL) {
+                       return ldb_oom(ldb);
+               }
+
+               *krbtgt_utf16 = data_blob_talloc_zero(krbtgt_utf16,
+                                                     (len+1)*2);
+               if (krbtgt_utf16->data == NULL) {
+                       return ldb_oom(ldb);
+               }
+               krbtgt_utf16->length = len * 2;
+               generate_secret_buffer(krbtgt_utf16->data,
+                                      krbtgt_utf16->length);
+               io->n.cleartext_utf16 = krbtgt_utf16;
+       }
+
        /* transform the old password (for password changes) */
        ret = setup_given_passwords(io, &io->og);
        if (ret != LDB_SUCCESS) {
@@ -3653,59 +3706,6 @@ static int setup_io(struct ph_context *ac,
                return ldb_operr(ldb);
        }
 
-       if (io->u.is_krbtgt) {
-               size_t min = 196;
-               size_t max = 255;
-               size_t diff = max - min;
-               size_t len = max;
-               struct ldb_val *krbtgt_utf16 = NULL;
-
-               if (!ac->pwd_reset) {
-                       return dsdb_module_werror(ac->module,
-                                       LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS,
-                                       WERR_DS_ATT_ALREADY_EXISTS,
-                                       "Password change on krbtgt not permitted!");
-               }
-
-               if (io->n.cleartext_utf16 == NULL) {
-                       return dsdb_module_werror(ac->module,
-                                       LDB_ERR_UNWILLING_TO_PERFORM,
-                                       WERR_DS_INVALID_ATTRIBUTE_SYNTAX,
-                                       "Password reset on krbtgt requires UTF16!");
-               }
-
-               /*
-                * Instead of taking the callers value,
-                * we just generate a new random value here.
-                *
-                * Include null termination in the array.
-                */
-               if (diff > 0) {
-                       size_t tmp;
-
-                       generate_random_buffer((uint8_t *)&tmp, sizeof(tmp));
-
-                       tmp %= diff;
-
-                       len = min + tmp;
-               }
-
-               krbtgt_utf16 = talloc_zero(io->ac, struct ldb_val);
-               if (krbtgt_utf16 == NULL) {
-                       return ldb_oom(ldb);
-               }
-
-               *krbtgt_utf16 = data_blob_talloc_zero(krbtgt_utf16,
-                                                     (len+1)*2);
-               if (krbtgt_utf16->data == NULL) {
-                       return ldb_oom(ldb);
-               }
-               krbtgt_utf16->length = len * 2;
-               generate_secret_buffer(krbtgt_utf16->data,
-                                      krbtgt_utf16->length);
-               io->n.cleartext_utf16 = krbtgt_utf16;
-       }
-
        if (existing_msg != NULL) {
                NTSTATUS status;