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.
From: Dmitry Timoshkov dmitry@baikal.ru
Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/kerberos/krb5_ap.c | 28 ++++++++++++++++++++++++---- dlls/kerberos/unixlib.c | 29 +++++++++++++++++++---------- dlls/kerberos/unixlib.h | 2 ++ 3 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/dlls/kerberos/krb5_ap.c b/dlls/kerberos/krb5_ap.c index 5a5878faf02..46c12586406 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) + if ((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, KERBEROS_MAX_BUF ); + if (!message->pBuffers[data_idx].pvBuffer) return STATUS_NO_MEMORY; + message->pBuffers[data_idx].cbBuffer = KERBEROS_MAX_BUF; + } + 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..b4379fe0c34 100644 --- a/dlls/kerberos/unixlib.c +++ b/dlls/kerberos/unixlib.c @@ -1012,16 +1012,25 @@ 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; + if (!(input.value = malloc( input.length ))) return SEC_E_INSUFFICIENT_MEMORY; + memcpy( input.value, params->stream, params->stream_length ); + } + 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 ); @@ -1030,7 +1039,7 @@ static NTSTATUS unseal_message_no_vector( gss_ctx_id_t ctx, const struct unseal_ 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 ); }
@@ -1042,7 +1051,7 @@ static NTSTATUS unseal_message( void *args ) struct unseal_message_params *params = args; gss_ctx_id_t ctx = ctxhandle_sspi_to_gss( params->context );
- if (is_dce_style_context( ctx )) return unseal_message_vector( ctx, params ); + if (is_dce_style_context( ctx ) && !params->stream_length) return unseal_message_vector( ctx, params ); return unseal_message_no_vector( ctx, params ); }
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;
Yes, this would allow us to simplify sasl_decode() but only if we add support to NTLM as well.
Hans Leidekker (@hans) commented about dlls/kerberos/krb5_ap.c:
{ 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)if ((token_idx = get_buffer_index( message, SECBUFFER_TOKEN )) == -1) return SEC_E_INVALID_TOKEN;
This introduces a compiler warning. We usually write this as an && condition BTW.
Hans Leidekker (@hans) commented about dlls/kerberos/unixlib.c:
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;if (!(input.value = malloc( input.length ))) return SEC_E_INSUFFICIENT_MEMORY;memcpy( input.value, params->stream, params->stream_length );- }
We can actually avoid the buffer copy in this case.
Hans Leidekker (@hans) commented about dlls/kerberos/unixlib.c:
struct unseal_message_params *params = args; gss_ctx_id_t ctx = ctxhandle_sspi_to_gss( params->context );
- if (is_dce_style_context( ctx )) return unseal_message_vector( ctx, params );
- if (is_dce_style_context( ctx ) && !params->stream_length) return unseal_message_vector( ctx, params );
This could use some testing. It should perhaps be an error if a SECBUFFER_STREAM is passed with a DCE context.
On Fri May 23 17:07:26 2025 +0000, Hans Leidekker wrote:
Yes, this would allow us to simplify sasl_decode() but only if we add support to NTLM as well.
Since I don't have a working NTLM setup I was planning to use new code path only for Kerberos.
On Fri May 23 17:07:27 2025 +0000, Hans Leidekker wrote:
This could use some testing. It should perhaps be an error if a SECBUFFER_STREAM is passed with a DCE context.
I tried to write some code to see how SECBUFFER_STREAM support works under Windows. However I couldn't even get the code snippet for DecryptMessage() from https://learn.microsoft.com/en-us/windows/win32/secauthn/sspi-kerberos-inter... work. I guess that's because of Negotiate wrappers that insist to see DATA+TOKEN buffers as input.
On Fri May 23 17:07:27 2025 +0000, Hans Leidekker wrote:
We can actually avoid the buffer copy in this case.
Indeed, thanks.
On Fri May 23 17:07:27 2025 +0000, Hans Leidekker wrote:
This introduces a compiler warning. We usually write this as an && condition BTW.
Thanks.
On Fri May 23 17:42:41 2025 +0000, Dmitry Timoshkov wrote:
Since I don't have a working NTLM setup I was planning to use new code path only for Kerberos.
That would complicate sasl_decode() further, I don't think we want that. If we have NTLM support too we can actually simplify sasl_decode().
On Fri May 23 17:49:48 2025 +0000, Hans Leidekker wrote:
That would complicate sasl_decode() further, I don't think we want that. If we have NTLM support too we can actually simplify sasl_decode().
Since I can't test NTLM code I'd rather leave that task to someone who actually could test it. Can you help with it?
On Fri May 23 17:48:45 2025 +0000, Dmitry Timoshkov wrote:
I tried to write some code to see how SECBUFFER_STREAM support works under Windows. However I couldn't even get the code snippet for DecryptMessage() from https://learn.microsoft.com/en-us/windows/win32/secauthn/sspi-kerberos-inter... work. I guess that's because of Negotiate wrappers that insist to see DATA+TOKEN buffers as input.
I know, this stuff is hard to test. Among other things I'm using a modified wldap32 test and wldap32 dll (which includes a modified libldap) on Windows. This allows me to see how native SSPI behaves.
On Fri May 23 17:56:25 2025 +0000, Dmitry Timoshkov wrote:
Since I can't test NTLM code I'd rather leave that task to someone who actually could test it. Can you help with it?
I can take a look.
On Fri May 23 18:01:00 2025 +0000, Hans Leidekker wrote:
I know, this stuff is hard to test. Among other things I'm using a modified wldap32 test and wldap32 dll (which includes a modified libldap) on Windows. This allows me to see how native SSPI behaves.
I also tried to convince EncryptMessage() to produce a SECBUFFER_STREAM with a variety of inputs but failed. It doesn't appear to support it, which is a shame.
On Fri May 23 18:14:57 2025 +0000, Hans Leidekker wrote:
I also tried to convince EncryptMessage() to produce a SECBUFFER_STREAM with a variety of inputs but failed. It doesn't appear to support it, which is a shame.
Would something like https://web.mit.edu/freebsd/head/contrib/wpa/src/crypto/tls_schannel.c,tls_c...) work? Presumably composing a buffer as SECBUFFER_STREAM_HEADER+SECBUFFER_DATA+SECBUFFER_STREAM_TRAILER could be decoded as a single SECBUFFER_STREAM buffer?
On Fri May 23 18:56:07 2025 +0000, Dmitry Timoshkov wrote:
Would something like https://web.mit.edu/freebsd/head/contrib/wpa/src/crypto/tls_schannel.c,tls_c...) work? Presumably composing a buffer as SECBUFFER_STREAM_HEADER+SECBUFFER_DATA+SECBUFFER_STREAM_TRAILER could be decoded as a single SECBUFFER_STREAM buffer?
I think that's the buffer layout for SECPKG_FLAG_STREAM providers such as Schannel, Kerberos is not one of them.
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Hans Leidekker (＠hans)