r9505: Work on GENSEC and the code that calls it, for tighter interface

requirements, and for better error reporting.

In particular, the composite session setup (extended security/SPNEGO)
code now returns errors, rather than NT_STATUS_NO_MEMORY.  This is
seen particularly when GENSEC fails to start.

The tighter interface rules apply to NTLMSSP, which must be called
exactly the right number of times.  This is to match some of our other
less-tested modules, where adding flexablity is harder.  (and this is
security code, so let's just get it right).  As such, the DCE/RPC and
LDAP clients have been updated.

Andrew Bartlett
(This used to be commit 134550cf752b9edad66c3368750bfb4bbd9d55d1)

1 files changed, 26 insertions, 8 deletions

diff --git a/source4/libcli/ldap/ldap_bind.c b/source4/libcli/ldap/ldap_bind.c
index e70a56779b..738222da86 100644
--- a/source4/libcli/ldap/ldap_bind.c
+++ b/source4/libcli/ldap/ldap_bind.c
@@ -141,6 +141,7 @@ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *cr
 {
 	NTSTATUS status;
 	TALLOC_CTX *tmp_ctx = NULL;
+
 	DATA_BLOB input = data_blob(NULL, 0);
 	DATA_BLOB output = data_blob(NULL, 0);
 
@@ -183,21 +184,35 @@ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *cr
 	tmp_ctx = talloc_new(conn);
 	if (tmp_ctx == NULL) goto failed;
 
-	status = gensec_update(conn->gensec, tmp_ctx, input, &output);
-
 	while (1) {
+		NTSTATUS gensec_status;
 		struct ldap_message *response;
 		struct ldap_message *msg;
 		struct ldap_request *req;
 		int result = LDAP_OTHER;
 	
-		if (NT_STATUS_IS_OK(status) && output.length == 0) {
-			break;
-		}
+		status = gensec_update(conn->gensec, tmp_ctx,
+				       input,
+				       &output);
+		/* The status value here, from GENSEC is vital to the security
+		 * of the system.  Even if the other end accepts, if GENSEC
+		 * claims 'MORE_PROCESSING_REQUIRED' then you must keep
+		 * feeding it blobs, or else the remote host/attacker might
+		 * avoid mutal authentication requirements.
+		 *
+		 * Likewise, you must not feed GENSEC too much (after the OK),
+		 * it doesn't like that either
+		 */
+
+		gensec_status = status;
+
 		if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED) && 
 		    !NT_STATUS_IS_OK(status)) {
 			break;
 		}
+		if (output.length == 0) {
+			break;
+		}
 
 		msg = new_ldap_sasl_bind_msg(tmp_ctx, "GSS-SPNEGO", &output);
 		if (msg == NULL) {
@@ -225,12 +240,15 @@ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *cr
 		result = response->r.BindResponse.response.resultcode;
 
 		if (result != LDAP_SUCCESS && result != LDAP_SASL_BIND_IN_PROGRESS) {
+			status = NT_STATUS_UNEXPECTED_NETWORK_ERROR;
 			break;
 		}
 
-		status = gensec_update(conn->gensec, tmp_ctx,
-				       response->r.BindResponse.SASL.secblob,
-				       &output);
+		/* This is where we check if GENSEC wanted to be fed more data */
+		if (!NT_STATUS_EQUAL(gensec_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
+			break;
+		}
+		input = response->r.BindResponse.SASL.secblob;
 	}
 
 	if (NT_STATUS_IS_OK(status) &&