Skip to content

Commit 880be81

Browse files
bluestreak01bluestreak01
committed
bugfix
1 parent c9f4aa7 commit 880be81

3 files changed

Lines changed: 360 additions & 10 deletions

File tree

‎core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,16 @@ public void startTlsSession(CharSequence peerName) throws TlsSessionInitFailedEx
331331
// there cannot be underflow since wrap() during handshake does not read from the input buffer at all
332332
throw new AssertionError("Buffer underflow during TLS handshake. This should not happen. please report as a bug");
333333
case BUFFER_OVERFLOW:
334-
if (wrapOutputBuffer.position() == 0) {
335-
// in theory, this can happen if the output buffer is too small to fit a single TLS handshake record,
336-
// but that would indicate our starting buffer is too small.
337-
growWrapOutputBuffer();
334+
if (wrapOutputBuffer.position() != 0) {
335+
// wrap() left bytes behind without producing a complete record. The OK
336+
// branch is the only place that drains and clears, so a non-empty
337+
// buffer here means we would re-enter NEED_WRAP with identical state
338+
// and spin forever. Fail loudly instead.
339+
throw new AssertionError("Buffer overflow during TLS handshake with non-empty output buffer. This should not happen, please report as a bug");
338340
}
341+
// in theory, this can happen if the output buffer is too small to fit a single TLS handshake record,
342+
// but that would indicate our starting buffer is too small.
343+
growWrapOutputBuffer();
339344
break;
340345
case OK:
341346
// wrapOutputBuffer: write mode
@@ -367,11 +372,15 @@ public void startTlsSession(CharSequence peerName) throws TlsSessionInitFailedEx
367372
// we need to receive more data from a socket, let's try again
368373
break;
369374
case BUFFER_OVERFLOW:
370-
if (unwrapOutputBuffer.position() == 0) {
371-
// in theory, this can happen if the output buffer is too small to fit a single TLS handshake record,
372-
// but that would indicate our starting buffer is too small.
373-
growUnwrapOutputBuffer();
375+
if (unwrapOutputBuffer.position() != 0) {
376+
// unwrap() produced plaintext but signalled overflow without consuming
377+
// the next record. Nothing in the handshake loop drains this buffer,
378+
// so re-entering NEED_UNWRAP would spin forever. Fail loudly.
379+
throw new AssertionError("Buffer overflow during TLS handshake with non-empty output buffer. This should not happen, please report as a bug");
374380
}
381+
// in theory, this can happen if the output buffer is too small to fit a single TLS handshake record,
382+
// but that would indicate our starting buffer is too small.
383+
growUnwrapOutputBuffer();
375384
break;
376385
case OK:
377386
// good, let's see what we need to do next

‎core/src/test/java/io/questdb/client/test/network/JavaTlsClientSocketHandshakeOverflowTest.java‎

Lines changed: 342 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,342 @@
1+
/*+*****************************************************************************
2+
* ___ _ ____ ____
3+
* / _ \ _ _ ___ ___| |_| _ \| __ )
4+
* | | | | | | |/ _ \/ __| __| | | | _ \
5+
* | |_| | |_| | __/\__ \ |_| |_| | |_) |
6+
* \__\_\\__,_|\___||___/\__|____/|____/
7+
*
8+
* Copyright (c) 2014-2019 Appsicle
9+
* Copyright (c) 2019-2026 QuestDB
10+
*
11+
* Licensed under the Apache License, Version 2.0 (the "License");
12+
* you may not use this file except in compliance with the License.
13+
* You may obtain a copy of the License at
14+
*
15+
* http://www.apache.org/licenses/LICENSE-2.0
16+
*
17+
* Unless required by applicable law or agreed to in writing, software
18+
* distributed under the License is distributed on an "AS IS" BASIS,
19+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
20+
* See the License for the specific language governing permissions and
21+
* limitations under the License.
22+
*
23+
******************************************************************************/
24+
25+
package io.questdb.client.test.network;
26+
27+
import io.questdb.client.ClientTlsConfiguration;
28+
import io.questdb.client.network.JavaTlsClientSocket;
29+
import io.questdb.client.network.NetworkFacade;
30+
import io.questdb.client.network.NetworkFacadeImpl;
31+
import org.junit.Assert;
32+
import org.junit.Test;
33+
import org.slf4j.LoggerFactory;
34+
35+
import javax.net.ssl.KeyManager;
36+
import javax.net.ssl.SSLContextSpi;
37+
import javax.net.ssl.SSLEngine;
38+
import javax.net.ssl.SSLEngineResult;
39+
import javax.net.ssl.SSLParameters;
40+
import javax.net.ssl.SSLServerSocketFactory;
41+
import javax.net.ssl.SSLSession;
42+
import javax.net.ssl.SSLSessionContext;
43+
import javax.net.ssl.SSLSocketFactory;
44+
import javax.net.ssl.TrustManager;
45+
import java.lang.reflect.Constructor;
46+
import java.nio.ByteBuffer;
47+
import java.security.Provider;
48+
import java.security.SecureRandom;
49+
import java.security.Security;
50+
import java.util.List;
51+
import java.util.concurrent.CountDownLatch;
52+
import java.util.concurrent.TimeUnit;
53+
import java.util.function.BiFunction;
54+
55+
public class JavaTlsClientSocketHandshakeOverflowTest {
56+
57+
private static final String PROVIDER_NAME = "HandshakeOverflowTestProvider";
58+
59+
/*
60+
* Demonstrates that startTlsSession() spins forever in the NEED_WRAP branch when
61+
* SSLEngine.wrap() returns BUFFER_OVERFLOW with wrapOutputBuffer.position() > 0
62+
* and handshakeStatus stays NEED_WRAP. The new code path
63+
*
64+
* case BUFFER_OVERFLOW:
65+
* if (wrapOutputBuffer.position() == 0) { growWrapOutputBuffer(); }
66+
* break;
67+
*
68+
* does not drain or grow when position > 0, so the outer while-loop re-enters
69+
* NEED_WRAP with identical state and never makes progress. The original code
70+
* threw an AssertionError here, which at least failed loudly.
71+
*/
72+
@Test
73+
public void testHandshakeWrapOverflowWithNonEmptyBufferShouldNotLoopForever() throws Exception {
74+
Provider provider = new HandshakeOverflowProvider();
75+
Security.insertProviderAt(provider, 1);
76+
Thread t = null;
77+
try {
78+
try (JavaTlsClientSocket socket = newSocket()) {
79+
socket.of(0); // -> STATE_PLAINTEXT
80+
81+
CountDownLatch done = new CountDownLatch(1);
82+
t = new Thread(() -> {
83+
try {
84+
socket.startTlsSession("test.host");
85+
} catch (Throwable ignored) {
86+
// Expected: a healthy handshake loop should fail loudly here,
87+
// not spin forever. Any exception (AssertionError, SSLException,
88+
// TlsSessionInitFailedException) counts as "did not hang".
89+
} finally {
90+
done.countDown();
91+
}
92+
});
93+
t.setDaemon(true);
94+
t.start();
95+
96+
boolean completed = done.await(2, TimeUnit.SECONDS);
97+
Assert.assertTrue(
98+
"startTlsSession looped without making progress — handshake BUFFER_OVERFLOW " +
99+
"with wrapOutputBuffer.position() > 0 has no break-out path",
100+
completed
101+
);
102+
}
103+
} finally {
104+
Security.removeProvider(PROVIDER_NAME);
105+
if (t != null && t.isAlive()) {
106+
t.interrupt();
107+
}
108+
}
109+
}
110+
111+
private static JavaTlsClientSocket newSocket() throws Exception {
112+
Constructor<JavaTlsClientSocket> ctor = JavaTlsClientSocket.class.getDeclaredConstructor(
113+
NetworkFacade.class,
114+
org.slf4j.Logger.class,
115+
ClientTlsConfiguration.class
116+
);
117+
ctor.setAccessible(true);
118+
return ctor.newInstance(
119+
new NoOpFacade(),
120+
LoggerFactory.getLogger(JavaTlsClientSocketHandshakeOverflowTest.class),
121+
ClientTlsConfiguration.INSECURE_NO_VALIDATION
122+
);
123+
}
124+
125+
public static final class HandshakeOverflowProvider extends Provider {
126+
public HandshakeOverflowProvider() {
127+
super(PROVIDER_NAME, "1.0", "test-only");
128+
put("SSLContext.TLS", HandshakeOverflowSslContextSpi.class.getName());
129+
}
130+
}
131+
132+
public static final class HandshakeOverflowSslContextSpi extends SSLContextSpi {
133+
public HandshakeOverflowSslContextSpi() {
134+
}
135+
136+
@Override
137+
protected SSLEngine engineCreateSSLEngine() {
138+
return new HandshakeOverflowEngine();
139+
}
140+
141+
@Override
142+
protected SSLEngine engineCreateSSLEngine(String host, int port) {
143+
return new HandshakeOverflowEngine();
144+
}
145+
146+
@Override
147+
protected SSLSessionContext engineGetClientSessionContext() {
148+
return null;
149+
}
150+
151+
@Override
152+
protected SSLServerSocketFactory engineGetServerSocketFactory() {
153+
throw new UnsupportedOperationException();
154+
}
155+
156+
@Override
157+
protected SSLSessionContext engineGetServerSessionContext() {
158+
return null;
159+
}
160+
161+
@Override
162+
protected SSLSocketFactory engineGetSocketFactory() {
163+
throw new UnsupportedOperationException();
164+
}
165+
166+
@Override
167+
protected void engineInit(KeyManager[] km, TrustManager[] tm, SecureRandom sr) {
168+
}
169+
}
170+
171+
private static final class HandshakeOverflowEngine extends SSLEngine {
172+
@Override
173+
public void beginHandshake() {
174+
}
175+
176+
@Override
177+
public void closeInbound() {
178+
}
179+
180+
@Override
181+
public void closeOutbound() {
182+
}
183+
184+
@Override
185+
public Runnable getDelegatedTask() {
186+
return null;
187+
}
188+
189+
@Override
190+
public boolean getEnableSessionCreation() {
191+
return false;
192+
}
193+
194+
@Override
195+
public String[] getEnabledCipherSuites() {
196+
return new String[0];
197+
}
198+
199+
@Override
200+
public String[] getEnabledProtocols() {
201+
return new String[0];
202+
}
203+
204+
@Override
205+
public SSLEngineResult.HandshakeStatus getHandshakeStatus() {
206+
return SSLEngineResult.HandshakeStatus.NEED_WRAP;
207+
}
208+
209+
@Override
210+
public boolean getNeedClientAuth() {
211+
return false;
212+
}
213+
214+
@Override
215+
public SSLParameters getSSLParameters() {
216+
return new SSLParameters();
217+
}
218+
219+
@Override
220+
public SSLSession getSession() {
221+
return null;
222+
}
223+
224+
@Override
225+
public String[] getSupportedCipherSuites() {
226+
return new String[0];
227+
}
228+
229+
@Override
230+
public String[] getSupportedProtocols() {
231+
return new String[0];
232+
}
233+
234+
@Override
235+
public boolean getUseClientMode() {
236+
return true;
237+
}
238+
239+
@Override
240+
public boolean getWantClientAuth() {
241+
return false;
242+
}
243+
244+
@Override
245+
public boolean isInboundDone() {
246+
return false;
247+
}
248+
249+
@Override
250+
public boolean isOutboundDone() {
251+
return false;
252+
}
253+
254+
@Override
255+
public void setEnableSessionCreation(boolean flag) {
256+
}
257+
258+
@Override
259+
public void setEnabledCipherSuites(String[] suites) {
260+
}
261+
262+
@Override
263+
public void setEnabledProtocols(String[] protocols) {
264+
}
265+
266+
@Override
267+
public void setNeedClientAuth(boolean need) {
268+
}
269+
270+
@Override
271+
public void setSSLParameters(SSLParameters params) {
272+
}
273+
274+
@Override
275+
public void setUseClientMode(boolean mode) {
276+
}
277+
278+
@Override
279+
public void setWantClientAuth(boolean want) {
280+
}
281+
282+
@Override
283+
public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer[] dsts, int offset, int length) {
284+
// Not used — handshake stays in NEED_WRAP.
285+
return new SSLEngineResult(
286+
SSLEngineResult.Status.OK,
287+
SSLEngineResult.HandshakeStatus.NEED_WRAP,
288+
0,
289+
0
290+
);
291+
}
292+
293+
@Override
294+
public SSLEngineResult wrap(ByteBuffer[] srcs, int offset, int length, ByteBuffer dst) {
295+
// Write one byte to dst to make wrapOutputBuffer.position() > 0 on the
296+
// very first call. From then on we always return BUFFER_OVERFLOW with
297+
// NEED_WRAP, modelling a (contrived but spec-permitted) engine where
298+
// wrap() advances dst.position but signals overflow.
299+
if (dst.remaining() > 0) {
300+
dst.put((byte) 0x42);
301+
}
302+
return new SSLEngineResult(
303+
SSLEngineResult.Status.BUFFER_OVERFLOW,
304+
SSLEngineResult.HandshakeStatus.NEED_WRAP,
305+
0,
306+
1
307+
);
308+
}
309+
310+
// Java 9+ overrides — stubbed so the test compiles on JDK 17.
311+
@Override
312+
public String getApplicationProtocol() {
313+
return null;
314+
}
315+
316+
@Override
317+
public String getHandshakeApplicationProtocol() {
318+
return null;
319+
}
320+
321+
@Override
322+
public BiFunction<SSLEngine, List<String>, String> getHandshakeApplicationProtocolSelector() {
323+
return null;
324+
}
325+
326+
@Override
327+
public void setHandshakeApplicationProtocolSelector(BiFunction<SSLEngine, List<String>, String> selector) {
328+
}
329+
}
330+
331+
private static final class NoOpFacade extends NetworkFacadeImpl {
332+
@Override
333+
public int recvRaw(int fd, long buffer, int bufferLen) {
334+
return 0;
335+
}
336+
337+
@Override
338+
public int sendRaw(int fd, long buffer, int bufferLen) {
339+
return bufferLen; // Pretend the bytes were sent so the OK send-loop terminates if reached.
340+
}
341+
}
342+
}

0 commit comments

Comments
 (0)