From 3a45a845750e009c9e42c1b9b8fc5b907db37e77 Mon Sep 17 00:00:00 2001 From: Kevin Geiszler Date: Tue, 9 Dec 2025 16:27:29 -0800 Subject: [PATCH 1/5] HBASE-29761: The HBase UI's Debug Dump is not redacting sensitive information Change-Id: I7f0cf9f096727272764252d8e7f6b8c6f5fc91c0 --- .../hbase/master/http/MasterDumpServlet.java | 10 +- .../regionserver/http/RSDumpServlet.java | 10 +- .../hbase/http/TestDebugDumpRedaction.java | 167 ++++++++++++++++++ .../master/http/TestMasterStatusPage.java | 33 +--- .../regionserver/http/TestRSStatusPage.java | 29 +-- .../hbase/util/TestServerHttpUtils.java | 53 ++++++ 6 files changed, 239 insertions(+), 63 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/http/MasterDumpServlet.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/http/MasterDumpServlet.java index 402c248072ff..822be1934d9d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/http/MasterDumpServlet.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/http/MasterDumpServlet.java @@ -18,9 +18,10 @@ package org.apache.hadoop.hbase.master.http; import java.io.IOException; -import java.io.OutputStream; +import java.io.OutputStreamWriter; import java.io.PrintStream; import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; import java.util.Date; import java.util.Map; import javax.servlet.http.HttpServletRequest; @@ -53,7 +54,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro assert master != null : "No Master in context!"; response.setContentType("text/plain"); - OutputStream os = response.getOutputStream(); + OutputStreamWriter os = + new OutputStreamWriter(response.getOutputStream(), StandardCharsets.UTF_8); try (PrintWriter out = new PrintWriter(os)) { out.println("Master status for " + master.getServerName() + " as of " + new Date()); @@ -81,7 +83,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro out.println("\n\nStacks:"); out.println(LINE); out.flush(); - PrintStream ps = new PrintStream(response.getOutputStream(), false, "UTF-8"); + PrintStream ps = new PrintStream(response.getOutputStream(), false, StandardCharsets.UTF_8); Threads.printThreadInfo(ps, ""); ps.flush(); @@ -89,7 +91,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro out.println(LINE); Configuration conf = master.getConfiguration(); out.flush(); - conf.writeXml(os); + conf.writeXml(null, os, conf); os.flush(); out.println("\n\nRecent regionserver aborts:"); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/http/RSDumpServlet.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/http/RSDumpServlet.java index 4c98c08b072c..5e1520dacee9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/http/RSDumpServlet.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/http/RSDumpServlet.java @@ -18,9 +18,10 @@ package org.apache.hadoop.hbase.regionserver.http; import java.io.IOException; -import java.io.OutputStream; +import java.io.OutputStreamWriter; import java.io.PrintStream; import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; import java.util.Date; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -58,7 +59,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro return; } - OutputStream os = response.getOutputStream(); + OutputStreamWriter os = + new OutputStreamWriter(response.getOutputStream(), StandardCharsets.UTF_8); try (PrintWriter out = new PrintWriter(os)) { out.println("RegionServer status for " + hrs.getServerName() + " as of " + new Date()); @@ -81,7 +83,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro out.println("\n\nStacks:"); out.println(LINE); - PrintStream ps = new PrintStream(response.getOutputStream(), false, "UTF-8"); + PrintStream ps = new PrintStream(response.getOutputStream(), false, StandardCharsets.UTF_8); Threads.printThreadInfo(ps, ""); ps.flush(); @@ -89,7 +91,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro out.println(LINE); Configuration conf = hrs.getConfiguration(); out.flush(); - conf.writeXml(os); + conf.writeXml(null, os, conf); os.flush(); out.println("\n\nLogs"); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java new file mode 100644 index 000000000000..af1efd6c3756 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java @@ -0,0 +1,167 @@ +/* + * 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.hadoop.hbase.http; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.util.List; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.LocalHBaseCluster; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.master.HMaster; +import org.apache.hadoop.hbase.master.ServerManager; +import org.apache.hadoop.hbase.testclassification.MiscTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.CommonFSUtils; +import org.apache.hadoop.hbase.util.TestServerHttpUtils; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +/** + * This class performs tests that ensure sensitive config values found in the HBase UI's Debug Dump + * are redacted. A config property name must follow one of the regex patterns specified in + * hadoop.security.sensitive-config-keys in order to have its value redacted. + */ +@Category({ MiscTests.class, SmallTests.class }) +public class TestDebugDumpRedaction { + private static final HBaseTestingUtil UTIL = new HBaseTestingUtil(); + private static final String XML_CONFIGURATION_START_TAG = ""; + private static final String XML_CONFIGURATION_END_TAG = ""; + private static final int SUBSTRING_OFFSET = XML_CONFIGURATION_END_TAG.length(); + private static final String PLAIN_TEXT_UTF8 = "text/plain;charset=utf-8"; + private static final String REDACTED = "******"; + private static final List SENSITIVE_CONF_PROPERTIES = + List.of("hbase.zookeeper.property.ssl.trustStore.password", "ssl.client.truststore.password", + "hbase.rpc.tls.truststore.password", "ssl.server.keystore.password", + "fs.s3a.server-side-encryption.key", "fs.s3a.encryption.algorithm", "fs.s3a.encryption.key", + "fs.s3a.secret.key", "fs.s3a.important.secret.key", "fs.s3a.session.key", + "fs.s3a.secret.session.key", "fs.s3a.session.token", "fs.s3a.secret.session.token", + "fs.azure.account.key.importantKey", "fs.azure.oauth2.token", "fs.adl.oauth2.token", + "fs.gs.encryption.sensitive", "fs.gs.proxy.important", "fs.gs.auth.sensitive.info", + "sensitive.credential", "oauth.important.secret", "oauth.important.password", + "oauth.important.token"); + private static LocalHBaseCluster CLUSTER; + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestDebugDumpRedaction.class); + + @BeforeClass + public static void beforeClass() throws Exception { + Configuration conf = UTIL.getConfiguration(); + + // Add various config properties with sensitive information that should be redacted + // when the Debug Dump is performed in the UI. These properties are following the + // regexes specified by the hadoop.security.sensitive-config-keys property. + for (String property : SENSITIVE_CONF_PROPERTIES) { + conf.set(property, "testPassword"); + } + + UTIL.startMiniZKCluster(); + + UTIL.startMiniDFSCluster(1); + Path rootdir = UTIL.getDataTestDirOnTestFS("TestDebugDumpServlet"); + CommonFSUtils.setRootDir(conf, rootdir); + + // The info servers do not run in tests by default. + // Set them to ephemeral ports so they will start + // setup configuration + conf.setInt(HConstants.MASTER_INFO_PORT, 0); + conf.setInt(HConstants.REGIONSERVER_INFO_PORT, 0); + + CLUSTER = new LocalHBaseCluster(conf, 1); + CLUSTER.startup(); + CLUSTER.getActiveMaster().waitForMetaOnline(); + } + + @AfterClass + public static void afterClass() throws Exception { + if (CLUSTER != null) { + CLUSTER.shutdown(); + CLUSTER.join(); + } + UTIL.shutdownMiniCluster(); + } + + @Test + public void testMasterPasswordsAreRedacted() throws IOException { + URL debugDumpUrl = + new URL(TestServerHttpUtils.getMasterInfoServerHostAndPort(CLUSTER) + "/dump"); + String response = TestServerHttpUtils.getPageContent(debugDumpUrl, PLAIN_TEXT_UTF8); + + // Verify this is the master server's debug dump + assertTrue( + response.startsWith("Master status for " + CLUSTER.getActiveMaster().getServerName())); + + verifyDebugDumpResponseConfig(response); + } + + @Test + public void testRegionServerPasswordsAreRedacted() throws IOException { + HMaster master = CLUSTER.getActiveMaster(); + + ServerManager serverManager = master.getServerManager(); + List onlineServersList = serverManager.getOnlineServersList(); + + assertEquals(1, onlineServersList.size()); + + ServerName regionServerName = onlineServersList.get(0); + int regionServerInfoPort = master.getRegionServerInfoPort(regionServerName); + String regionServerHostname = regionServerName.getHostname(); + + URL debugDumpUrl = + new URL("http://" + regionServerHostname + ":" + regionServerInfoPort + "/dump"); + String response = TestServerHttpUtils.getPageContent(debugDumpUrl, PLAIN_TEXT_UTF8); + + // Verify this is the region server's debug dump + assertTrue(response.startsWith("RegionServer status for " + regionServerName)); + + verifyDebugDumpResponseConfig(response); + } + + private void verifyDebugDumpResponseConfig(String response) throws IOException { + // Grab the server's config from the Debug Dump. + String xmlString = response.substring(response.indexOf(XML_CONFIGURATION_START_TAG), + response.indexOf(XML_CONFIGURATION_END_TAG) + SUBSTRING_OFFSET); + + // Convert the XML string into an InputStream + Configuration conf = new Configuration(false); + try (InputStream is = new ByteArrayInputStream(xmlString.getBytes(StandardCharsets.UTF_8))) { + // Add the InputStream as a resource to the Configuration object + conf.addResource(is, "DebugDumpXmlConfig"); + } + + // Verify all sensitive properties have had their values redacted + for (String property : SENSITIVE_CONF_PROPERTIES) { + assertEquals(REDACTED, conf.get(property)); + } + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/http/TestMasterStatusPage.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/http/TestMasterStatusPage.java index 481de91ea0e3..a7496c78aa9a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/http/TestMasterStatusPage.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/http/TestMasterStatusPage.java @@ -21,10 +21,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import java.io.BufferedReader; import java.io.IOException; -import java.io.InputStreamReader; -import java.net.HttpURLConnection; import java.net.URL; import java.util.ArrayList; import java.util.List; @@ -45,6 +42,7 @@ import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.CommonFSUtils; +import org.apache.hadoop.hbase.util.TestServerHttpUtils; import org.apache.hadoop.hbase.util.VersionInfo; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -108,7 +106,9 @@ public void testMasterStatusPage() throws Exception { createTestTables(master); - String page = getMasterStatusPageContent(); + URL url = + new URL(TestServerHttpUtils.getMasterInfoServerHostAndPort(CLUSTER) + "/master-status"); + String page = TestServerHttpUtils.getPageContent(url, "text/html;charset=utf-8"); String hostname = master.getServerName().getHostname(); assertTrue(page.contains("

Master " + hostname + "

")); @@ -127,17 +127,6 @@ public void testMasterStatusPage() throws Exception { assertTrue(page.contains(VersionInfo.getVersion())); } - private String getMasterStatusPageContent() throws IOException { - URL url = new URL(getInfoServerHostAndPort() + "/master-status"); - HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - conn.connect(); - - assertEquals(200, conn.getResponseCode()); - assertEquals("text/html;charset=utf-8", conn.getContentType()); - - return getResponseBody(conn); - } - private static void createTestTables(HMaster master) throws IOException { ColumnFamilyDescriptor cf = ColumnFamilyDescriptorBuilder.of("CF"); TableDescriptor tableDescriptor1 = TableDescriptorBuilder @@ -149,20 +138,6 @@ private static void createTestTables(HMaster master) throws IOException { master.flushMasterStore(); } - private String getInfoServerHostAndPort() { - return "http://localhost:" + CLUSTER.getActiveMaster().getInfoServer().getPort(); - } - - private static String getResponseBody(HttpURLConnection conn) throws IOException { - StringBuilder sb = new StringBuilder(); - BufferedReader br = new BufferedReader(new InputStreamReader(conn.getInputStream())); - String output; - while ((output = br.readLine()) != null) { - sb.append(output); - } - return sb.toString(); - } - private static void assertRegionServerLinks(HMaster master, String responseBody) { ServerManager serverManager = master.getServerManager(); List servers = serverManager.getOnlineServersList(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/http/TestRSStatusPage.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/http/TestRSStatusPage.java index b4b7214b037d..12b7e0d78579 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/http/TestRSStatusPage.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/http/TestRSStatusPage.java @@ -20,10 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import java.io.BufferedReader; import java.io.IOException; -import java.io.InputStreamReader; -import java.net.HttpURLConnection; import java.net.URL; import java.util.List; import org.apache.hadoop.conf.Configuration; @@ -43,6 +40,7 @@ import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.RegionServerTests; import org.apache.hadoop.hbase.util.CommonFSUtils; +import org.apache.hadoop.hbase.util.TestServerHttpUtils; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.ClassRule; @@ -123,7 +121,8 @@ public void testStatusPage() throws Exception { String hostname = firstServerName.getHostname(); int port = firstServerName.getPort(); - String page = getRegionServerStatusPageContent(hostname, infoPort); + URL url = new URL("http://" + hostname + ":" + infoPort + "/regionserver.jsp"); + String page = TestServerHttpUtils.getPageContent(url, "text/html;charset=utf-8"); assertTrue(page.contains("HBase Region Server: " + masterHostname + "")); @@ -153,26 +152,4 @@ private static void createTestTables(HMaster master) throws IOException { master.createTable(tableDescriptor2, null, 0, 0); master.flushMasterStore(); } - - private static String getRegionServerStatusPageContent(String hostname, int infoPort) - throws IOException { - URL url = new URL("http://" + hostname + ":" + infoPort + "/regionserver.jsp"); - HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - conn.connect(); - - assertEquals(200, conn.getResponseCode()); - assertEquals("text/html;charset=utf-8", conn.getContentType()); - - return getResponseBody(conn); - } - - private static String getResponseBody(HttpURLConnection conn) throws IOException { - StringBuilder sb = new StringBuilder(); - BufferedReader br = new BufferedReader(new InputStreamReader(conn.getInputStream())); - String output; - while ((output = br.readLine()) != null) { - sb.append(output); - } - return sb.toString(); - } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java new file mode 100644 index 000000000000..f970ce3d0c37 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java @@ -0,0 +1,53 @@ +/* + * 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.hadoop.hbase.util; + +import static org.junit.Assert.assertEquals; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.net.HttpURLConnection; +import java.net.URL; +import org.apache.hadoop.hbase.LocalHBaseCluster; + +public class TestServerHttpUtils { + public static String getMasterInfoServerHostAndPort(LocalHBaseCluster cluster) { + return "http://localhost:" + cluster.getActiveMaster().getInfoServer().getPort(); + } + + public static String getPageContent(URL url, String mimeType) throws IOException { + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.connect(); + + assertEquals(200, conn.getResponseCode()); + assertEquals(mimeType, conn.getContentType()); + + return TestServerHttpUtils.getResponseBody(conn); + } + + private static String getResponseBody(HttpURLConnection conn) throws IOException { + StringBuilder sb = new StringBuilder(); + BufferedReader br = new BufferedReader(new InputStreamReader(conn.getInputStream())); + String output; + while ((output = br.readLine()) != null) { + sb.append(output); + } + return sb.toString(); + } +} From dcb218558a9f0d905842003f9856b250d4e45597 Mon Sep 17 00:00:00 2001 From: Kevin Geiszler Date: Mon, 15 Dec 2025 16:54:06 -0800 Subject: [PATCH 2/5] Manually perform redaction to ensure backward compatibility Change-Id: I961ff80833c4da6f879cfeafa2620ecfaa0e7e84 --- .../hbase/master/http/MasterDumpServlet.java | 4 ++-- .../hbase/monitoring/StateDumpServlet.java | 19 +++++++++++++++++ .../regionserver/http/RSDumpServlet.java | 6 +++--- .../hbase/http/TestDebugDumpRedaction.java | 21 ++++++++++--------- .../hbase/util/TestServerHttpUtils.java | 3 +++ 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/http/MasterDumpServlet.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/http/MasterDumpServlet.java index 822be1934d9d..daabfad937c7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/http/MasterDumpServlet.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/http/MasterDumpServlet.java @@ -89,9 +89,9 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro out.println("\n\nMaster configuration:"); out.println(LINE); - Configuration conf = master.getConfiguration(); + Configuration redactedConf = getRedactedConfiguration(master.getConfiguration()); out.flush(); - conf.writeXml(null, os, conf); + redactedConf.writeXml(os); os.flush(); out.println("\n\nRecent regionserver aborts:"); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/StateDumpServlet.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/StateDumpServlet.java index ff6d2939f178..17943db96206 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/StateDumpServlet.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/StateDumpServlet.java @@ -22,6 +22,7 @@ import java.util.Map; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; +import org.apache.hadoop.conf.ConfigRedactor; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.executor.ExecutorService; import org.apache.hadoop.hbase.executor.ExecutorService.ExecutorStatus; @@ -32,6 +33,7 @@ public abstract class StateDumpServlet extends HttpServlet { static final long DEFAULT_TAIL_KB = 100; private static final long serialVersionUID = 1L; + protected static final String REDACTED_TEXT = ""; protected void dumpVersionInfo(PrintWriter out) { VersionInfo.writeTo(out); @@ -66,4 +68,21 @@ protected void dumpExecutors(ExecutorService service, PrintWriter out) throws IO status.dumpTo(out, " "); } } + + protected Configuration getRedactedConfiguration(Configuration conf) { + // YARN-11308 introduced a new method signature to the overloaded Configuration.writeXml() + // method. Within this new method, the ConfigRedactor is used on the Configuration object if + // that object is not null. This allows the XML output to have sensitive content redacted + // automatically. However, this new method is only available in Hadoop 3.4 and later, so we are + // performing the redaction here manually in order to ensure backward compatibility. + ConfigRedactor redactor = new ConfigRedactor(conf); + String redactResult; + for (Map.Entry entry : conf) { + redactResult = redactor.redact(entry.getKey(), entry.getValue()); + if (REDACTED_TEXT.equals(redactResult)) { + conf.set(entry.getKey(), REDACTED_TEXT); + } + } + return conf; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/http/RSDumpServlet.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/http/RSDumpServlet.java index 5e1520dacee9..3bdf7b61cdac 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/http/RSDumpServlet.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/http/RSDumpServlet.java @@ -89,9 +89,9 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro out.println("\n\nRS Configuration:"); out.println(LINE); - Configuration conf = hrs.getConfiguration(); + Configuration redactedConf = getRedactedConfiguration(hrs.getConfiguration()); out.flush(); - conf.writeXml(null, os, conf); + redactedConf.writeXml(os); os.flush(); out.println("\n\nLogs"); @@ -101,7 +101,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro out.println("\n\nRS Queue:"); out.println(LINE); - if (isShowQueueDump(conf)) { + if (isShowQueueDump(hrs.getConfiguration())) { dumpQueue(hrs, out); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java index af1efd6c3756..17e4f8080a91 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java @@ -25,6 +25,7 @@ import java.io.InputStream; import java.net.URL; import java.nio.charset.StandardCharsets; +import java.util.Arrays; import java.util.List; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -57,17 +58,17 @@ public class TestDebugDumpRedaction { private static final String XML_CONFIGURATION_END_TAG = ""; private static final int SUBSTRING_OFFSET = XML_CONFIGURATION_END_TAG.length(); private static final String PLAIN_TEXT_UTF8 = "text/plain;charset=utf-8"; - private static final String REDACTED = "******"; + private static final String REDACTED = ""; private static final List SENSITIVE_CONF_PROPERTIES = - List.of("hbase.zookeeper.property.ssl.trustStore.password", "ssl.client.truststore.password", - "hbase.rpc.tls.truststore.password", "ssl.server.keystore.password", - "fs.s3a.server-side-encryption.key", "fs.s3a.encryption.algorithm", "fs.s3a.encryption.key", - "fs.s3a.secret.key", "fs.s3a.important.secret.key", "fs.s3a.session.key", - "fs.s3a.secret.session.key", "fs.s3a.session.token", "fs.s3a.secret.session.token", - "fs.azure.account.key.importantKey", "fs.azure.oauth2.token", "fs.adl.oauth2.token", - "fs.gs.encryption.sensitive", "fs.gs.proxy.important", "fs.gs.auth.sensitive.info", - "sensitive.credential", "oauth.important.secret", "oauth.important.password", - "oauth.important.token"); + Arrays.asList("hbase.zookeeper.property.ssl.trustStore.password", + "ssl.client.truststore.password", "hbase.rpc.tls.truststore.password", + "ssl.server.keystore.password", "fs.s3a.server-side-encryption.key", + "fs.s3a.encryption.algorithm", "fs.s3a.encryption.key", "fs.s3a.secret.key", + "fs.s3a.important.secret.key", "fs.s3a.session.key", "fs.s3a.secret.session.key", + "fs.s3a.session.token", "fs.s3a.secret.session.token", "fs.azure.account.key.importantKey", + "fs.azure.oauth2.token", "fs.adl.oauth2.token", "fs.gs.encryption.sensitive", + "fs.gs.proxy.important", "fs.gs.auth.sensitive.info", "sensitive.credential", + "oauth.important.secret", "oauth.important.password", "oauth.important.token"); private static LocalHBaseCluster CLUSTER; @ClassRule diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java index f970ce3d0c37..60d914047882 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java @@ -27,6 +27,9 @@ import org.apache.hadoop.hbase.LocalHBaseCluster; public class TestServerHttpUtils { + private TestServerHttpUtils() { + } + public static String getMasterInfoServerHostAndPort(LocalHBaseCluster cluster) { return "http://localhost:" + cluster.getActiveMaster().getInfoServer().getPort(); } From 2aa66e3d407503ba954b9c2bc7ecd8f3b909ac24 Mon Sep 17 00:00:00 2001 From: Kevin Geiszler Date: Tue, 16 Dec 2025 08:56:07 -0800 Subject: [PATCH 3/5] Make redacted text be '******' instead of '' Change-Id: Ic040fa865690d3dfd421f32a4afdd8e5a011f2d8 --- .../org/apache/hadoop/hbase/monitoring/StateDumpServlet.java | 5 +++-- .../org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/StateDumpServlet.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/StateDumpServlet.java index 17943db96206..fa84eb6853d3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/StateDumpServlet.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/StateDumpServlet.java @@ -33,7 +33,8 @@ public abstract class StateDumpServlet extends HttpServlet { static final long DEFAULT_TAIL_KB = 100; private static final long serialVersionUID = 1L; - protected static final String REDACTED_TEXT = ""; + protected static final String REDACTED = ""; + protected static final String REDACTED_TEXT = "******"; protected void dumpVersionInfo(PrintWriter out) { VersionInfo.writeTo(out); @@ -79,7 +80,7 @@ protected Configuration getRedactedConfiguration(Configuration conf) { String redactResult; for (Map.Entry entry : conf) { redactResult = redactor.redact(entry.getKey(), entry.getValue()); - if (REDACTED_TEXT.equals(redactResult)) { + if (REDACTED.equals(redactResult)) { conf.set(entry.getKey(), REDACTED_TEXT); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java index 17e4f8080a91..b064513a44e0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java @@ -58,7 +58,7 @@ public class TestDebugDumpRedaction { private static final String XML_CONFIGURATION_END_TAG = ""; private static final int SUBSTRING_OFFSET = XML_CONFIGURATION_END_TAG.length(); private static final String PLAIN_TEXT_UTF8 = "text/plain;charset=utf-8"; - private static final String REDACTED = ""; + private static final String REDACTED_TEXT = "******"; private static final List SENSITIVE_CONF_PROPERTIES = Arrays.asList("hbase.zookeeper.property.ssl.trustStore.password", "ssl.client.truststore.password", "hbase.rpc.tls.truststore.password", @@ -162,7 +162,7 @@ private void verifyDebugDumpResponseConfig(String response) throws IOException { // Verify all sensitive properties have had their values redacted for (String property : SENSITIVE_CONF_PROPERTIES) { - assertEquals(REDACTED, conf.get(property)); + assertEquals(REDACTED_TEXT, conf.get(property)); } } } From 8b0b6ff36b8ae61e093b582608dfbda71970c0a1 Mon Sep 17 00:00:00 2001 From: Kevin Geiszler Date: Wed, 17 Dec 2025 17:49:29 -0800 Subject: [PATCH 4/5] Add getMasterPageContent() and getRegionServerPageContent(); Test int and boolean redaction; Verify other props are not redacted Change-Id: I27f4fc1105025327dd52701a9073066767fc7da6 --- .../hbase/http/TestDebugDumpRedaction.java | 79 ++++++++++++++++--- .../hbase/util/TestServerHttpUtils.java | 12 +++ 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java index b064513a44e0..d1b49ebd4244 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java @@ -18,15 +18,18 @@ package org.apache.hadoop.hbase.http; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.Collection; import java.util.List; +import java.util.Map; +import java.util.stream.Stream; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -45,6 +48,8 @@ import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This class performs tests that ensure sensitive config values found in the HBase UI's Debug Dump @@ -53,12 +58,14 @@ */ @Category({ MiscTests.class, SmallTests.class }) public class TestDebugDumpRedaction { + private static final Logger LOG = LoggerFactory.getLogger(TestDebugDumpRedaction.class); private static final HBaseTestingUtil UTIL = new HBaseTestingUtil(); private static final String XML_CONFIGURATION_START_TAG = ""; private static final String XML_CONFIGURATION_END_TAG = ""; private static final int SUBSTRING_OFFSET = XML_CONFIGURATION_END_TAG.length(); - private static final String PLAIN_TEXT_UTF8 = "text/plain;charset=utf-8"; private static final String REDACTED_TEXT = "******"; + + // These are typical configuration properties whose values we would want to see redacted. private static final List SENSITIVE_CONF_PROPERTIES = Arrays.asList("hbase.zookeeper.property.ssl.trustStore.password", "ssl.client.truststore.password", "hbase.rpc.tls.truststore.password", @@ -68,7 +75,27 @@ public class TestDebugDumpRedaction { "fs.s3a.session.token", "fs.s3a.secret.session.token", "fs.azure.account.key.importantKey", "fs.azure.oauth2.token", "fs.adl.oauth2.token", "fs.gs.encryption.sensitive", "fs.gs.proxy.important", "fs.gs.auth.sensitive.info", "sensitive.credential", - "oauth.important.secret", "oauth.important.password", "oauth.important.token"); + "oauth.important.secret", "oauth.important.password", "oauth.important.token", + "fs.adl.oauth2.access.token.provider.type", "hadoop.security.sensitive-config-keys"); + + // These are not typical configuration properties whose values we would want to see redacted, + // but we are testing their redaction anyway because we want to see how the redaction behaves + // with booleans and ints. + private static final List NON_SENSITIVE_KEYS_WITH_DEFAULT_VALUES = Arrays.asList( + "hbase.zookeeper.quorum", "hbase.cluster.distributed", "hbase.master.logcleaner.ttl", + "hbase.master.hfilecleaner.plugins", "hbase.master.infoserver.redirect", + "hbase.thrift.minWorkerThreads", "hbase.table.lock.enable"); + + // We also want to verify the behavior for a string with value "null" and an empty string. + // (giving a config property an actual null value will throw an error) + private static final String NULL_CONFIG_KEY = "null.key"; + private static final String EMPTY_CONFIG_KEY = "empty.key"; + + // Combine all properties we want to redact into one list + private static final List REDACTED_PROPS = + Stream.of(SENSITIVE_CONF_PROPERTIES, NON_SENSITIVE_KEYS_WITH_DEFAULT_VALUES, + List.of(NULL_CONFIG_KEY, EMPTY_CONFIG_KEY)).flatMap(Collection::stream).toList(); + private static LocalHBaseCluster CLUSTER; @ClassRule @@ -86,6 +113,25 @@ public static void beforeClass() throws Exception { conf.set(property, "testPassword"); } + // Also verify a null string and empty string will get redacted. + // Setting the config to use an actual null value throws an error. + conf.set(NULL_CONFIG_KEY, "null"); + conf.set(EMPTY_CONFIG_KEY, ""); + + // Config properties following these regex patterns will have their values redacted in the + // Debug Dump + String sensitiveKeyRegexes = "secret$,password$,ssl.keystore.pass$," + + "fs.s3a.server-side-encryption.key,fs.s3a.*.server-side-encryption.key," + + "fs.s3a.encryption.algorithm,fs.s3a.encryption.key,fs.s3a.secret.key," + + "fs.s3a.*.secret.key,fs.s3a.session.key,fs.s3a.*.session.key,fs.s3a.session.token," + + "fs.s3a.*.session.token,fs.azure.account.key.*,fs.azure.oauth2.*,fs.adl.oauth2.*," + + "fs.gs.encryption.*,fs.gs.proxy.*,fs.gs.auth.*,credential$,oauth.*secret," + + "oauth.*password,oauth.*token,hadoop.security.sensitive-config-keys," + + String.join(",", NON_SENSITIVE_KEYS_WITH_DEFAULT_VALUES) + "," + NULL_CONFIG_KEY + "," + + EMPTY_CONFIG_KEY; + + conf.set("hadoop.security.sensitive-config-keys", sensitiveKeyRegexes); + UTIL.startMiniZKCluster(); UTIL.startMiniDFSCluster(1); @@ -114,9 +160,7 @@ public static void afterClass() throws Exception { @Test public void testMasterPasswordsAreRedacted() throws IOException { - URL debugDumpUrl = - new URL(TestServerHttpUtils.getMasterInfoServerHostAndPort(CLUSTER) + "/dump"); - String response = TestServerHttpUtils.getPageContent(debugDumpUrl, PLAIN_TEXT_UTF8); + String response = TestServerHttpUtils.getMasterPageContent(CLUSTER); // Verify this is the master server's debug dump assertTrue( @@ -138,9 +182,8 @@ public void testRegionServerPasswordsAreRedacted() throws IOException { int regionServerInfoPort = master.getRegionServerInfoPort(regionServerName); String regionServerHostname = regionServerName.getHostname(); - URL debugDumpUrl = - new URL("http://" + regionServerHostname + ":" + regionServerInfoPort + "/dump"); - String response = TestServerHttpUtils.getPageContent(debugDumpUrl, PLAIN_TEXT_UTF8); + String response = + TestServerHttpUtils.getRegionServerPageContent(regionServerHostname, regionServerInfoPort); // Verify this is the region server's debug dump assertTrue(response.startsWith("RegionServer status for " + regionServerName)); @@ -160,9 +203,21 @@ private void verifyDebugDumpResponseConfig(String response) throws IOException { conf.addResource(is, "DebugDumpXmlConfig"); } - // Verify all sensitive properties have had their values redacted - for (String property : SENSITIVE_CONF_PROPERTIES) { - assertEquals(REDACTED_TEXT, conf.get(property)); + // Verify the expected properties had their values redacted + for (String property : REDACTED_PROPS) { + LOG.info("Verifying property has been redacted: {}", property); + assertEquals("Expected " + property + " to have its value redacted", REDACTED_TEXT, + conf.get(property)); + } + + String propertyName; + for (Map.Entry property : conf) { + propertyName = property.getKey(); + if (!REDACTED_PROPS.contains(propertyName)) { + LOG.info("Verifying {} property has not had its value redacted", propertyName); + assertNotEquals("Expected property " + propertyName + " to not have its value redacted", + REDACTED_TEXT, conf.get(propertyName)); + } } } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java index 60d914047882..a69971e1f36f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestServerHttpUtils.java @@ -27,6 +27,8 @@ import org.apache.hadoop.hbase.LocalHBaseCluster; public class TestServerHttpUtils { + private static final String PLAIN_TEXT_UTF8 = "text/plain;charset=utf-8"; + private TestServerHttpUtils() { } @@ -44,6 +46,16 @@ public static String getPageContent(URL url, String mimeType) throws IOException return TestServerHttpUtils.getResponseBody(conn); } + public static String getMasterPageContent(LocalHBaseCluster cluster) throws IOException { + URL debugDumpUrl = new URL(getMasterInfoServerHostAndPort(cluster) + "/dump"); + return getPageContent(debugDumpUrl, PLAIN_TEXT_UTF8); + } + + public static String getRegionServerPageContent(String hostName, int port) throws IOException { + URL debugDumpUrl = new URL("http://" + hostName + ":" + port + "/dump"); + return getPageContent(debugDumpUrl, PLAIN_TEXT_UTF8); + } + private static String getResponseBody(HttpURLConnection conn) throws IOException { StringBuilder sb = new StringBuilder(); BufferedReader br = new BufferedReader(new InputStreamReader(conn.getInputStream())); From 1ad06f1f9b3aa2467feae2a9ebc3ae9450b324d1 Mon Sep 17 00:00:00 2001 From: Kevin Geiszler Date: Thu, 18 Dec 2025 09:26:58 -0800 Subject: [PATCH 5/5] Add comment Change-Id: Iec6fdb39931bceafdb10713d0f38314defd2c629 --- .../org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java index d1b49ebd4244..0b4dd4d00abd 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java @@ -196,20 +196,21 @@ private void verifyDebugDumpResponseConfig(String response) throws IOException { String xmlString = response.substring(response.indexOf(XML_CONFIGURATION_START_TAG), response.indexOf(XML_CONFIGURATION_END_TAG) + SUBSTRING_OFFSET); - // Convert the XML string into an InputStream + // Convert the XML string into an InputStream. Configuration conf = new Configuration(false); try (InputStream is = new ByteArrayInputStream(xmlString.getBytes(StandardCharsets.UTF_8))) { // Add the InputStream as a resource to the Configuration object conf.addResource(is, "DebugDumpXmlConfig"); } - // Verify the expected properties had their values redacted + // Verify the expected properties had their values redacted. for (String property : REDACTED_PROPS) { LOG.info("Verifying property has been redacted: {}", property); assertEquals("Expected " + property + " to have its value redacted", REDACTED_TEXT, conf.get(property)); } + // Verify all other props have not had their values redacted. String propertyName; for (Map.Entry property : conf) { propertyName = property.getKey();