r19465: Rather than use the non-standard API for determining the signature

length, use the amount the wapped message expanded by.

This works, because GSSAPI doesn't do AEAD (signing of headers), and
so changing the signature length after the fact is valid.

Andrew Bartlett
(This used to be commit bd1e0f679c8f2b9755051b8d34114fa127a7cf26)

3 files changed, 87 insertions, 65 deletions

diff --git a/source4/auth/gensec/gensec_gssapi.c b/source4/auth/gensec/gensec_gssapi.c
index d9f6edc7d7..364c0c3499 100644
--- a/source4/auth/gensec/gensec_gssapi.c
+++ b/source4/auth/gensec/gensec_gssapi.c
@@ -818,46 +818,6 @@ static NTSTATUS gensec_gssapi_unwrap(struct gensec_security *gensec_security,
 	return NT_STATUS_OK;
 }
 
-/* Find out the size of the signature, assuming (incorrectly) that it
- * GSSAPI provides any guarantees as to it's size.
- *
- * This is needed by the DCE/RPC code, which uses AEAD 
- * (signed headers, including signature legnth and a sealed body)
- */
-static size_t gensec_gssapi_sig_size(struct gensec_security *gensec_security, size_t data_size) 
-{
-	struct gensec_gssapi_state *gensec_gssapi_state = gensec_security->private_data;
-	OM_uint32 maj_stat, min_stat;
-	OM_uint32 output_size;
-	if ((gensec_gssapi_state->gss_oid->length != gss_mech_krb5->length)
-	    || (memcmp(gensec_gssapi_state->gss_oid->elements, gss_mech_krb5->elements, 
-		       gensec_gssapi_state->gss_oid->length) != 0)) {
-		DEBUG(1, ("NO sig size available for this mech\n"));
-		return 0;
-	}
-		
-	maj_stat = gsskrb5_wrap_size(&min_stat, 
-				     gensec_gssapi_state->gssapi_context,
-				     gensec_have_feature(gensec_security, GENSEC_FEATURE_SEAL),
-				     GSS_C_QOP_DEFAULT,
-				     data_size, 
-				     &output_size);
-	if (GSS_ERROR(maj_stat)) {
-		TALLOC_CTX *mem_ctx = talloc_new(NULL); 
-		DEBUG(1, ("gensec_gssapi_sig_size: determinaing signature size with gsskrb5_wrap_size failed: %s\n", 
-			  gssapi_error_string(mem_ctx, maj_stat, min_stat)));
-		talloc_free(mem_ctx);
-		return 0;
-	}
-
-	if (output_size < data_size) {
-		return 0;
-	}
-
-	/* The difference between the max output and the max input must be the signature */
-	return output_size - data_size;
-}
-
 /* Find out the maximum input size negotiated on this connection */
 
 static size_t gensec_gssapi_max_input_size(struct gensec_security *gensec_security) 
