JiriOndrusek commented on code in PR #559:
URL:
https://github.com/apache/camel-quarkus-examples/pull/559#discussion_r3735777853
##########
http-pqc-j21/src/test/java/org/acme/http/pqc/AbstractPqcTest.java:
##########
@@ -84,54 +76,31 @@ void testRestAssuredConnection() throws Exception {
}
void testHttpClientConnection(String securityProvider, boolean
expectFailure) throws Exception {
- boolean failedAsExpected = false;
-
try {
SSLContext sslContext = createSslContext(securityProvider);
-
- // Create custom SSLConnectionSocketFactory that explicitly sets
named groups
- SSLConnectionSocketFactory sslSocketFactory = new
SSLConnectionSocketFactory(sslContext) {
- @Override
- protected void prepareSocket(javax.net.ssl.SSLSocket socket)
throws java.io.IOException {
- super.prepareSocket(socket);
- // Explicitly set named groups on the socket's SSL
parameters
- String configuredGroups =
System.getProperty("jdk.tls.namedGroups", "X25519MLKEM768");
- try {
- SSLParameters sslParams = socket.getSSLParameters();
- String[] namedGroupsArray =
configuredGroups.split(",");
- for (int i = 0; i < namedGroupsArray.length; i++) {
- namedGroupsArray[i] = namedGroupsArray[i].trim();
- }
- sslParams.setNamedGroups(namedGroupsArray);
- sslParams.setProtocols(new String[] { "TLSv1.3" });
- socket.setSSLParameters(sslParams);
- LOG.info("Set named groups on socket: " +
Arrays.toString(namedGroupsArray));
- } catch (Exception e) {
- LOG.warn("Could not set named groups on socket: " +
e.getMessage());
- }
- }
- };
-
- HttpClientConnectionManager connectionManager =
PoolingHttpClientConnectionManagerBuilder.create()
- .setSSLSocketFactory(sslSocketFactory)
- .build();
-
- try (CloseableHttpClient httpClient = HttpClients.custom()
- .setConnectionManager(connectionManager)
- .build()) {
-
- HttpGet request = new HttpGet("https://localhost:"; +
RestAssured.port + "/pqc/secure");
- int responseStatus = httpClient.execute(request,
HttpResponse::getCode);
-
- if (expectFailure) {
- fail(securityProvider + " should have failed but got
response status : " + responseStatus);
- } else {
- assertTrue(responseStatus == 200, "Expected response
status is 200");
- }
+ SSLSocketFactory sslSocketFactory = new
SSLSocketFactory(sslContext);
Review Comment:
`new SSLSocketFactory(sslContext)` has two issues:
1. Defaults to `BROWSER_COMPATIBLE_HOSTNAME_VERIFIER` (lenient) instead of
strict
2. Does not constrain the socket to TLS 1.3 — client could negotiate TLS
1.2, bypassing PQC
```suggestion
SSLSocketFactory sslSocketFactory = new SSLSocketFactory(
sslContext,
new String[] { "TLSv1.3" },
null,
SSLSocketFactory.STRICT_HOSTNAME_VERIFIER);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
