HBASE-29761: The HBase UI's Debug Dump is not redacting sensitive information

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,15 +83,15 @@ 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();
 
       out.println("\n\nMaster configuration:");
       out.println(LINE);
-      Configuration conf = master.getConfiguration();
+      Configuration redactedConf = getRedactedConfiguration(master.getConfiguration());
       out.flush();
-      conf.writeXml(os);
+      redactedConf.writeXml(os);
       os.flush();
 
       out.println("\n\nRecent regionserver aborts:");

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,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 = "<redacted>";
+  protected static final String REDACTED_TEXT = "******";
 
   protected void dumpVersionInfo(PrintWriter out) {
     VersionInfo.writeTo(out);
@@ -66,4 +69,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<String, String> entry : conf) {
+      redactResult = redactor.redact(entry.getKey(), entry.getValue());
+      if (REDACTED.equals(redactResult)) {
+        conf.set(entry.getKey(), REDACTED_TEXT);
+      }
+    }
+    return conf;
+  }
 }

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,15 +83,15 @@ 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();
 
       out.println("\n\nRS Configuration:");
       out.println(LINE);
-      Configuration conf = hrs.getConfiguration();
+      Configuration redactedConf = getRedactedConfiguration(hrs.getConfiguration());
       out.flush();
-      conf.writeXml(os);
+      redactedConf.writeXml(os);
       os.flush();
 
       out.println("\n\nLogs");
@@ -99,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);
       }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/http/TestDebugDumpRedaction.java

@@ -0,0 +1,168 @@
+/*
+ * 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.Arrays;
+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 = "<configuration>";
+  private static final String XML_CONFIGURATION_END_TAG = "</configuration>";
+  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 = "******";
+  private static final List<String> SENSITIVE_CONF_PROPERTIES =
+    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
+  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<ServerName> 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_TEXT, conf.get(property));
+    }
+  }
+}

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("<h1>Master <small>" + hostname + "</small></h1>"));
@@ -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<ServerName> servers = serverManager.getOnlineServersList();