From 5d20988f9ff8b46acee65aca36278309d6371f66 Mon Sep 17 00:00:00 2001 From: slfan1989 <55643692+slfan1989@users.noreply.github.com> Date: Tue, 27 Sep 2022 06:54:17 +0800 Subject: [PATCH] YARN-11308. Router Page display the db username and password in mask mode. (#4908) --- .../org/apache/hadoop/conf/ConfServlet.java | 2 +- .../apache/hadoop/conf/ConfigRedactor.java | 16 +++++ .../org/apache/hadoop/conf/Configuration.java | 28 +++++--- .../fs/CommonConfigurationKeysPublic.java | 1 + .../apache/hadoop/conf/TestConfServlet.java | 65 +++++++++++++++++++ 5 files changed, 102 insertions(+), 10 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfServlet.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfServlet.java index 67d0e63013..b427038fdd 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfServlet.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfServlet.java @@ -98,7 +98,7 @@ static void writeResponse(Configuration conf, if (FORMAT_JSON.equals(format)) { Configuration.dumpConfiguration(conf, propertyName, out); } else if (FORMAT_XML.equals(format)) { - conf.writeXml(propertyName, out); + conf.writeXml(propertyName, out, conf); } else { throw new BadFormatException("Bad format: " + format); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfigRedactor.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfigRedactor.java index 881a2ce811..1e74077c13 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfigRedactor.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfigRedactor.java @@ -37,6 +37,7 @@ public class ConfigRedactor { private static final String REDACTED_TEXT = ""; + private static final String REDACTED_XML = "******"; private List compiledPatterns; @@ -84,4 +85,19 @@ private boolean configIsSensitive(String key) { } return false; } + + /** + * Given a key / value pair, decides whether or not to redact and returns + * either the original value or text indicating it has been redacted. + * + * @param key param key. + * @param value param value, will return if conditions permit. + * @return Original value, or text indicating it has been redacted + */ + public String redactXml(String key, String value) { + if (configIsSensitive(key)) { + return REDACTED_XML; + } + return value; + } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java index d8ceb58aba..0a1e8868e3 100755 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java @@ -3593,11 +3593,13 @@ public void writeXml(Writer out) throws IOException { * * @param propertyName xml property name. * @param out the writer to write to. + * @param config configuration. * @throws IOException raised on errors performing I/O. */ - public void writeXml(@Nullable String propertyName, Writer out) + public void writeXml(@Nullable String propertyName, Writer out, Configuration config) throws IOException, IllegalArgumentException { - Document doc = asXmlDocument(propertyName); + ConfigRedactor redactor = config != null ? new ConfigRedactor(this) : null; + Document doc = asXmlDocument(propertyName, redactor); try { DOMSource source = new DOMSource(doc); @@ -3614,11 +3616,16 @@ public void writeXml(@Nullable String propertyName, Writer out) } } + public void writeXml(@Nullable String propertyName, Writer out) + throws IOException, IllegalArgumentException { + writeXml(propertyName, out, null); + } + /** * Return the XML DOM corresponding to this Configuration. */ - private synchronized Document asXmlDocument(@Nullable String propertyName) - throws IOException, IllegalArgumentException { + private synchronized Document asXmlDocument(@Nullable String propertyName, + ConfigRedactor redactor) throws IOException, IllegalArgumentException { Document doc; try { doc = DocumentBuilderFactory @@ -3641,13 +3648,13 @@ private synchronized Document asXmlDocument(@Nullable String propertyName) propertyName + " not found"); } else { // given property is found, write single property - appendXMLProperty(doc, conf, propertyName); + appendXMLProperty(doc, conf, propertyName, redactor); conf.appendChild(doc.createTextNode("\n")); } } else { // append all elements for (Enumeration e = properties.keys(); e.hasMoreElements();) { - appendXMLProperty(doc, conf, (String)e.nextElement()); + appendXMLProperty(doc, conf, (String)e.nextElement(), redactor); conf.appendChild(doc.createTextNode("\n")); } } @@ -3663,7 +3670,7 @@ private synchronized Document asXmlDocument(@Nullable String propertyName) * @param propertyName */ private synchronized void appendXMLProperty(Document doc, Element conf, - String propertyName) { + String propertyName, ConfigRedactor redactor) { // skip writing if given property name is empty or null if (!Strings.isNullOrEmpty(propertyName)) { String value = properties.getProperty(propertyName); @@ -3676,8 +3683,11 @@ private synchronized void appendXMLProperty(Document doc, Element conf, propNode.appendChild(nameNode); Element valueNode = doc.createElement("value"); - valueNode.appendChild(doc.createTextNode( - properties.getProperty(propertyName))); + String propertyValue = properties.getProperty(propertyName); + if (redactor != null) { + propertyValue = redactor.redactXml(propertyName, propertyValue); + } + valueNode.appendChild(doc.createTextNode(propertyValue)); propNode.appendChild(valueNode); Element finalNode = doc.createElement("final"); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java index 7a458e8f3f..67cd81ee91 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java @@ -1000,6 +1000,7 @@ public class CommonConfigurationKeysPublic { String.join(",", "secret$", "password$", + "username$", "ssl.keystore.pass$", "fs.s3.*[Ss]ecret.?[Kk]ey", "fs.s3a.*.server-side-encryption.key", diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfServlet.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfServlet.java index eae9a1fc74..9d7f425597 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfServlet.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfServlet.java @@ -59,6 +59,7 @@ public class TestConfServlet { new HashMap(); private static final Map TEST_FORMATS = new HashMap(); + private static final Map MASK_PROPERTIES = new HashMap<>(); @BeforeClass public static void initTestProperties() { @@ -67,6 +68,8 @@ public static void initTestProperties() { TEST_PROPERTIES.put("test.key3", "value3"); TEST_FORMATS.put(ConfServlet.FORMAT_XML, "application/xml"); TEST_FORMATS.put(ConfServlet.FORMAT_JSON, "application/json"); + MASK_PROPERTIES.put("yarn.federation.state-store.sql.username", "admin"); + MASK_PROPERTIES.put("yarn.federation.state-store.sql.password", "123456"); } private Configuration getTestConf() { @@ -80,6 +83,9 @@ private Configuration getMultiPropertiesConf() { for(String key : TEST_PROPERTIES.keySet()) { testConf.set(key, TEST_PROPERTIES.get(key)); } + for(String key : MASK_PROPERTIES.keySet()) { + testConf.set(key, MASK_PROPERTIES.get(key)); + } return testConf; } @@ -247,4 +253,63 @@ public void testBadFormat() throws Exception { } assertEquals("", sw.toString()); } + + private void verifyReplaceProperty(Configuration conf, String format, + String propertyName) throws Exception { + StringWriter sw = null; + PrintWriter pw = null; + ConfServlet service = null; + try { + service = new ConfServlet(); + ServletConfig servletConf = mock(ServletConfig.class); + ServletContext context = mock(ServletContext.class); + service.init(servletConf); + when(context.getAttribute(HttpServer2.CONF_CONTEXT_ATTRIBUTE)).thenReturn(conf); + when(service.getServletContext()).thenReturn(context); + + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getHeader(HttpHeaders.ACCEPT)).thenReturn(TEST_FORMATS.get(format)); + when(request.getParameter("name")).thenReturn(propertyName); + + HttpServletResponse response = mock(HttpServletResponse.class); + sw = new StringWriter(); + pw = new PrintWriter(sw); + when(response.getWriter()).thenReturn(pw); + + // response request + service.doGet(request, response); + String result = sw.toString().trim(); + + // For example, for the property yarn.federation.state-store.sql.username, + // we set the value to test-user, + // which should be replaced by a mask, which should be ****** + // MASK_PROPERTIES.get("property yarn.federation.state-store.sql.username") + // is the value before replacement, test-user + // result contains the replaced value, which should be ****** + assertTrue(result.contains(propertyName)); + assertFalse(result.contains(MASK_PROPERTIES.get(propertyName))); + + } finally { + if (sw != null) { + sw.close(); + } + if (pw != null) { + pw.close(); + } + if (service != null) { + service.destroy(); + } + } + } + + @Test + public void testReplaceProperty() throws Exception { + Configuration configurations = getMultiPropertiesConf(); + + for(String format : TEST_FORMATS.keySet()) { + for(String key : MASK_PROPERTIES.keySet()) { + verifyReplaceProperty(configurations, format, key); + } + } + } } \ No newline at end of file