[patch] clean up sid when writing to tdb

[Philipp Gesang]

Hey guys,

I’d appreciate feedback on the attached patch.

CI: https://gitlab.com/samba-team/devel/samba/pipelines/32800919

Thank you,
Philipp

-------------- next part --------------
From fb44f4b931e8105ebd19936a2c2eb34a8069532e Mon Sep 17 00:00:00 2001
From: Philipp Gesang <philipp.gesang at intra2net.com>
Date: Thu, 4 Oct 2018 09:25:14 +0200
Subject: [PATCH] s3:secrets: clean up sid before storing

SIDs may contain non-zero memory beyond SubAuthorityCount:

    {
    key(15) = "SECRETS/SID/FOO"
    data(68) = "\01\04\00\00\00\00\00\05\15\00\00\00}u@\8C\08\A3\06nx\95\16\FE\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00`F\92\B7\03\00\00\00\18e\92\B7\03\00\00\00 at H\92\B7\00\00\00\00"
    }

These parts are lost when converting to ``string format syntax``
so a roundtrip conversion does not result in the same binary
representation.

Ensure that these never reach the tdb by using an initialized
copy. This allows bitwise comparisons of secrets.tdb after
dumping SIDs as text and reading them back.

Signed-off-by: Philipp Gesang <philipp.gesang at intra2net.com>
---
 source3/passdb/machine_account_secrets.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c
index a96bf1c0b6a..d8ffcaa7fb6 100644
--- a/source3/passdb/machine_account_secrets.c
+++ b/source3/passdb/machine_account_secrets.c
@@ -114,6 +114,7 @@ bool secrets_store_domain_sid(const char *domain, const struct dom_sid  *sid)
 {
 	char *protect_ids;
 	bool ret;
+	struct dom_sid clean_sid = { 0 };
 
 	protect_ids = secrets_fetch(protect_ids_keystr(domain), NULL);
 	if (protect_ids) {
@@ -126,7 +127,15 @@ bool secrets_store_domain_sid(const char *domain, const struct dom_sid  *sid)
 	}
 	SAFE_FREE(protect_ids);
 
-	ret = secrets_store(domain_sid_keystr(domain), sid, sizeof(struct dom_sid ));
+	/*
+	 * use a copy to prevent uninitialized memory from being carried over
+	 * to the tdb
+	 */
+	sid_copy(&clean_sid, sid);
+
+	ret = secrets_store(domain_sid_keystr(domain),
+			    &clean_sid,
+			    sizeof(struct dom_sid));
 
 	/* Force a re-query, in the case where we modified our domain */
 	if (ret) {
-- 
2.17.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181012/0d80ccdd/signature.sig>