From a6d480e25467da6f9ba0514409a437850fd83fba Mon Sep 17 00:00:00 2001 From: magnum Date: Fri, 29 May 2026 17:30:50 +0900 Subject: [PATCH] HIVE-29636: Add SSL keystore auto-reloading for HiveServer2 WebUI --- .../org/apache/hadoop/hive/conf/HiveConf.java | 3 + .../java/org/apache/hive/http/HttpServer.java | 38 ++++ .../org/apache/hive/http/TestHttpServer.java | 206 ++++++++++++++++++ 3 files changed, 247 insertions(+) create mode 100644 common/src/test/org/apache/hive/http/TestHttpServer.java diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java index 31b5e32c2ddb..f85fcc04e578 100644 --- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java +++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java @@ -3856,6 +3856,9 @@ public static enum ConfVars { "SSL certificate keystore location for HiveServer2 WebUI."), HIVE_SERVER2_WEBUI_SSL_KEYSTORE_PASSWORD("hive.server2.webui.keystore.password", "", "SSL certificate keystore password for HiveServer2 WebUI."), + HIVE_SERVER2_WEBUI_SSL_KEYSTORE_RELOAD_INTERVAL("hive.server2.webui.keystore.reload.interval", "60s", + new TimeValidator(TimeUnit.MILLISECONDS), + "The refresh interval used to check if either of the keystore certificate file has changed."), HIVE_SERVER2_WEBUI_SSL_KEYSTORE_TYPE("hive.server2.webui.keystore.type", "", "SSL certificate keystore type for HiveServer2 WebUI."), HIVE_SERVER2_WEBUI_SSL_INCLUDE_CIPHERSUITES("hive.server2.webui.include.ciphersuites", "", diff --git a/common/src/java/org/apache/hive/http/HttpServer.java b/common/src/java/org/apache/hive/http/HttpServer.java index dd9e66f92b6b..88a8d4cf75f6 100644 --- a/common/src/java/org/apache/hive/http/HttpServer.java +++ b/common/src/java/org/apache/hive/http/HttpServer.java @@ -35,6 +35,8 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.Timer; +import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -66,6 +68,8 @@ import org.apache.hadoop.security.authorize.AccessControlList; import org.apache.hadoop.hive.common.classification.InterfaceAudience; import org.apache.hadoop.security.http.CrossOriginFilter; +import org.apache.hadoop.security.ssl.FileBasedKeyStoresFactory; +import org.apache.hadoop.security.ssl.FileMonitoringTimerTask; import org.apache.hive.http.security.PamAuthenticator; import org.apache.hive.http.security.PamConstraint; import org.apache.hive.http.security.PamConstraintMapping; @@ -140,6 +144,7 @@ public class HttpServer { private Server webServer; private QueuedThreadPool threadPool; private PortHandlerWrapper portHandlerWrapper; + private Optional configurationChangeMonitor = Optional.empty(); /** * Create a status server on the given port. @@ -360,6 +365,9 @@ public void start() throws Exception { } public void stop() throws Exception { + if (this.configurationChangeMonitor.isPresent()) { + this.configurationChangeMonitor.get().cancel(); + } webServer.stop(); } @@ -695,6 +703,12 @@ ServerConnector createAndAddChannelConnector(int queueSize, Builder b) { new String[excludedSSLProtocols.size()])); sslContextFactory.setKeyStorePassword(b.keyStorePassword); connector = new ServerConnector(webServer, sslContextFactory, http); + + long storesReloadInterval = b.conf.getTimeVar(ConfVars.HIVE_SERVER2_WEBUI_SSL_KEYSTORE_RELOAD_INTERVAL, TimeUnit.MILLISECONDS); + if (storesReloadInterval > 0) { + this.configurationChangeMonitor = Optional.of( + this.makeConfigurationChangeMonitor(storesReloadInterval, b.keyStorePath, sslContextFactory)); + } } connector.setAcceptQueueSize(queueSize); @@ -706,6 +720,30 @@ ServerConnector createAndAddChannelConnector(int queueSize, Builder b) { return connector; } + private Timer makeConfigurationChangeMonitor(long reloadInterval, String keyStorePath, + SslContextFactory sslContextFactory) { + LOG.info("Starting SSL Certificates Store Monitor. reload interval: {}ms, keyStorePath: {}", reloadInterval, keyStorePath); + Timer timer = new Timer("SSL Certificates Store Monitor", true); + // + // The Jetty SSLContextFactory provides a 'reload' method which will reload both + // truststore and keystore certificates. + // + timer.schedule(new FileMonitoringTimerTask( + Paths.get(keyStorePath), + path -> { + LOG.info("Reloading certificates from store keystore " + keyStorePath); + try { + sslContextFactory.reload(factory -> { }); + } catch (Exception ex) { + LOG.error("Failed to reload SSL keystore certificates", ex); + } + },null), + reloadInterval, + reloadInterval + ); + return timer; + } + /** * Secure the web server with PAM. */ diff --git a/common/src/test/org/apache/hive/http/TestHttpServer.java b/common/src/test/org/apache/hive/http/TestHttpServer.java new file mode 100644 index 000000000000..ba6d23379a97 --- /dev/null +++ b/common/src/test/org/apache/hive/http/TestHttpServer.java @@ -0,0 +1,206 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hive.http; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.conf.HiveConf.ConfVars; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileTime; +import java.util.Optional; +import java.util.Timer; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.withSettings; + +/** + * Tests for the SSL keystore auto-reload feature wired in via + * {@code HttpServer#makeConfigurationChangeMonitor} and the surrounding + * {@code configurationChangeMonitor} field. See HiveConf + * {@code hive.server2.webui.keystore.reload.interval}. + */ +public class TestHttpServer { + + private Path keystore; + private Timer timer; + + @Before + public void setUp() throws Exception { + keystore = Files.createTempFile("test-keystore-", ".jks"); + Files.write(keystore, "initial-content".getBytes()); + } + + @After + public void tearDown() throws Exception { + if (timer != null) { + timer.cancel(); + } + if (keystore != null) { + Files.deleteIfExists(keystore); + } + } + + /** + * The reload-interval ConfVar defaults to 60s so the feature is on out of the box. + * Wiring in {@code createAndAddChannelConnector} treats <= 0 as "disabled". + */ + @Test + public void testReloadIntervalConfDefaultIs60Seconds() { + HiveConf conf = new HiveConf(); + long ms = conf.getTimeVar( + ConfVars.HIVE_SERVER2_WEBUI_SSL_KEYSTORE_RELOAD_INTERVAL, TimeUnit.MILLISECONDS); + assertEquals(60_000L, ms); + } + + /** + * When the watched keystore file is modified, the scheduled + * {@code FileMonitoringTimerTask} must invoke + * {@code SslContextFactory#reload}. + */ + @Test(timeout = 10_000) + public void testMonitorReloadsSslContextOnKeystoreModification() throws Exception { + SslContextFactory sslContextFactory = mock(SslContextFactory.class); + CountDownLatch reloadCalled = new CountDownLatch(1); + doAnswer(invocation -> { + reloadCalled.countDown(); + return null; + }).when(sslContextFactory).reload(any()); + + timer = invokeMakeMonitor(100L, keystore.toString(), sslContextFactory); + + // Bump mtime to guarantee a detected change (FileMonitoringTimerTask compares mtimes). + Files.setLastModifiedTime(keystore, FileTime.fromMillis(System.currentTimeMillis() + 5_000)); + + assertTrue("SslContextFactory#reload was not called within 5s of keystore mtime change", + reloadCalled.await(5, TimeUnit.SECONDS)); + verify(sslContextFactory, atLeastOnce()).reload(any()); + } + + /** + * Reload failures must be swallowed so a transient bad keystore can't take HS2 down; + * the next mtime change should still trigger another reload attempt. + */ + @Test(timeout = 10_000) + public void testMonitorSurvivesReloadException() throws Exception { + SslContextFactory sslContextFactory = mock(SslContextFactory.class); + CountDownLatch reloadCalled = new CountDownLatch(2); + doAnswer(invocation -> { + reloadCalled.countDown(); + throw new RuntimeException("simulated keystore reload failure"); + }).when(sslContextFactory).reload(any()); + + timer = invokeMakeMonitor(100L, keystore.toString(), sslContextFactory); + + Files.setLastModifiedTime(keystore, FileTime.fromMillis(System.currentTimeMillis() + 5_000)); + Thread.sleep(300); + Files.setLastModifiedTime(keystore, FileTime.fromMillis(System.currentTimeMillis() + 10_000)); + + assertTrue("Monitor should keep firing reload attempts even after exceptions", + reloadCalled.await(5, TimeUnit.SECONDS)); + } + + /** + * {@code stop()} must cancel the monitor Timer when one was installed, + * so the daemon thread does not outlive HS2. + */ + @Test + public void testStopCancelsConfigurationChangeMonitor() throws Exception { + HttpServer server = mock(HttpServer.class, withSettings().defaultAnswer(CALLS_REAL_METHODS)); + + // Track whether cancel() was invoked on the installed timer. + boolean[] cancelled = {false}; + Timer installed = new Timer("test-monitor", true) { + @Override + public void cancel() { + cancelled[0] = true; + super.cancel(); + } + }; + setField(server, "configurationChangeMonitor", Optional.of(installed)); + + // stop() also calls webServer.stop(); webServer is null on a mock, so we expect + // a NullPointerException after the cancel path runs. + try { + server.stop(); + } catch (NullPointerException expected) { + // intentionally ignored — we only assert the monitor was cancelled + } + assertTrue("Timer#cancel should have been invoked from stop()", cancelled[0]); + } + + /** + * No monitor installed → stop() must not blow up trying to cancel a missing Timer. + * (Mockito skips field initializers, so we re-establish the production default + * {@code Optional.empty()} on the mock before exercising stop().) + */ + @Test + public void testStopWithoutMonitorDoesNotThrowFromCancelPath() throws Exception { + HttpServer server = mock(HttpServer.class, withSettings().defaultAnswer(CALLS_REAL_METHODS)); + setField(server, "configurationChangeMonitor", Optional.empty()); + + Optional current = getField(server, "configurationChangeMonitor"); + assertFalse("configurationChangeMonitor should be empty for this case", current.isPresent()); + + try { + server.stop(); + } catch (NullPointerException expectedFromWebServerStop) { + // ok — the monitor branch must not have thrown before reaching webServer.stop() + } + } + + // ---- reflection helpers ------------------------------------------------ + + private static Timer invokeMakeMonitor(long intervalMs, String keystorePath, + SslContextFactory sslContextFactory) throws Exception { + HttpServer server = mock(HttpServer.class, withSettings().defaultAnswer(CALLS_REAL_METHODS)); + Method m = HttpServer.class.getDeclaredMethod( + "makeConfigurationChangeMonitor", long.class, String.class, SslContextFactory.class); + m.setAccessible(true); + return (Timer) m.invoke(server, intervalMs, keystorePath, sslContextFactory); + } + + private static void setField(Object target, String name, Object value) throws Exception { + Field f = HttpServer.class.getDeclaredField(name); + f.setAccessible(true); + f.set(target, value); + } + + @SuppressWarnings("unchecked") + private static T getField(Object target, String name) throws Exception { + Field f = HttpServer.class.getDeclaredField(name); + f.setAccessible(true); + return (T) f.get(target); + } +}