Skip to content

Commit a1690ef

Browse files
whiter001baiyunfengCopilot
authored
net.vschannel: chunk large TLS payloads to fix 16KB EncryptMessage crash on Windows (#26681)
* fix(vschannel): chunk large TLS request payloads on Windows Split request payloads into cbMaximumMessage-sized chunks before EncryptMessage and send each encrypted record fully to avoid large-body crashes in Schannel path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(tools): add vschannel 16kb httpbin probe script Add a reproducible probe script that posts payloads around the 16KB boundary to https://httpbin.org/post and supports before/after expectation modes for regression verification. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style(tools): vfmt vschannel_16kb_httpbin_probe.v * refactor(tools): apply Copilot suggestions to vschannel probe - use os.new_process instead of os.execute to avoid shell escaping issues on Windows and separate stdout/stderr capture - add explicit 'none' branch in expected_success_for_mode to make intent clear and guard against future unexpected mode values * fix(vschannel): cast send result before DWORD accumulation --------- Co-authored-by: baiyunfeng <baiyunfeng@bailian.ai> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 89ee154 commit a1690ef

2 files changed

Lines changed: 211 additions & 35 deletions

File tree

‎cmd/tools/vschannel_16kb_httpbin_probe.v‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import flag
2+
import net.http
3+
import os
4+
import time
5+
6+
const probe_url = 'https://httpbin.org/post'
7+
const vschannel_16kb_boundary = 16 * 1024
8+
const default_sizes_csv = '12000,15000,17000,24000,32768'
9+
10+
fn main() {
11+
if os.args.len >= 2 && os.args[1] == '--worker' {
12+
run_worker_mode()
13+
return
14+
}
15+
run_parent_mode()
16+
}
17+
18+
fn run_worker_mode() {
19+
if os.args.len < 3 {
20+
eprintln('missing payload size for --worker')
21+
exit(2)
22+
}
23+
size := os.args[2].int()
24+
if size <= 0 {
25+
eprintln('invalid payload size `${os.args[2]}`')
26+
exit(2)
27+
}
28+
payload := '{"size":${size},"payload":"' + 'x'.repeat(size) + '"}'
29+
mut headers := http.new_header()
30+
headers.add(.content_type, 'application/json')
31+
mut req := http.Request{
32+
method: .post
33+
url: probe_url
34+
header: headers
35+
data: payload
36+
read_timeout: 60 * time.second
37+
write_timeout: 30 * time.second
38+
}
39+
resp := req.do() or {
40+
eprintln('request failed at size=${size}: ${err}')
41+
exit(1)
42+
}
43+
if resp.status_code != 200 {
44+
eprintln('unexpected status at size=${size}: ${resp.status_code}')
45+
exit(1)
46+
}
47+
if !resp.body.contains('httpbin.org/post') {
48+
eprintln('unexpected response body at size=${size}')
49+
exit(1)
50+
}
51+
println('OK size=${size} payload_bytes=${payload.len}')
52+
}
53+
54+
fn run_parent_mode() {
55+
mut fp := flag.new_flag_parser(os.args)
56+
fp.application('vschannel_16kb_httpbin_probe')
57+
fp.version('0.0.1')
58+
fp.description('Probe net.http HTTPS POST behavior around 16KB payloads using https://httpbin.org/post.')
59+
fp.skip_executable()
60+
61+
expect_mode := fp.string('expect', `e`, 'after', 'Expected behavior mode: before|after|none. before expects >16KB failures, after expects all succeed.')
62+
sizes_csv := fp.string('sizes', `s`, default_sizes_csv, 'Comma separated payload sizes in bytes.')
63+
verbose := fp.bool('verbose', `v`, false, 'Print child process output for all sizes.')
64+
show_help := fp.bool('help', `h`, false, 'Show this help screen.')
65+
free_args := fp.finalize() or {
66+
eprintln('flag parse failed: ${err}')
67+
exit(2)
68+
}
69+
if free_args.len > 0 {
70+
eprintln('unexpected positional args: ${free_args}')
71+
exit(2)
72+
}
73+
if show_help {
74+
println(fp.usage())
75+
return
76+
}
77+
78+
mode := expect_mode.to_lower()
79+
if mode !in ['before', 'after', 'none'] {
80+
eprintln('invalid --expect `${expect_mode}` (allowed: before|after|none)')
81+
exit(2)
82+
}
83+
sizes := parse_sizes_csv(sizes_csv) or {
84+
eprintln(err)
85+
exit(2)
86+
}
87+
88+
self_exe := os.executable()
89+
println('probe url: ${probe_url}')
90+
println('expect mode: ${mode}')
91+
println('sizes: ${sizes}')
92+
93+
mut mismatches := 0
94+
for size in sizes {
95+
// Launch worker directly without going through a shell, and capture stdout/stderr separately.
96+
mut p := os.new_process(self_exe)
97+
p.set_args(['--worker', size.str()])
98+
p.set_redirect_stdio()
99+
p.run()
100+
p.wait()
101+
exit_code := p.code
102+
stdout := p.stdout_read().trim_space()
103+
stderr := p.stderr_read().trim_space()
104+
p.close()
105+
success := exit_code == 0
106+
expected_success := expected_success_for_mode(mode, size)
107+
status := if success { 'OK' } else { 'FAIL' }
108+
expect_text := if expected_success { 'expect=OK' } else { 'expect=FAIL' }
109+
println('${status:4} size=${size:6} exit=${exit_code:3} ${expect_text}')
110+
if verbose || !success {
111+
if stdout.len > 0 {
112+
println(stdout)
113+
}
114+
if stderr.len > 0 {
115+
eprintln(stderr)
116+
}
117+
}
118+
if mode != 'none' && success != expected_success {
119+
mismatches++
120+
}
121+
}
122+
123+
if mismatches > 0 {
124+
eprintln('mismatch count: ${mismatches}')
125+
exit(1)
126+
}
127+
println('probe completed without mismatches')
128+
}
129+
130+
fn parse_sizes_csv(raw string) ![]int {
131+
mut sizes := []int{}
132+
for part in raw.split(',') {
133+
item := part.trim_space()
134+
if item.len == 0 {
135+
continue
136+
}
137+
size := item.int()
138+
if size <= 0 {
139+
return error('invalid size entry `${item}` in --sizes')
140+
}
141+
sizes << size
142+
}
143+
if sizes.len == 0 {
144+
return error('no sizes provided in --sizes')
145+
}
146+
return sizes
147+
}
148+
149+
fn expected_success_for_mode(mode string, size int) bool {
150+
return match mode {
151+
'before' {
152+
size <= vschannel_16kb_boundary
153+
}
154+
'after' {
155+
true
156+
}
157+
'none' {
158+
true
159+
}
160+
else {
161+
eprintln('unexpected mode `${mode}` in expected_success_for_mode')
162+
false
163+
}
164+
}
165+
}

‎thirdparty/vschannel/vschannel.c‎

Lines changed: 46 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -726,8 +726,12 @@ static SECURITY_STATUS https_make_request(TlsContext *tls_ctx, CHAR *req, DWORD
726726
PBYTE pbMessage;
727727
DWORD cbMessage;
728728

729-
DWORD cbData;
729+
INT cbData;
730730
INT i;
731+
DWORD req_offset;
732+
DWORD chunk_len;
733+
DWORD to_send;
734+
DWORD sent;
731735

732736

733737
// Read stream encryption properties.
@@ -748,50 +752,57 @@ static SECURITY_STATUS https_make_request(TlsContext *tls_ctx, CHAR *req, DWORD
748752
return SEC_E_INTERNAL_ERROR;
749753
}
750754

751-
// Build an HTTP request to send to the server.
752-
753-
// Build the HTTP request offset into the data buffer by "header size"
754-
// bytes. This enables Schannel to perform the encryption in place,
755-
// which is a significant performance win.
755+
// Build and send HTTP request in chunks no larger than cbMaximumMessage.
756+
// EncryptMessage expects plaintext <= cbMaximumMessage.
756757
pbMessage = pbIoBuffer + Sizes.cbHeader;
758+
req_offset = 0;
759+
while(req_offset < req_len) {
760+
chunk_len = req_len - req_offset;
761+
if(chunk_len > Sizes.cbMaximumMessage) {
762+
chunk_len = Sizes.cbMaximumMessage;
763+
}
757764

758-
// Build HTTP request. Note that I'm assuming that this is less than
759-
// the maximum message size. If it weren't, it would have to be broken up.
760-
memcpy(pbMessage, req, req_len);
761-
cbMessage = req_len;
765+
memcpy(pbMessage, req + req_offset, chunk_len);
766+
cbMessage = chunk_len;
762767

763-
// Encrypt the HTTP request.
764-
Buffers[0].pvBuffer = pbIoBuffer;
765-
Buffers[0].cbBuffer = Sizes.cbHeader;
766-
Buffers[0].BufferType = SECBUFFER_STREAM_HEADER;
768+
Buffers[0].pvBuffer = pbIoBuffer;
769+
Buffers[0].cbBuffer = Sizes.cbHeader;
770+
Buffers[0].BufferType = SECBUFFER_STREAM_HEADER;
767771

768-
Buffers[1].pvBuffer = pbMessage;
769-
Buffers[1].cbBuffer = cbMessage;
770-
Buffers[1].BufferType = SECBUFFER_DATA;
772+
Buffers[1].pvBuffer = pbMessage;
773+
Buffers[1].cbBuffer = cbMessage;
774+
Buffers[1].BufferType = SECBUFFER_DATA;
771775

772-
Buffers[2].pvBuffer = pbMessage + cbMessage;
773-
Buffers[2].cbBuffer = Sizes.cbTrailer;
774-
Buffers[2].BufferType = SECBUFFER_STREAM_TRAILER;
776+
Buffers[2].pvBuffer = pbMessage + cbMessage;
777+
Buffers[2].cbBuffer = Sizes.cbTrailer;
778+
Buffers[2].BufferType = SECBUFFER_STREAM_TRAILER;
775779

776-
Buffers[3].BufferType = SECBUFFER_EMPTY;
780+
Buffers[3].BufferType = SECBUFFER_EMPTY;
777781

778-
Message.ulVersion = SECBUFFER_VERSION;
779-
Message.cBuffers = 4;
780-
Message.pBuffers = Buffers;
782+
Message.ulVersion = SECBUFFER_VERSION;
783+
Message.cBuffers = 4;
784+
Message.pBuffers = Buffers;
781785

782-
scRet = tls_ctx->sspi->EncryptMessage(&tls_ctx->h_context, 0, &Message, 0);
786+
scRet = tls_ctx->sspi->EncryptMessage(&tls_ctx->h_context, 0, &Message, 0);
787+
if(FAILED(scRet)) {
788+
wprintf(L"Error 0x%x returned by EncryptMessage\n", scRet);
789+
return scRet;
790+
}
783791

784-
if(FAILED(scRet)) {
785-
wprintf(L"Error 0x%x returned by EncryptMessage\n", scRet);
786-
return scRet;
787-
}
792+
// Send all encrypted bytes for this chunk.
793+
to_send = Buffers[0].cbBuffer + Buffers[1].cbBuffer + Buffers[2].cbBuffer;
794+
sent = 0;
795+
while(sent < to_send) {
796+
cbData = send(tls_ctx->socket, (char*)pbIoBuffer + sent, (int)(to_send - sent), 0);
797+
if(cbData == SOCKET_ERROR || cbData == 0) {
798+
wprintf(L"Error %d sending data to server (3)\n", WSAGetLastError());
799+
tls_ctx->sspi->DeleteSecurityContext(&tls_ctx->h_context);
800+
return SEC_E_INTERNAL_ERROR;
801+
}
802+
sent += (DWORD)cbData;
803+
}
788804

789-
// Send the encrypted data to the server.
790-
cbData = send(tls_ctx->socket, pbIoBuffer, Buffers[0].cbBuffer + Buffers[1].cbBuffer + Buffers[2].cbBuffer, 0);
791-
if(cbData == SOCKET_ERROR || cbData == 0) {
792-
wprintf(L"Error %d sending data to server (3)\n", WSAGetLastError());
793-
tls_ctx->sspi->DeleteSecurityContext(&tls_ctx->h_context);
794-
return SEC_E_INTERNAL_ERROR;
805+
req_offset += chunk_len;
795806
}
796807

797808
// Read data from server until done.

0 commit comments

Comments
 (0)