CVE-2021-23192: dcesrv_core: only the first fragment specifies the auth_contexts

All other fragments blindly inherit it.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Samuel Cabrero <scabrero@samba.org>

6 files changed, 109 insertions, 46 deletions

diff --git a/librpc/rpc/dcerpc_pkt_auth.c b/librpc/rpc/dcerpc_pkt_auth.c
index 322d7497893..1cb191468b5 100644
--- a/librpc/rpc/dcerpc_pkt_auth.c
+++ b/librpc/rpc/dcerpc_pkt_auth.c
@@ -39,6 +39,7 @@
 
 NTSTATUS dcerpc_ncacn_pull_pkt_auth(const struct dcerpc_auth *auth_state,
 				    struct gensec_security *gensec,
+				    bool check_pkt_auth_fields,
 				    TALLOC_CTX *mem_ctx,
 				    enum dcerpc_pkt_type ptype,
 				    uint8_t required_flags,
@@ -115,16 +116,18 @@ NTSTATUS dcerpc_ncacn_pull_pkt_auth(const struct dcerpc_auth *auth_state,
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	if (auth.auth_type != auth_state->auth_type) {
-		return NT_STATUS_ACCESS_DENIED;
-	}
+	if (check_pkt_auth_fields) {
+		if (auth.auth_type != auth_state->auth_type) {
+			return NT_STATUS_ACCESS_DENIED;
+		}
 
-	if (auth.auth_level != auth_state->auth_level) {
-		return NT_STATUS_ACCESS_DENIED;
-	}
+		if (auth.auth_level != auth_state->auth_level) {
+			return NT_STATUS_ACCESS_DENIED;
+		}
 
-	if (auth.auth_context_id != auth_state->auth_context_id) {
-		return NT_STATUS_ACCESS_DENIED;
+		if (auth.auth_context_id != auth_state->auth_context_id) {
+			return NT_STATUS_ACCESS_DENIED;
+		}
 	}
 
 	/* check signature or unseal the packet */
diff --git a/librpc/rpc/dcerpc_pkt_auth.h b/librpc/rpc/dcerpc_pkt_auth.h
index c0d23b91c05..1dcee12f53c 100644
--- a/librpc/rpc/dcerpc_pkt_auth.h
+++ b/librpc/rpc/dcerpc_pkt_auth.h
@@ -31,6 +31,7 @@
 
 NTSTATUS dcerpc_ncacn_pull_pkt_auth(const struct dcerpc_auth *auth_state,
 				    struct gensec_security *gensec,
+				    bool check_pkt_auth_fields,
 				    TALLOC_CTX *mem_ctx,
 				    enum dcerpc_pkt_type ptype,
 				    uint8_t required_flags,
diff --git a/librpc/rpc/dcesrv_auth.c b/librpc/rpc/dcesrv_auth.c
index 62f69696dad..fec8df513a8 100644
--- a/librpc/rpc/dcesrv_auth.c
+++ b/librpc/rpc/dcesrv_auth.c
@@ -443,6 +443,10 @@ bool dcesrv_auth_prepare_auth3(struct dcesrv_call_state *call)
 		return false;
 	}
 
+	if (auth->auth_invalid) {
+		return false;
+	}
+
 	/* We can't work without an existing gensec state */
 	if (auth->gensec_security == NULL) {
 		return false;
@@ -529,6 +533,10 @@ bool dcesrv_auth_alter(struct dcesrv_call_state *call)
 		return false;
 	}
 
+	if (auth->auth_invalid) {
+		return false;
+	}
+
 	if (call->in_auth_info.auth_type != auth->auth_type) {
 		return false;
 	}
@@ -595,6 +603,7 @@ bool dcesrv_auth_pkt_pull(struct dcesrv_call_state *call,
 		.auth_level = auth->auth_level,
 		.auth_context_id = auth->auth_context_id,
 	};
+	bool check_pkt_auth_fields;
 	NTSTATUS status;
 
 	if (!auth->auth_started) {
@@ -610,8 +619,27 @@ bool dcesrv_auth_pkt_pull(struct dcesrv_call_state *call,
 		return false;
 	}
 
+	if (call->pkt.pfc_flags & DCERPC_PFC_FLAG_FIRST) {
+		/*
+		 * The caller most likely checked this
+		 * already, but we better double check.
+		 */
+		check_pkt_auth_fields = true;
+	} else {
+		/*
+		 * The caller already found first fragment
+		 * and is passing the auth_state of it.
+		 * A server is supposed to use the
+		 * setting of the first fragment and
+		 * completely ignore the values
+		 * on the remaining fragments
+		 */
+		check_pkt_auth_fields = false;
+	}
+
 	status = dcerpc_ncacn_pull_pkt_auth(&tmp_auth,
 					    auth->gensec_security,
+					    check_pkt_auth_fields,
 					    call,
 					    pkt->ptype,
 					    required_flags,
diff --git a/librpc/rpc/dcesrv_core.c b/librpc/rpc/dcesrv_core.c
index fdd56aa0874..d16159b0b6c 100644
--- a/librpc/rpc/dcesrv_core.c
+++ b/librpc/rpc/dcesrv_core.c
@@ -1790,6 +1790,10 @@ static NTSTATUS dcesrv_request(struct dcesrv_call_state *call)
 	struct ndr_pull *pull;
 	NTSTATUS status;
 
+	if (auth->auth_invalid) {
+		return dcesrv_fault_disconnect(call, DCERPC_NCA_S_PROTO_ERROR);
+	}
+
 	if (!auth->auth_finished) {
 		return dcesrv_fault_disconnect(call, DCERPC_NCA_S_PROTO_ERROR);
 	}
@@ -1953,6 +1957,7 @@ static NTSTATUS dcesrv_process_ncacn_packet(struct dcesrv_connection *dce_conn,
 	enum dcerpc_AuthType auth_type = 0;
 	enum dcerpc_AuthLevel auth_level = 0;
 	uint32_t auth_context_id = 0;
+	bool auth_invalid = false;
 
 	call = talloc_zero(dce_conn, struct dcesrv_call_state);
 	if (!call) {
@@ -1984,12 +1989,16 @@ static NTSTATUS dcesrv_process_ncacn_packet(struct dcesrv_connection *dce_conn,
 
 	if (call->auth_state == NULL) {
 		struct dcesrv_auth *a = NULL;
+		bool check_type_level = true;
 
 		auth_type = dcerpc_get_auth_type(&blob);
 		auth_level = dcerpc_get_auth_level(&blob);
 		auth_context_id = dcerpc_get_auth_context_id(&blob);
 
 		if (call->pkt.ptype == DCERPC_PKT_REQUEST) {
+			if (!(call->pkt.pfc_flags & DCERPC_PFC_FLAG_FIRST)) {
+				check_type_level = false;
+			}
 			dce_conn->default_auth_level_connect = NULL;
 			if (auth_level == DCERPC_AUTH_LEVEL_CONNECT) {
 				dce_conn->got_explicit_auth_level_connect = true;
@@ -1999,14 +2008,19 @@ static NTSTATUS dcesrv_process_ncacn_packet(struct dcesrv_connection *dce_conn,
 		for (a = dce_conn->auth_states; a != NULL; a = a->next) {
 			num_auth_ctx++;
 
-			if (a->auth_type != auth_type) {
+			if (a->auth_context_id != auth_context_id) {
 				continue;
 			}
-			if (a->auth_finished && a->auth_level != auth_level) {
-				continue;
+
+			if (a->auth_type != auth_type) {
+				auth_invalid = true;
 			}
-			if (a->auth_context_id != auth_context_id) {
-				continue;
+			if (a->auth_level != auth_level) {
+				auth_invalid = true;
+			}
+
+			if (check_type_level && auth_invalid) {
+				a->auth_invalid = true;
 			}
 
 			DLIST_PROMOTE(dce_conn->auth_states, a);
@@ -2033,6 +2047,7 @@ static NTSTATUS dcesrv_process_ncacn_packet(struct dcesrv_connection *dce_conn,
 			/*
 			 * This can never be valid.
 			 */
+			auth_invalid = true;
 			a->auth_invalid = true;
 		}
 		call->auth_state = a;
@@ -2101,6 +2116,18 @@ static NTSTATUS dcesrv_process_ncacn_packet(struct dcesrv_connection *dce_conn,
 			}
 			/* only one request is possible in the fragmented list */
 			if (dce_conn->incoming_fragmented_call_list != NULL) {
+				call->fault_code = DCERPC_NCA_S_PROTO_ERROR;
+
+				existing = dcesrv_find_fragmented_call(dce_conn,
+								       call->pkt.call_id);
+				if (existing != NULL && call->auth_state != existing->auth_state) {
+					call->context = dcesrv_find_context(call->conn,
+								call->pkt.u.request.context_id);
+
+					if (call->pkt.auth_length != 0 && existing->context == call->context) {
+						call->fault_code = DCERPC_FAULT_SEC_PKG_ERROR;
+					}
+				}
 				if (!(dce_conn->state_flags & DCESRV_CALL_STATE_FLAG_MULTIPLEXED)) {
 					/*
 					 * Without DCERPC_PFC_FLAG_CONC_MPX
@@ -2110,11 +2137,14 @@ static NTSTATUS dcesrv_process_ncacn_packet(struct dcesrv_connection *dce_conn,
 					 * This is important to get the
 					 * call_id and context_id right.
 					 */
+					dce_conn->incoming_fragmented_call_list->fault_code = call->fault_code;
 					TALLOC_FREE(call);
 					call = dce_conn->incoming_fragmented_call_list;
 				}
-				return dcesrv_fault_disconnect0(call,
-						DCERPC_NCA_S_PROTO_ERROR);
+				if (existing != NULL) {
+					call->context = existing->context;
+				}
+				return dcesrv_fault_disconnect0(call, call->fault_code);
 			}
 			if (call->pkt.pfc_flags & DCERPC_PFC_FLAG_PENDING_CANCEL) {
 				return dcesrv_fault_disconnect(call,
@@ -2127,17 +2157,43 @@ static NTSTATUS dcesrv_process_ncacn_packet(struct dcesrv_connection *dce_conn,
 					DCERPC_PFC_FLAG_DID_NOT_EXECUTE);
 			}
 		} else {
-			const struct dcerpc_request *nr = &call->pkt.u.request;
-			const struct dcerpc_request *er = NULL;
 			int cmp;
 
 			existing = dcesrv_find_fragmented_call(dce_conn,
 							call->pkt.call_id);
 			if (existing == NULL) {
+				if (!(dce_conn->state_flags & DCESRV_CALL_STATE_FLAG_MULTIPLEXED)) {
+					/*
+					 * Without DCERPC_PFC_FLAG_CONC_MPX
+					 * we need to return the FAULT on the
+					 * already existing call.
+					 *
+					 * This is important to get the
+					 * call_id and context_id right.
+					 */
+					if (dce_conn->incoming_fragmented_call_list != NULL) {
+						TALLOC_FREE(call);
+						call = dce_conn->incoming_fragmented_call_list;
+					}
+					return dcesrv_fault_disconnect0(call,
+							DCERPC_NCA_S_PROTO_ERROR);
+				}
+				if (dce_conn->incoming_fragmented_call_list != NULL) {
+					return dcesrv_fault_disconnect0(call, DCERPC_NCA_S_PROTO_ERROR);
+				}
+				call->context = dcesrv_find_context(call->conn,
+							call->pkt.u.request.context_id);
+				if (call->context == NULL) {
+					return dcesrv_fault_with_flags(call, DCERPC_NCA_S_UNKNOWN_IF,
+						DCERPC_PFC_FLAG_DID_NOT_EXECUTE);
+				}
+				if (auth_invalid) {
+					return dcesrv_fault_disconnect0(call,
+									DCERPC_FAULT_ACCESS_DENIED);
+				}
 				return dcesrv_fault_disconnect0(call,
 						DCERPC_NCA_S_PROTO_ERROR);
 			}
-			er = &existing->pkt.u.request;
 
 			if (call->pkt.ptype != existing->pkt.ptype) {
 				/* trying to play silly buggers are we? */
@@ -2150,14 +2206,8 @@ static NTSTATUS dcesrv_process_ncacn_packet(struct dcesrv_connection *dce_conn,
 				return dcesrv_fault_disconnect(existing,
 						DCERPC_NCA_S_PROTO_ERROR);
 			}
-			if (nr->context_id != er->context_id)  {
-				return dcesrv_fault_disconnect(existing,
-						DCERPC_NCA_S_PROTO_ERROR);
-			}
-			if (nr->opnum != er->opnum)  {
-				return dcesrv_fault_disconnect(existing,
-						DCERPC_NCA_S_PROTO_ERROR);
-			}
+			call->auth_state = existing->auth_state;
+			call->context = existing->context;
 		}
 	}
 
diff --git a/selftest/knownfail.d/dcerpc-auth-fraq b/selftest/knownfail.d/dcerpc-auth-fraq
deleted file mode 100644
index f3c62b65e9e..00000000000
--- a/selftest/knownfail.d/dcerpc-auth-fraq
+++ /dev/null
@@ -1,20 +0,0 @@
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_auth_MPX_middle_all_111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_auth_MPX_middle_alone
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_auth_MPX_middle_auth_all_111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_auth_MPX_middle_auth_context_111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_auth_MPX_middle_auth_level_111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_auth_MPX_middle_auth_type_111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_MPX_first1_firstSame111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_MPX_first1_firstSameNone
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_MPX_first1_firstSameNone111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_MPX_first1_lastSameNone
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_MPX_first1_lastSameNone111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_firstSame2
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_lastNext111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_lastNext2
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_lastNextNone
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_lastNextNone111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_lastSame111
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_lastSame2
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_lastSameNone
-^samba.tests.dcerpc.raw_protocol.samba.tests.dcerpc.raw_protocol.TestDCERPC_BIND.test_ntlmssp_multi_auth_first1_lastSameNone111
diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index 4847e8a0200..baf6df6e498 100644
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -726,6 +726,7 @@ static NTSTATUS ncacn_pull_pkt_auth(struct dcecli_connection *c,
 
 	status = dcerpc_ncacn_pull_pkt_auth(&tmp_auth,
 					    c->security_state.generic_state,
+					    true, /* check_pkt_auth_fields */
 					    mem_ctx,
 					    ptype,
 					    required_flags,