s4:dsdb:audit_log log auth info changes

Log changes to altSecurityIdentities, dNSHostName, msDS-additionalDnsHostNames
and servicePrincipal name in the same way that changes to mdDS-keyCredentialLink
changes are logged.

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

diff --git a/selftest/knownfail b/selftest/knownfail
index d187dd9b787b516e52bb8286dab42f0a59a4827c..ab2d79d7114edb6ee29eafd4ef5e4ce84921471b 100644 (file)
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -338,8 +338,3 @@
 
 # We currently don't send referrals for LDAP modify of non-replicated attrs
 ^samba4.ldap.rodc.python\(rodc\).__main__.RodcTests.test_modify_nonreplicated.*
-
-^samba.tests.audit_log_pass_change.samba.tests.audit_log_pass_change.AuditLogPassChangeTests.test_ldap_altSecurityIdentities
-^samba.tests.audit_log_pass_change.samba.tests.audit_log_pass_change.AuditLogPassChangeTests.test_ldap_service_principal_name
-^samba.tests.audit_log_pass_change.samba.tests.audit_log_pass_change.AuditLogPassChangeTests.test_ldap_msDS_AdditionalDnsHostName
-^samba.tests.audit_log_pass_change.samba.tests.audit_log_pass_change.AuditLogPassChangeTests.test_ldap_dns_host_name

diff --git a/source4/dsdb/common/util.h b/source4/dsdb/common/util.h
index 78adb91a37c16fb92aa60bfcd1deb9550cda7f0a..9a3613047b818e2b63d188ca0054d590e52c83ba 100644 (file)
--- a/source4/dsdb/common/util.h
+++ b/source4/dsdb/common/util.h
@@ -73,6 +73,18 @@
        "unicodePwd", \
        "dBCSPwd"
 
+/*
+ * Attributes that are used in the authentication of an account,
+ * but are not secret. However changes to them should be logged
+ * with password changes.
+ */
+#define DSDB_AUTHENTICATION_ATTRIBUTES \
+       "msDS-KeyCredentialLink", \
+       "altSecurityIdentities", \
+       "dNSHostName", \
+       "msDS-AdditionalDnsHostName", \
+       "servicePrincipalName"
+
 /*
  * ldb opaque values used to pass the user session information to ldb modules
  */

diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
index dd730c82e0aad22263950f27893f9d4cc88a28bf..bf00a9597e94492842b2ed941601031aabac254b 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
@@ -120,38 +120,41 @@ static bool has_password_changed(const struct ldb_message *message)
        return false;
 }
 /*
- * @brief Has a public key been set or unset in this message.
+ * @brief Has authentication information changed.
  *
- * We treat msDS-KeyCredentialLink a bit like a password change,
- * because it changes the remote certificate that is accepted.
+ * authentication information is any non secret information used for
+ * authentication purposes, i.e. msDS-KeyCredentialLink, altSecurityIdentities
  *
- * While this is not a secret, it is significant from a security point
- * of view because, as openssh likes to say, IT IS POSSIBLE THAT
- * SOMEONE IS DOING SOMETHING NASTY by changing trusted keys.
+ * We treat changes to authentication information a bit like a password change,
+ * because it changes the way a user is authenticated.
+ *
+ * While this information is not a secret, it is significant from a security
+ * point of view because, as openssh likes to say, IT IS POSSIBLE THAT
+ * SOMEONE IS DOING SOMETHING NASTY by changing it.
  *
  * A real password change only matters for this reason too. But a
  * *read* of the password hash is a security event in a way that a
- * read of msDS-KeyCredentialLink is not.
+ * read of the authentication information is not.
  *
- * That's why we don't add just public keys to DSDB_PASSWORD_ATTRIBUTES,
+ * That's why we don't add just them to DSDB_PASSWORD_ATTRIBUTES,
  * which is used elsewhere to check secrecy.
  *
  * This does not actually check that the message will change the
- * database -- a message setting msDS-KeyCredentialLink to its current
+ * database -- a message setting an attribute to its current
  * value will still be logged as a change.
  *
- * @return true if the message contains a public key, which currently
- * just means msDS-KeyCredentialLink.
+ * @return true if the message contains authentication information
  */
-static bool has_public_key_changed(const struct ldb_message *message)
+static bool has_authentication_information_changed(
+       const struct ldb_message *message)
 {
        unsigned int i;
        if (message == NULL) {
                return false;
        }
        for (i = 0; i<message->num_elements; i++) {
-               if (ldb_attr_cmp(message->elements[i].name,
-                                "msDS-KeyCredentialLink") == 0) {
+               if (dsdb_audit_is_authentication_information(
+                       message->elements[i].name)) {
                        return true;
                }
        }
@@ -1188,7 +1191,8 @@ static void log_standard_operation(
 
        const struct ldb_message *message = dsdb_audit_get_message(request);
        bool password_changed = has_password_changed(message);
-       bool public_key_changed = has_public_key_changed(message);
+       bool authentication_information_changed
+               = has_authentication_information_changed(message);
        struct audit_private *audit_private =
                talloc_get_type_abort(ldb_module_get_private(module),
                                      struct audit_private);
@@ -1225,7 +1229,7 @@ static void log_standard_operation(
                                PASSWORD_LOG_LVL);
                        TALLOC_FREE(entry);
                }
-               if (public_key_changed) {
+               if (authentication_information_changed) {
                        char *entry = NULL;
                        entry = password_change_human_readable(
                                ctx,
@@ -1279,7 +1283,7 @@ static void log_standard_operation(
                        }
                        json_free(&json);
                }
-               if (public_key_changed) {
+               if (authentication_information_changed) {
                        struct json_object json;
                        json = password_change_json(module, request, reply, true);
                        audit_log_json(

diff --git a/source4/dsdb/samdb/ldb_modules/audit_util.c b/source4/dsdb/samdb/ldb_modules/audit_util.c
index ea212469995d88ac3a1486229d3d33960defefb0..11e1b75561627977f0b4aebac63308087f8852ba 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/audit_util.c
+++ b/source4/dsdb/samdb/ldb_modules/audit_util.c
@@ -22,6 +22,7 @@
  *
  */
 
+#include "common/util.h"
 #include "includes.h"
 #include "ldb_module.h"
 #include "lib/audit_logging/audit_logging.h"
@@ -50,6 +51,14 @@ static const char * const password_attributes[] = {
        DSDB_PASSWORD_ATTRIBUTES,
        NULL};
 
+/*
+ * List of attributes that while not secret, do control the authentication
+ * of an account.
+ */
+static const char * const  authentication_attributes[] = {
+       DSDB_AUTHENTICATION_ATTRIBUTES,
+       NULL};
+
 /*
  * @brief Should the value of the specified value be redacted.
  *
@@ -87,6 +96,21 @@ bool dsdb_audit_is_password_attribute(const char * name)
        return is_password;
 }
 
+
+/*
+ * @brief is the attribute an authentication information attribute?
+ *
+ * Is the attribute a authentication information attribute.
+ *
+ * @return True if the attribute is an "Authentication Information" attribute.
+ */
+bool dsdb_audit_is_authentication_information(const char * name)
+{
+
+       bool is_auth_info = ldb_attr_in_list(authentication_attributes, name);
+       return is_auth_info;
+}
+
 /*
  * @brief Get the remote address from the ldb context.
  *