oscerd commented on code in PR #27820:
URL: https://github.com/apache/camel/pull/27820#discussion_r4269743837


##########
components/camel-netty/src/main/java/org/apache/camel/component/netty/DefaultServerInitializerFactory.java:
##########
@@ -130,6 +130,26 @@ private SSLContext createSSLContext(CamelContext 
camelContext, NettyServerBootst
         return answer;
     }
 
+    /**
+     * Creates the engine, carrying the host and port when this consumer 
connects as a client with host name
+     * verification enabled.
+     * <p/>
+     * The JDK reads the name to match from {@code SSLSession.getPeerHost()}, 
which the no-arg factory method leaves
+     * {@code null} - and then {@code HostnameChecker.match} throws
+     * {@code CertificateException: Hostname or IP address is undefined} 
rather than checking anything. See
+     * {@code DefaultClientInitializerFactory} for the producer side.
+     * <p/>
+     * In server mode the peer is deliberately not attached: there the 
algorithm would ask the JDK to match the client's
+     * certificate against a host name the server does not have, which is not 
what the option means.
+     */
+    private SSLEngine createServerSSLEngine() {
+        NettyConfiguration configuration = consumer.getConfiguration();
+        if (configuration.isClientMode() && 
configuration.isHostnameVerification()) {

Review Comment:
   Done — guarded with `isClientMode()`, so server mode never sets the 
algorithm.
   
   I checked your reasoning against the JDK rather than take it on trust, and 
it holds with one correction to the message. The SNI part is exactly right — 
`X509TrustManagerImpl.checkIdentity`, offsets 53/54:
   
   ```
   53: iload_3                     // checkClientTrusted
   54: ifne 107                    // jump PAST the SNI block
   57: invokestatic getRequestedServerNames
   65: invokestatic getHostNameInSNI
   ...
   107: (the getPeerHost() path)
   ```
   
   So a client certificate never consults SNI and goes straight to 
`getPeerHost()`, which is `null` on a server engine.
   
   What a user actually sees is not `Hostname or IP address is undefined`, 
though. That is the inner cause, and the JDK rewrites it. Offsets 126–152 of 
the same method catch the `CertificateException` and, when it is checking a 
**client** certificate with algorithm `HTTPS`, replace it:
   
   ```
   126: astore 7
   128: iload_3                    // checkClientTrusted
   129: ifeq 153                   // not a client cert -> rethrow the original
   132: ldc "HTTPS"
   136: equalsIgnoreCase
   139: ifeq 153
   142: new CertificateException
   146: ldc "Endpoint Identification Algorithm HTTPS is not supported on the 
server side"
   152: athrow
   ```
   
   Confirmed with a server-engine probe on 21.0.10:
   
   | server engine | result |
   |---|---|
   | `EIA=HTTPS` + `needClientAuth` (today) | `CertificateException: Endpoint 
Identification Algorithm HTTPS is not supported on the server side` |
   | `EIA=HTTPS`, no client auth | handshake completes, nothing checked |
   | no `EIA` + `needClientAuth` (after this change) | handshake completes |
   
   Which makes your conclusion stronger than stated: the JDK does not merely 
lack the name, it explicitly refuses the algorithm on that side. So removing 
the line cannot relax a check — there was never a check to relax. I quoted the 
real message in the javadoc, the component page and the upgrade guide.
   
   I also took the test you called optional, because it was not: 
`createServerSSLEngine()`'s `isClientMode()` branch had no coverage at all, and 
I had claimed the consumer was fixed. 
`NettySSLConsumerClientModeHostnameVerificationTest` points a `clientMode=true` 
consumer at a Netty TLS server presenting `keystore-localhost.jks` and asserts 
the message is routed. Revert-checked: dropping the peer fails it on all 3 
runs, and `target/camel-netty-test.log` shows why —
   
   ```
   Caused by: java.security.cert.CertificateException: Hostname or IP address 
is undefined.
   WARN  ntModeHostnameVerificationTest - Unhandled exception caught: 
javax.net.ssl.SSLHandshakeException: (certificate_unknown) Received fatal 
alert: certificate_unknown
   ```
   
   Worth noting for anyone reading the console output of that test: nothing 
appears there, because `src/test/resources/log4j2.properties` points the root 
logger at `target/camel-netty-test.log` only.
   
   The two `camel-netty-http` server initializers are left for a separate issue 
as you suggested, so this stays a clean backport.
   
   _Claude Code on behalf of oscerd_
   



##########
components/camel-netty/src/main/docs/netty-component.adoc:
##########
@@ -674,6 +674,21 @@ YAML::
 ----
 ====
 
+=== Verifying the host name of the server
+
+A producer, and a consumer with `clientMode=true`, connects as a TLS client 
when `ssl=true`. By default it accepts any
+certificate the JVM trusts, whatever host that certificate was issued for. Set 
`hostnameVerification=true` to also
+check that the certificate matches the host the endpoint connects to, with the 
same rules as an HTTPS client: a
+connection to an IP address needs a certificate that lists that address as a 
subject alternative name of its own.
+
+A certificate that does not match makes the producer or consumer fail with an
+`SSLHandshakeException: No subject alternative names matching IP address ... 
found` (or
+`No subject alternative DNS name matching ... found` for a host name). 
Enabling the option also makes the JDK send the
+host as an SNI server name.
+
+The option applies to the client side. On a consumer that acts as a server it 
does not verify the host of a client

Review Comment:
   You were right that it did not describe what happens today — thanks for 
catching it, since I wrote that paragraph before deciding to leave server mode 
alone, and it quietly became a promise the code did not keep.
   
   I took your preferred route (the `isClientMode()` guard, see the other 
thread), so the paragraph is now true. I went a bit further than leaving it as 
it was, because "it does not verify the host of a client that connects" states 
the outcome without the reason, and the reason is the useful part — the JDK 
refuses the algorithm on that side outright. It now reads:
   
   > The option applies to the client side only. On a consumer that acts as a 
server it is ignored, because the JDK does not support host name verification 
on that side of a handshake: asked to do it anyway, it rejects the client 
certificate with `Endpoint Identification Algorithm HTTPS is not supported on 
the server side`.
   
   That is the message I got from a server engine with `needClientAuth` on 
21.0.10, not the `Hostname or IP address is undefined` we had both expected — 
the JDK catches that one and rewrites it for the client-certificate case. 
Details and the bytecode are on the other thread.
   
   The 4.24 upgrade guide entry gained a matching paragraph: a consumer that 
asks for client certificates and had the option set now completes the handshake 
instead of failing it. The catalog copy is re-synced and `diff` against the 
source is empty.
   
   _Claude Code on behalf of oscerd_
   



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

Reply via email to