joseluisll commented on code in PR #8717:
URL: https://github.com/apache/hadoop/pull/8717#discussion_r4093299735


##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestSSLHttpServerMTLS.java:
##########
@@ -145,6 +147,21 @@ public void testUntrustedClientIsRejected() throws 
Exception {
     HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
     // presents untrustedCert; server cert is trusted via no-op TrustManager
     KeyStoreTestUtil.setAllowAllSSL(conn, untrustedCert, untrustedKeyPair);
-    assertThrows(SSLHandshakeException.class, () -> conn.getInputStream());
+    // The server rejects the certificate as soon as it arrives and drops the
+    // connection, which races the client's own last handshake flight, and how
+    // the refusal surfaces depends on who wins and on the protocol in play.
+    // Under TLSv1.2 it is an SSLHandshakeException; when the close wins the
+    // client fails writing that flight and gets a SocketException instead;
+    // under TLSv1.3 the handshake completes client-side before the server has
+    // verified the cert, so the failure lands on the request write as a bare
+    // IOException with no cause.  What the server guarantees is that the
+    // request is refused, not which of those the client gets to see.  Assert
+    // the refusal and exclude only ConnectException, which would mean we
+    // never reached the server at all.
+    IOException e =
+        assertThrows(IOException.class, () -> conn.getInputStream());

Review Comment:
   Adopted, and it is a real hole rather than a theoretical one: with 
`getInputStream()` the assertion is satisfied by any `IOException`, and an HTTP 
error status raises one, so a 403 or a 500 reached over a handshake the server 
should have refused would have passed. `getResponseCode()` returns such a 
status instead of throwing, and only throws when no status line was ever read.
   
   Now `assertThrows(IOException.class, () -> conn.getResponseCode())`, comment 
updated to say why. Ran it 30x under `hadoop.ssl.enabled.protocols=TLSv1.2` and 
30x under `TLSv1.3` on JDK 21: 30/30 both times. (The plumbing was worth 
checking - a bogus protocol value fails `testTrustedClientCanConnect`, 
confirming the setting reaches the server.)



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to