CVE-2017-5646

@@ -36,10 +36,12 @@
 import java.nio.charset.Charset;
 import java.security.Principal;
 import java.util.ArrayList;
+import java.util.List;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 
 public class IdentityAsserterHttpServletRequestWrapper extends HttpServletRequestWrapper {
@@ -121,7 +123,7 @@ private Map<String, String[]> getParams( String qString ) {
   private Map<String, String[]> getParams() {
     return getParams( super.getQueryString() );
   }
-  
+
   @Override
   public String getQueryString() {
     String q = null;
@@ -135,13 +137,15 @@ public String getQueryString() {
     al.add(username);
     String[] a = { "" };
 
+    List<String> principalParamNames = getImpersonationParamNames();
+    params = scrubOfExistingPrincipalParams(params, principalParamNames);
+
     if ("true".equals(System.getProperty(GatewayConfig.HADOOP_KERBEROS_SECURED))) {
       params.put(DOAS_PRINCIPAL_PARAM, al.toArray(a));
-      params.remove(PRINCIPAL_PARAM);
     } else {
       params.put(PRINCIPAL_PARAM, al.toArray(a));
     }
-    
+
     String encoding = getCharacterEncoding();
     if (encoding == null) {
       encoding = Charset.defaultCharset().name();
@@ -150,6 +154,32 @@ public String getQueryString() {
     return q;
   }
 
+  private List<String> getImpersonationParamNames() {
+    // TODO: let's have service definitions register their impersonation
+    // params in a future release and get this list from a central registry.
+    // This will provide better coverage of protection by removing any
+    // prepopulated impersonation params.
+    ArrayList<String> principalParamNames = new ArrayList<String>();
+    principalParamNames.add(DOAS_PRINCIPAL_PARAM);
+    principalParamNames.add(PRINCIPAL_PARAM);
+    return principalParamNames;
+  }
+
+  private Map<String, String[]> scrubOfExistingPrincipalParams(
+      Map<String, String[]> params, List<String> principalParamNames) {
+    HashSet<String> remove = new HashSet<String>();
+    for (String paramKey : params.keySet()) {
+      for (String p : principalParamNames) {
+        if (p.equalsIgnoreCase(paramKey)) {
+          remove.add(paramKey);
+          log.possibleIdentitySpoofingAttempt(paramKey);
+        }
+      }
+    }
+    params.keySet().removeAll(remove);
+    return params;
+  }
+
   @Override
   public int getContentLength() {
     int len;

@@ -18,12 +18,14 @@
 package org.apache.hadoop.gateway.identityasserter.filter;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.hadoop.gateway.config.GatewayConfig;
 import org.apache.hadoop.gateway.identityasserter.common.filter.IdentityAsserterHttpServletRequestWrapper;
 import org.apache.hadoop.test.category.FastTests;
 import org.apache.hadoop.test.category.UnitTests;
 import org.apache.hadoop.test.mock.MockHttpServletRequest;
 import org.apache.hadoop.test.mock.MockServletInputStream;
 import org.junit.Test;
+import org.junit.After;
 import org.junit.experimental.categories.Category;
 
 import java.io.ByteArrayInputStream;
@@ -38,6 +40,11 @@
 @Category( { UnitTests.class, FastTests.class } )
 public class IdentityAssertionHttpServletRequestWrapperTest {
 
+  @After
+  public void resetSystemProps() {
+    System.setProperty(GatewayConfig.HADOOP_KERBEROS_SECURED, "false");
+  }
+
   @Test
   public void testInsertUserNameInPostMethod() throws IOException {
     String inputBody = "jar=%2Ftmp%2FGatewayWebHdfsFuncTest%2FtestJavaMapReduceViaWebHCat%2Fhadoop-examples.jar&class=org.apache.org.apache.hadoop.examples.WordCount&arg=%2Ftmp%2FGatewayWebHdfsFuncTest%2FtestJavaMapReduceViaTempleton%2Finput&arg=%2Ftmp%2FGatewayWebHdfsFuncTest%2FtestJavaMapReduceViaTempleton%2Foutput";
@@ -143,6 +150,19 @@ public void testInsertUserNameInQueryString() {
     assertThat( output, containsString( "user.name=output-user" ) );
   }
 
+  @Test
+  public void testInsertDoAsInQueryString() {
+    System.setProperty(GatewayConfig.HADOOP_KERBEROS_SECURED, "true");
+    MockHttpServletRequest request = new MockHttpServletRequest();
+    request.setQueryString("op=LISTSTATUS&user.name=jack&User.Name=jill&DOas=admin&doas=root");
+
+    IdentityAsserterHttpServletRequestWrapper wrapper
+        = new IdentityAsserterHttpServletRequestWrapper( request, "output-user" );
+
+    String output = wrapper.getQueryString();
+    assertThat(output, is("op=LISTSTATUS&doAs=output-user"));
+  }
+
   @Test
   public void testInsertUserNameInNullQueryString() {
     String input = null;

@@ -67,4 +67,7 @@ public interface SpiGatewayMessages {
 
   @Message( level = MessageLevel.DEBUG, text = "Inbound response entity content type: {0}" )
   void inboundResponseEntityContentType( String fullContentType );
+
+  @Message( level = MessageLevel.WARN, text = "Possible identity spoofing attempt - impersonation parameter removed: {0}" )
+  void possibleIdentitySpoofingAttempt( String impersonationParam );
 }