[PATCH 0/6] MR8203: SECBUFFER_STREAM fixes
From: Hans Leidekker <hans(a)codeweavers.com> --- dlls/msv1_0/main.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/dlls/msv1_0/main.c b/dlls/msv1_0/main.c index a8c5dab8b0a..44f079249e2 100644 --- a/dlls/msv1_0/main.c +++ b/dlls/msv1_0/main.c @@ -1322,11 +1322,11 @@ static void hmac_md5_final( struct hmac_md5_ctx *ctx, char *digest ) memcpy( digest, outer_ctx.digest, 16 ); } -static SECURITY_STATUS create_signature( struct ntlm_ctx *ctx, unsigned int flags, SecBufferDesc *msg, int idx, - enum sign_direction dir, BOOL encrypt ) +static SECURITY_STATUS create_signature( struct ntlm_ctx *ctx, unsigned int flags, SecBufferDesc *msg, + SecBuffer *sig_buf, enum sign_direction dir, BOOL encrypt ) { unsigned int i, sign_version = 1; - char *sig = msg->pBuffers[idx].pvBuffer; + char *sig = sig_buf->pvBuffer; if (flags & FLAG_NEGOTIATE_NTLM2 && flags & FLAG_NEGOTIATE_SIGN) { @@ -1377,7 +1377,7 @@ static SECURITY_STATUS create_signature( struct ntlm_ctx *ctx, unsigned int flag memcpy( sig + 4, digest, 8 ); memcpy( sig + 12, seq_no, 4 ); - msg->pBuffers[idx].cbBuffer = 16; + sig_buf->cbBuffer = 16; return SEC_E_OK; } @@ -1413,9 +1413,9 @@ static SECURITY_STATUS create_signature( struct ntlm_ctx *ctx, unsigned int flag if (flags & FLAG_NEGOTIATE_ALWAYS_SIGN || !flags) { /* create dummy signature */ - memset( msg->pBuffers[idx].pvBuffer, 0, 16 ); - memset( msg->pBuffers[idx].pvBuffer, 1, 1 ); - msg->pBuffers[idx].cbBuffer = 16; + memset( sig_buf->pvBuffer, 0, 16 ); + memset( sig_buf->pvBuffer, 1, 1 ); + sig_buf->cbBuffer = 16; return SEC_E_OK; } @@ -1436,7 +1436,7 @@ static NTSTATUS NTAPI ntlm_SpMakeSignature( LSA_SEC_HANDLE handle, ULONG qop, Se return SEC_E_INVALID_TOKEN; if (msg->pBuffers[idx].cbBuffer < 16) return SEC_E_BUFFER_TOO_SMALL; - return create_signature( ctx, ctx->flags, msg, idx, SIGN_SEND, TRUE ); + return create_signature( ctx, ctx->flags, msg, &msg->pBuffers[idx], SIGN_SEND, TRUE ); } static NTSTATUS verify_signature( struct ntlm_ctx *ctx, unsigned int flags, SecBufferDesc *msg, int idx ) @@ -1468,7 +1468,7 @@ static NTSTATUS verify_signature( struct ntlm_ctx *ctx, unsigned int flags, SecB } } - if ((status = create_signature( ctx, flags, &desc, idx, SIGN_RECV, TRUE )) == SEC_E_OK) + if ((status = create_signature( ctx, flags, &desc, &buf[idx], SIGN_RECV, TRUE )) == SEC_E_OK) { if (memcmp( (char *)buf[idx].pvBuffer + 8, (char *)msg->pBuffers[idx].pvBuffer + 8, 8 )) status = SEC_E_MESSAGE_ALTERED; @@ -1514,7 +1514,7 @@ static NTSTATUS NTAPI ntlm_SpSealMessage( LSA_SEC_HANDLE handle, ULONG qop, SecB ctx = (struct ntlm_ctx *)handle; if (ctx->flags & FLAG_NEGOTIATE_NTLM2 && ctx->flags & FLAG_NEGOTIATE_SEAL) { - create_signature( ctx, ctx->flags, msg, token_idx, SIGN_SEND, FALSE ); + create_signature( ctx, ctx->flags, msg, &msg->pBuffers[token_idx], SIGN_SEND, FALSE ); arc4_process( &ctx->crypt.ntlm2.send_arc4info, msg->pBuffers[data_idx].pvBuffer, msg->pBuffers[data_idx].cbBuffer ); @@ -1525,7 +1525,7 @@ static NTSTATUS NTAPI ntlm_SpSealMessage( LSA_SEC_HANDLE handle, ULONG qop, SecB { char *sig = msg->pBuffers[token_idx].pvBuffer; - create_signature( ctx, ctx->flags | FLAG_NEGOTIATE_SIGN, msg, token_idx, SIGN_SEND, FALSE ); + create_signature( ctx, ctx->flags | FLAG_NEGOTIATE_SIGN, msg, &msg->pBuffers[token_idx], SIGN_SEND, FALSE ); arc4_process( &ctx->crypt.ntlm.arc4info, msg->pBuffers[data_idx].pvBuffer, msg->pBuffers[data_idx].cbBuffer ); arc4_process( &ctx->crypt.ntlm.arc4info, sig + 4, 12 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8203
From: Hans Leidekker <hans(a)codeweavers.com> --- dlls/msv1_0/main.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/msv1_0/main.c b/dlls/msv1_0/main.c index 44f079249e2..904fa922e3b 100644 --- a/dlls/msv1_0/main.c +++ b/dlls/msv1_0/main.c @@ -1439,10 +1439,10 @@ static NTSTATUS NTAPI ntlm_SpMakeSignature( LSA_SEC_HANDLE handle, ULONG qop, Se return create_signature( ctx, ctx->flags, msg, &msg->pBuffers[idx], SIGN_SEND, TRUE ); } -static NTSTATUS verify_signature( struct ntlm_ctx *ctx, unsigned int flags, SecBufferDesc *msg, int idx ) +static NTSTATUS verify_signature( struct ntlm_ctx *ctx, unsigned int flags, SecBufferDesc *msg, SecBuffer *sig_buf ) { NTSTATUS status; - unsigned int i; + unsigned int i, sig_idx = 0; SecBufferDesc desc; SecBuffer *buf; char sig[16]; @@ -1459,6 +1459,7 @@ static NTSTATUS verify_signature( struct ntlm_ctx *ctx, unsigned int flags, SecB buf[i].BufferType = SECBUFFER_TOKEN; buf[i].cbBuffer = 16; buf[i].pvBuffer = sig; + sig_idx = i; } else { @@ -1468,9 +1469,9 @@ static NTSTATUS verify_signature( struct ntlm_ctx *ctx, unsigned int flags, SecB } } - if ((status = create_signature( ctx, flags, &desc, &buf[idx], SIGN_RECV, TRUE )) == SEC_E_OK) + if ((status = create_signature( ctx, flags, &desc, &buf[sig_idx], SIGN_RECV, TRUE )) == SEC_E_OK) { - if (memcmp( (char *)buf[idx].pvBuffer + 8, (char *)msg->pBuffers[idx].pvBuffer + 8, 8 )) + if (memcmp( (char *)buf[sig_idx].pvBuffer + 8, (char *)sig_buf->pvBuffer + 8, 8 )) status = SEC_E_MESSAGE_ALTERED; } @@ -1491,7 +1492,7 @@ static NTSTATUS NTAPI ntlm_SpVerifySignature( LSA_SEC_HANDLE handle, SecBufferDe return SEC_E_INVALID_TOKEN; if (msg->pBuffers[idx].cbBuffer < 16) return SEC_E_BUFFER_TOO_SMALL; - return verify_signature( ctx, ctx->flags, msg, idx ); + return verify_signature( ctx, ctx->flags, msg, &msg->pBuffers[idx] ); } static NTSTATUS NTAPI ntlm_SpSealMessage( LSA_SEC_HANDLE handle, ULONG qop, SecBufferDesc *msg, ULONG msg_seq_no ) @@ -1562,7 +1563,7 @@ static NTSTATUS NTAPI ntlm_SpUnsealMessage( LSA_SEC_HANDLE handle, SecBufferDesc /* make sure we use a session key for the signature check, SealMessage always does that, even in the dummy case */ - return verify_signature( ctx, ctx->flags | FLAG_NEGOTIATE_SIGN, msg, token_idx ); + return verify_signature( ctx, ctx->flags | FLAG_NEGOTIATE_SIGN, msg, &msg->pBuffers[token_idx] ); } static SECPKG_USER_FUNCTION_TABLE ntlm_user_table = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8203
From: Hans Leidekker <hans(a)codeweavers.com> --- dlls/msv1_0/main.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/dlls/msv1_0/main.c b/dlls/msv1_0/main.c index 904fa922e3b..b5aa4e8d445 100644 --- a/dlls/msv1_0/main.c +++ b/dlls/msv1_0/main.c @@ -1454,7 +1454,7 @@ static NTSTATUS verify_signature( struct ntlm_ctx *ctx, unsigned int flags, SecB for (i = 0; i < msg->cBuffers; i++) { - if (msg->pBuffers[i].BufferType == SECBUFFER_TOKEN) + if (msg->pBuffers[i].BufferType == SECBUFFER_TOKEN || msg->pBuffers[i].BufferType == SECBUFFER_STREAM) { buf[i].BufferType = SECBUFFER_TOKEN; buf[i].cbBuffer = 16; @@ -1539,7 +1539,10 @@ static NTSTATUS NTAPI ntlm_SpSealMessage( LSA_SEC_HANDLE handle, ULONG qop, SecB static NTSTATUS NTAPI ntlm_SpUnsealMessage( LSA_SEC_HANDLE handle, SecBufferDesc *msg, ULONG msg_seq_no, ULONG *qop ) { - int token_idx, data_idx; + int data_idx = get_buffer_index( msg, SECBUFFER_DATA ); + int stream_idx = get_buffer_index( msg, SECBUFFER_STREAM ); + int token_idx = get_buffer_index( msg, SECBUFFER_TOKEN ); + SecBuffer token_buf; struct ntlm_ctx *ctx; TRACE( "%#Ix, %p, %lu, %p\n", handle, msg, msg_seq_no, qop ); @@ -1547,11 +1550,25 @@ static NTSTATUS NTAPI ntlm_SpUnsealMessage( LSA_SEC_HANDLE handle, SecBufferDesc if (!handle) return SEC_E_INVALID_HANDLE; - if (!msg || !msg->pBuffers || msg->cBuffers < 2 || - (token_idx = get_buffer_index( msg, SECBUFFER_TOKEN )) == -1 || - (data_idx = get_buffer_index( msg, SECBUFFER_DATA )) == -1) return SEC_E_INVALID_TOKEN; + if (!msg || !msg->pBuffers || msg->cBuffers < 2 || (token_idx == -1 && stream_idx == -1) || + (stream_idx != -1 && token_idx != -1) || data_idx == -1) return SEC_E_INVALID_TOKEN; - if (msg->pBuffers[token_idx].cbBuffer < 16) return SEC_E_BUFFER_TOO_SMALL; + if (stream_idx != -1) + { + if (msg->pBuffers[stream_idx].cbBuffer < 16) return SEC_E_BUFFER_TOO_SMALL; + token_buf.BufferType = SECBUFFER_TOKEN; + token_buf.cbBuffer = 16; + token_buf.pvBuffer = msg->pBuffers[stream_idx].pvBuffer; + + /* native decrypts in-place even when an appropriately sized data buffer is supplied */ + msg->pBuffers[data_idx].pvBuffer = (char *)msg->pBuffers[stream_idx].pvBuffer + 16; + msg->pBuffers[data_idx].cbBuffer = msg->pBuffers[stream_idx].cbBuffer - 16; + } + else + { + if (msg->pBuffers[token_idx].cbBuffer < 16) return SEC_E_BUFFER_TOO_SMALL; + token_buf = msg->pBuffers[token_idx]; + } ctx = (struct ntlm_ctx *)handle; if (ctx->flags & FLAG_NEGOTIATE_NTLM2 && ctx->flags & FLAG_NEGOTIATE_SEAL) @@ -1563,7 +1580,7 @@ static NTSTATUS NTAPI ntlm_SpUnsealMessage( LSA_SEC_HANDLE handle, SecBufferDesc /* make sure we use a session key for the signature check, SealMessage always does that, even in the dummy case */ - return verify_signature( ctx, ctx->flags | FLAG_NEGOTIATE_SIGN, msg, &msg->pBuffers[token_idx] ); + return verify_signature( ctx, ctx->flags | FLAG_NEGOTIATE_SIGN, msg, &token_buf ); } static SECPKG_USER_FUNCTION_TABLE ntlm_user_table = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8203
From: Hans Leidekker <hans(a)codeweavers.com> --- dlls/kerberos/unixlib.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/kerberos/unixlib.c b/dlls/kerberos/unixlib.c index 254db4f950e..f934d64a3b4 100644 --- a/dlls/kerberos/unixlib.c +++ b/dlls/kerberos/unixlib.c @@ -1393,6 +1393,8 @@ static NTSTATUS wow64_unseal_message( void *args ) struct { UINT64 context; + PTR32 stream; + ULONG stream_length; PTR32 data; ULONG data_length; PTR32 token; @@ -1402,6 +1404,8 @@ static NTSTATUS wow64_unseal_message( void *args ) struct unseal_message_params params = { params32->context, + ULongToPtr(params32->stream), + params32->stream_length, ULongToPtr(params32->data), params32->data_length, ULongToPtr(params32->token), -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8203
From: Hans Leidekker <hans(a)codeweavers.com> Native decrypts SECBUFFER_STREAM in-place even if a data buffer of sufficient size is supplied. --- dlls/kerberos/krb5_ap.c | 10 ++-------- dlls/kerberos/unixlib.c | 41 ++++++++++++++++++----------------------- dlls/kerberos/unixlib.h | 4 ++-- 3 files changed, 22 insertions(+), 33 deletions(-) diff --git a/dlls/kerberos/krb5_ap.c b/dlls/kerberos/krb5_ap.c index 4ae13094889..27c91b265f2 100644 --- a/dlls/kerberos/krb5_ap.c +++ b/dlls/kerberos/krb5_ap.c @@ -884,19 +884,13 @@ static NTSTATUS NTAPI kerberos_SpUnsealMessage( LSA_SEC_HANDLE context, SecBuffe } 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.data_length = &message->pBuffers[data_idx].cbBuffer; + params.data = (BYTE **)&message->pBuffers[data_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 f934d64a3b4..51580458811 100644 --- a/dlls/kerberos/unixlib.c +++ b/dlls/kerberos/unixlib.c @@ -976,7 +976,7 @@ static NTSTATUS seal_message( void *args ) return seal_message_no_vector( ctx, params ); } -static NTSTATUS unseal_message_vector( gss_ctx_id_t ctx, const struct unseal_message_params *params ) +static NTSTATUS unseal_message_vector( gss_ctx_id_t ctx, struct unseal_message_params *params ) { gss_iov_buffer_desc iov[4]; OM_uint32 ret, minor_status; @@ -986,9 +986,7 @@ static NTSTATUS unseal_message_vector( gss_ctx_id_t ctx, const struct unseal_mes { 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[0].buffer.value = params->stream; iov[1].type = GSS_IOV_BUFFER_TYPE_DATA; iov[1].buffer.length = 0; @@ -999,17 +997,12 @@ static NTSTATUS unseal_message_vector( gss_ctx_id_t ctx, const struct unseal_mes 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; - } + *params->data = iov[1].buffer.value; + *params->data_length = iov[1].buffer.length; - memcpy( params->data, iov[1].buffer.value, iov[1].buffer.length ); if (params->qop) *params->qop = conf_state ? 0 : SECQOP_WRAP_NO_ENCRYPT; } - free( iov[0].buffer.value ); return status_gss_to_sspi( ret ); } @@ -1018,8 +1011,8 @@ static NTSTATUS unseal_message_vector( gss_ctx_id_t ctx, const struct unseal_mes iov[0].buffer.value = NULL; iov[1].type = GSS_IOV_BUFFER_TYPE_DATA; - iov[1].buffer.length = params->data_length; - iov[1].buffer.value = params->data; + iov[1].buffer.length = *params->data_length; + iov[1].buffer.value = *params->data; iov[2].type = GSS_IOV_BUFFER_TYPE_SIGN_ONLY; iov[2].buffer.length = 0; @@ -1052,14 +1045,10 @@ static NTSTATUS unseal_message_no_vector( gss_ctx_id_t ctx, const struct unseal_ } else { - DWORD len_data, len_token; - - len_data = params->data_length; - len_token = params->token_length; - input.length = len_data + len_token; + input.length = *params->data_length + params->token_length; if (!(input.value = malloc( input.length ))) return STATUS_NO_MEMORY; - memcpy( input.value, params->data, len_data ); - memcpy( (char *)input.value + len_data, params->token, len_token ); + memcpy( input.value, *params->data, *params->data_length ); + memcpy( (char *)input.value + *params->data_length, params->token, params->token_length ); } ret = pgss_unwrap( &minor_status, ctx, &input, &output, &conf_state, NULL ); @@ -1069,7 +1058,13 @@ 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, output.length ); + if (params->stream_length) + { + memcpy( params->stream, output.value, output.length ); + *params->data = params->stream; + *params->data_length = output.length; + } + else memcpy( *params->data, output.value, output.length ); pgss_release_buffer( &minor_status, &output ); } @@ -1396,7 +1391,7 @@ static NTSTATUS wow64_unseal_message( void *args ) PTR32 stream; ULONG stream_length; PTR32 data; - ULONG data_length; + PTR32 data_length; PTR32 token; ULONG token_length; PTR32 qop; @@ -1407,7 +1402,7 @@ static NTSTATUS wow64_unseal_message( void *args ) ULongToPtr(params32->stream), params32->stream_length, ULongToPtr(params32->data), - params32->data_length, + ULongToPtr(params32->data_length), ULongToPtr(params32->token), params32->token_length, ULongToPtr(params32->qop), diff --git a/dlls/kerberos/unixlib.h b/dlls/kerberos/unixlib.h index 1599d7de4f8..60a8c2f243c 100644 --- a/dlls/kerberos/unixlib.h +++ b/dlls/kerberos/unixlib.h @@ -108,8 +108,8 @@ struct unseal_message_params UINT64 context; BYTE *stream; ULONG stream_length; - BYTE *data; - ULONG data_length; + BYTE **data; + ULONG *data_length; BYTE *token; ULONG token_length; ULONG *qop; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8203
From: Hans Leidekker <hans(a)codeweavers.com> --- libs/ldap/libldap/sasl_w.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/libs/ldap/libldap/sasl_w.c b/libs/ldap/libldap/sasl_w.c index 429c21ba438..5c3cd591531 100644 --- a/libs/ldap/libldap/sasl_w.c +++ b/libs/ldap/libldap/sasl_w.c @@ -68,37 +68,28 @@ int sasl_decode( sasl_conn_t *handle, const char *input, unsigned int inputlen, unsigned int len; SecBuffer bufs[2] = { - { 0, SECBUFFER_DATA, NULL }, - { conn->trailer_size, SECBUFFER_TOKEN, NULL } + { 0, SECBUFFER_STREAM, NULL }, + { 0, SECBUFFER_DATA, NULL } }; SecBufferDesc buf_desc = { SECBUFFER_VERSION, ARRAYSIZE(bufs), bufs }; SECURITY_STATUS status; int ret; - if (inputlen < sizeof(len) + conn->trailer_size) return SASL_FAIL; + if (inputlen < sizeof(len)) return SASL_FAIL; len = ntohl( *(unsigned int *)input ); if (inputlen < sizeof(len) + len) return SASL_FAIL; if ((ret = grow_buffer( conn, len )) < 0) return ret; memcpy( conn->buf, input + sizeof(len), len ); - bufs[0].cbBuffer = len - conn->trailer_size; - if (conn->package_id == RPC_C_AUTHN_GSS_KERBEROS) - { - bufs[0].pvBuffer = conn->buf; - bufs[1].pvBuffer = conn->buf + bufs[0].cbBuffer; - } - else - { - bufs[0].pvBuffer = conn->buf + conn->trailer_size; - bufs[1].pvBuffer = conn->buf; - } + bufs[0].pvBuffer = conn->buf; + bufs[0].cbBuffer = len; status = DecryptMessage( &conn->ctxt_handle, &buf_desc, 0, NULL ); if (status == SEC_E_OK) { - *output = bufs[0].pvBuffer; - *outputlen = bufs[0].cbBuffer; + *output = bufs[1].pvBuffer; + *outputlen = bufs[1].cbBuffer; return SASL_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8203
Looks (and works) good to me, thanks! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8203#note_105355
participants (3)
-
Dmitry Timoshkov (＠dmitry) -
Hans Leidekker
-
Hans Leidekker (＠hans)