r26192: Handle, test and implement the style of extended_dn requiest that MMC uses.

It appears that the control value is optional, implying type 0 responses.

Failing to parse this was causing LDAP disconnects with 'unavailable
critical extension'.

Andrew Bartlett
(This used to be commit 833dfc2f2af84c45f954e428c9ea6babf100ba92)

7 files changed, 136 insertions, 38 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn.c b/source4/dsdb/samdb/ldb_modules/extended_dn.c
index d64673fdd5..b62e806398 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn.c
@@ -104,35 +104,58 @@ static bool inject_extended_dn(struct ldb_message *msg,
 	const struct ldb_val *val;
 	struct GUID guid;
 	struct dom_sid *sid;
+	const DATA_BLOB *guid_blob;
+	const DATA_BLOB *sid_blob;
 	char *object_guid;
 	char *object_sid;
 	char *new_dn;
 
-	/* retrieve object_guid */
-	guid = samdb_result_guid(msg, "objectGUID");
-	object_guid = GUID_string(msg, &guid);
-	if (!object_guid)
-		return false;
-
-	if (remove_guid)
-		ldb_msg_remove_attr(msg, "objectGUID");
+	guid_blob = ldb_msg_find_ldb_val(msg, "objectGUID");
+	sid_blob = ldb_msg_find_ldb_val(msg, "objectSID");
 
-	/* retrieve object_sid */
-	object_sid = NULL;
-	sid = samdb_result_dom_sid(msg, msg, "objectSID");
-	if (sid) {
-		object_sid = dom_sid_string(msg, sid);
-		if (!object_sid)
-			return false;
-
-		if (remove_sid)
-			ldb_msg_remove_attr(msg, "objectSID");
-	}
+	if (!guid_blob)
+		return false;
 
-	/* TODO: handle type */
 	switch (type) {
 		case 0:
+			/* return things in hexadecimal format */
+			if (sid_blob) {
+				const char *lower_guid_hex = strlower_talloc(msg, data_blob_hex_string(msg, guid_blob));
+				const char *lower_sid_hex = strlower_talloc(msg, data_blob_hex_string(msg, sid_blob));
+				if (!lower_guid_hex || !lower_sid_hex) {
+					return false;
+				}
+				new_dn = talloc_asprintf(msg, "<GUID=%s>;<SID=%s>;%s",
+							 lower_guid_hex, 
+							 lower_sid_hex,
+							 ldb_dn_get_linearized(msg->dn));
+			} else {
+				const char *lower_guid_hex = strlower_talloc(msg, data_blob_hex_string(msg, guid_blob));
+				if (!lower_guid_hex) {
+					return false;
+				}
+				new_dn = talloc_asprintf(msg, "<GUID=%s>;%s",
+							 lower_guid_hex, 
+							 ldb_dn_get_linearized(msg->dn));
+			}
+
+			break;
 		case 1:
+			/* retrieve object_guid */
+			guid = samdb_result_guid(msg, "objectGUID");
+			object_guid = GUID_string(msg, &guid);
+			
+			/* retrieve object_sid */
+			object_sid = NULL;
+			sid = samdb_result_dom_sid(msg, msg, "objectSID");
+			if (sid) {
+				object_sid = dom_sid_string(msg, sid);
+				if (!object_sid)
+					return false;
+				
+			}
+			
+			/* Normal, sane format */
 			if (object_sid) {
 				new_dn = talloc_asprintf(msg, "<GUID=%s>;<SID=%s>;%s",
 							 object_guid, object_sid,
@@ -147,8 +170,17 @@ static bool inject_extended_dn(struct ldb_message *msg,
 			return false;
 	}
 
-	if (!new_dn)
+	if (!new_dn) {
 		return false;
+	}
+
+	if (remove_guid) {
+		ldb_msg_remove_attr(msg, "objectGUID");
+	}
+
+	if (sid_blob && remove_sid) {
+		ldb_msg_remove_attr(msg, "objectSID");
+	}
 
 	msg->dn = ldb_dn_new(msg, ldb, new_dn);
 	if (! ldb_dn_validate(msg->dn))
@@ -201,7 +233,7 @@ error:
 static int extended_search(struct ldb_module *module, struct ldb_request *req)
 {
 	struct ldb_control *control;
-	struct ldb_extended_dn_control *extended_ctrl;
+	struct ldb_extended_dn_control *extended_ctrl = NULL;
 	struct ldb_control **saved_controls;
 	struct extended_context *ac;
 	struct ldb_request *down_req;
@@ -215,9 +247,11 @@ static int extended_search(struct ldb_module *module, struct ldb_request *req)
 		return ldb_next_request(module, req);
 	}
 
-	extended_ctrl = talloc_get_type(control->data, struct ldb_extended_dn_control);
-	if (!extended_ctrl) {
-		return LDB_ERR_PROTOCOL_ERROR;
+	if (control->data) {
+		extended_ctrl = talloc_get_type(control->data, struct ldb_extended_dn_control);
+		if (!extended_ctrl) {
+			return LDB_ERR_PROTOCOL_ERROR;
+		}
 	}
 
 	ac = talloc(req, struct extended_context);
@@ -231,7 +265,11 @@ static int extended_search(struct ldb_module *module, struct ldb_request *req)
 	ac->attrs = req->op.search.attrs;
 	ac->remove_guid = false;
 	ac->remove_sid = false;
-	ac->extended_type = extended_ctrl->type;
+	if (extended_ctrl) {
+		ac->extended_type = extended_ctrl->type;
+	} else {
+		ac->extended_type = 0;
+	}
 
 	down_req = talloc_zero(req, struct ldb_request);
 	if (down_req == NULL) {
diff --git a/source4/lib/ldb/common/ldb_controls.c b/source4/lib/ldb/common/ldb_controls.c
index 137d35ac4e..e3f8551407 100644
--- a/source4/lib/ldb/common/ldb_controls.c
+++ b/source4/lib/ldb/common/ldb_controls.c
@@ -291,12 +291,22 @@ struct ldb_control **ldb_parse_control_strings(struct ldb_context *ldb, void *me
 			p = &(control_strings[i][12]);
 			ret = sscanf(p, "%d:%d", &crit, &type);
 			if ((ret != 2) || (crit < 0) || (crit > 1) || (type < 0) || (type > 1)) {
-				error_string = talloc_asprintf(mem_ctx, "invalid extended_dn control syntax\n");
-				error_string = talloc_asprintf_append(error_string, " syntax: crit(b):type(b)\n");
-				error_string = talloc_asprintf_append(error_string, "   note: b = boolean");
-				ldb_set_errstring(ldb, error_string);
-				talloc_free(error_string);
-				return NULL;
+				ret = sscanf(p, "%d", &crit);
+				if ((ret != 1) || (crit < 0) || (crit > 1)) {
+					error_string = talloc_asprintf(mem_ctx, "invalid extended_dn control syntax\n");
+					error_string = talloc_asprintf_append(error_string, " syntax: crit(b)[:type(i)]\n");
+					error_string = talloc_asprintf_append(error_string, "   note: b = boolean\n");
+					error_string = talloc_asprintf_append(error_string, "         i = integer\n");
+					error_string = talloc_asprintf_append(error_string, "   valid values are: 0 - hexadecimal representation\n");
+					error_string = talloc_asprintf_append(error_string, "                     1 - normal string representation");
+					ldb_set_errstring(ldb, error_string);
+					talloc_free(error_string);
+					return NULL;
+				}
+				control = NULL;
+			} else {
+				control = talloc(ctrl, struct ldb_extended_dn_control);
+				control->type = type;
 			}
 
 			ctrl[i] = talloc(ctrl, struct ldb_control);
@@ -306,9 +316,7 @@ struct ldb_control **ldb_parse_control_strings(struct ldb_context *ldb, void *me
 			}
 			ctrl[i]->oid = LDB_CONTROL_EXTENDED_DN_OID;
 			ctrl[i]->critical = crit;
-			control = talloc(ctrl[i], struct ldb_extended_dn_control);
-			control->type = type;
-			ctrl[i]->data = control;
+			ctrl[i]->data = talloc_steal(ctrl[i], control);
 
 			continue;
 		}
diff --git a/source4/libcli/ldap/ldap.c b/source4/libcli/ldap/ldap.c
index 11689fbd79..34d715e3e5 100644
--- a/source4/libcli/ldap/ldap.c
+++ b/source4/libcli/ldap/ldap.c
@@ -1325,10 +1325,12 @@ NTSTATUS ldap_decode(struct asn1_data *data, struct ldap_message *msg)
 	}
 
 	msg->controls = NULL;
+	msg->controls_decoded = NULL;
 
 	if (asn1_peek_tag(data, ASN1_CONTEXT(0))) {
 		int i = 0;
 		struct ldb_control **ctrl = NULL;
+		bool *decoded = NULL;
 
 		asn1_start_tag(data, ASN1_CONTEXT(0));
 
@@ -1341,6 +1343,11 @@ NTSTATUS ldap_decode(struct asn1_data *data, struct ldap_message *msg)
 				return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR);
 			}
 
+			decoded = talloc_realloc(msg, decoded, bool, i+1);
+			if (!decoded) {
+				return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR);
+			}
+
 			ctrl[i] = talloc(ctrl, struct ldb_control);
 			if (!ctrl[i]) {
 				return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR);
@@ -1352,12 +1359,15 @@ NTSTATUS ldap_decode(struct asn1_data *data, struct ldap_message *msg)
 			
 			if (!ldap_decode_control_value(ctrl, value, ctrl[i])) {
 				if (ctrl[i]->critical) {
-					return NT_STATUS_LDAP(LDAP_UNAVAILABLE_CRITICAL_EXTENSION);
+					ctrl[i]->data = NULL;
+					decoded[i] = false;
+					i++;
 				} else {
 					talloc_free(ctrl[i]);
 					ctrl[i] = NULL;
 				}
 			} else {
+				decoded[i] = true;
 				i++;
 			}
 		}
@@ -1367,6 +1377,7 @@ NTSTATUS ldap_decode(struct asn1_data *data, struct ldap_message *msg)
 		}
 
 		msg->controls = ctrl;
+		msg->controls_decoded = decoded;
 
 		asn1_end_tag(data);
 	}
