[PATCH v4 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. -- v4: 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 | 54 +++++++++++++++++++++++++++++++++-------- dlls/kerberos/unixlib.h | 2 ++ 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/dlls/kerberos/krb5_ap.c b/dlls/kerberos/krb5_ap.c index 5a5878faf02..5b2d831fa26 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; + 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 (stream_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..96e44f2a5df 100644 --- a/dlls/kerberos/unixlib.c +++ b/dlls/kerberos/unixlib.c @@ -982,6 +982,32 @@ 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 = params->stream; + + 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) + return SEC_E_BUFFER_TOO_SMALL; + + memcpy( params->data, iov[1].buffer.value, iov[1].buffer.length ); + + if (params->qop) + *params->qop = (conf_state ? 0 : SECQOP_WRAP_NO_ENCRYPT); + } + 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 +1038,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:
+ iov[0].buffer.length = params->stream_length; + iov[0].buffer.value = params->stream; + + 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) + return SEC_E_BUFFER_TOO_SMALL; + + memcpy( params->data, iov[1].buffer.value, iov[1].buffer.length ); Buffer should be freed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8116#note_104570
On Mon May 26 14:52:08 2025 +0000, Hans Leidekker wrote:
Buffer should be freed. I used https://web.mit.edu/kerberos/krb5-1.17/doc/appdev/gssapi.html as a reference, and it says "The DATA buffer will be initialized to point to the decrypted data within the STREAM buffer, unless it has the **GSS_C_BUFFER_FLAG_ALLOCATE** flag set, in which case it will be initialized with a copy of the decrypted data."
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8116#note_104571
On Mon May 26 14:55:27 2025 +0000, Dmitry Timoshkov wrote:
I used https://web.mit.edu/kerberos/krb5-1.17/doc/appdev/gssapi.html as a reference, and it says "The DATA buffer will be initialized to point to the decrypted data within the STREAM buffer, unless it has the **GSS_C_BUFFER_FLAG_ALLOCATE** flag set, in which case it will be initialized with a copy of the decrypted data." We should use GSS_C_BUFFER_FLAG_ALLOCATE then, the app may not expect the stream buffer to be modified.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8116#note_104572
On Mon May 26 15:05:36 2025 +0000, Hans Leidekker wrote:
We should use GSS_C_BUFFER_FLAG_ALLOCATE then, the app may not expect the stream buffer to be modified. Then we have to make a copy of the input buffer because "The STREAM buffer will be modified in-place to decrypt its contents."
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8116#note_104581
On Mon May 26 15:10:27 2025 +0000, Dmitry Timoshkov wrote:
Then we have to make a copy of the input buffer because "The STREAM buffer will be modified in-place to decrypt its contents." The language is somewhat ambiguous. It doesn't make sense to me that the data is both decrypted in-place and copied to the data buffer, but we can ignore that by giving it a copy of the full stream buffer and free it afterwards.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8116#note_104582
participants (3)
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Hans Leidekker (＠hans)