Skip to content

Commit 5e7ae28

Browse files
committed
8373677: Clear text HttpServer connection could fail fast if receiving SSL ClientHello
Reviewed-by: jpai, djelinski
1 parent e4636d6 commit 5e7ae28

File tree

3 files changed

+206
-2
lines changed

3 files changed

+206
-2
lines changed

src/jdk.httpserver/share/classes/sun/net/httpserver/Request.java

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
package sun.net.httpserver;
2727

28+
import java.net.ProtocolException;
2829
import java.nio.*;
2930
import java.io.*;
3031
import java.nio.channels.*;
@@ -39,15 +40,18 @@ class Request {
3940
static final int BUF_LEN = 2048;
4041
static final byte CR = 13;
4142
static final byte LF = 10;
43+
static final byte FIRST_CHAR = 32;
4244

4345
private String startLine;
4446
private SocketChannel chan;
4547
private InputStream is;
4648
private OutputStream os;
4749
private final int maxReqHeaderSize;
50+
private final boolean firstClearRequest;
4851

49-
Request(InputStream rawInputStream, OutputStream rawout) throws IOException {
52+
Request(InputStream rawInputStream, OutputStream rawout, boolean firstClearRequest) throws IOException {
5053
this.maxReqHeaderSize = ServerConfig.getMaxReqHeaderSize();
54+
this.firstClearRequest = firstClearRequest;
5155
is = rawInputStream;
5256
os = rawout;
5357
do {
@@ -78,6 +82,25 @@ public String readLine() throws IOException {
7882
boolean gotCR = false, gotLF = false;
7983
pos = 0; lineBuf = new StringBuffer();
8084
long lsize = 32;
85+
86+
// For the first request that comes on a clear connection
87+
// we will check that the first non CR/LF char on the
88+
// request line is eligible. This should be the first char
89+
// of a method name, so it should be at least greater or equal
90+
// to 32 (FIRST_CHAR) which is the space character.
91+
// The main goal here is to fail fast if we receive 0x16 (22) which
92+
// happens to be the first byte of a TLS handshake record.
93+
// This is typically what would be received if a TLS client opened
94+
// a TLS connection on a non-TLS server.
95+
// If we receive 0x16 we should close the connection immediately as
96+
// it indicates we're receiving a ClientHello on a clear
97+
// connection, and we will never receive the expected CRLF that
98+
// terminates the first request line.
99+
// Though we could check only for 0x16, any characters < 32
100+
// (excluding CRLF) is not expected at this position in a
101+
// request line, so we can still fail here early if any of
102+
// those are detected.
103+
int offset = 0;
81104
while (!gotLF) {
82105
int c = is.read();
83106
if (c == -1) {
@@ -89,13 +112,25 @@ public String readLine() throws IOException {
89112
} else {
90113
gotCR = false;
91114
consume(CR);
115+
if (firstClearRequest && offset == 0) {
116+
if (c < FIRST_CHAR) {
117+
throw new ProtocolException("Unexpected start of request line");
118+
}
119+
offset++;
120+
}
92121
consume(c);
93122
lsize = lsize + 2;
94123
}
95124
} else {
96125
if (c == CR) {
97126
gotCR = true;
98127
} else {
128+
if (firstClearRequest && offset == 0) {
129+
if (c < FIRST_CHAR) {
130+
throw new ProtocolException("Unexpected start of request line");
131+
}
132+
offset++;
133+
}
99134
consume(c);
100135
lsize = lsize + 1;
101136
}

src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import java.lang.System.Logger.Level;
4646
import java.net.BindException;
4747
import java.net.InetSocketAddress;
48+
import java.net.ProtocolException;
4849
import java.net.ServerSocket;
4950
import java.net.URI;
5051
import java.net.URISyntaxException;
@@ -733,7 +734,16 @@ public void run() {
733734
connection.raw = rawin;
734735
connection.rawout = rawout;
735736
}
736-
Request req = new Request(rawin, rawout);
737+
738+
Request req;
739+
try {
740+
req = new Request(rawin, rawout, newconnection && !https);
741+
} catch (ProtocolException pe) {
742+
logger.log(Level.DEBUG, "closing due to: " + pe);
743+
reject(Code.HTTP_BAD_REQUEST, "", pe.getMessage());
744+
return;
745+
}
746+
737747
requestLine = req.requestLine();
738748
if (requestLine == null) {
739749
/* connection closed */
Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8373677
27+
* @summary Tests for verifying that a non-SSL server can detect
28+
* when a client attempts to use SSL.
29+
* @library /test/lib
30+
* @run junit/othervm ${test.main.class}
31+
*/
32+
33+
import com.sun.net.httpserver.HttpExchange;
34+
import com.sun.net.httpserver.HttpHandler;
35+
import com.sun.net.httpserver.HttpServer;
36+
import jdk.test.lib.net.SimpleSSLContext;
37+
import jdk.test.lib.net.URIBuilder;
38+
import org.junit.jupiter.api.Assertions;
39+
import org.junit.jupiter.api.BeforeAll;
40+
import org.junit.jupiter.api.Test;
41+
42+
import java.io.IOException;
43+
import java.io.InputStreamReader;
44+
import java.net.InetAddress;
45+
import java.net.InetSocketAddress;
46+
import java.net.Socket;
47+
import java.net.URI;
48+
import java.net.URISyntaxException;
49+
import java.net.http.HttpClient;
50+
import java.net.http.HttpRequest;
51+
import java.net.http.HttpResponse;
52+
import java.util.logging.ConsoleHandler;
53+
import java.util.logging.Level;
54+
import java.util.logging.Logger;
55+
56+
import javax.net.ssl.SSLException;
57+
58+
import static com.sun.net.httpserver.HttpExchange.RSPBODY_EMPTY;
59+
import static java.net.http.HttpClient.Builder.NO_PROXY;
60+
import static org.junit.jupiter.api.Assertions.*;
61+
62+
public class ClearTextServerSSL {
63+
64+
static final InetAddress LOOPBACK_ADDR = InetAddress.getLoopbackAddress();
65+
static final boolean ENABLE_LOGGING = true;
66+
static final Logger logger = Logger.getLogger("com.sun.net.httpserver");
67+
68+
static final String CTXT_PATH = "/ClearTextServerSSL";
69+
70+
@BeforeAll
71+
public static void setup() {
72+
if (ENABLE_LOGGING) {
73+
ConsoleHandler ch = new ConsoleHandler();
74+
logger.setLevel(Level.ALL);
75+
ch.setLevel(Level.ALL);
76+
logger.addHandler(ch);
77+
}
78+
}
79+
80+
@Test
81+
public void test() throws Exception {
82+
var sslContext = new SimpleSSLContext().get();
83+
var handler = new TestHandler();
84+
var server = HttpServer.create(new InetSocketAddress(LOOPBACK_ADDR, 0), 0);
85+
server.createContext(path(""), handler);
86+
server.start();
87+
try (var client = HttpClient.newBuilder()
88+
.sslContext(sslContext)
89+
.proxy(NO_PROXY)
90+
.build()) {
91+
var request = HttpRequest.newBuilder()
92+
.uri(uri("http", server, path("/clear")))
93+
.build();
94+
var response = client.send(request, HttpResponse.BodyHandlers.ofString());
95+
assertEquals(200, response.statusCode());
96+
var sslRequest = HttpRequest.newBuilder()
97+
.uri(uri("https", server, path("/ssl")))
98+
.build();
99+
Assertions.assertThrows(SSLException.class, () -> {
100+
client.send(sslRequest, HttpResponse.BodyHandlers.ofString());
101+
});
102+
try (var socket = new Socket()) {
103+
socket.connect(server.getAddress());
104+
byte[] badRequest = {
105+
22, 'B', 'A', 'D', ' ',
106+
'/', ' ' ,
107+
'H', 'T', 'T', 'P', '/', '1', '.', '1' };
108+
socket.getOutputStream().write(badRequest);
109+
socket.getOutputStream().flush();
110+
var reader = new InputStreamReader(socket.getInputStream());
111+
var line = reader.readAllLines();
112+
Assertions.assertEquals("HTTP/1.1 400 Bad Request", line.get(0));
113+
System.out.println("Got expected response:");
114+
line.stream().map(l -> "\t" + l).forEach(System.out::println);
115+
}
116+
117+
} finally {
118+
server.stop(0);
119+
}
120+
}
121+
122+
// --- infra ---
123+
124+
static String path(String path) {
125+
assert CTXT_PATH.startsWith("/");
126+
assert !CTXT_PATH.endsWith("/");
127+
if (path.startsWith("/")) {
128+
return CTXT_PATH + path;
129+
} else {
130+
return CTXT_PATH + "/" + path;
131+
}
132+
}
133+
134+
static URI uri(String scheme, HttpServer server, String path) throws URISyntaxException {
135+
return URIBuilder.newBuilder()
136+
.scheme(scheme)
137+
.loopback()
138+
.port(server.getAddress().getPort())
139+
.path(path)
140+
.build();
141+
}
142+
143+
/**
144+
* A test handler that reads any request bytes and sends
145+
* an empty 200 response
146+
*/
147+
static class TestHandler implements HttpHandler {
148+
@java.lang.Override
149+
public void handle(HttpExchange exchange) throws IOException {
150+
try (var reqBody = exchange.getRequestBody()) {
151+
reqBody.readAllBytes();
152+
exchange.sendResponseHeaders(200, RSPBODY_EMPTY);
153+
} catch (Throwable t) {
154+
t.printStackTrace();
155+
exchange.sendResponseHeaders(500, RSPBODY_EMPTY);
156+
}
157+
}
158+
}
159+
}

0 commit comments

Comments
 (0)