From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 30 Mar 2021 03:48:31 +0000 (+1300)
Subject: CVE-2021-20251 auth4: Avoid reading the database twice by precaculating some variables
X-Git-Tag: talloc-2.4.0~1076
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b5f78b7b895a6b92cfdc9221b18d67ab18bc2a24;p=thirdparty%2Fsamba.git

CVE-2021-20251 auth4: Avoid reading the database twice by precaculating some variables

These variables are not important to protect against a race with
and a double-read can easily be avoided by moving them up the file
a little.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
---

diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam
deleted file mode 100644
index 438cea46415..00000000000
--- a/selftest/knownfail.d/auth-sam
+++ /dev/null
@@ -1,12 +0,0 @@
-^samba.unittests.auth.sam.test_success_accounting_add_control_failed.none
-^samba.unittests.auth.sam.test_success_accounting_build_mod_req_failed.none
-^samba.unittests.auth.sam.test_success_accounting_commit_failed.none
-^samba.unittests.auth.sam.test_success_accounting_ldb_msg_new_failed.none
-^samba.unittests.auth.sam.test_success_accounting_ldb_request_failed.none
-^samba.unittests.auth.sam.test_success_accounting_ldb_wait_failed.none
-^samba.unittests.auth.sam.test_success_accounting_reread_failed.none
-^samba.unittests.auth.sam.test_success_accounting_rollback_failed.none
-^samba.unittests.auth.sam.test_success_accounting_samdb_rodc_failed.none
-^samba.unittests.auth.sam.test_success_accounting_spurious_bad_pwd_indicator.none
-^samba.unittests.auth.sam.test_success_accounting_start_txn_failed.none
-^samba.unittests.auth.sam.test_success_accounting_update_lastlogon_failed.none
diff --git a/source4/auth/sam.c b/source4/auth/sam.c
index 698324e5cc0..219ee10d5bd 100644
--- a/source4/auth/sam.c
+++ b/source4/auth/sam.c
@@ -1408,6 +1408,8 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx,
 	struct timeval tv_now;
 	NTTIME now;
 	NTTIME lastLogonTimestamp;
+	int64_t lockOutObservationWindow;
+	NTTIME sync_interval_nt = 0;
 	bool am_rodc = false;
 	bool txn_active = false;
 	bool need_db_reread;
@@ -1436,6 +1438,36 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx,
 		return status;
 	}
 
+	if (interactive_or_kerberos == false) {
+		/*
+		 * Avoid calculating this twice, it reads the PSO.  A
+		 * race on this is unimportant.
+		 */
+		lockOutObservationWindow
+			= samdb_result_msds_LockoutObservationWindow(
+				sam_ctx, mem_ctx, domain_dn, msg);
+	}
+
+	ret = samdb_rodc(sam_ctx, &am_rodc);
+	if (ret != LDB_SUCCESS) {
+		status = NT_STATUS_INTERNAL_ERROR;
+		goto error;
+	}
+
+	if (!am_rodc) {
+		/*
+		 * Avoid reading the main domain DN twice.  A race on
+		 * this is unimportant.
+		 */
+		status = authsam_calculate_lastlogon_sync_interval(
+			sam_ctx, mem_ctx, domain_dn, &sync_interval_nt);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			status = NT_STATUS_INTERNAL_ERROR;
+			goto error;
+		}
+	}
+
 get_transaction:
 
 	if (need_db_reread) {
@@ -1485,9 +1517,10 @@ get_transaction:
 	if (interactive_or_kerberos) {
 		badPwdCount = dbBadPwdCount;
 	} else {
-		int64_t lockOutObservationWindow =
-			samdb_result_msds_LockoutObservationWindow(
-				sam_ctx, mem_ctx, domain_dn, msg);
+		/*
+		 * We get lockOutObservationWindow above, before the
+		 * transaction
+		 */
 		badPwdCount = dsdb_effective_badPwdCount(
 			msg, lockOutObservationWindow, now);
 	}
@@ -1562,23 +1595,7 @@ get_transaction:
 		}
 	}
 
-	ret = samdb_rodc(sam_ctx, &am_rodc);
-	if (ret != LDB_SUCCESS) {
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto error;
-	}
-
 	if (!am_rodc) {
-		NTTIME sync_interval_nt;
-
-		status = authsam_calculate_lastlogon_sync_interval(
-			sam_ctx, mem_ctx, domain_dn, &sync_interval_nt);
-
-		if (!NT_STATUS_IS_OK(status)) {
-			status = NT_STATUS_INTERNAL_ERROR;
-			goto error;
-		}
-
 		status = authsam_update_lastlogon_timestamp(
 			sam_ctx,
 			msg_mod,