From da950b9be21f0bad1616b9327890e17e405a0724 Mon Sep 17 00:00:00 2001 From: Bryan Beaudreault Date: Wed, 4 Aug 2021 21:45:47 -0400 Subject: [PATCH] HBASE-26160: Configurable disallowlist for live editing of loglevels (#3549) Signed-off-by: Wei-Chiu Chuang --- .../hadoop/hbase/http/log/LogLevel.java | 47 ++++++++++++++++--- .../hadoop/hbase/http/log/TestLogLevel.java | 40 +++++++++++++--- 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java index 91b2615b81b..611316d9ec6 100644 --- a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java +++ b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java @@ -22,8 +22,8 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintWriter; +import java.net.HttpURLConnection; import java.net.URL; -import java.net.URLConnection; import java.nio.charset.StandardCharsets; import java.util.Objects; import java.util.regex.Pattern; @@ -41,6 +41,7 @@ import org.apache.hadoop.hbase.logging.Log4jUtils; import org.apache.hadoop.security.authentication.client.AuthenticatedURL; import org.apache.hadoop.security.authentication.client.KerberosAuthenticator; import org.apache.hadoop.security.ssl.SSLFactory; +import org.apache.hadoop.util.HttpExceptionUtils; import org.apache.hadoop.util.ServletUtil; import org.apache.hadoop.util.Tool; import org.apache.yetus.audience.InterfaceAudience; @@ -59,6 +60,8 @@ public final class LogLevel { public static final String PROTOCOL_HTTP = "http"; public static final String PROTOCOL_HTTPS = "https"; + public static final String READONLY_LOGGERS_CONF_KEY = "hbase.ui.logLevels.readonly.loggers"; + /** * A command line implementation */ @@ -247,11 +250,11 @@ public final class LogLevel { * @return a connected connection * @throws Exception if it can not establish a connection. */ - private URLConnection connect(URL url) throws Exception { + private HttpURLConnection connect(URL url) throws Exception { AuthenticatedURL.Token token = new AuthenticatedURL.Token(); AuthenticatedURL aUrl; SSLFactory clientSslFactory; - URLConnection connection; + HttpURLConnection connection; // If https is chosen, configures SSL client. if (PROTOCOL_HTTPS.equals(url.getProtocol())) { clientSslFactory = new SSLFactory(SSLFactory.Mode.CLIENT, this.getConf()); @@ -280,7 +283,9 @@ public final class LogLevel { URL url = new URL(urlString); System.out.println("Connecting to " + url); - URLConnection connection = connect(url); + HttpURLConnection connection = connect(url); + + HttpExceptionUtils.validateResponse(connection, 200); // read from the servlet @@ -317,8 +322,10 @@ public final class LogLevel { Configuration conf = (Configuration) getServletContext().getAttribute( HttpServer.CONF_CONTEXT_ATTRIBUTE); if (conf.getBoolean("hbase.master.ui.readonly", false)) { - response.sendError(HttpServletResponse.SC_FORBIDDEN, "Modification of HBase via" - + " the UI is disallowed in configuration."); + sendError( + response, + HttpServletResponse.SC_FORBIDDEN, + "Modification of HBase via the UI is disallowed in configuration."); return; } response.setContentType("text/html"); @@ -336,6 +343,8 @@ public final class LogLevel { String logName = ServletUtil.getParameter(request, "log"); String level = ServletUtil.getParameter(request, "level"); + String[] readOnlyLogLevels = conf.getStrings(READONLY_LOGGERS_CONF_KEY); + if (logName != null) { out.println("

Results:

"); out.println(MARKER @@ -345,6 +354,14 @@ public final class LogLevel { out.println(MARKER + "Log Class: " + log.getClass().getName() +"
"); if (level != null) { + if (!isLogLevelChangeAllowed(logName, readOnlyLogLevels)) { + sendError( + response, + HttpServletResponse.SC_PRECONDITION_FAILED, + "Modification of logger " + logName + " is disallowed in configuration."); + return; + } + out.println(MARKER + "Submitted Level: " + level + "
"); } process(log, level, out); @@ -360,6 +377,24 @@ public final class LogLevel { out.close(); } + private boolean isLogLevelChangeAllowed(String logger, String[] readOnlyLogLevels) { + if (readOnlyLogLevels == null) { + return true; + } + for (String readOnlyLogLevel : readOnlyLogLevels) { + if (logger.startsWith(readOnlyLogLevel)) { + return false; + } + } + return true; + } + + private void sendError(HttpServletResponse response, int code, String message) + throws IOException { + response.setStatus(code, message); + response.sendError(code, message); + } + static final String FORMS = "
\n" + "
\n" + "
\n" + "

Get/Set Log Level

\n" + "
\n" + "
\n" + "Actions:" + "

" diff --git a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/log/TestLogLevel.java b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/log/TestLogLevel.java index 421cbbde693..acbcb55d5db 100644 --- a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/log/TestLogLevel.java +++ b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/log/TestLogLevel.java @@ -24,12 +24,14 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.File; +import java.io.IOException; import java.net.BindException; import java.net.SocketException; import java.net.URI; import java.security.PrivilegedExceptionAction; import java.util.Properties; import javax.net.ssl.SSLException; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.FileUtils; import org.apache.hadoop.HadoopIllegalArgumentException; import org.apache.hadoop.conf.Configuration; @@ -73,6 +75,8 @@ public class TestLogLevel { private static Configuration clientConf; private static Configuration sslConf; private static final String logName = TestLogLevel.class.getName(); + private static final String protectedPrefix = "protected"; + private static final String protectedLogName = protectedPrefix + "." + logName; private static final org.apache.logging.log4j.Logger log = org.apache.logging.log4j.LogManager.getLogger(logName); private final static String PRINCIPAL = "loglevel.principal"; @@ -89,6 +93,7 @@ public class TestLogLevel { @BeforeClass public static void setUp() throws Exception { serverConf = new Configuration(); + serverConf.setStrings(LogLevel.READONLY_LOGGERS_CONF_KEY, protectedPrefix); HTU = new HBaseCommonTestingUtil(serverConf); File keystoreDir = new File(HTU.getDataTestDir("keystore").toString()); @@ -259,9 +264,17 @@ public class TestLogLevel { private void testDynamicLogLevel(final String bindProtocol, final String connectProtocol, final boolean isSpnego) throws Exception { testDynamicLogLevel(bindProtocol, connectProtocol, isSpnego, + logName, org.apache.logging.log4j.Level.DEBUG.toString()); } + private void testDynamicLogLevel(final String bindProtocol, final String connectProtocol, + final boolean isSpnego, final String newLevel) throws Exception { + testDynamicLogLevel(bindProtocol, connectProtocol, isSpnego, + logName, + newLevel); + } + /** * Run both client and server using the given protocol. * @param bindProtocol specify either http or https for server @@ -270,13 +283,14 @@ public class TestLogLevel { * @throws Exception if client can't accesss server. */ private void testDynamicLogLevel(final String bindProtocol, final String connectProtocol, - final boolean isSpnego, final String newLevel) throws Exception { + final boolean isSpnego, final String loggerName, final String newLevel) throws Exception { if (!LogLevel.isValidProtocol(bindProtocol)) { throw new Exception("Invalid server protocol " + bindProtocol); } if (!LogLevel.isValidProtocol(connectProtocol)) { throw new Exception("Invalid client protocol " + connectProtocol); } + org.apache.logging.log4j.Logger log = org.apache.logging.log4j.LogManager.getLogger(loggerName); org.apache.logging.log4j.Level oldLevel = log.getLevel(); assertNotEquals("Get default Log Level which shouldn't be ERROR.", org.apache.logging.log4j.Level.ERROR, oldLevel); @@ -305,8 +319,8 @@ public class TestLogLevel { try { clientUGI.doAs((PrivilegedExceptionAction) () -> { // client command line - getLevel(connectProtocol, authority); - setLevel(connectProtocol, authority, newLevel); + getLevel(connectProtocol, authority, loggerName); + setLevel(connectProtocol, authority, loggerName, newLevel); return null; }); } finally { @@ -324,7 +338,7 @@ public class TestLogLevel { * @param authority daemon's web UI address * @throws Exception if unable to connect */ - private void getLevel(String protocol, String authority) throws Exception { + private void getLevel(String protocol, String authority, String logName) throws Exception { String[] getLevelArgs = { "-getlevel", authority, logName, "-protocol", protocol }; CLI cli = new CLI(protocol.equalsIgnoreCase("https") ? sslConf : clientConf); cli.run(getLevelArgs); @@ -336,13 +350,27 @@ public class TestLogLevel { * @param authority daemon's web UI address * @throws Exception if unable to run or log level does not change as expected */ - private void setLevel(String protocol, String authority, String newLevel) throws Exception { + private void setLevel(String protocol, String authority, String logName, String newLevel) throws Exception { String[] setLevelArgs = { "-setlevel", authority, logName, newLevel, "-protocol", protocol }; CLI cli = new CLI(protocol.equalsIgnoreCase("https") ? sslConf : clientConf); cli.run(setLevelArgs); + org.apache.logging.log4j.Logger logger = org.apache.logging.log4j.LogManager.getLogger(logName); + assertEquals("new level not equal to expected: ", newLevel.toUpperCase(), - log.getLevel().toString()); + logger.getLevel().toString()); + } + + @Test + public void testSettingProtectedLogLevel() throws Exception { + try { + testDynamicLogLevel(LogLevel.PROTOCOL_HTTP, LogLevel.PROTOCOL_HTTP, true, protectedLogName, + "DEBUG"); + fail("Expected IO exception due to protected logger"); + } catch (IOException e) { + assertTrue(e.getMessage().contains("" + HttpServletResponse.SC_PRECONDITION_FAILED)); + assertTrue(e.getMessage().contains("Modification of logger " + protectedLogName + " is disallowed in configuration.")); + } } /**