Add interceptor for rejecting TRACE and other invalid verbs

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/RequestTypeEnum.java

@@ -21,5 +21,5 @@ package ca.uhn.fhir.rest.api;
  */
 
 public enum RequestTypeEnum {
-	DELETE, GET, OPTIONS, POST, PUT
+	CONNECT, DELETE, GET, OPTIONS, PATCH, POST, PUT, TRACE, TRACK
 }

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java

@@ -93,6 +93,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 	private static final ExceptionHandlingInterceptor DEFAULT_EXCEPTION_HANDLER = new ExceptionHandlingInterceptor();
 
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(RestfulServer.class);
+
 	private static final long serialVersionUID = 1L;
 	/**
 	 * Requests will have an HttpServletRequest attribute set with this name, containing the servlet
@@ -263,31 +264,6 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 		return resourceMethod;
 	}
 
-	@Override
-	protected void doDelete(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
-		handleRequest(RequestTypeEnum.DELETE, request, response);
-	}
-
-	@Override
-	protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
-		handleRequest(RequestTypeEnum.GET, request, response);
-	}
-
-	@Override
-	protected void doOptions(HttpServletRequest theReq, HttpServletResponse theResp) throws ServletException, IOException {
-		handleRequest(RequestTypeEnum.OPTIONS, theReq, theResp);
-	}
-
-	@Override
-	protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
-		handleRequest(RequestTypeEnum.POST, request, response);
-	}
-
-	@Override
-	protected void doPut(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
-		handleRequest(RequestTypeEnum.PUT, request, response);
-	}
-
 	/**
 	 * Count length of URL string, but treating unescaped sequences (e.g. ' ') as their unescaped equivalent (%20)
 	 */
@@ -1071,7 +1047,8 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 					}
 					String resourceName = getFhirContext().getResourceDefinition(resourceType).getName();
 					if (myTypeToProvider.containsKey(resourceName)) {
-						throw new ConfigurationException("Multiple resource providers return resource type[" + resourceName + "]: First[" + myTypeToProvider.get(resourceName).getClass().getCanonicalName() + "] and Second[" + rsrcProvider.getClass().getCanonicalName() + "]");
+						throw new ConfigurationException("Multiple resource providers return resource type[" + resourceName + "]: First[" + myTypeToProvider.get(resourceName).getClass().getCanonicalName()
+								+ "] and Second[" + rsrcProvider.getClass().getCanonicalName() + "]");
 					}
 					if (!inInit) {
 						myResourceProviders.add(rsrcProvider);
@@ -1183,6 +1160,63 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 		return null;
 	}
 
+	@Override
+	protected void service(HttpServletRequest theReq, HttpServletResponse theResp) throws ServletException, IOException {
+		RequestTypeEnum method;
+		try {
+			method = RequestTypeEnum.valueOf(theReq.getMethod());
+		} catch (IllegalArgumentException e) {
+			super.service(theReq, theResp);
+			return;
+		}
+
+		switch (method) {
+		case DELETE:
+			doDelete(theReq, theResp);
+			break;
+		case GET:
+			doGet(theReq, theResp);
+			break;
+		case OPTIONS:
+			doOptions(theReq, theResp);
+			break;
+		case POST:
+			doPost(theReq, theResp);
+			break;
+		case PUT:
+			doPut(theReq, theResp);
+			break;
+		default:
+			handleRequest(method, theReq, theResp);
+			break;
+		}
+	}
+
+	@Override
+	protected void doDelete(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		handleRequest(RequestTypeEnum.DELETE, request, response);
+	}
+
+	@Override
+	protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		handleRequest(RequestTypeEnum.GET, request, response);
+	}
+
+	@Override
+	protected void doOptions(HttpServletRequest theReq, HttpServletResponse theResp) throws ServletException, IOException {
+		handleRequest(RequestTypeEnum.OPTIONS, theReq, theResp);
+	}
+
+	@Override
+	protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		handleRequest(RequestTypeEnum.POST, request, response);
+	}
+
+	@Override
+	protected void doPut(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		handleRequest(RequestTypeEnum.PUT, request, response);
+	}
+
 	/**
 	 * Sets the profile tagging behaviour for the server. When set to a value other than {@link AddProfileTagEnum#NEVER}
 	 * (which is the default), the server will automatically add a profile tag based on

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/BanUnsupprtedHttpMethodsInterceptor.java

@@ -0,0 +1,39 @@
+package ca.uhn.fhir.rest.server.interceptor;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import ca.uhn.fhir.rest.api.RequestTypeEnum;
+import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException;
+
+/**
+ * This interceptor causes the server to reject invocations for HTTP methods
+ * other than those supported by the server with an HTTP 405. This is a requirement
+ * of some security assessments.
+ */
+public class BanUnsupprtedHttpMethodsInterceptor extends InterceptorAdapter {
+
+	private Set<RequestTypeEnum> myAllowedMethods = new HashSet<RequestTypeEnum>();
+	
+	public BanUnsupprtedHttpMethodsInterceptor() {
+		myAllowedMethods.add(RequestTypeEnum.GET);
+		myAllowedMethods.add(RequestTypeEnum.OPTIONS);
+		myAllowedMethods.add(RequestTypeEnum.DELETE);
+		myAllowedMethods.add(RequestTypeEnum.PUT);
+		myAllowedMethods.add(RequestTypeEnum.POST);
+	}
+	
+	@Override
+	public boolean incomingRequestPreProcessed(HttpServletRequest theRequest, HttpServletResponse theResponse) {
+		RequestTypeEnum requestType = RequestTypeEnum.valueOf(theRequest.getMethod());
+		if (myAllowedMethods.contains(requestType)) {
+			return true;
+		}
+		
+		throw new MethodNotAllowedException("Method not supported: " + theRequest.getMethod());
+	}
+
+}

hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/TestRestfulServer.java

@@ -32,6 +32,7 @@ import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider;
 import ca.uhn.fhir.rest.server.HardcodedServerAddressStrategy;
 import ca.uhn.fhir.rest.server.IResourceProvider;
 import ca.uhn.fhir.rest.server.RestfulServer;
+import ca.uhn.fhir.rest.server.interceptor.BanUnsupprtedHttpMethodsInterceptor;
 import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor;
 import ca.uhn.fhir.rest.server.interceptor.ResponseHighlighterInterceptor;
 import ca.uhn.fhirtest.config.TestDstu3Config;
@@ -175,6 +176,7 @@ public class TestRestfulServer extends RestfulServer {
 		 * makes things a little easier for testers.
 		 */
 		registerInterceptor(new ResponseHighlighterInterceptor());
+		registerInterceptor(new BanUnsupprtedHttpMethodsInterceptor());
 		
 		/*
 		 * Default to JSON with pretty printing

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ServerMimetypeDstu3Test.java

@@ -5,14 +5,18 @@ import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpRequestBase;
+import org.apache.http.client.methods.HttpTrace;
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.CloseableHttpClient;
@@ -69,6 +73,36 @@ public class ServerMimetypeDstu3Test {
 		assertEquals("<OperationOutcome xmlns=\"http://hl7.org/fhir\"><issue><diagnostics value=\"FAMILY\"/></issue></OperationOutcome>", responseContent);
 	}
 
+	@Test
+	public void testHttpTraceNotEnabled() throws Exception {
+		HttpTrace req = new HttpTrace("http://localhost:" + ourPort + "/Patient");
+		CloseableHttpResponse status = ourClient.execute(req);
+		try {
+			ourLog.info(status.toString());
+			assertEquals(400, status.getStatusLine().getStatusCode());
+		} finally {
+			IOUtils.closeQuietly(status.getEntity().getContent());
+		}
+	}
+
+	@Test
+	public void testHttpTrackNotEnabled() throws Exception {
+		HttpRequestBase req = new HttpRequestBase() {
+			@Override
+			public String getMethod() {
+				return "TRACK";
+			}};
+		req.setURI(new URI("http://localhost:" + ourPort + "/Patient"));
+			
+		CloseableHttpResponse status = ourClient.execute(req);
+		try {
+			ourLog.info(status.toString());
+			assertEquals(400, status.getStatusLine().getStatusCode());
+		} finally {
+			IOUtils.closeQuietly(status.getEntity().getContent());
+		}
+	}
+
 	@Test
 	public void testCreateWithXmlNewNoAcceptHeader() throws Exception {
 		Patient p = new Patient();

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/BanUnsupprtedHttpMethodsInterceptorDstu3Test.java

@@ -0,0 +1,187 @@
+package ca.uhn.fhir.rest.server.interceptor;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import java.net.URI;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpRequestBase;
+import org.apache.http.client.methods.HttpTrace;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.servlet.ServletHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.hamcrest.core.StringContains;
+import org.hl7.fhir.dstu3.model.IdType;
+import org.hl7.fhir.dstu3.model.Patient;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.slf4j.Logger;
+
+import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.rest.annotation.IdParam;
+import ca.uhn.fhir.rest.annotation.Read;
+import ca.uhn.fhir.rest.server.IResourceProvider;
+import ca.uhn.fhir.rest.server.RestfulServer;
+import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
+import ca.uhn.fhir.util.PortUtil;
+import ca.uhn.fhir.util.TestUtil;
+
+public class BanUnsupprtedHttpMethodsInterceptorDstu3Test {
+
+	private static CloseableHttpClient ourClient;
+	private static final FhirContext ourCtx = FhirContext.forDstu3();
+	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BanUnsupprtedHttpMethodsInterceptorDstu3Test.class);
+	private static int ourPort;
+	private static Server ourServer;
+
+	private static RestfulServer servlet;
+
+	@Test
+	public void testHttpTraceNotEnabled() throws Exception {
+		HttpTrace req = new HttpTrace("http://localhost:" + ourPort + "/Patient");
+		CloseableHttpResponse status = ourClient.execute(req);
+		try {
+			ourLog.info(status.toString());
+			assertEquals(405, status.getStatusLine().getStatusCode());
+		} finally {
+			IOUtils.closeQuietly(status.getEntity().getContent());
+		}
+	}
+
+	@Test
+	public void testHttpTrackNotEnabled() throws Exception {
+		HttpRequestBase req = new HttpRequestBase() {
+			@Override
+			public String getMethod() {
+				return "TRACK";
+			}
+		};
+		req.setURI(new URI("http://localhost:" + ourPort + "/Patient"));
+
+		CloseableHttpResponse status = ourClient.execute(req);
+		try {
+			ourLog.info(status.toString());
+			assertEquals(405, status.getStatusLine().getStatusCode());
+		} finally {
+			IOUtils.closeQuietly(status.getEntity().getContent());
+		}
+	}
+
+	@Test
+	public void testHttpFooNotEnabled() throws Exception {
+		HttpRequestBase req = new HttpRequestBase() {
+			@Override
+			public String getMethod() {
+				return "FOO";
+			}
+		};
+		req.setURI(new URI("http://localhost:" + ourPort + "/Patient"));
+
+		CloseableHttpResponse status = ourClient.execute(req);
+		try {
+			ourLog.info(status.toString());
+			assertEquals(501, status.getStatusLine().getStatusCode());
+		} finally {
+			IOUtils.closeQuietly(status.getEntity().getContent());
+		}
+	}
+
+	@Test
+	public void testRead() throws Exception {
+
+		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1");
+		HttpResponse status = ourClient.execute(httpGet);
+		IOUtils.closeQuietly(status.getEntity().getContent());
+		
+		assertEquals(200, status.getStatusLine().getStatusCode());
+	}
+	
+	@AfterClass
+	public static void afterClassClearContext() throws Exception {
+		ourServer.stop();
+		TestUtil.clearAllStaticFieldsForUnitTest();
+	}
+
+	@BeforeClass
+	public static void beforeClass() throws Exception {
+		ourPort = PortUtil.findFreePort();
+		ourServer = new Server(ourPort);
+
+		ServletHandler proxyHandler = new ServletHandler();
+		servlet = new RestfulServer(ourCtx);
+
+		servlet.setResourceProviders(new DummyPatientResourceProvider());
+		servlet.registerInterceptor(new BanUnsupprtedHttpMethodsInterceptor());
+
+		ServletHolder servletHolder = new ServletHolder(servlet);
+		proxyHandler.addServletWithMapping(servletHolder, "/*");
+		ourServer.setHandler(proxyHandler);
+		ourServer.start();
+
+		PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS);
+		HttpClientBuilder builder = HttpClientBuilder.create();
+		builder.setConnectionManager(connectionManager);
+		ourClient = builder.build();
+
+	}
+
+	public static class DummyPatientResourceProvider implements IResourceProvider {
+
+		private Patient createPatient1() {
+			Patient patient = new Patient();
+			patient.addName();
+			patient.getName().get(0).addFamily("Test");
+			patient.getName().get(0).addGiven("PatientOne");
+			return patient;
+		}
+
+		public Map<String, Patient> getIdToPatient() {
+			Map<String, Patient> idToPatient = new HashMap<String, Patient>();
+			{
+				Patient patient = createPatient1();
+				idToPatient.put("1", patient);
+			}
+			return idToPatient;
+		}
+
+		/**
+		 * Retrieve the resource by its identifier
+		 * 
+		 * @param theId
+		 *           The resource identity
+		 * @return The resource
+		 */
+		@Read()
+		public Patient getResourceById(@IdParam IdType theId) {
+			if (theId.getIdPart().equals("EX")) {
+				throw new InvalidRequestException("FOO");
+			}
+			String key = theId.getIdPart();
+			Patient retVal = getIdToPatient().get(key);
+			return retVal;
+		}
+
+		@Override
+		public Class<Patient> getResourceType() {
+			return Patient.class;
+		}
+
+	}
+
+}

src/site/xdoc/doc_rest_server_interceptor.xml

@@ -290,6 +290,18 @@
 				
 			</subsection>
 
+			<subsection name="Rejecting Unsupported HTTP Verbs">
+			
+				<p>
+					Some security audit tools require that servers return an HTTP 405 if
+					an unsupported HTTP verb is received (e.g. TRACE). The 
+					<a href="./apidocs/ca/uhn/fhir/rest/server/interceptor/BanUnsupprtedHttpMethodsInterceptor.html">BanUnsupprtedHttpMethodsInterceptor</a>
+					(<a href="./xref/ca/uhn/fhir/rest/server/interceptor/BanUnsupprtedHttpMethodsInterceptor.html">code</a>)
+					can be used to accomplish this.
+				</p>
+				
+			</subsection>
+
 		</section>
 				
 		<section name="Creating Interceptors">