[PATCH v8 0/1] MR8116: kerberos: Add support for SECBUFFER_STREAM to SpUnsealMessage().
This is an alternative approach to https://gitlab.winehq.org/wine/wine/-/merge_requests/8114. Once this is accepted SECBUFFER_STREAM could be used for for Kerberos/DecryptMessage() in libs/ldap. -- v8: kerberos: Add support for SECBUFFER_STREAM to SpUnsealMessage(). https://gitlab.winehq.org/wine/wine/-/merge_requests/8116
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/kerberos/krb5_ap.c | 28 +++++++++++++++--- dlls/kerberos/unixlib.c | 63 ++++++++++++++++++++++++++++++++++------- dlls/kerberos/unixlib.h | 2 ++ 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/dlls/kerberos/krb5_ap.c b/dlls/kerberos/krb5_ap.c index 5a5878faf02..4ae13094889 100644 --- a/dlls/kerberos/krb5_ap.c +++ b/dlls/kerberos/krb5_ap.c @@ -867,16 +867,36 @@ static NTSTATUS NTAPI kerberos_SpUnsealMessage( LSA_SEC_HANDLE context, SecBuffe { struct context_handle *context_handle = (void *)context; struct unseal_message_params params; - int data_idx, token_idx; + int stream_idx, data_idx, token_idx = -1; + if ((stream_idx = get_buffer_index( message, SECBUFFER_STREAM )) == -1 && + (token_idx = get_buffer_index( message, SECBUFFER_TOKEN )) == -1) return SEC_E_INVALID_TOKEN; if ((data_idx = get_buffer_index( message, SECBUFFER_DATA )) == -1) return SEC_E_INVALID_TOKEN; - if ((token_idx = get_buffer_index( message, SECBUFFER_TOKEN )) == -1) return SEC_E_INVALID_TOKEN; params.context = context_handle->handle; + + if (token_idx != -1) + { + params.stream_length = 0; + params.stream = NULL; + params.token_length = message->pBuffers[token_idx].cbBuffer; + params.token = message->pBuffers[token_idx].pvBuffer; + } + else + { + if (!message->pBuffers[data_idx].pvBuffer) + { + message->pBuffers[data_idx].pvBuffer = RtlAllocateHeap( GetProcessHeap(), 0, message->pBuffers[stream_idx].cbBuffer ); + if (!message->pBuffers[data_idx].pvBuffer) return STATUS_NO_MEMORY; + message->pBuffers[data_idx].cbBuffer = message->pBuffers[stream_idx].cbBuffer; + } + params.stream_length = message->pBuffers[stream_idx].cbBuffer; + params.stream = message->pBuffers[stream_idx].pvBuffer; + params.token_length = 0; + params.token = NULL; + } params.data_length = message->pBuffers[data_idx].cbBuffer; params.data = message->pBuffers[data_idx].pvBuffer; - params.token_length = message->pBuffers[token_idx].cbBuffer; - params.token = message->pBuffers[token_idx].pvBuffer; params.qop = quality_of_protection; return KRB5_CALL( unseal_message, ¶ms ); diff --git a/dlls/kerberos/unixlib.c b/dlls/kerberos/unixlib.c index 93acecc1567..b5a04cdb53e 100644 --- a/dlls/kerberos/unixlib.c +++ b/dlls/kerberos/unixlib.c @@ -982,6 +982,41 @@ static NTSTATUS unseal_message_vector( gss_ctx_id_t ctx, const struct unseal_mes OM_uint32 ret, minor_status; int conf_state; + if (params->stream_length) + { + iov[0].type = GSS_IOV_BUFFER_TYPE_STREAM; + iov[0].buffer.length = params->stream_length; + iov[0].buffer.value = malloc( params->stream_length ); + if (!iov[0].buffer.value) return STATUS_NO_MEMORY; + memcpy( iov[0].buffer.value, params->stream, params->stream_length ); + + iov[1].type = GSS_IOV_BUFFER_TYPE_DATA; + iov[1].buffer.length = 0; + iov[1].buffer.value = NULL; + + ret = pgss_unwrap_iov( &minor_status, ctx, &conf_state, NULL, iov, 2 ); + TRACE( "gss_unwrap_iov returned %#x minor status %#x\n", ret, minor_status ); + if (GSS_ERROR( ret )) trace_gss_status( ret, minor_status ); + if (ret == GSS_S_COMPLETE) + { + if (params->data_length < iov[1].buffer.length) + { + free( iov[0].buffer.value ); + return SEC_E_BUFFER_TOO_SMALL; + } + + memcpy( params->data, iov[1].buffer.value, iov[1].buffer.length ); + /* DATA buffer is supposed to point within STREAM buffer, so + * there's no need to release it. + */ + + if (params->qop) + *params->qop = conf_state ? 0 : SECQOP_WRAP_NO_ENCRYPT; + } + free( iov[0].buffer.value ); + return status_gss_to_sspi( ret ); + } + iov[0].type = GSS_IOV_BUFFER_TYPE_SIGN_ONLY; iov[0].buffer.length = 0; iov[0].buffer.value = NULL; @@ -1012,25 +1047,33 @@ static NTSTATUS unseal_message_no_vector( gss_ctx_id_t ctx, const struct unseal_ { gss_buffer_desc input, output; OM_uint32 ret, minor_status; - DWORD len_data, len_token; int conf_state; - len_data = params->data_length; - len_token = params->token_length; - - input.length = len_data + len_token; - if (!(input.value = malloc( input.length ))) return SEC_E_INSUFFICIENT_MEMORY; - memcpy( input.value, params->data, len_data ); - memcpy( (char *)input.value + len_data, params->token, len_token ); + if (params->stream_length) + { + input.length = params->stream_length; + input.value = params->stream; + } + else + { + DWORD len_data, len_token; + + len_data = params->data_length; + len_token = params->token_length; + input.length = len_data + len_token; + if (!(input.value = malloc( input.length ))) return SEC_E_INSUFFICIENT_MEMORY; + memcpy( input.value, params->data, len_data ); + memcpy( (char *)input.value + len_data, params->token, len_token ); + } ret = pgss_unwrap( &minor_status, ctx, &input, &output, &conf_state, NULL ); - free( input.value ); + if (input.value != params->stream) free( input.value ); TRACE( "gss_unwrap returned %#x minor status %#x\n", ret, minor_status ); if (GSS_ERROR( ret )) trace_gss_status( ret, minor_status ); if (ret == GSS_S_COMPLETE) { if (params->qop) *params->qop = (conf_state ? 0 : SECQOP_WRAP_NO_ENCRYPT); - memcpy( params->data, output.value, len_data ); + memcpy( params->data, output.value, output.length ); pgss_release_buffer( &minor_status, &output ); } diff --git a/dlls/kerberos/unixlib.h b/dlls/kerberos/unixlib.h index 87516859991..1599d7de4f8 100644 --- a/dlls/kerberos/unixlib.h +++ b/dlls/kerberos/unixlib.h @@ -106,6 +106,8 @@ struct seal_message_params struct unseal_message_params { UINT64 context; + BYTE *stream; + ULONG stream_length; BYTE *data; ULONG data_length; BYTE *token; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8116
Hans Leidekker (@hans) commented about dlls/kerberos/unixlib.c:
}
memcpy( params->data, iov[1].buffer.value, iov[1].buffer.length ); - - /* check whether DATA buffer points within STREAM buffer */ - if (!((char *)iov[1].buffer.value >= (char *)iov[0].buffer.value && (char *)iov[1].buffer.value < (char *)iov[0].buffer.value + iov[0].buffer.length)) - pgss_release_buffer( &minor_status, &iov[1].buffer ); + /* DATA buffer is supposed to point within STREAM buffer, so + * there's no need to release it. + */
This comment seems unnecessary. Releasing buffers is only needed when GSS_C_BUFFER_FLAG_ALLOCATE is passed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8116#note_105213
On Tue Jun 3 07:03:30 2025 +0000, Hans Leidekker wrote:
This comment seems unnecessary. Releasing buffers is only needed when GSS_C_BUFFER_FLAG_ALLOCATE is passed. While I find this comment helpful because it explains why DATA buffer doesn't need to be released, and avoid a confusion in future, it's not principial, and I wouldn't insist on it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8116#note_105214
participants (3)
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Hans Leidekker (＠hans)