diff --git a/source4/libcli/ldap/ldap.h b/source4/libcli/ldap/ldap.h
index e89322213a..6f5e86744e 100644
--- a/source4/libcli/ldap/ldap.h
+++ b/source4/libcli/ldap/ldap.h
@@ -240,11 +240,13 @@ union ldap_Request {
 	struct ldap_ExtendedResponse 	ExtendedResponse;
 };
 
+
 struct ldap_message {
 	int                     messageid;
 	enum ldap_request_tag   type;
 	union ldap_Request      r;
 	struct ldb_control    **controls;
+	bool                   *controls_decoded;
 };
 
 struct event_context;
diff --git a/source4/libcli/ldap/ldap_client.c b/source4/libcli/ldap/ldap_client.c
index fcb2d92214..41e9c37196 100644
--- a/source4/libcli/ldap/ldap_client.c
+++ b/source4/libcli/ldap/ldap_client.c
@@ -116,6 +116,7 @@ static void ldap_error_handler(void *private_data, NTSTATUS status)
 static void ldap_match_message(struct ldap_connection *conn, struct ldap_message *msg)
 {
 	struct ldap_request *req;
+	int i;
 
 	for (req=conn->pending; req; req=req->next) {
 		if (req->messageid == msg->messageid) break;
@@ -132,6 +133,20 @@ static void ldap_match_message(struct ldap_connection *conn, struct ldap_message
 		return;
 	}
 
+	/* Check for undecoded critical extensions */
+	for (i=0; msg->controls && msg->controls[i]; i++) {
+		if (!msg->controls_decoded[i] && 
+		    msg->controls[i]->critical) {
+			req->status = NT_STATUS_LDAP(LDAP_UNAVAILABLE_CRITICAL_EXTENSION);
+			req->state = LDAP_REQUEST_DONE;
+			DLIST_REMOVE(conn->pending, req);
+			if (req->async.fn) {
+				req->async.fn(req);
+			}
+			return;
+		}
+	}
+
 	/* add to the list of replies received */
 	talloc_steal(req, msg);
 	req->replies = talloc_realloc(req, req->replies, 
diff --git a/source4/libcli/ldap/ldap_controls.c b/source4/libcli/ldap/ldap_controls.c
index b7fd1ce178..34e5cccf75 100644
--- a/source4/libcli/ldap/ldap_controls.c
+++ b/source4/libcli/ldap/ldap_controls.c
@@ -156,9 +156,16 @@ static bool decode_server_sort_request(void *mem_ctx, DATA_BLOB in, void **out)
 
 static bool decode_extended_dn_request(void *mem_ctx, DATA_BLOB in, void **out)
 {
-	struct asn1_data *data = asn1_init(mem_ctx);
+	struct asn1_data *data;
 	struct ldb_extended_dn_control *ledc;
 
+	/* The content of this control is optional */
+	if (in.length == 0) {
+		*out = NULL;
+		return true;
+	}
+
+	data = asn1_init(mem_ctx);
 	if (!data) return false;
 
 	if (!asn1_load(data, in)) {
@@ -717,7 +724,14 @@ static bool encode_server_sort_request(void *mem_ctx, void *in, DATA_BLOB *out)
 static bool encode_extended_dn_request(void *mem_ctx, void *in, DATA_BLOB *out)
 {
 	struct ldb_extended_dn_control *ledc = talloc_get_type(in, struct ldb_extended_dn_control);
-	struct asn1_data *data = asn1_init(mem_ctx);
+	struct asn1_data *data;
+
+	if (!in) {
+		*out = data_blob(NULL, 0);
+		return true;
+	}
+
+	data = asn1_init(mem_ctx);
 
 	if (!data) return false;
 
diff --git a/testprogs/blackbox/test_ldb.sh b/testprogs/blackbox/test_ldb.sh
index fd925fc99b..e35d3547d9 100755
--- a/testprogs/blackbox/test_ldb.sh
+++ b/testprogs/blackbox/test_ldb.sh
@@ -71,11 +71,21 @@ failed=`expr $failed + 1`
 fi
 
 echo "Test Extended DN Control"
+nentries=`bin/ldbsearch $options $CONFIGURATION -H $p://$SERVER --controls=extended_dn:1 '(objectclass=user)' | grep sAMAccountName | wc -l`
+if [ $nentries -lt 1 ]; then
+echo "Extended DN Control test returned 0 items"
+failed=`expr $failed + 1`
+fi
 nentries=`bin/ldbsearch $options $CONFIGURATION -H $p://$SERVER --controls=extended_dn:1:0 '(objectclass=user)' | grep sAMAccountName | wc -l`
 if [ $nentries -lt 1 ]; then
 echo "Extended DN Control test returned 0 items"
 failed=`expr $failed + 1`
 fi
+nentries=`bin/ldbsearch $options $CONFIGURATION -H $p://$SERVER --controls=extended_dn:1:1 '(objectclass=user)' | grep sAMAccountName | wc -l`
+if [ $nentries -lt 1 ]; then
+echo "Extended DN Control test returned 0 items"
+failed=`expr $failed + 1`
+fi
 
 echo "Test Domain scope Control"
 nentries=`bin/ldbsearch $options $CONFIGURATION -H $p://$SERVER --controls=domain_scope:1 '(objectclass=user)' | grep sAMAccountName | wc -l`