r19502: fixed the RPC-SECRETS test with kerberos. Andrew, can you look at this

as well?

The server side change is needed to fix a valgrind error, which was
possibly exploitable if the client sent deliberately bad data
(This used to be commit e3c04cf165fe15739197b2713e78046399aa7653)

2 files changed, 20 insertions, 18 deletions

diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index 28e48c4a5a..bda07066ff 100644
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -272,7 +272,6 @@ static NTSTATUS ncacn_pull_request_auth(struct dcerpc_connection *c, TALLOC_CTX
 		return status;
 	}
 	
-	
 	/* check signature or unseal the packet */
 	switch (c->security_state.auth_info->auth_level) {
 	case DCERPC_AUTH_LEVEL_PRIVACY:
@@ -433,6 +432,13 @@ static NTSTATUS ncacn_push_request_sign(struct dcerpc_connection *c,
 			return status;
 		}
 		dcerpc_set_auth_length(blob, creds2.length);
+		if (c->security_state.auth_info->credentials.length == 0) {
+			/* this is needed for krb5 only, to correct the total packet
+			   length */
+			dcerpc_set_frag_length(blob, 
+					       dcerpc_get_frag_length(blob)
+					       +creds2.length);
+		}
 		break;
 
 	case DCERPC_AUTH_LEVEL_INTEGRITY:
@@ -454,6 +460,13 @@ static NTSTATUS ncacn_push_request_sign(struct dcerpc_connection *c,
 			return status;
 		}
 		dcerpc_set_auth_length(blob, creds2.length);
+		if (c->security_state.auth_info->credentials.length == 0) {
+			/* this is needed for krb5 only, to correct the total packet
+			   length */
+			dcerpc_set_frag_length(blob, 
+					       dcerpc_get_frag_length(blob)
+					       +creds2.length);
+		}
 		break;
 
 	case DCERPC_AUTH_LEVEL_CONNECT:
diff --git a/source4/rpc_server/dcesrv_auth.c b/source4/rpc_server/dcesrv_auth.c
index e6e9bb7fc5..b73143ce34 100644
--- a/source4/rpc_server/dcesrv_auth.c
+++ b/source4/rpc_server/dcesrv_auth.c
@@ -470,19 +470,14 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
 					    &creds2);
 
 		if (NT_STATUS_IS_OK(status)) {
-			status = data_blob_realloc(call, blob,
-						   blob->length - dce_conn->auth_state.auth_info->credentials.length + 
-						   creds2.length);
-		}
-
-		if (NT_STATUS_IS_OK(status)) {
-			memcpy(blob->data + blob->length - dce_conn->auth_state.auth_info->credentials.length,
-			       creds2.data, creds2.length);
+			blob->length -= dce_conn->auth_state.auth_info->credentials.length;
+			status = data_blob_append(call, blob, creds2.data, creds2.length);
 		}
 
 		/* If we did AEAD signing of the packet headers, then we hope
 		 * this value didn't change... */
 		dcerpc_set_auth_length(blob, creds2.length);
+		dcerpc_set_frag_length(blob, dcerpc_get_frag_length(blob)+creds2.length);
 		data_blob_free(&creds2);
 		break;
 
@@ -495,20 +490,14 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
 					    blob->length - dce_conn->auth_state.auth_info->credentials.length,
 					    &creds2);
 		if (NT_STATUS_IS_OK(status)) {
-			status = data_blob_realloc(call, blob,
-						   blob->length - dce_conn->auth_state.auth_info->credentials.length + 
-						   creds2.length);
-		}
-
-		if (NT_STATUS_IS_OK(status)) {
-			memcpy(blob->data + blob->length - dce_conn->auth_state.auth_info->credentials.length,
-			       creds2.data, creds2.length);
+			blob->length -= dce_conn->auth_state.auth_info->credentials.length;
+			status = data_blob_append(call, blob, creds2.data, creds2.length);
 		}
 
 		/* If we did AEAD signing of the packet headers, then we hope
 		 * this value didn't change... */
 		dcerpc_set_auth_length(blob, creds2.length);
-
+		dcerpc_set_frag_length(blob, dcerpc_get_frag_length(blob)+creds2.length);
 		data_blob_free(&creds2);
 		break;