Attachment 16229 Details for Bug 14497 – Back-port of Metze's _netr_ServerPasswordSet2 protections to s3.

[patch] Back-port of Metze's _netr_ServerPasswordSet2 protections to s3.

s3-netr_ServerPasswordSet2.patch (text/plain), 5.58 KB, created by Jeremy Allison on 2020-09-16 19:56:02 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2020-09-16 19:56:02 UTC

Size: 5.58 KB

patch

obsolete

>From 0d1f4ebfc59eebab959a83117ed59f8d4e115067 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 16 Sep 2020 12:48:21 -0700
>Subject: [PATCH 1/3] s3: rpc_server: Fix mem leak onto p->mem_ctx in error
> path of _netr_ServerPasswordSet2().
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/rpc_server/netlogon/srv_netlog_nt.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c
>index 2a2e2d0ac6e..0c068c6ef17 100644
>--- a/source3/rpc_server/netlogon/srv_netlog_nt.c
>+++ b/source3/rpc_server/netlogon/srv_netlog_nt.c
>@@ -1387,6 +1387,7 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> 						      516);
> 	}
> 	if (!NT_STATUS_IS_OK(status)) {
>+		TALLOC_FREE(creds);
> 		return status;
> 	}
> 
>-- 
>2.25.1
>
>
>From 2e112f6c5b3f70228cc04a44aaf8842090ba1c62 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 16 Sep 2020 12:49:46 -0700
>Subject: [PATCH 2/3] s3: rpc: netlogon: Rename DATA_BLOB plaintext ->
> new_password in _netr_ServerPasswordSet2()
>
>Initialize to data_blob_null. No logic change but will make
>us look closer to the s4 ServerPasswordSet2 code.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/rpc_server/netlogon/srv_netlog_nt.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c
>index 0c068c6ef17..c5ce580bb11 100644
>--- a/source3/rpc_server/netlogon/srv_netlog_nt.c
>+++ b/source3/rpc_server/netlogon/srv_netlog_nt.c
>@@ -1345,7 +1345,7 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> {
> 	NTSTATUS status;
> 	struct netlogon_creds_CredentialState *creds = NULL;
>-	DATA_BLOB plaintext;
>+	DATA_BLOB new_password = data_blob_null;
> 	struct samr_CryptPassword password_buf;
> 	struct _samr_Credentials_t cr = { CRED_TYPE_PLAIN_TEXT, {0}};
> 
>@@ -1393,8 +1393,8 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> 
> 	if (!decode_pw_buffer(p->mem_ctx,
> 			      password_buf.data,
>-			      (char**) &plaintext.data,
>-			      &plaintext.length,
>+			      (char**) &new_password.data,
>+			      &new_password.length,
> 			      CH_UTF16)) {
> 		DEBUG(2,("_netr_ServerPasswordSet2: unable to extract password "
> 			 "from a buffer. Rejecting auth request as a wrong password\n"));
>@@ -1402,7 +1402,7 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> 		return NT_STATUS_WRONG_PASSWORD;
> 	}
> 
>-	cr.creds.password = (const char*) plaintext.data;
>+	cr.creds.password = (const char*) new_password.data;
> 	status = netr_set_machine_account_password(p->mem_ctx,
> 						   p->session_info,
> 						   p->msg_ctx,
>-- 
>2.25.1
>
>
>From eb4f15e9b2ec9f65dfefb5d91948e8bcad8244c1 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 16 Sep 2020 12:53:50 -0700
>Subject: [PATCH 3/3] s3: rpc: Netlogon. Back-port of Metze's
> netr_ServerPasswordSet2() protection.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/rpc_server/netlogon/srv_netlog_nt.c | 62 +++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
>diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c
>index c5ce580bb11..df98b78e71a 100644
>--- a/source3/rpc_server/netlogon/srv_netlog_nt.c
>+++ b/source3/rpc_server/netlogon/srv_netlog_nt.c
>@@ -1346,6 +1346,8 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> 	NTSTATUS status;
> 	struct netlogon_creds_CredentialState *creds = NULL;
> 	DATA_BLOB new_password = data_blob_null;
>+	DATA_BLOB dec_blob = data_blob_null;
>+	DATA_BLOB enc_blob = data_blob_null;
> 	struct samr_CryptPassword password_buf;
> 	struct _samr_Credentials_t cr = { CRED_TYPE_PLAIN_TEXT, {0}};
> 
>@@ -1402,6 +1404,66 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> 		return NT_STATUS_WRONG_PASSWORD;
> 	}
> 
>+	/*
>+	 * Make sure the length field was encrypted,
>+	 * otherwise we are under attack.
>+	 */
>+	if (new_password.length == r->in.new_password->length) {
>+		DBG_WARNING("Length[%zu] field not encrypted\n",
>+			new_password.length);
>+		TALLOC_FREE(creds);
>+		return NT_STATUS_WRONG_PASSWORD;
>+	}
>+
>+	/*
>+	 * Make sure the CryptPassword buffer was encrypted,
>+	 * otherwise we are under attack.
>+	 */
>+	enc_blob = data_blob_const(r->in.new_password->data, 512);
>+	dec_blob = data_blob_const(password_buf.data, 512);
>+	if (data_blob_cmp(&dec_blob, &enc_blob) == 0) {
>+		DBG_WARNING("CryptPassword buffer not encrypted Length[%zu]\n",
>+			new_password.length);
>+		TALLOC_FREE(creds);
>+		return NT_STATUS_WRONG_PASSWORD;
>+	}
>+
>+	/*
>+	 * We don't allow empty passwords for machine accounts.
>+	 */
>+	if (new_password.length < 2) {
>+		DBG_WARNING("Empty password Length[%zu]\n",
>+			new_password.length);
>+		TALLOC_FREE(creds);
>+		return NT_STATUS_WRONG_PASSWORD;
>+	}
>+
>+	/*
>+	 * Check that the password part was actually encrypted,
>+	 * otherwise we are under attack.
>+	 */
>+
>+	memcpy(password_buf.data, r->in.new_password->data, 512);
>+	SIVAL(password_buf.data, 512, new_password.length);
>+
>+	if (!decode_pw_buffer(p->mem_ctx,
>+				password_buf.data,
>+				(char**) &new_password.data,
>+				&new_password.length,
>+				CH_UTF16)) {
>+		DBG_WARNING("Failed extract encrypted password Length[%zu]\n",
>+			new_password.length);
>+		TALLOC_FREE(creds);
>+		return NT_STATUS_WRONG_PASSWORD;
>+	}
>+
>+	if (data_blob_cmp(&new_password, &enc_blob) == 0) {
>+		DBG_WARNING("Password buffer not encrypted Length[%zu]\n",
>+			new_password.length);
>+		TALLOC_FREE(creds);
>+		return NT_STATUS_WRONG_PASSWORD;
>+	}
>+
> 	cr.creds.password = (const char*) new_password.data;
> 	status = netr_set_machine_account_password(p->mem_ctx,
> 						   p->session_info,
>-- 
>2.25.1
>

Flags:

metze: review-

Actions: View

Attachments on bug 14497: 16228 | 16229 | 16230 | 16231 | 16232 | 16233 | 16234 | 16235 | 16236 | 16237 | 16238 | 16239 | 16240 | 16241 | 16242 | 16243 | 16244 | 16245 | 16246 | 16247 | 16248 | 16249 | 16250 | 16251 | 16268 | 16269