@@ -918,14 +878,12 @@ static NTSTATUS gensec_gssapi_seal_packet(struct gensec_security *gensec_securit
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
-	sig_length = gensec_gssapi_sig_size(gensec_security, length);
-
-	/* Caller must pad to right boundary */
-	if (output_token.length != (length + sig_length)) {
-		DEBUG(1, ("gensec_gssapi_seal_packet: GSS Wrap length [%ld] does not match caller length [%ld] plus sig size [%ld] = [%ld]\n", 
-			  (long)output_token.length, (long)length, (long)sig_length, (long)(length + sig_length)));
+	if (output_token.length < input_token.length) {
+		DEBUG(1, ("gensec_gssapi_seal_packet: GSS Wrap length [%ld] *less* than caller length [%ld]\n", 
+			  (long)output_token.length, (long)length));
 		return NT_STATUS_INTERNAL_ERROR;
 	}
+	sig_length = output_token.length - input_token.length;
 
 	memcpy(data, ((uint8_t *)output_token.value) + sig_length, length);
 	*sig = data_blob_talloc(mem_ctx, (uint8_t *)output_token.value, sig_length);
@@ -1021,18 +979,14 @@ static NTSTATUS gensec_gssapi_sign_packet(struct gensec_security *gensec_securit
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
-	if (output_token.length < length) {
+	if (output_token.length < input_token.length) {
+		DEBUG(1, ("gensec_gssapi_sign_packet: GSS Wrap length [%ld] *less* than caller length [%ld]\n", 
+			  (long)output_token.length, (long)length));
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	sig_length = gensec_gssapi_sig_size(gensec_security, length);
-
 	/* Caller must pad to right boundary */
-	if (output_token.length != (length + sig_length)) {
-		DEBUG(1, ("gensec_gssapi_sign_packet: GSS Wrap length [%ld] does not match caller length [%ld] plus sig size [%ld] = [%ld]\n", 
-			  (long)output_token.length, (long)length, (long)sig_length, (long)(length + sig_length)));
-		return NT_STATUS_INTERNAL_ERROR;
-	}
+	sig_length = output_token.length - input_token.length;
 
 	*sig = data_blob_talloc(mem_ctx, (uint8_t *)output_token.value, sig_length);
 
@@ -1352,7 +1306,6 @@ static const struct gensec_security_ops gensec_gssapi_krb5_security_ops = {
 	.update 	= gensec_gssapi_update,
 	.session_key	= gensec_gssapi_session_key,
 	.session_info	= gensec_gssapi_session_info,
-	.sig_size	= gensec_gssapi_sig_size,
 	.sign_packet	= gensec_gssapi_sign_packet,
 	.check_packet	= gensec_gssapi_check_packet,
 	.seal_packet	= gensec_gssapi_seal_packet,
diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index bfdf93a0f3..53d0e1b65b 100644
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -369,6 +369,9 @@ static NTSTATUS ncacn_push_request_sign(struct dcerpc_connection *c,
 	switch (c->security_state.auth_info->auth_level) {
 	case DCERPC_AUTH_LEVEL_PRIVACY:
 	case DCERPC_AUTH_LEVEL_INTEGRITY:
+		/* We hope this length is accruate.  If must be if the
+		 * GENSEC mech does AEAD signing of the packet
+		 * headers */
 		c->security_state.auth_info->credentials
 			= data_blob_talloc(mem_ctx, NULL, gensec_sig_size(c->security_state.generic_state, 
 									  payload_length));
@@ -405,6 +408,8 @@ static NTSTATUS ncacn_push_request_sign(struct dcerpc_connection *c,
 	   in these earlier as we don't know the signature length (it
 	   could be variable length) */
 	dcerpc_set_frag_length(blob, blob->length);
+	/* We hope this value is accruate.  If must be if the GENSEC
+	 * mech does AEAD signing of the packet headers */
 	dcerpc_set_auth_length(blob, c->security_state.auth_info->credentials.length);
 
 	/* sign or seal the packet */
@@ -421,7 +426,20 @@ static NTSTATUS ncacn_push_request_sign(struct dcerpc_connection *c,
 		if (!NT_STATUS_IS_OK(status)) {
 			return status;
 		}
-		memcpy(blob->data + blob->length - creds2.length, creds2.data, creds2.length);
+		status = data_blob_realloc(mem_ctx, blob,
+					   blob->length - c->security_state.auth_info->credentials.length + 
+					   creds2.length);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+		memcpy(blob->data + blob->length - c->security_state.auth_info->credentials.length,
+		       creds2.data, creds2.length);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+		dcerpc_set_auth_length(blob, creds2.length);
 		break;
 
 	case DCERPC_AUTH_LEVEL_INTEGRITY:
@@ -436,7 +454,20 @@ static NTSTATUS ncacn_push_request_sign(struct dcerpc_connection *c,
 		if (!NT_STATUS_IS_OK(status)) {
 			return status;
 		}
-		memcpy(blob->data + blob->length - creds2.length, creds2.data, creds2.length);
+		status = data_blob_realloc(mem_ctx, blob,
+					   blob->length - c->security_state.auth_info->credentials.length + 
+					   creds2.length);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+		memcpy(blob->data + blob->length - c->security_state.auth_info->credentials.length,
+		       creds2.data, creds2.length);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+		dcerpc_set_auth_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 3f848ca381..e6e9bb7fc5 100644
--- a/source4/rpc_server/dcesrv_auth.c
+++ b/source4/rpc_server/dcesrv_auth.c
@@ -393,6 +393,7 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
 	NTSTATUS status;
 	struct ndr_push *ndr;
 	uint32_t payload_length;
+	DATA_BLOB creds2;
 
 	/* non-signed packets are simple */
 	if (!dce_conn->auth_state.auth_info || !dce_conn->auth_state.gensec_security) {
@@ -427,14 +428,20 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
 			return False;
 		}
 	} else {
+
+		/* We hope this length is accruate.  If must be if the
+		 * GENSEC mech does AEAD signing of the packet
+		 * headers */
 		dce_conn->auth_state.auth_info->credentials
 			= data_blob_talloc(call, NULL, 
 					   gensec_sig_size(dce_conn->auth_state.gensec_security, 
 							   payload_length));
+		data_blob_clear(&dce_conn->auth_state.auth_info->credentials);
 	}
 
 	/* add the auth verifier */
-	status = ndr_push_dcerpc_auth(ndr, NDR_SCALARS|NDR_BUFFERS, dce_conn->auth_state.auth_info);
+	status = ndr_push_dcerpc_auth(ndr, NDR_SCALARS|NDR_BUFFERS, 
+				      dce_conn->auth_state.auth_info);
 	if (!NT_STATUS_IS_OK(status)) {
 		return False;
 	}
@@ -446,6 +453,9 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
 	   in these earlier as we don't know the signature length (it
 	   could be variable length) */
 	dcerpc_set_frag_length(blob, blob->length);
+
+	/* We hope this value is accruate.  If must be if the GENSEC
+	 * mech does AEAD signing of the packet headers */
 	dcerpc_set_auth_length(blob, dce_conn->auth_state.auth_info->credentials.length);
 
 	/* sign or seal the packet */
@@ -457,7 +467,23 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
 					    payload_length,
 					    blob->data,
 					    blob->length - dce_conn->auth_state.auth_info->credentials.length,
-					    &dce_conn->auth_state.auth_info->credentials);
+					    &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);
+		}
+
+		/* If we did AEAD signing of the packet headers, then we hope
+		 * this value didn't change... */
+		dcerpc_set_auth_length(blob, creds2.length);
+		data_blob_free(&creds2);
 		break;
 
 	case DCERPC_AUTH_LEVEL_INTEGRITY:
@@ -467,8 +493,23 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
 					    payload_length,
 					    blob->data,
 					    blob->length - dce_conn->auth_state.auth_info->credentials.length,
-					    &dce_conn->auth_state.auth_info->credentials);
+					    &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);
+		}
+
+		/* If we did AEAD signing of the packet headers, then we hope
+		 * this value didn't change... */
+		dcerpc_set_auth_length(blob, creds2.length);
+
+		data_blob_free(&creds2);
 		break;
 
 	case DCERPC_AUTH_LEVEL_CONNECT:
@@ -479,14 +520,11 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
 		break;
 	}
 
+	data_blob_free(&dce_conn->auth_state.auth_info->credentials);
+
 	if (!NT_STATUS_IS_OK(status)) {
 		return False;
 	}	
 
-	memcpy(blob->data + blob->length - dce_conn->auth_state.auth_info->credentials.length, 
-	       dce_conn->auth_state.auth_info->credentials.data, dce_conn->auth_state.auth_info->credentials.length);
-	
-	data_blob_free(&dce_conn->auth_state.auth_info->credentials);
-
 	return True;
 }