smengcl commented on code in PR #11567:
URL: https://github.com/apache/ozone/pull/11567#discussion_r4311446025
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/HttpServer2.java:
##########
@@ -638,6 +639,22 @@ private ServerConnector createHttpsChannelConnector(
SslContextFactory.Server sslContextFactory =
new SslContextFactory.Server();
sslContextFactory.setNeedClientAuth(needsClientAuth);
+ // With a combined keystore (for example separate TLS-server-auth and
+ // client-auth certificates issued under single-purpose EKU), Jetty's
+ // default SNI key manager picks the server certificate by matching the
+ // SNI host against each certificate's SAN/CN, ignoring the extended key
+ // usage. That selection fails to pick the server-auth certificate for
+ // requests that carry no matching SNI. When enabled, delegate alias
Review Comment:
This does not match Jetty 12 behavior. `SslContextFactory.Server#sniSelect`
already returns `DELEGATE` when there is no SNI, or when the SNI host matches
no certificate. The only case the custom selector changes is an SNI host that
matches a certificate in the keystore (e.g. the clientAuth cert with the same
SAN), where Jetty returns that alias directly. The no SNI failure comes from
the `SunX509` key manager, not from SNI matching. Please reword the comment to
describe that.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/HttpServer2.java:
##########
@@ -638,6 +639,22 @@ private ServerConnector createHttpsChannelConnector(
SslContextFactory.Server sslContextFactory =
new SslContextFactory.Server();
sslContextFactory.setNeedClientAuth(needsClientAuth);
+ // With a combined keystore (for example separate TLS-server-auth and
+ // client-auth certificates issued under single-purpose EKU), Jetty's
+ // default SNI key manager picks the server certificate by matching the
+ // SNI host against each certificate's SAN/CN, ignoring the extended key
+ // usage. That selection fails to pick the server-auth certificate for
+ // requests that carry no matching SNI. When enabled, delegate alias
+ // selection to the JDK key manager using the EKU-aware PKIX algorithm,
+ // so the server-auth certificate is chosen regardless of SNI.
+ boolean sniKeyManagerDelegate = conf.getBoolean(
+ OzoneConfigKeys.OZONE_HTTP_SNI_KEYMANAGER_DELEGATE_ENABLED,
+ OzoneConfigKeys.OZONE_HTTP_SNI_KEYMANAGER_DELEGATE_ENABLED_DEFAULT);
+ if (sniKeyManagerDelegate) {
+ sslContextFactory.setSNISelector(
Review Comment:
This sets the `DELEGATE` selector but keeps Jetty's default key manager
algorithm, `KeyManagerFactory.getDefaultAlgorithm()`, which is `SunX509` on a
standard JDK. `SunX509` ignores EKU, so with a combined keystore it can still
pick the clientAuth only entry. Jetty already delegates when the client sends
no SNI, so without `PKIX` the flag does not help that case at all.
I reproduced this on Jetty 12.1.14 and JDK 21 with a CA signed keystore
holding `a-client` (clientAuth) and `z-server` (serverAuth). The handshake
failed with "Extended key usage does not permit use for TLS server
authentication", with and without SNI. Setting `PKIX` here made both pass.
```suggestion
sslContextFactory.setKeyManagerFactoryAlgorithm("PKIX");
sslContextFactory.setSNISelector(
```
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/http/TestHttpServer2SSL.java:
##########
@@ -252,6 +253,42 @@ public void
testEnabledProtocolAppliedWhenConfigSetToNonDefault() throws Excepti
assertServerAppliesEnabledProtocol(serverConf, "TLSv1.3");
}
+ /**
+ * With {@link OzoneConfigKeys#OZONE_HTTP_SNI_KEYMANAGER_DELEGATE_ENABLED}
+ * enabled, the HTTPS connector switches to the EKU-aware PKIX key manager
and
+ * installs an SNI selector that always delegates alias selection to that key
+ * manager instead of Jetty's SNI host matching. The server still serves
+ * requests over TLS.
+ */
+ @Test
+ public void testSniKeyManagerDelegateAppliesDelegateSelector() throws
Exception {
+ OzoneConfiguration serverConf = new OzoneConfiguration(conf);
+
serverConf.setBoolean(OzoneConfigKeys.OZONE_HTTP_SNI_KEYMANAGER_DELEGATE_ENABLED,
true);
+ HttpServer2 server = buildServer(serverConf, null, null, null);
+ server.start();
+ try {
+ ServerConnector listener = server.getListeners().get(0);
+ SslConnectionFactory connectionFactory =
+ listener.getConnectionFactory(SslConnectionFactory.class);
+ assertNotNull(connectionFactory,
+ "Expected HTTPS listener with an SSL connection factory");
+ SslContextFactory.Server sslContextFactory =
+ (SslContextFactory.Server) connectionFactory.getSslContextFactory();
+
+ SniX509ExtendedKeyManager.SniSelector selector =
sslContextFactory.getSNISelector();
+ assertNotNull(selector, "Expected a custom SNI selector");
+ assertEquals(SniX509ExtendedKeyManager.SniSelector.DELEGATE,
Review Comment:
The javadoc says the connector switches to `PKIX`, but nothing checks that,
and calling the lambda with nulls only shows it returns `DELEGATE`. Please:
- assert `sslContextFactory.getKeyManagerFactoryAlgorithm()` is `PKIX`
- add the negative case: with the flag off, the selector stays null and the
algorithm stays the JDK default
- add a handshake against a combined keystore (separate clientAuth and
serverAuth entries). The current single certificate keystore passes with or
without this change, so it cannot catch the bug above.
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -3880,6 +3880,17 @@
any load balancer, virtual IP or DNS alias in front of the daemon.
</description>
</property>
+ <property>
+ <name>ozone.http.sni.keymanager.delegate.enabled</name>
+ <value>false</value>
+ <tag>OZONE, SECURITY, MANAGEMENT</tag>
+ <description>
+ Whether the HTTPS endpoints of Ozone daemons delegate SNI certificate
selection to the
+ JDK key manager. When the server keystore combines multiple
certificates, enabling this
+ lets the key manager pick the certificate that matches the request's TLS
SNI name instead
+ of always serving the default certificate.
Review Comment:
This describes the opposite of what the flag does. With delegation Jetty
stops matching the SNI name, and the JDK key manager picks the alias by key
type, issuers and (with `PKIX`) EKU. Maybe something like:
```suggestion
Whether the HTTPS endpoints of Ozone daemons skip Jetty's SNI based
certificate selection
and let the JDK PKIX key manager choose the server certificate,
preferring one whose
extended key usage allows TLS server authentication. Enable this when
the server keystore
combines certificates with different extended key usages, for example
separate serverAuth
and clientAuth certificates.
```
--
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]
