[PATCH v3 0/1] MR10916: secur32: Return SEC_E_INVALID_TOKEN for invalid TLS record headers
I encountered an issue with establishing an encrypted connection to the mail server in Outlook. I managed to find out that Outlook tries various connection setup options — in particular, it sends a ClientHello to server port 587, and in response receives an SMTP banner, which in the establish_context function (https://gitlab.winehq.org/wine/wine/-/blame/master/dlls/secur32/schannel.c?r...) is mistakenly interpreted as TLS and returns a SEC_E_INCOMPLETE_MESSAGE error. This causes further reading of data from the server → an error and connection break. I managed to fix it by adding validation of the first bytes of the buffer to match the TLS header. -- v3: secur32: Return SEC_E_INVALID_TOKEN for invalid TLS record headers https://gitlab.winehq.org/wine/wine/-/merge_requests/10916
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10916
From: Ivan Lyugaev <valy@etersoft.ru> --- dlls/secur32/schannel.c | 19 +++++++++++++++++++ dlls/secur32/tests/schannel.c | 2 -- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c index 19eda9e0026..1e3bbb9f31c 100644 --- a/dlls/secur32/schannel.c +++ b/dlls/secur32/schannel.c @@ -841,6 +841,10 @@ static void dump_buffer_desc(SecBufferDesc *desc) #define HEADER_SIZE_TLS 5 #define HEADER_SIZE_DTLS 13 +#define TLS_CONTENT_TYPE_CHANGE_CIPHER_SPEC 20 +#define TLS_CONTENT_TYPE_APPLICATION_DATA 23 +#define TLS_RECORD_TYPE_OFFSET 0 +#define TLS_RECORD_VERSION_MAJOR_OFFSET 1 static inline SIZE_T read_record_size(const BYTE *buf, SIZE_T header_size) { @@ -852,6 +856,13 @@ static inline BOOL is_dtls_context(const struct schan_context *ctx) return ctx->header_size == HEADER_SIZE_DTLS; } +static inline BOOL is_tls_record_header(BYTE *ptr) +{ + return ptr[TLS_RECORD_TYPE_OFFSET] >= TLS_CONTENT_TYPE_CHANGE_CIPHER_SPEC && + ptr[TLS_RECORD_TYPE_OFFSET] <= TLS_CONTENT_TYPE_APPLICATION_DATA && + ptr[TLS_RECORD_VERSION_MAJOR_OFFSET] == 3; +} + static void fill_missing_sec_buffer(SecBufferDesc *input, DWORD size) { int idx = schan_find_sec_buffer_idx(input, 0, SECBUFFER_EMPTY); @@ -1017,6 +1028,14 @@ static SECURITY_STATUS establish_context( return SEC_E_INCOMPLETE_MESSAGE; } + if (!is_dtls_context(ctx) && !is_tls_record_header(ptr)) + { + WARN("Invalid TLS record header: %02x %02x %02x %02x %02x.\n", + ptr[0], ptr[1], ptr[2], ptr[3], ptr[4]); + pOutput->pBuffers[idx].cbBuffer = 0; + return SEC_E_INVALID_TOKEN; + } + while (buffer->cbBuffer >= expected_size + ctx->header_size) { record_size = ctx->header_size + read_record_size(ptr, ctx->header_size); diff --git a/dlls/secur32/tests/schannel.c b/dlls/secur32/tests/schannel.c index 3c4f938f556..b5736a2ce39 100644 --- a/dlls/secur32/tests/schannel.c +++ b/dlls/secur32/tests/schannel.c @@ -1040,9 +1040,7 @@ static void test_communication(void) status = InitializeSecurityContextA(&cred_handle, &context, (SEC_CHAR *)"localhost", ISC_REQ_CONFIDENTIALITY|ISC_REQ_STREAM, 0, 0, &buffers[1], 0, NULL, &buffers[0], &attrs, NULL); - todo_wine ok(status == SEC_E_INVALID_TOKEN, "Expected SEC_E_INVALID_TOKEN, got %08lx\n", status); - todo_wine ok(buffers[0].pBuffers[0].cbBuffer == 0, "Output buffer size was not set to 0.\n"); buffers[1].cBuffers = 1; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10916
On Sat May 30 13:23:36 2026 +0000, Hans Leidekker wrote:
Please fix the failures first. I fixed it
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10916#note_141779
participants (3)
-
Hans Leidekker (＠hans) -
Ivan Lyugaev
-
Ivan Lyugaev (＠valy)