The branch, master has been updated
via 70bd05c2251 dsdb:audit log: cmocka unit tests for KCL
via c9e752ab18f dsdb:audit: bump minor version of password logs (1.1 ->
1.2)
via 139caa8adb2 dsdb:audit: log if msDS-KeyCredentialLink changed
via 0c4853fd858 dsdb:audit: password_change loggers take a new flag
argument
via 976ff207da3 dsdb: audit: replace local min() with libreplace MIN()
via 387731687bf dsdb:audit:test: test msDS-KeyCredentialLink change
logging
via d094cd9fea8 librpc:idl: add windows event 5136 object change
from 3d9b1dd3552 winbindd: Simplify parse_domain_user()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 70bd05c225153222c889fb4ef5e5c611d6631a18
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Sun Aug 31 21:24:34 2025 +1200
dsdb:audit log: cmocka unit tests for KCL
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Gary Lockyer <g...@catalyst.net.nz>
Autobuild-User(master): Douglas Bagnall <dbagn...@samba.org>
Autobuild-Date(master): Wed Sep 3 03:13:47 UTC 2025 on atb-devel-224
commit c9e752ab18f43758d704951f7f31e39dafa6fdb4
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Thu Aug 28 17:09:43 2025 +1200
dsdb:audit: bump minor version of password logs (1.1 -> 1.2)
In https://wiki.samba.org/index.php/Interpreting_JSON_Audit_Logs we say:
> a version number for the JSON format. It has two parts.
>
> major: incremented if fields change meaning
> minor: incremented if a field is added
>
> A change in possible values does not usually trigger a version
> change. This is obviously true for client supplied data, but also
> applies to e.g. passwordType, where the set of supported password
> formats can change over time without changing the JSON version.
The last paragraph explicitly exempts us from a version change for
adding a new possible value ("Public key change") to the "action"
field. On the other hand we have expanded the scope of the log, which
deserves some version recognition.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Gary Lockyer <g...@catalyst.net.nz>
commit 139caa8adb2396e36eb195c593e91d7e37038315
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Thu Aug 28 17:02:34 2025 +1200
dsdb:audit: log if msDS-KeyCredentialLink changed
As noted in the comments, by "changed" we mean "set" or "unset".
Explicitly re-setting to the current value will be logged as if it
were a change.
This follows the behaviour of the password fields.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Gary Lockyer <g...@catalyst.net.nz>
commit 0c4853fd8580413542a1a5fcc5fcb82191983689
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Thu Aug 28 17:03:51 2025 +1200
dsdb:audit: password_change loggers take a new flag argument
This will allow a key credential link change to be logged, but we
don't do that anywhere in this commit.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Gary Lockyer <g...@catalyst.net.nz>
commit 976ff207da315f05ed1cc967f1eb3c1e16aadbe6
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Wed Aug 27 16:02:56 2025 +1200
dsdb: audit: replace local min() with libreplace MIN()
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Gary Lockyer <g...@catalyst.net.nz>
commit 387731687bfcaddf7c1190899ad64a5ee35891ef
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Fri Aug 29 09:57:48 2025 +1200
dsdb:audit:test: test msDS-KeyCredentialLink change logging
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Gary Lockyer <g...@catalyst.net.nz>
commit d094cd9fea8cfeb49bb6eeebef8dd706fc47ce51
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Thu Aug 28 12:37:13 2025 +1200
librpc:idl: add windows event 5136 object change
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Gary Lockyer <g...@catalyst.net.nz>
-----------------------------------------------------------------------
Summary of changes:
librpc/idl/windows_event_ids.idl | 12 +-
python/samba/tests/audit_log_pass_change.py | 289 ++++++++++++++++++++-
source4/dsdb/samdb/ldb_modules/audit_log.c | 104 +++++++-
source4/dsdb/samdb/ldb_modules/audit_util.c | 4 +-
.../dsdb/samdb/ldb_modules/tests/test_audit_log.c | 163 +++++++++++-
.../ldb_modules/tests/test_audit_log_errors.c | 10 +-
6 files changed, 555 insertions(+), 27 deletions(-)
Changeset truncated at 500 lines:
diff --git a/librpc/idl/windows_event_ids.idl b/librpc/idl/windows_event_ids.idl
index f482800d897..04f0cc46720 100644
--- a/librpc/idl/windows_event_ids.idl
+++ b/librpc/idl/windows_event_ids.idl
@@ -25,7 +25,17 @@ interface windows_events
EVT_ID_USER_ADDED_TO_UNIVERSAL_SEC_GROUP = 4756,
EVT_ID_USER_REMOVED_FROM_UNIVERSAL_SEC_GROUP = 4757,
EVT_ID_USER_ADDED_TO_UNIVERSAL_GROUP = 4761,
- EVT_ID_USER_REMOVED_FROM_UNIVERSAL_GROUP = 4762
+ EVT_ID_USER_REMOVED_FROM_UNIVERSAL_GROUP = 4762,
+ /*
+ * Any change to any object will cause event 5136 in
+ * Windows AD -- if that object has a SACL asking for
+ * auditing.
+ *
+ * This event is used for msDS-KeyCredentialLink
+ * changes which do not have a specific event code.
+ */
+ EVT_ID_DIRECTORY_OBJECT_CHANGE = 5136
+
} event_id_type;
/* See
https://learn.microsoft.com/en-us/windows-server/security/credentials-protection-and-management/authentication-policies-and-authentication-policy-silos#BKMK_ErrorandEvents
*/
diff --git a/python/samba/tests/audit_log_pass_change.py
b/python/samba/tests/audit_log_pass_change.py
index 1039e17a180..2aa4be48ce0 100644
--- a/python/samba/tests/audit_log_pass_change.py
+++ b/python/samba/tests/audit_log_pass_change.py
@@ -26,10 +26,14 @@ import os
from samba.tests.audit_log_base import AuditLogTestBase
from samba.tests import delete_force
from samba.net import Net
-from ldb import ERR_INSUFFICIENT_ACCESS_RIGHTS
+from ldb import (LdbError,
+ ERR_INSUFFICIENT_ACCESS_RIGHTS,
+ ERR_INVALID_DN_SYNTAX,
+ ERR_NO_SUCH_ATTRIBUTE)
from samba.dcerpc.windows_event_ids import (
EVT_ID_PASSWORD_CHANGE,
- EVT_ID_PASSWORD_RESET
+ EVT_ID_PASSWORD_RESET,
+ EVT_ID_DIRECTORY_OBJECT_CHANGE
)
@@ -329,3 +333,284 @@ class AuditLogPassChangeTests(AuditLogTestBase):
self.assertEqual(service_description, "LDAP")
self.assertTrue(self.is_guid(audit["sessionId"]))
self.assertTrue(self.is_guid(audit["transactionId"]))
+
+
+ # These are some valid BinaryDN prefixes for
+ # msDS-KeyCredentialLink values.
+ kcl_prefixes = [
+ ("B:772:000200002000012849ABF23C432F5428756A641827A6CD2E4A4F40"
+ "9F4DE57BA152B7C2F31E46EC200002A5499B70EC0FBEF4BCB78AF3878D08"
+ "2B2D43527BBAA230462707947C128BAF4226010330820122300D06092A86"
+ "4886F70D01010105000382010F003082010A02820101009A16283EA51D94"
+ "11FD54C73615D90E4B3B8DAF152D699A79C3B217DDBA7787DE419E3E31E6"
+ "012088FD9B4EC0EA9199E91EBE2F99F48A4E7084D819CF76C67C58F5050E"
+ "E5428B09676CA475EADFAD91FBEF2835820BE4CDE3F3A3B33DB6A0C75956"
+ "47CA8489566EBCF6B748DC637EC2D34D255E008DAF93F3DC77478B7526DE"
+ "5A6B4638FC50A622233C2BBA935A0E2BBA6DDCE32D539E57AD12FAA7BA33"
+ "1C92D32E54E05290132E66D25476458FDDFBF4940DA97F19B63C30EFE0EF"
+ "1F77A2751CD8EAA7EF6DFA54FF0B3500D9E66A1ECC9D435FF0206EA27047"
+ "8A37CD9F949EB6CA82925A138C5852C5C763AD6EA60003FCE5C7D486382B"
+ "8E347B8CF6645F449902030100010100040108000900CB9A52E817DC01:"),
+
+ ("B:772:00020000200001E251886DDA8335FA6A9B311F1FE4ABCDA7C04F05"
+ "33668BA7B143760C675703BB200002E324A0CCE2393C603686A0C7828B53"
+ "77A9B5E8C639131F657C3CEB39EDED7D4F26010330820122300D06092A86"
+ "4886F70D01010105000382010F003082010A0282010100A5DF595B4E0F36"
+ "9A4EF8CD5A2F6AF9322460C14175DDB7DE5F3A494A7591EC1096A5150800"
+ "F80401F7D8739C8900165035E231E85AA0E39A9C75BE760764BCDC82A4CB"
+ "955D84AFA47A8CACC35BAB6775478BF214A81726263D79CAA1623EA60DAB"
+ "2C61CAE29905FE4A89467736F47ADCA27170DBC77F5A82E28075E4D44FB9"
+ "76FFF5400B1AFC21B57C52718864253D57E780DEF8F40C990D30FB253831"
+ "BC95B01EEFFB312E52F7B773CFB1FF9E48A40B9C82E6B176464087C862F5"
+ "EA1D38809CF6454A8F169B993FFA857D1D928E4488EB13C947EE847D30F1"
+ "7CF1FAA85DE6AFD07ED82504C024E0CEA5B47E02515E57C3258963E80137"
+ "15EB0D3B126067A79F02030100010100040108000900CB9A52E817DC01:"),
+
+ ("B:772:0002000020000118ADEEC2CBCC63FCA44F332CAE39275E12CB881F"
+ "68B582ED2F3982718832126120000269F60B033A8697DB69835A356C9263"
+ "1CF87FDE6E448F3426285B3978E85C385C26010330820122300D06092A86"
+ "4886F70D01010105000382010F003082010A0282010100CADFE83AC8FCE1"
+ "B7A999AE162DC6BD6CC53D686E6F0CB866AFADCF64F736249E2CB4F438E2"
+ "78B636C4151C540BC4677821E66E6D88E6875A6B9B4F473D41C1C05376A4"
+ "F929076E515698F076F1F1BBEB25AAE062C9973D6436E3E4B48F74B5C0A5"
+ "B1FAD5026B8AB9E021849FFF8D18E54643C5F5FB57590BDC1CBC747C53CE"
+ "A696D5379EE3678E8AE015E2AE4AEA5B79F03E953D415986197F00D876E1"
+ "D23A28CC944901FCE5A902C9671CC46D7AA7F349F4F80CEF74FF035AEAEE"
+ "30D6FD8F907B08C9A23031B9E44AC84A4010BFA06A468D94B034DDF202A7"
+ "1B20663864703AC8205CE7C60B5FC18253C94CB8389381E05DB70B08BCCC"
+ "EE58A5E81BA2FE18AD02030100010100040108000980613353E817DC01:"),
+ ]
+
+ def test_ldap_key_credential_link(self):
+ """Test logging of msDS-KeyCredentialLink public key changes.
+ """
+ # The restrictions on msDS-KeyCredentialLink changes work
+ # different from those on the password attributes. We don't
+ # care about dsHeuristics, minPwdAge, etc, but we do care
+ # about things like GUID_DRS_DS_VALIDATED_WRITE_COMPUTER and
+ # ACLs.
+ #
+ # To avoid all the set-up cost of making a fresh DB and user,
+ # we use sub-tests in this test.
+ #
+ # See source4/dsdb/tests/pythpn/key_credential_link.py for
+ # deeper tests of the restrictions on setting this attribute.
+
+ dn = f"cn={USER_NAME},cn=users,{self.base_dn}"
+ self.discardSetupMessages(dn)
+
+ kcls = [f"{x}{dn}" for x in self.kcl_prefixes]
+ session_id = self.get_session()
+ service_description = self.get_service_description()
+ self.assertEqual(service_description, "LDAP")
+ transactions_seen = set()
+
+ with self.subTest("initial setup"):
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "add: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {kcls[0]}\n")
+ messages = self.waitForMessages(1, dn=dn)
+ print("Received %d messages" % len(messages))
+ self.assertEqual(1, len(messages))
+ audit = messages[0]["passwordChange"]
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE, audit["eventId"])
+ self.assertEqual("Public key change", audit["action"])
+ self.assertEqual(dn, audit["dn"])
+ self.assertIn(self.remoteAddress, audit["remoteAddress"])
+ self.assertEqual(session_id, audit["sessionId"])
+ transactions_seen.add(audit["transactionId"])
+ self.assertEqual(0, audit["statusCode"])
+ self.discardMessages()
+
+ with self.subTest("replace"):
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "replace: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {kcls[1]}\n")
+ messages = self.waitForMessages(1, dn=dn)
+ print("Received %d messages" % len(messages))
+ self.assertEqual(1, len(messages))
+ audit = messages[0]["passwordChange"]
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE, audit["eventId"])
+ self.assertEqual("Public key change", audit["action"])
+ self.assertEqual(dn, audit["dn"])
+ self.assertIn(self.remoteAddress, audit["remoteAddress"])
+ self.assertTrue(self.is_guid(audit["sessionId"]))
+ transactions_seen.add(audit["transactionId"])
+ self.discardMessages()
+
+ with self.subTest("constrained replace"):
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "delete: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {kcls[1]}\n"
+ "add: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {kcls[2]}\n")
+ messages = self.waitForMessages(1, dn=dn)
+ print("Received %d messages" % len(messages))
+ self.assertEqual(1, len(messages))
+ audit = messages[0]["passwordChange"]
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE, audit["eventId"])
+ self.assertEqual("Public key change", audit["action"])
+ self.assertEqual(dn, audit["dn"])
+ self.assertIn(self.remoteAddress, audit["remoteAddress"])
+ self.assertEqual(session_id, audit["sessionId"])
+ self.assertEqual(0, audit["statusCode"])
+ transactions_seen.add(audit["transactionId"])
+ self.discardMessages()
+
+ with self.subTest("identical replace"):
+ # replacing the KCL with itself still sends the message.
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "replace: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {kcls[2]}\n")
+ messages = self.waitForMessages(1, dn=dn)
+ print("Received %d messages" % len(messages))
+ self.assertEqual(1, len(messages))
+ audit = messages[0]["passwordChange"]
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE, audit["eventId"])
+ self.assertEqual("Public key change", audit["action"])
+ self.assertEqual(dn, audit["dn"])
+ self.assertIn(self.remoteAddress, audit["remoteAddress"])
+ self.assertEqual(session_id, audit["sessionId"])
+ self.assertEqual(0, audit["statusCode"])
+ transactions_seen.add(audit["transactionId"])
+ self.discardMessages()
+
+ with self.subTest("replace KCL AND password"):
+ # there should be two messages
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "replace: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {kcls[0]}\n"
+ "replace: userPassword\n"
+ "userPassword: gruffalo3.\n")
+ messages = self.waitForMessages(2, dn=dn)
+ self.assertEqual(2, len(messages))
+ pwd_audit = messages[0]["passwordChange"]
+ kcl_audit = messages[1]["passwordChange"]
+ # we send the password message first, but we don't need to
+ # depend on that.
+ if pwd_audit["eventId"] == EVT_ID_DIRECTORY_OBJECT_CHANGE:
+ kcl_audit, pwd_audit = pwd_audit, kcl_audit
+ del audit
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE,
kcl_audit["eventId"])
+ self.assertEqual(EVT_ID_PASSWORD_RESET, pwd_audit["eventId"])
+ self.assertEqual("Public key change", kcl_audit["action"])
+ self.assertEqual("Reset", pwd_audit["action"])
+ # if we delete the action and eventId, the rest of
+ # structures should be the same (sessionId, transactionId,
+ # version, etc). Timestamps are in the outer message.
+ del pwd_audit["eventId"]
+ del pwd_audit["action"]
+ del kcl_audit["eventId"]
+ del kcl_audit["action"]
+ self.assertEqual(kcl_audit, pwd_audit)
+ transactions_seen.add(pwd_audit["transactionId"])
+ self.discardMessages()
+
+ with self.subTest("delete"):
+ # replacing the KCL with itself still sends the message.
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "delete: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {kcls[0]}\n")
+ messages = self.waitForMessages(1, dn=dn)
+ print("Received %d messages" % len(messages))
+ self.assertEqual(1, len(messages))
+ audit = messages[0]["passwordChange"]
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE, audit["eventId"])
+ self.assertEqual("Public key change", audit["action"])
+ self.assertEqual(dn, audit["dn"])
+ self.assertIn(self.remoteAddress, audit["remoteAddress"])
+ self.assertEqual(session_id, audit["sessionId"])
+ self.assertEqual(0, audit["statusCode"])
+ transactions_seen.add(audit["transactionId"])
+ self.discardMessages()
+
+ with self.subTest("delete that which does not exist"):
+ # still sends a message
+ with self.assertRaises(LdbError) as e:
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "delete: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {kcls[2]}\n")
+ self.assertEqual(e.exception.args[0], ERR_NO_SUCH_ATTRIBUTE)
+ messages = self.waitForMessages(1, dn=dn)
+ print("Received %d messages" % len(messages))
+ # We still get the message on a failed attempt
+ self.assertEqual(1, len(messages))
+ audit = messages[0]["passwordChange"]
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE, audit["eventId"])
+ self.assertEqual("Public key change", audit["action"])
+ self.assertEqual(dn, audit["dn"])
+ self.assertIn(self.remoteAddress, audit["remoteAddress"])
+ self.assertEqual(session_id, audit["sessionId"])
+ transactions_seen.add(audit["transactionId"])
+ self.discardMessages()
+ with self.subTest("check status code"):
+ self.assertEqual(ERR_NO_SUCH_ATTRIBUTE, audit["statusCode"])
+ self.assertEqual("No such attribute", audit["status"])
+
+ with self.subTest("add bad KCL DN value"):
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "replace: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: B:4:f1ea:{dn}\n")
+ messages = self.waitForMessages(1, dn=dn)
+ self.discardMessages()
+ audit = messages[0]["passwordChange"]
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE, audit["eventId"])
+ self.assertEqual("Public key change", audit["action"])
+ self.assertEqual(dn, audit["dn"])
+ self.assertIn(self.remoteAddress, audit["remoteAddress"])
+ self.assertEqual(session_id, audit["sessionId"])
+ self.assertEqual(0, audit["statusCode"])
+ transactions_seen.add(audit["transactionId"])
+
+ # these should all have been separate transactions
+ with self.subTest("check transactions"):
+ self.assertEqual(len(transactions_seen), 8)
+ for t in transactions_seen:
+ self.assertTrue(self.is_guid(t))
+
+ with self.subTest("add a second DN value"):
+ # should this fail?
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "add: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: B:4:f1ee:{dn}\n")
+ messages = self.waitForMessages(1, dn=dn)
+ self.discardMessages()
+ audit = messages[0]["passwordChange"]
+ self.assertEqual(EVT_ID_DIRECTORY_OBJECT_CHANGE, audit["eventId"])
+ self.assertEqual("Public key change", audit["action"])
+ self.assertEqual(dn, audit["dn"])
+ self.assertIn(self.remoteAddress, audit["remoteAddress"])
+ self.assertEqual(session_id, audit["sessionId"])
+ self.assertEqual(0, audit["statusCode"])
+
+ with self.subTest("add bad Binary DN value"):
+ for bad_dn in ('B:6:f1ea:{dn}', 'flea', dn):
+ with self.assertRaises(LdbError) as e:
+ self.ldb.modify_ldif(
+ f"dn: {dn}\n"
+ "changetype: modify\n"
+ "replace: msDS-keyCredentialLink\n"
+ f"msDS-keyCredentialLink: {bad_dn}\n")
+ self.assertEqual(e.exception.args[0], ERR_INVALID_DN_SYNTAX)
+ # no messages from those the 3 bad DNs
+ # because DN syntax check comes first
+ messages = self.waitForMessages(1, dn=dn)
+ self.assertEqual(0, len(messages))
diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c
b/source4/dsdb/samdb/ldb_modules/audit_log.c
index 7cc3ff67d77..dd730c82e0a 100644
--- a/source4/dsdb/samdb/ldb_modules/audit_log.c
+++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
@@ -44,7 +44,7 @@
#define PASSWORD_JSON_TYPE "passwordChange"
#define PASSWORD_HR_TAG "Password Change"
#define PASSWORD_MAJOR 1
-#define PASSWORD_MINOR 1
+#define PASSWORD_MINOR 2
#define PASSWORD_LOG_LVL 5
#define TRANSACTION_JSON_TYPE "dsdbTransaction"
@@ -65,8 +65,6 @@
*/
#define MAX_LENGTH 1024
-#define min(a, b) (((a)>(b))?(b):(a))
-
/*
* Private data for the module, stored in the ldb_module private data
*/
@@ -121,6 +119,44 @@ static bool has_password_changed(const struct ldb_message
*message)
}
return false;
}
+/*
+ * @brief Has a public key been set or unset in this message.
+ *
+ * We treat msDS-KeyCredentialLink a bit like a password change,
+ * because it changes the remote certificate that is accepted.
+ *
+ * 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.
+ *
+ * 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.
+ *
+ * That's why we don't add just public keys 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
+ * value will still be logged as a change.
+ *
+ * @return true if the message contains a public key, which currently
+ * just means msDS-KeyCredentialLink.
+ */
+static bool has_public_key_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) {
+ return true;
+ }
+ }
+ return false;
+}
/*
* @brief get the password change windows event id
@@ -483,7 +519,8 @@ failure:
static struct json_object password_change_json(
struct ldb_module *module,
const struct ldb_request *request,
- const struct ldb_reply *reply)
+ const struct ldb_reply *reply,
+ bool public_key_changed)
{
struct ldb_context *ldb = NULL;
const struct dom_sid *sid = NULL;
@@ -504,10 +541,14 @@ static struct json_object password_change_json(
remote = dsdb_audit_get_remote_address(ldb);
sid = dsdb_audit_get_user_sid(module);
dn = dsdb_audit_get_primary_dn(request);
- action = get_password_action(request, reply);
unique_session_token = dsdb_audit_get_unique_session_token(module);
- event_id = get_password_windows_event_id(request, reply);
-
+ if (public_key_changed) {
+ action = "Public key change";
+ event_id = EVT_ID_DIRECTORY_OBJECT_CHANGE;
+ } else {
+ action = get_password_action(request, reply);
+ event_id = get_password_windows_event_id(request, reply);
+ }
audit = json_new_object();
if (json_is_invalid(&audit)) {
goto failure;
@@ -773,7 +814,8 @@ static char *password_change_human_readable(
TALLOC_CTX *mem_ctx,
struct ldb_module *module,
const struct ldb_request *request,
- const struct ldb_reply *reply)
+ const struct ldb_reply *reply,
+ bool is_public_key_change)
{
struct ldb_context *ldb = NULL;
const char *remote_host = NULL;
@@ -791,7 +833,12 @@ static char *password_change_human_readable(
remote_host = dsdb_audit_get_remote_host(ldb, ctx);
sid = dsdb_audit_get_user_sid(module);
timestamp = audit_get_timestamp(ctx);
- action = get_password_action(request, reply);
+
+ if (is_public_key_change) {
+ action = "Public key change";
+ } else {
+ action = get_password_action(request, reply);
+ }
dn = dsdb_audit_get_primary_dn(request);
log_entry = talloc_asprintf(
@@ -881,7 +928,7 @@ static char *log_attributes(
}
v = message->elements[i].values[j];
- length = min(MAX_LENGTH, v.length);
+ length = MIN(MAX_LENGTH, v.length);
use_b64_encode = ldb_should_b64_encode(ldb, &v);
if (use_b64_encode) {
const char *encoded = ldb_base64_encode(
@@ -1141,6 +1188,7 @@ 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);
struct audit_private *audit_private =
talloc_get_type_abort(ldb_module_get_private(module),
struct audit_private);
@@ -1168,7 +1216,23 @@ static void log_standard_operation(
ctx,
module,
request,
- reply);
+ reply,
+ false);
+ audit_log_human_text(
+ PASSWORD_HR_TAG,
+ entry,
+ DBGC_DSDB_PWD_AUDIT,
+ PASSWORD_LOG_LVL);
+ TALLOC_FREE(entry);
+ }
+ if (public_key_changed) {
+ char *entry = NULL;
+ entry = password_change_human_readable(
+ ctx,
+ module,
+ request,
+ reply,
+ true);
audit_log_human_text(
PASSWORD_HR_TAG,
entry,
@@ -1201,7 +1265,23 @@ static void log_standard_operation(
&& audit_private->send_password_events)) {
if (password_changed) {
struct json_object json;
- json = password_change_json(module, request, reply);
+ json = password_change_json(module, request, reply,
false);
+ audit_log_json(
+ &json,
+ DBGC_DSDB_PWD_AUDIT_JSON,
+ PASSWORD_LOG_LVL);
--
Samba Shared Repository
