From 2ce4ebdb05172097b01ab8f03ed5a76b0494e7d5 Mon Sep 17 00:00:00 2001 From: Michael Han Date: Mon, 6 Mar 2017 12:58:01 -0800 Subject: [PATCH 1/2] ZOOKEEPER-2693: DOS attack on wchp/wchc four letter words (4lw). Initial commit for branch-3.4. --- .../content/xdocs/zookeeperAdmin.xml | 34 +++ .../zookeeper/server/NIOServerCnxn.java | 33 ++- .../zookeeper/server/NettyServerCnxn.java | 32 ++- .../apache/zookeeper/server/ServerCnxn.java | 94 ++++++- .../test/org/apache/zookeeper/ZKTestCase.java | 4 + .../test/FourLetterWordsWhiteListTest.java | 252 ++++++++++++++++++ 6 files changed, 439 insertions(+), 10 deletions(-) create mode 100644 src/java/test/org/apache/zookeeper/test/FourLetterWordsWhiteListTest.java diff --git a/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml b/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml index 5aefa9a1168..56697b923a9 100644 --- a/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml +++ b/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml @@ -1042,6 +1042,40 @@ server.3=zoo3:2888:3888 + + + 4lw.commands.whitelist + + + (Java system property: zookeeper.4lw.commands.whitelist) + + New in 3.4.10: + This property contains a list of comma separated + Four Letter Words commands. It is introduced + to provide fine grained control over the set of commands ZooKeeper can execute, + so users can turn off certain commands if necessary. + By default it contains all supported four letter word commands except "wchp" and "wchc", + if the property is not specified. If the property is specified, then only commands listed + in the whitelist are enabled. + + + Here's an example of the configuration that enables stat, ruok, conf, and isro + command while disabling the rest of Four Letter Words command: + + 4lw.commands.whitelist=stat, ruok, conf, isro + + + Users can also use asterisk option so they don't have to include every command one by one in the list. + As an example, this will enable all four letter word commands: + + + 4lw.commands.whitelist=* + + + + + diff --git a/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java b/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java index 4ea7fb2736d..456d4c2f14b 100644 --- a/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java +++ b/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java @@ -825,18 +825,30 @@ public void commandRun() { } } + private class NopCommand extends CommandThread { + private String msg; + + public NopCommand(PrintWriter pw, String msg) { + super(pw); + this.msg = msg; + } + + @Override + public void commandRun() { + pw.println(msg); + } + } + /** Return if four letter word found and responded to, otw false **/ private boolean checkFourLetterWord(final SelectionKey k, final int len) throws IOException { // We take advantage of the limited size of the length to look // for cmds. They are all 4-bytes which fits inside of an int - String cmd = cmd2String.get(len); - if (cmd == null) { + if (!ServerCnxn.isKnown(len)) { return false; } - LOG.info("Processing " + cmd + " command from " - + sock.socket().getRemoteSocketAddress()); + packetReceived(); /** cancel the selection key to remove the socket handling @@ -858,6 +870,19 @@ private boolean checkFourLetterWord(final SelectionKey k, final int len) final PrintWriter pwriter = new PrintWriter( new BufferedWriter(new SendBufferWriter())); + + String cmd = ServerCnxn.getCommandString(len); + // ZOOKEEPER-2693: don't execute 4lw if it's not enabled. + if (!ServerCnxn.isEnabled(cmd)) { + LOG.debug("Command {} is not executed because it is not in the whitelist.", cmd); + NopCommand nopCmd = new NopCommand(pwriter, cmd + " is not executed because it is not in the whitelist."); + nopCmd.start(); + return true; + } + + LOG.info("Processing " + cmd + " command from " + + sock.socket().getRemoteSocketAddress()); + if (len == ruokCmd) { RuokCommand ruok = new RuokCommand(pwriter); ruok.start(); diff --git a/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java index 32fc371e6de..203f0e60b86 100644 --- a/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java +++ b/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java @@ -618,23 +618,47 @@ public void commandRun() { } } + private class NopCommand extends CommandThread { + private String msg; + + public NopCommand(PrintWriter pw, String msg) { + super(pw); + this.msg = msg; + } + + @Override + public void commandRun() { + pw.println(msg); + } + } + /** Return if four letter word found and responded to, otw false **/ private boolean checkFourLetterWord(final Channel channel, ChannelBuffer message, final int len) throws IOException { // We take advantage of the limited size of the length to look // for cmds. They are all 4-bytes which fits inside of an int - String cmd = cmd2String.get(len); - if (cmd == null) { + if (!ServerCnxn.isKnown(len)) { return false; } + channel.setInterestOps(0).awaitUninterruptibly(); - LOG.info("Processing " + cmd + " command from " - + channel.getRemoteAddress()); packetReceived(); final PrintWriter pwriter = new PrintWriter( new BufferedWriter(new SendBufferWriter())); + + String cmd = ServerCnxn.getCommandString(len); + // ZOOKEEPER-2693: don't execute 4lw if it's not enabled. + if (!ServerCnxn.isEnabled(cmd)) { + LOG.debug("Command {} is not executed because it is not in the whitelist.", cmd); + NopCommand nopCmd = new NopCommand(pwriter, cmd + " is not executed because it is not in the whitelist."); + nopCmd.start(); + return true; + } + + LOG.info("Processing " + cmd + " command from " + channel.getRemoteAddress()); + if (len == ruokCmd) { RuokCommand ruok = new RuokCommand(pwriter); ruok.start(); diff --git a/src/java/main/org/apache/zookeeper/server/ServerCnxn.java b/src/java/main/org/apache/zookeeper/server/ServerCnxn.java index 6dd509b167d..6b93e93f140 100644 --- a/src/java/main/org/apache/zookeeper/server/ServerCnxn.java +++ b/src/java/main/org/apache/zookeeper/server/ServerCnxn.java @@ -26,10 +26,17 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Date; +import java.util.Map; import java.util.HashMap; +import java.util.Set; +import java.util.HashSet; +import java.util.Arrays; import java.util.List; import java.util.concurrent.atomic.AtomicLong; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import org.apache.jute.Record; import org.apache.zookeeper.WatchedEvent; import org.apache.zookeeper.Watcher; @@ -227,8 +234,91 @@ public String toString() { protected final static int isroCmd = ByteBuffer.wrap("isro".getBytes()) .getInt(); - protected final static HashMap cmd2String = - new HashMap(); + protected final static Map cmd2String = new HashMap(); + + private static final String ZOOKEEPER_4LW_COMMANDS_WHITELIST = "zookeeper.4lw.commands.whitelist"; + + private static final Logger LOG = LoggerFactory.getLogger(ServerCnxn.class); + + private static final Set whiteListedCommands = new HashSet(); + + private static boolean whiteListInitialized = false; + + // @VisibleForTesting + public static void resetWhiteList() { + whiteListInitialized = false; + whiteListedCommands.clear(); + } + + /** + * Return the string representation of the specified command code. + */ + public static String getCommandString(int command) { + return cmd2String.get(command); + } + + /** + * Check if the specified command code is from a known command. + * + * @param command The integer code of command. + * @return true if the specified command is known, false otherwise. + */ + public static boolean isKnown(int command) { + return cmd2String.containsKey(command); + } + + /** + * Check if the specified command is enabled. + * + * In ZOOKEEPER-2693 we introduce a configuration option to only + * allow a specific set of white listed commands to execute. + * A command will only be executed if it is also configured + * in the white list. + * + * @param command The command string. + * @return true if the specified command is enabled. + */ + public static boolean isEnabled(String command) { + if (whiteListInitialized) { + return whiteListedCommands.contains(command); + } + + String commands = System.getProperty(ZOOKEEPER_4LW_COMMANDS_WHITELIST); + if (commands != null) { + String[] list = commands.split(","); + for (String cmd : list) { + if (cmd.trim().equals("*")) { + for (Map.Entry entry : cmd2String.entrySet()) { + whiteListedCommands.add(entry.getValue()); + } + break; + } + if (!cmd.trim().isEmpty()) { + whiteListedCommands.add(cmd.trim()); + } + } + } else { + for (Map.Entry entry : cmd2String.entrySet()) { + String cmd = entry.getValue(); + if (cmd.equals("wchc") || cmd.equals("wchp")) { + // ZOOKEEPER-2693 / disable these exploitable commands by default. + continue; + } + whiteListedCommands.add(cmd); + } + } + + // Readonly mode depends on "isro". + if (System.getProperty("readonlymode.enabled", "false").equals("true")) { + whiteListedCommands.add("isro"); + } + // zkServer.sh depends on "srvr". + whiteListedCommands.add("srvr"); + whiteListInitialized = true; + LOG.info("The list of known four letter word commands is : {}", Arrays.asList(cmd2String)); + LOG.info("The list of enabled four letter word commands is : {}", Arrays.asList(whiteListedCommands)); + return whiteListedCommands.contains(command); + } // specify all of the commands that are available static { diff --git a/src/java/test/org/apache/zookeeper/ZKTestCase.java b/src/java/test/org/apache/zookeeper/ZKTestCase.java index 97e2db6ee21..9098fc47ebf 100644 --- a/src/java/test/org/apache/zookeeper/ZKTestCase.java +++ b/src/java/test/org/apache/zookeeper/ZKTestCase.java @@ -52,6 +52,10 @@ protected String getTestName() { @Override public void starting(FrameworkMethod method) { testName = method.getName(); + // ZOOKEEPER-2693 disables all 4lw by default. + // Here we enable the 4lw which ZooKeeper tests depends. + System.setProperty("zookeeper.4lw.commands.whitelist", "*"); + LOG.info("STARTING " + testName); } diff --git a/src/java/test/org/apache/zookeeper/test/FourLetterWordsWhiteListTest.java b/src/java/test/org/apache/zookeeper/test/FourLetterWordsWhiteListTest.java new file mode 100644 index 00000000000..613346f2ece --- /dev/null +++ b/src/java/test/org/apache/zookeeper/test/FourLetterWordsWhiteListTest.java @@ -0,0 +1,252 @@ +/** + * 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.zookeeper.test; + +import java.io.IOException; + +import org.apache.zookeeper.TestableZooKeeper; +import org.apache.zookeeper.server.ServerCnxn; +import static org.apache.zookeeper.client.FourLetterWordMain.send4LetterWord; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.Timeout; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class FourLetterWordsWhiteListTest extends ClientBase { + protected static final Logger LOG = + LoggerFactory.getLogger(FourLetterWordsTest.class); + + @Rule + public Timeout timeout = new Timeout(30000); + + /* + * ZOOKEEPER-2693: test white list of four letter words. + * For 3.5.x default white list is empty. Verify that is + * the case (except 'stat' command which is enabled in ClientBase + * which other tests depend on.). + */ + @Test(timeout=30000) + public void testFourLetterWordsAllDisabledByDefault() throws Exception { + stopServer(); + ServerCnxn.resetWhiteList(); + System.setProperty("zookeeper.4lw.commands.whitelist", "stat"); + startServer(); + + // Default white list for 3.5.x is empty, so all command should fail. + verifyAllCommandsFail(); + + TestableZooKeeper zk = createClient(); + + verifyAllCommandsFail(); + + zk.getData("/", true, null); + + verifyAllCommandsFail(); + + zk.close(); + + verifyFuzzyMatch("stat", "Outstanding"); + verifyAllCommandsFail(); + } + + @Test(timeout=30000) + public void testFourLetterWordsEnableSomeCommands() throws Exception { + stopServer(); + ServerCnxn.resetWhiteList(); + System.setProperty("zookeeper.4lw.commands.whitelist", "stat, ruok, isro"); + startServer(); + // stat, ruok and isro are white listed. + verifyFuzzyMatch("stat", "Outstanding"); + verifyExactMatch("ruok", "imok"); + verifyExactMatch("isro", "rw"); + + // Rest of commands fail. + verifyExactMatch("conf", generateExpectedMessage("conf")); + verifyExactMatch("cons", generateExpectedMessage("cons")); + verifyExactMatch("crst", generateExpectedMessage("crst")); + verifyExactMatch("dump", generateExpectedMessage("dump")); + verifyExactMatch("envi", generateExpectedMessage("envi")); + verifyExactMatch("gtmk", generateExpectedMessage("gtmk")); + verifyExactMatch("stmk", generateExpectedMessage("stmk")); + verifyExactMatch("srst", generateExpectedMessage("srst")); + verifyExactMatch("wchc", generateExpectedMessage("wchc")); + verifyExactMatch("wchp", generateExpectedMessage("wchp")); + verifyExactMatch("wchs", generateExpectedMessage("wchs")); + verifyExactMatch("mntr", generateExpectedMessage("mntr")); + } + + @Test(timeout=30000) + public void testISROEnabledWhenReadOnlyModeEnabled() throws Exception { + stopServer(); + ServerCnxn.resetWhiteList(); + System.setProperty("zookeeper.4lw.commands.whitelist", "stat"); + System.setProperty("readonlymode.enabled", "true"); + startServer(); + verifyExactMatch("isro", "rw"); + System.clearProperty("readonlymode.enabled"); + } + + @Test(timeout=30000) + public void testFourLetterWordsInvalidConfiguration() throws Exception { + stopServer(); + ServerCnxn.resetWhiteList(); + System.setProperty("zookeeper.4lw.commands.whitelist", "foo bar" + + " foo,,, " + + "bar :.,@#$%^&*() , , , , bar, bar, stat, "); + startServer(); + + // Just make sure we are good when admin made some mistakes in config file. + verifyAllCommandsFail(); + // But still, what's valid in white list will get through. + verifyFuzzyMatch("stat", "Outstanding"); + } + + @Test(timeout=30000) + public void testFourLetterWordsEnableAllCommandsThroughAsterisk() throws Exception { + stopServer(); + ServerCnxn.resetWhiteList(); + System.setProperty("zookeeper.4lw.commands.whitelist", "*"); + startServer(); + verifyAllCommandsSuccess(); + } + + @Test(timeout=30000) + public void testFourLetterWordsEnableAllCommandsThroughExplicitList() throws Exception { + stopServer(); + ServerCnxn.resetWhiteList(); + System.setProperty("zookeeper.4lw.commands.whitelist", + "ruok, envi, conf, stat, srvr, cons, dump," + + "wchs, wchp, wchc, srst, crst, " + + "mntr, gtmk, isro, stmk"); + startServer(); + verifyAllCommandsSuccess(); + } + + private void verifyAllCommandsSuccess() throws Exception { + verifyExactMatch("ruok", "imok"); + verifyFuzzyMatch("envi", "java.version"); + verifyFuzzyMatch("conf", "clientPort"); + verifyFuzzyMatch("stat", "Outstanding"); + verifyFuzzyMatch("srvr", "Outstanding"); + verifyFuzzyMatch("cons", "queued"); + verifyFuzzyMatch("dump", "Session"); + verifyFuzzyMatch("wchs", "watches"); + verifyFuzzyMatch("wchp", ""); + verifyFuzzyMatch("wchc", ""); + + verifyFuzzyMatch("srst", "reset"); + verifyFuzzyMatch("crst", "reset"); + + verifyFuzzyMatch("stat", "Outstanding"); + verifyFuzzyMatch("srvr", "Outstanding"); + verifyFuzzyMatch("cons", "queued"); + verifyFuzzyMatch("gtmk", "306"); + verifyFuzzyMatch("isro", "rw"); + + TestableZooKeeper zk = createClient(); + String sid = getHexSessionId(zk.getSessionId()); + + verifyFuzzyMatch("stat", "queued"); + verifyFuzzyMatch("srvr", "Outstanding"); + verifyFuzzyMatch("cons", sid); + verifyFuzzyMatch("dump", sid); + + zk.getData("/", true, null); + + verifyFuzzyMatch("stat", "queued"); + verifyFuzzyMatch("srvr", "Outstanding"); + verifyFuzzyMatch("cons", sid); + verifyFuzzyMatch("dump", sid); + + verifyFuzzyMatch("wchs", "watching 1"); + verifyFuzzyMatch("wchp", sid); + verifyFuzzyMatch("wchc", sid); + zk.close(); + + verifyExactMatch("ruok", "imok"); + verifyFuzzyMatch("envi", "java.version"); + verifyFuzzyMatch("conf", "clientPort"); + verifyFuzzyMatch("stat", "Outstanding"); + verifyFuzzyMatch("srvr", "Outstanding"); + verifyFuzzyMatch("cons", "queued"); + verifyFuzzyMatch("dump", "Session"); + verifyFuzzyMatch("wchs", "watch"); + verifyFuzzyMatch("wchp", ""); + verifyFuzzyMatch("wchc", ""); + + verifyFuzzyMatch("srst", "reset"); + verifyFuzzyMatch("crst", "reset"); + + verifyFuzzyMatch("stat", "Outstanding"); + verifyFuzzyMatch("srvr", "Outstanding"); + verifyFuzzyMatch("cons", "queued"); + verifyFuzzyMatch("mntr", "zk_server_state\tstandalone"); + verifyFuzzyMatch("mntr", "num_alive_connections"); + verifyFuzzyMatch("stat", "Connections"); + verifyFuzzyMatch("srvr", "Connections"); + } + + private void verifyAllCommandsFail() throws Exception { + verifyExactMatch("ruok", generateExpectedMessage("ruok")); + verifyExactMatch("conf", generateExpectedMessage("conf")); + verifyExactMatch("cons", generateExpectedMessage("cons")); + verifyExactMatch("crst", generateExpectedMessage("crst")); + verifyExactMatch("dump", generateExpectedMessage("dump")); + verifyExactMatch("envi", generateExpectedMessage("envi")); + verifyExactMatch("gtmk", generateExpectedMessage("gtmk")); + verifyExactMatch("stmk", generateExpectedMessage("stmk")); + verifyExactMatch("srst", generateExpectedMessage("srst")); + verifyExactMatch("wchc", generateExpectedMessage("wchc")); + verifyExactMatch("wchp", generateExpectedMessage("wchp")); + verifyExactMatch("wchs", generateExpectedMessage("wchs")); + verifyExactMatch("mntr", generateExpectedMessage("mntr")); + verifyExactMatch("isro", generateExpectedMessage("isro")); + + // srvr is enabled by default due to the sad fact zkServer.sh uses it. + verifyFuzzyMatch("srvr", "Outstanding"); + } + + private void verifyFuzzyMatch(String cmd, String expected) throws IOException { + String resp = sendRequest(cmd); + LOG.info("cmd " + cmd + " expected " + expected + " got " + resp); + Assert.assertTrue(resp.contains(expected)); + } + + private String generateExpectedMessage(String command) { + return command + " is not executed because it is not in the whitelist."; + } + + private void verifyExactMatch(String cmd, String expected) throws IOException { + String resp = sendRequest(cmd); + LOG.info("cmd " + cmd + " expected an exact match of " + expected + "; got " + resp); + Assert.assertTrue(resp.trim().equals(expected)); + } + + private String sendRequest(String cmd) throws IOException { + HostPort hpobj = ClientBase.parseHostPortList(hostPort).get(0); + return send4LetterWord(hpobj.host, hpobj.port, cmd); + } + + private String sendRequest(String cmd, int timeout) throws IOException { + HostPort hpobj = ClientBase.parseHostPortList(hostPort).get(0); + return send4LetterWord(hpobj.host, hpobj.port, cmd, timeout); + } +} From d060ddcfe9f3b24eebd7b2b0742be3070a1f245f Mon Sep 17 00:00:00 2001 From: Michael Han Date: Mon, 6 Mar 2017 19:08:09 -0800 Subject: [PATCH 2/2] update doc. --- .../src/documentation/content/xdocs/zookeeperAdmin.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml b/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml index 56697b923a9..fb00fae2494 100644 --- a/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml +++ b/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml @@ -1701,6 +1701,16 @@ imok usage limit that would cause the system to swap. + + + Publicly accessible deployment + + + A ZooKeeper ensemble is expected to operate in a trusted computing environment. + It is thus recommended to deploy ZooKeeper behind a firewall. + + +