petersomogyi commented on code in PR #8681:
URL: https://github.com/apache/hbase/pull/8681#discussion_r4094252450


##########
hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestRESTServerSSL.java:
##########
@@ -160,6 +161,98 @@ public void testSslConnectionUsingKeystoreFormatPKCS12() 
throws Exception {
     assertEquals(200, response.getCode());
   }
 
+  // 
---------------------------------------------------------------------------
+  // Role-scoped configuration + mTLS (single-EKU certificate support).
+  // 
---------------------------------------------------------------------------
+
+  /**
+   * Server started with only the role-scoped hbase.rest.ssl.server.* keys 
(and legacy keys unset).
+   * A successful SSL connection proves the server-scoped keys were consulted.
+   */
+  @Test
+  public void testSslConnectionUsingRoleScopedServerKeys() throws Exception {
+    // Move the legacy passwords set in beforeEachTest onto the role-scoped 
keys and clear the
+    // legacy passwords so a successful start proves the server-scoped keys 
are what the code
+    // actually picked up.
+    conf.unset(Constants.REST_SSL_KEYSTORE_PASSWORD);
+    conf.unset(Constants.REST_SSL_KEYSTORE_KEYPASSWORD);
+    conf.unset(Constants.REST_SSL_TRUSTSTORE_PASSWORD);
+    conf.set(Constants.REST_SSL_SERVER_KEYSTORE_PASSWORD, KEY_STORE_PASSWORD);
+    conf.set(Constants.REST_SSL_SERVER_KEYSTORE_KEYPASSWORD, 
KEY_STORE_PASSWORD);
+    conf.set(Constants.REST_SSL_SERVER_TRUSTSTORE_PASSWORD, 
TRUST_STORE_PASSWORD);
+    conf.set(Constants.REST_SSL_SERVER_KEYSTORE_STORE, 
getKeystoreFilePath("jks"));
+    conf.set(Constants.REST_SSL_SERVER_TRUSTSTORE_STORE, 
getTruststoreFilePath("jks"));
+
+    REST_TEST_UTIL.startServletContainer(conf);
+    Cluster localCluster = new Cluster().add("localhost", 
REST_TEST_UTIL.getServletPort());
+    sslClient = new Client(localCluster, getTruststoreFilePath("jks"),
+      Optional.of(TRUST_STORE_PASSWORD), Optional.empty());
+
+    Response response = sslClient.get("/version", Constants.MIMETYPE_TEXT);
+    assertEquals(200, response.getCode());
+  }
+
+  /**
+   * Backward-compatibility regression: existing deployments that know only 
about the legacy
+   * unscoped keys must continue to work exactly as before. This mirrors 
{@link #testSslConnection}
+   * but names the intent explicitly.
+   */
+  @Test
+  public void testSslConnectionFallsBackToLegacyKeystoreKeys() throws 
Exception {
+    // Make sure no role-scoped key is set — the beforeEachTest configures 
only legacy passwords,
+    // so this is a fresh state check.
+    conf.unset(Constants.REST_SSL_SERVER_KEYSTORE_STORE);
+    conf.unset(Constants.REST_SSL_SERVER_KEYSTORE_PASSWORD);
+    conf.unset(Constants.REST_SSL_SERVER_KEYSTORE_KEYPASSWORD);
+    conf.unset(Constants.REST_SSL_SERVER_KEYSTORE_TYPE);
+    conf.unset(Constants.REST_SSL_SERVER_TRUSTSTORE_STORE);
+    conf.unset(Constants.REST_SSL_SERVER_TRUSTSTORE_PASSWORD);
+    conf.unset(Constants.REST_SSL_SERVER_TRUSTSTORE_TYPE);
+
+    startRESTServerWithDefaultKeystoreType();
+
+    Response response = sslClient.get("/version", Constants.MIMETYPE_TEXT);
+    assertEquals(200, response.getCode());
+  }
+
+  /**
+   * With {@code client.auth.mode=NONE} (the default), a client that presents 
no client certificate
+   * is accepted — matching today's behavior.
+   */
+  @Test
+  public void testClientAuthNoneAcceptsClientWithoutCert() throws Exception {
+    conf.set(Constants.REST_SSL_CLIENT_AUTH_MODE, "NONE");
+    startRESTServerWithDefaultKeystoreType();
+
+    Response response = sslClient.get("/version", Constants.MIMETYPE_TEXT);
+    assertEquals(200, response.getCode());
+  }
+
+  /**
+   * With {@code client.auth.mode=WANT}, an anonymous client (no client cert) 
is still accepted; the
+   * server requests a cert but does not require it.
+   */
+  @Test
+  public void testClientAuthWantAllowsAnonymousClient() throws Exception {
+    conf.set(Constants.REST_SSL_CLIENT_AUTH_MODE, "WANT");
+    startRESTServerWithDefaultKeystoreType();
+
+    Response response = sslClient.get("/version", Constants.MIMETYPE_TEXT);
+    assertEquals(200, response.getCode());
+  }
+
+  /**
+   * With {@code client.auth.mode=NEED}, an anonymous client (no client cert) 
is rejected by the
+   * server. The base {@link Client} configures truststore-only, so it 
presents no key material to
+   * the server.
+   */
+  @Test
+  public void testClientAuthNeedRejectsClientWithoutCert() throws Exception {
+    conf.set(Constants.REST_SSL_CLIENT_AUTH_MODE, "NEED");
+    startRESTServerWithDefaultKeystoreType();
+    assertThrows(SSLException.class, () -> sslClient.get("/version"));

Review Comment:
   This should be changed to IOException like in TestThriftServerSSLMutualAuth.



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