From 4ae539a753aa867c59356ff9573b39dae188edd1 Mon Sep 17 00:00:00 2001 From: Bryan Beaudreault Date: Sun, 1 Aug 2021 09:55:20 -0400 Subject: [PATCH] HBASE-26160: Configurable disallowlist for live editing of loglevels --- .../hadoop/hbase/http/log/LogLevel.java | 47 ++++++++++++++++--- .../hadoop/hbase/http/log/TestLogLevel.java | 40 +++++++++++++--- 2 files changed, 74 insertions(+), 13 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 1fcfa1390c2c..819581735a89 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.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.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; @@ -60,6 +61,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 */ @@ -248,11 +251,11 @@ private void doSetLevel() throws Exception { * @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()); @@ -281,7 +284,9 @@ private void process(String urlString) throws Exception { 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 @@ -319,8 +324,10 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) 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"); @@ -338,6 +345,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) 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 @@ -347,6 +356,14 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) 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); @@ -362,6 +379,24 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) 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 2c5d0c42b6da..b52129ccdbf3 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 @@ -23,12 +23,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; @@ -75,6 +77,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 Logger log = LogManager.getLogger(logName); private final static String PRINCIPAL = "loglevel.principal"; private final static String KEYTAB = "loglevel.keytab"; @@ -90,6 +94,7 @@ public class TestLogLevel { @BeforeClass public static void setUp() throws Exception { serverConf = new Configuration(); + serverConf.setStrings(LogLevel.READONLY_LOGGERS_CONF_KEY, protectedPrefix); HTU = new HBaseCommonTestingUtility(serverConf); File keystoreDir = new File(HTU.getDataTestDir("keystore").toString()); @@ -276,7 +281,13 @@ private HttpServer createServer(String protocol, boolean isSpnego) private void testDynamicLogLevel(final String bindProtocol, final String connectProtocol, final boolean isSpnego) throws Exception { - testDynamicLogLevel(bindProtocol, connectProtocol, isSpnego, Level.DEBUG.toString()); + testDynamicLogLevel(bindProtocol, connectProtocol, isSpnego, logName, 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); } /** @@ -288,7 +299,7 @@ private void testDynamicLogLevel(final String bindProtocol, final String connect * @throws Exception if client can't accesss server. */ private void testDynamicLogLevel(final String bindProtocol, final String connectProtocol, - final boolean isSpnego, final String newLevel) + final boolean isSpnego, final String loggerName, final String newLevel) throws Exception { if (!LogLevel.isValidProtocol(bindProtocol)) { throw new Exception("Invalid server protocol " + bindProtocol); @@ -296,7 +307,8 @@ private void testDynamicLogLevel(final String bindProtocol, final String connect if (!LogLevel.isValidProtocol(connectProtocol)) { throw new Exception("Invalid client protocol " + connectProtocol); } - Level oldLevel = log.getEffectiveLevel(); + Logger log = LogManager.getLogger(loggerName); + Level oldLevel = log.getLevel(); assertNotEquals("Get default Log Level which shouldn't be ERROR.", Level.ERROR, oldLevel); @@ -324,8 +336,8 @@ private void testDynamicLogLevel(final String bindProtocol, final String connect 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 { @@ -345,7 +357,7 @@ private void testDynamicLogLevel(final String bindProtocol, final String connect * @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); @@ -359,16 +371,30 @@ private void getLevel(String protocol, String authority) throws Exception { * @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) + 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); + Logger log = LogManager.getLogger(logName); + assertEquals("new level not equal to expected: ", newLevel.toUpperCase(), log.getEffectiveLevel().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.")); + } + } + /** * Test setting log level to "Info". *