From: Gary Lockyer Date: Mon, 6 Oct 2025 00:06:12 +0000 (+1300) Subject: s4:dsdb:audit_log log auth info changes X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9159e8080c99d0cc5e55247d039a07cb7ef2ec03;p=thirdparty%2Fsamba.git 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 Reviewed-by: Douglas Bagnall --- diff --git a/selftest/knownfail b/selftest/knownfail index d187dd9b787..ab2d79d7114 100644 --- 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 78adb91a37c..9a3613047b8 100644 --- 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 dd730c82e0a..bf00a9597e9 100644 --- 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; inum_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 ea212469995..11e1b755616 100644 --- 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. *