r13508: some ASN.1 element in LDAP are optional,

make it possible to code the difference between a zero length and a NULL DATA_BLOB...

metze
(This used to be commit 54f0b19c55df8ad3882f31a114e2ea0e4cf940ae)

6 files changed, 137 insertions, 66 deletions

diff --git a/source4/ldap_server/ldap_backend.c b/source4/ldap_server/ldap_backend.c
index 562263371b..37e45ce3e6 100644
--- a/source4/ldap_server/ldap_backend.c
+++ b/source4/ldap_server/ldap_backend.c
@@ -139,9 +139,8 @@ NTSTATUS ldapsrv_unwilling(struct ldapsrv_call *call, int error)
 	r->response.dn = NULL;
 	r->response.errormessage = NULL;
 	r->response.referral = NULL;
-	r->name = NULL;
-	r->value.data = NULL;
-	r->value.length = 0;
+	r->oid = NULL;
+	r->value = NULL;
 
 	ldapsrv_queue_reply(call, reply);
 	return NT_STATUS_OK;
diff --git a/source4/ldap_server/ldap_bind.c b/source4/ldap_server/ldap_bind.c
index b42fe51b38..5341b9f741 100644
--- a/source4/ldap_server/ldap_bind.c
+++ b/source4/ldap_server/ldap_bind.c
@@ -49,8 +49,6 @@ static NTSTATUS ldapsrv_BindSimple(struct ldapsrv_call *call)
 						  req->creds.password, &session_info);
 	}
 
