You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by el...@apache.org on 2017/06/21 15:36:47 UTC

[3/6] accumulo git commit: ACCUMULO-4660 sanitize incoming values from HTTP parameters

ACCUMULO-4660 sanitize incoming values from HTTP parameters

By only accepting alphabetical data, we can be reasonable certain
about what we can and cannot safely do with that data.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/6f187823
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/6f187823
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/6f187823

Branch: refs/heads/master
Commit: 6f187823e4309ed79d6fa3f24b049731126f5263
Parents: bc55724
Author: Josh Elser <el...@apache.org>
Authored: Tue Jun 20 19:08:38 2017 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Tue Jun 20 19:33:08 2017 -0400

----------------------------------------------------------------------
 .../main/asciidoc/chapters/administration.txt   |  5 ++
 server/monitor/pom.xml                          |  5 ++
 .../accumulo/monitor/servlets/trace/Basic.java  |  2 +-
 .../monitor/servlets/trace/BasicTest.java       | 55 ++++++++++++++++++++
 4 files changed, 66 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/6f187823/docs/src/main/asciidoc/chapters/administration.txt
----------------------------------------------------------------------
diff --git a/docs/src/main/asciidoc/chapters/administration.txt b/docs/src/main/asciidoc/chapters/administration.txt
index 54cc1e1..8d0e03a 100644
--- a/docs/src/main/asciidoc/chapters/administration.txt
+++ b/docs/src/main/asciidoc/chapters/administration.txt
@@ -501,6 +501,11 @@ The Traces page displays data for recent traces performed (see the following sec
 The Recent Logs page displays warning and error logs forwarded to the monitor from all Accumulo processes.
 Also, the XML and JSON links provide metrics in XML and JSON formats, respectively.
 
+The Accumulo monitor does a best-effort to not display any sensitive information to users; however,
+the monitor is intended to be a tool used with care. It is not a production-grade webservice. It is
+a good idea to whitelist access to the monitor via an authentication proxy or firewall. It
+is strongly recommended that the Monitor is not exposed to any publicly-accessible networks.
+
 ==== SSL
 SSL may be enabled for the monitor page by setting the following properties in the +accumulo-site.xml+ file:
 

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6f187823/server/monitor/pom.xml
----------------------------------------------------------------------
diff --git a/server/monitor/pom.xml b/server/monitor/pom.xml
index 7312907..1724078 100644
--- a/server/monitor/pom.xml
+++ b/server/monitor/pom.xml
@@ -128,6 +128,11 @@
       <artifactId>junit</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.easymock</groupId>
+      <artifactId>easymock</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
   <build>
     <pluginManagement>

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6f187823/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/Basic.java
----------------------------------------------------------------------
diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/Basic.java b/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/Basic.java
index 712b273..c98aeb6 100644
--- a/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/Basic.java
+++ b/server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/Basic.java
@@ -50,7 +50,7 @@ abstract class Basic extends BasicServlet {
   private static final long serialVersionUID = 1L;
 
   public static String getStringParameter(HttpServletRequest req, String name, String defaultValue) {
-    String result = req.getParameter(name);
+    String result = req.getParameter(name).replaceAll("[^A-Za-z]", "");
     if (result == null) {
       return defaultValue;
     }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6f187823/server/monitor/src/test/java/org/apache/accumulo/monitor/servlets/trace/BasicTest.java
----------------------------------------------------------------------
diff --git a/server/monitor/src/test/java/org/apache/accumulo/monitor/servlets/trace/BasicTest.java b/server/monitor/src/test/java/org/apache/accumulo/monitor/servlets/trace/BasicTest.java
new file mode 100644
index 0000000..720ff53
--- /dev/null
+++ b/server/monitor/src/test/java/org/apache/accumulo/monitor/servlets/trace/BasicTest.java
@@ -0,0 +1,55 @@
+/*
+ * 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.accumulo.monitor.servlets.trace;
+
+import static org.junit.Assert.*;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.easymock.EasyMock;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Test class for {@link Basic}.
+ */
+public class BasicTest {
+  private HttpServletRequest request;
+
+  @Before
+  public void setupMocks() {
+    request = EasyMock.createMock(HttpServletRequest.class);
+  }
+
+  @Test
+  public void testSanitizedStringParameters() {
+    final String safeKey = "safeKey";
+    final String bogusKey = "bogusKey";
+    final String safeValue = "value";
+    final String bogusValue = "value<script>alert('ohnoes');</script>";
+    final String sanitizedBogusValue = "valuescriptalertohnoesscript";
+
+    EasyMock.expect(request.getParameter(safeKey)).andReturn(safeValue).atLeastOnce();
+    EasyMock.expect(request.getParameter(bogusKey)).andReturn(bogusValue).atLeastOnce();
+    EasyMock.replay(request);
+    String actualValue = Basic.getStringParameter(request, safeKey, null);
+    assertEquals(safeValue, actualValue);
+    actualValue = Basic.getStringParameter(request, bogusKey, null);
+    assertEquals(sanitizedBogusValue, actualValue);
+    EasyMock.verify(request);
+  }
+}