-	/* When we add authentication here, we also need to handle telling the backends */
-
 	reply = ldapsrv_init_reply(call, LDAP_TAG_BindResponse);
 	if (!reply) {
 		return NT_STATUS_NO_MEMORY;
@@ -84,9 +82,7 @@ static NTSTATUS ldapsrv_BindSimple(struct ldapsrv_call *call)
 	resp->response.errormessage = errstr;
 	resp->response.dn = NULL;
 	resp->response.referral = NULL;
-
-	/* This looks wrong... */
-	resp->SASL.secblob = data_blob(NULL, 0);
+	resp->SASL.secblob = NULL;
 
 	ldapsrv_queue_reply(call, reply);
 	return NT_STATUS_OK;
@@ -145,10 +141,29 @@ static NTSTATUS ldapsrv_BindSASL(struct ldapsrv_call *call)
 	}
 
 	if (NT_STATUS_IS_OK(status)) {
+		DATA_BLOB input = data_blob(NULL, 0);
+		DATA_BLOB output = data_blob(NULL, 0);
+
+		if (req->creds.SASL.secblob) {
+			input = *req->creds.SASL.secblob;
+		}
+
+		resp->SASL.secblob = talloc(reply, DATA_BLOB);
+		NT_STATUS_HAVE_NO_MEMORY(resp->SASL.secblob);
+
 		status = gensec_update(conn->gensec, reply,
-				       req->creds.SASL.secblob, &resp->SASL.secblob);
+				       input, &output);
+
+		/* TODO: gensec should really handle the difference between NULL and length=0 better! */
+		if (output.data) {
+			resp->SASL.secblob = talloc(reply, DATA_BLOB);
+			NT_STATUS_HAVE_NO_MEMORY(resp->SASL.secblob);
+			*resp->SASL.secblob = output;
+		} else {
+			resp->SASL.secblob = NULL;
+		}
 	} else {
-		resp->SASL.secblob = data_blob(NULL, 0);	
+		resp->SASL.secblob = NULL;
 	}
 
 	if (NT_STATUS_EQUAL(NT_STATUS_MORE_PROCESSING_REQUIRED, status)) {
@@ -223,7 +238,7 @@ NTSTATUS ldapsrv_BindRequest(struct ldapsrv_call *call)
 	resp->response.dn = NULL;
 	resp->response.errormessage = talloc_asprintf(reply, "Bad AuthenticationChoice [%d]", req->mechanism);
 	resp->response.referral = NULL;
-	resp->SASL.secblob = data_blob(NULL, 0);
+	resp->SASL.secblob = NULL;
 
 	ldapsrv_queue_reply(call, reply);
 	return NT_STATUS_OK;
diff --git a/source4/libcli/ldap/ldap.c b/source4/libcli/ldap/ldap.c
index 496fec527f..55f1361e16 100644
--- a/source4/libcli/ldap/ldap.c
+++ b/source4/libcli/ldap/ldap.c
@@ -219,14 +219,9 @@ BOOL ldap_encode(struct ldap_message *msg, DATA_BLOB *result, TALLOC_CTX *mem_ct
 			asn1_push_tag(&data, ASN1_CONTEXT(3));
 			asn1_write_OctetString(&data, r->creds.SASL.mechanism,
 					       strlen(r->creds.SASL.mechanism));
-			/* The value of data indicates if this
-			 * optional element exists at all.  In SASL
-			 * there is a difference between NULL and
-			 * zero-legnth, but our APIs don't express it
-			 * well */
-			if (r->creds.SASL.secblob.data) {
-				asn1_write_OctetString(&data, r->creds.SASL.secblob.data,
-						       r->creds.SASL.secblob.length);
+			if (r->creds.SASL.secblob) {
+				asn1_write_OctetString(&data, r->creds.SASL.secblob->data,
+						       r->creds.SASL.secblob->length);
 			}
 			asn1_pop_tag(&data);
 			break;
@@ -241,13 +236,8 @@ BOOL ldap_encode(struct ldap_message *msg, DATA_BLOB *result, TALLOC_CTX *mem_ct
 		struct ldap_BindResponse *r = &msg->r.BindResponse;
 		asn1_push_tag(&data, ASN1_APPLICATION(msg->type));
 		ldap_encode_response(&data, &r->response);
-		/* The value of data indicates if this
-		 * optional element exists at all.  In SASL
-		 * there is a difference between NULL and
-		 * zero-legnth, but our APIs don't express it
-		 * well */
-		if (r->SASL.secblob.data) {
-			asn1_write_ContextSimple(&data, 7, &r->SASL.secblob);
+		if (r->SASL.secblob) {
+			asn1_write_ContextSimple(&data, 7, r->SASL.secblob);
 		}
 		asn1_pop_tag(&data);
 		break;
@@ -396,7 +386,7 @@ BOOL ldap_encode(struct ldap_message *msg, DATA_BLOB *result, TALLOC_CTX *mem_ct
 		asn1_write_OctetString(&data, r->dn, strlen(r->dn));
 		asn1_write_OctetString(&data, r->newrdn, strlen(r->newrdn));
 		asn1_write_BOOLEAN(&data, r->deleteolddn);
-		if (r->newsuperior != NULL) {
+		if (r->newsuperior) {
 			asn1_push_tag(&data, ASN1_CONTEXT_SIMPLE(0));
 			asn1_write(&data, r->newsuperior,
 				   strlen(r->newsuperior));
@@ -452,9 +442,11 @@ BOOL ldap_encode(struct ldap_message *msg, DATA_BLOB *result, TALLOC_CTX *mem_ct
 		asn1_push_tag(&data, ASN1_CONTEXT_SIMPLE(0));
 		asn1_write(&data, r->oid, strlen(r->oid));
 		asn1_pop_tag(&data);
-		asn1_push_tag(&data, ASN1_CONTEXT_SIMPLE(1));
-		asn1_write(&data, r->value.data, r->value.length);
-		asn1_pop_tag(&data);
+		if (r->value) {
+			asn1_push_tag(&data, ASN1_CONTEXT_SIMPLE(1));
+			asn1_write(&data, r->value->data, r->value->length);
+			asn1_pop_tag(&data);
+		}
 		asn1_pop_tag(&data);
 		break;
 	}
@@ -462,6 +454,16 @@ BOOL ldap_encode(struct ldap_message *msg, DATA_BLOB *result, TALLOC_CTX *mem_ct
 		struct ldap_ExtendedResponse *r = &msg->r.ExtendedResponse;
 		asn1_push_tag(&data, ASN1_APPLICATION(msg->type));
 		ldap_encode_response(&data, &r->response);
+		if (r->oid) {
+			asn1_push_tag(&data, ASN1_CONTEXT_SIMPLE(10));
+			asn1_write(&data, r->oid, strlen(r->oid));
+			asn1_pop_tag(&data);
+		}
+		if (r->value) {
+			asn1_push_tag(&data, ASN1_CONTEXT_SIMPLE(11));
+			asn1_write(&data, r->value->data, r->value->length);
+			asn1_pop_tag(&data);
+		}
 		asn1_pop_tag(&data);
 		break;
 	}
@@ -960,12 +962,17 @@ BOOL ldap_decode(struct asn1_data *data, struct ldap_message *msg)
 			r->mechanism = LDAP_AUTH_MECH_SASL;
 			asn1_read_OctetString_talloc(msg, data, &r->creds.SASL.mechanism);
 			if (asn1_peek_tag(data, ASN1_OCTET_STRING)) { /* optional */
-				asn1_read_OctetString(data, &r->creds.SASL.secblob);
-				if (r->creds.SASL.secblob.data) {
-					talloc_steal(msg, r->creds.SASL.secblob.data);
+				DATA_BLOB tmp_blob = data_blob(NULL, 0);
+				asn1_read_OctetString(data, &tmp_blob);
+				r->creds.SASL.secblob = talloc(msg, DATA_BLOB);
+				if (!r->creds.SASL.secblob) {
+					return False;
 				}
+				*r->creds.SASL.secblob = data_blob_talloc(r->creds.SASL.secblob,
+									  tmp_blob.data, tmp_blob.length);
+				data_blob_free(&tmp_blob);
 			} else {
-				r->creds.SASL.secblob = data_blob(NULL, 0);
+				r->creds.SASL.secblob = NULL;
 			}
 			asn1_end_tag(data);
 		}
@@ -981,10 +988,15 @@ BOOL ldap_decode(struct asn1_data *data, struct ldap_message *msg)
 		if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(7))) {
 			DATA_BLOB tmp_blob = data_blob(NULL, 0);
 			asn1_read_ContextSimple(data, 7, &tmp_blob);
-			r->SASL.secblob = data_blob_talloc(msg, tmp_blob.data, tmp_blob.length);
+			r->SASL.secblob = talloc(msg, DATA_BLOB);
+			if (!r->SASL.secblob) {
+				return False;
+			}
+			*r->SASL.secblob = data_blob_talloc(r->SASL.secblob,
+							    tmp_blob.data, tmp_blob.length);
 			data_blob_free(&tmp_blob);
 		} else {
-			r->SASL.secblob = data_blob(NULL, 0);
+			r->SASL.secblob = NULL;
 		}
 		asn1_end_tag(data);
 		break;
@@ -1241,10 +1253,14 @@ BOOL ldap_decode(struct asn1_data *data, struct ldap_message *msg)
 
 		if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(1))) {
 			asn1_read_ContextSimple(data, 1, &tmp_blob);
-			r->value = data_blob_talloc(msg, tmp_blob.data, tmp_blob.length);
+			r->value = talloc(msg, DATA_BLOB);
+			if (!r->value) {
+				return False;
+			}
+			*r->value = data_blob_talloc(r->value, tmp_blob.data, tmp_blob.length);
 			data_blob_free(&tmp_blob);
 		} else {
-			r->value = data_blob(NULL, 0);
+			r->value = NULL;
 		}
 
 		asn1_end_tag(data);
@@ -1253,15 +1269,35 @@ BOOL ldap_decode(struct asn1_data *data, struct ldap_message *msg)
 
 	case ASN1_APPLICATION(LDAP_TAG_ExtendedResponse): {
 		struct ldap_ExtendedResponse *r = &msg->r.ExtendedResponse;
+		DATA_BLOB tmp_blob = data_blob(NULL, 0);
+
 		msg->type = LDAP_TAG_ExtendedResponse;
 		asn1_start_tag(data, tag);		
 		ldap_decode_response(msg, data, &r->response);
-		/* I have to come across an operation that actually sends
-		 * something back to really see what's going on. The currently
-		 * needed pwdchange does not send anything back. */
-		r->name = NULL;
-		r->value.data = NULL;
-		r->value.length = 0;
+
+		if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(10))) {
+			asn1_read_ContextSimple(data, 1, &tmp_blob);
+			r->oid = blob2string_talloc(msg, tmp_blob);
+			data_blob_free(&tmp_blob);
+			if (!r->oid) {
+				return False;
+			}
+		} else {
+			r->oid = NULL;
+		}
+
+		if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(11))) {
+			asn1_read_ContextSimple(data, 1, &tmp_blob);
+			r->value = talloc(msg, DATA_BLOB);
+			if (!r->value) {
+				return False;
+			}
+			*r->value = data_blob_talloc(r->value, tmp_blob.data, tmp_blob.length);
+			data_blob_free(&tmp_blob);
+		} else {
+			r->value = NULL;
+		}
+
 		asn1_end_tag(data);
 		break;
 	}
diff --git a/source4/libcli/ldap/ldap.h b/source4/libcli/ldap/ldap.h
index 5283553f13..de284d23d1 100644
--- a/source4/libcli/ldap/ldap.h
+++ b/source4/libcli/ldap/ldap.h
@@ -109,7 +109,7 @@ struct ldap_BindRequest {
 		const char *password;
 		struct {
 			const char *mechanism;
-			DATA_BLOB secblob;
+			DATA_BLOB *secblob;/* optional */
 		} SASL;
 	} creds;
 };
@@ -117,7 +117,7 @@ struct ldap_BindRequest {
 struct ldap_BindResponse {
 	struct ldap_Result response;
 	union {
-		DATA_BLOB secblob;
+		DATA_BLOB *secblob;/* optional */
 	} SASL;
 };
 
@@ -192,7 +192,7 @@ struct ldap_ModifyDNRequest {
 	const char *dn;
 	const char *newrdn;
 	BOOL deleteolddn;
-	const char *newsuperior;
+	const char *newsuperior;/* optional */
 };
 
 struct ldap_CompareRequest {
@@ -207,13 +207,13 @@ struct ldap_AbandonRequest {
 
 struct ldap_ExtendedRequest {
 	const char *oid;
-	DATA_BLOB value;
+	DATA_BLOB *value;/* optional */
 };
 
 struct ldap_ExtendedResponse {
 	struct ldap_Result response;
-	const char *name;
-	DATA_BLOB value;
+	const char *oid;/* optional */
+	DATA_BLOB *value;/* optional */
 };
 
 union ldap_Request {
diff --git a/source4/libcli/ldap/ldap_bind.c b/source4/libcli/ldap/ldap_bind.c
index 2880298dd5..cacb0d150e 100644
--- a/source4/libcli/ldap/ldap_bind.c
+++ b/source4/libcli/ldap/ldap_bind.c
@@ -129,7 +129,16 @@ static struct ldap_message *new_ldap_sasl_bind_msg(struct ldap_connection *conn,
 	res->r.BindRequest.dn = "";
 	res->r.BindRequest.mechanism = LDAP_AUTH_MECH_SASL;
 	res->r.BindRequest.creds.SASL.mechanism = talloc_strdup(res, sasl_mechanism);
-	res->r.BindRequest.creds.SASL.secblob = *secblob;
+	if (secblob) {
+		res->r.BindRequest.creds.SASL.secblob = talloc(res, DATA_BLOB);
+		if (!res->r.BindRequest.creds.SASL.secblob) {
+			talloc_free(res);
+			return NULL;
+		}
+		*res->r.BindRequest.creds.SASL.secblob = *secblob;
+	} else {
+		res->r.BindRequest.creds.SASL.secblob = NULL;
+	}
 	res->controls = NULL;
 
 	return res;
@@ -262,7 +271,7 @@ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *cr
 		}
 
 		/* Perhaps we should make gensec_start_mech_by_sasl_list() return the name we got? */
-		msg = new_ldap_sasl_bind_msg(tmp_ctx, conn->gensec->ops->sasl_name, &output);
+		msg = new_ldap_sasl_bind_msg(tmp_ctx, conn->gensec->ops->sasl_name, (output.data?&output:NULL));
 		if (msg == NULL) {
 			status = NT_STATUS_NO_MEMORY;
 			goto failed;
@@ -297,7 +306,11 @@ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *cr
 		if (!NT_STATUS_EQUAL(gensec_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
 			break;
 		}
-		input = response->r.BindResponse.SASL.secblob;
+		if (response->r.BindResponse.SASL.secblob) {
+			input = *response->r.BindResponse.SASL.secblob;
+		} else {
+			input = data_blob(NULL, 0);
+		}
 	}
 
 	if (NT_STATUS_IS_OK(status) &&
diff --git a/source4/libcli/ldap/ldap_controls.c b/source4/libcli/ldap/ldap_controls.c
index 4a28fa510b..5bd46cf7a9 100644
--- a/source4/libcli/ldap/ldap_controls.c
+++ b/source4/libcli/ldap/ldap_controls.c
@@ -304,7 +304,7 @@ static BOOL decode_asq_control(void *mem_ctx, DATA_BLOB in, void **out)
 		}
 		lac->src_attr_len = source_attribute.length;
 		if (lac->src_attr_len) {
-			lac->source_attribute = talloc_strndup(lac, (char *)source_attribute.data, source_attribute.length);
+			lac->source_attribute = talloc_strndup(lac, (const char *)source_attribute.data, source_attribute.length);
 
 			if (!(lac->source_attribute)) {
 				return False;
@@ -864,7 +864,7 @@ BOOL ldap_decode_control(void *mem_ctx, struct asn1_data *data, struct ldap_Cont
 		return False;
 	}
 	ctrl->oid = talloc_strndup(mem_ctx, (char *)oid.data, oid.length);
-	if (!(ctrl->oid)) {
+	if (!ctrl->oid) {
 		return False;
 	}
 
@@ -878,13 +878,17 @@ BOOL ldap_decode_control(void *mem_ctx, struct asn1_data *data, struct ldap_Cont
 
 	ctrl->value = NULL;
 
+	if (!asn1_peek_tag(data, ASN1_OCTET_STRING)) {
+		goto end_tag;
+	}
+
+	if (!asn1_read_OctetString(data, &value)) {
+		return False;
+	}
+
 	for (i = 0; ldap_known_controls[i].oid != NULL; i++) {
 		if (strcmp(ldap_known_controls[i].oid, ctrl->oid) == 0) {
-			
-			if (!asn1_read_OctetString(data, &value)) {
-				return False;
-			}
-			if (!ldap_known_controls[i].decode(mem_ctx, value, &(ctrl->value))) {
+			if (!ldap_known_controls[i].decode(mem_ctx, value, &ctrl->value)) {
 				return False;
 			}
 			break;
@@ -894,6 +898,7 @@ BOOL ldap_decode_control(void *mem_ctx, struct asn1_data *data, struct ldap_Cont
 		return False;
 	}
 
+end_tag:
 	if (!asn1_end_tag(data)) {
 		return False;
 	}
@@ -909,17 +914,21 @@ BOOL ldap_encode_control(void *mem_ctx, struct asn1_data *data, struct ldap_Cont
 	if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) {
 		return False;
 	}
-	
+
 	if (!asn1_write_OctetString(data, ctrl->oid, strlen(ctrl->oid))) {
 		return False;
 	}
-	
+
 	if (ctrl->critical) {
 		if (!asn1_write_BOOLEAN(data, ctrl->critical)) {
 			return False;
 		}
 	}
 
+	if (!ctrl->value) {
+		goto pop_tag;
+	}
+
 	for (i = 0; ldap_known_controls[i].oid != NULL; i++) {
 		if (strcmp(ldap_known_controls[i].oid, ctrl->oid) == 0) {
 			if (!ldap_known_controls[i].encode(mem_ctx, ctrl->value, &value)) {
@@ -932,12 +941,11 @@ BOOL ldap_encode_control(void *mem_ctx, struct asn1_data *data, struct ldap_Cont
 		return False;
 	}
 
-	if (value.length != 0) {
-		if (!asn1_write_OctetString(data, value.data, value.length)) {
-			return False;
-		}
+	if (!asn1_write_OctetString(data, value.data, value.length)) {
+		return False;
 	}
 
+pop_tag:
 	if (!asn1_pop_tag(data)) {
 		return False;
 	}