From fbd0d3cbce42e1066b3d64c229c1e59599cd9628 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 27 May 2020 13:57:52 -0700 Subject: [PATCH] GUACAMOLE-1298: Enforce default limit on request size. --- .../apache/guacamole/rest/APIException.java | 13 +- .../rest/LimitedRequestInputStream.java | 152 ++++++++++++++++++ .../guacamole/rest/RESTExceptionMapper.java | 5 + .../guacamole/rest/RESTServiceModule.java | 13 +- .../guacamole/rest/RequestSizeFilter.java | 89 ++++++++++ .../rest/history/APISortPredicate.java | 5 +- 6 files changed, 262 insertions(+), 15 deletions(-) create mode 100644 guacamole/src/main/java/org/apache/guacamole/rest/LimitedRequestInputStream.java create mode 100644 guacamole/src/main/java/org/apache/guacamole/rest/RequestSizeFilter.java diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java b/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java index 01103b0ed..e37b106a9 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/APIException.java @@ -33,18 +33,15 @@ import org.apache.guacamole.GuacamoleException; public class APIException extends WebApplicationException { /** - * Construct a new APIException based on the given GuacamoleException and - * HTTP status. The details of the GuacamoleException relevant to the REST - * API will be exposed via an APIError. - * - * @param status - * The HTTP status which corresponds to the GuacamoleException. + * Construct a new APIException based on the given GuacamoleException. The + * details of the GuacamoleException relevant to the REST API will be + * exposed via an APIError. * * @param exception * The GuacamoleException that occurred. */ - public APIException(Response.Status status, GuacamoleException exception) { - super(Response.status(status) + public APIException(GuacamoleException exception) { + super(Response.status(exception.getHttpStatusCode()) .type(MediaType.APPLICATION_JSON) .entity(new APIError(exception)) .build()); diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/LimitedRequestInputStream.java b/guacamole/src/main/java/org/apache/guacamole/rest/LimitedRequestInputStream.java new file mode 100644 index 000000000..840ef1917 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/rest/LimitedRequestInputStream.java @@ -0,0 +1,152 @@ +/* + * 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.guacamole.rest; + +import java.io.IOException; +import java.io.InputStream; +import org.apache.guacamole.GuacamoleClientOverrunException; + +/** + * InputStream implementation which limits the body of REST API requests to + * a particular maximum size. If an attempt is made to read from a REST API + * request which exceeds this limit, the read attempt will be aborted by + * throwing an APIException. + */ +public class LimitedRequestInputStream extends InputStream { + + /** + * The InputStream being limited. + */ + private final InputStream stream; + + /** + * The maximum number of bytes to allow to be read or skipped. + */ + private final long maxLength; + + /** + * The total number of bytes that have been read or skipped from the stream + * thus far. + */ + private long bytesRead = 0; + + /** + * Wraps the given InputStream, ensuring that the overall number of bytes + * read or skipped does not exceed the given maximum length. + * + * @param stream + * The InputStream to limit. + * + * @param maxLength + * The maximum number of bytes to allow to be read or skipped. + */ + public LimitedRequestInputStream(InputStream stream, long maxLength) { + this.stream = stream; + this.maxLength = maxLength; + } + + /** + * Immediately verifies that the stream length limit has not been exceeded. + * If the length limit has been exceeded, an APIException is thrown + * indicating that the request body is too large. + * + * @throws APIException + * If the length limit has been exceeded. + */ + private synchronized void recheckLength() throws APIException { + if (bytesRead > maxLength) + throw new APIException(new GuacamoleClientOverrunException("Request body/entity too large.")); + } + + /** + * Updates the current number of bytes read based on the return value of a + * read-like operation such as read() or skip(). If the maximum stream + * length is exceeded as a result of the read, an APIException indicating + * this is thrown. + * + * NOTE: To avoid unnecessary read operations, recheckLength() should be + * manually called before performing any read operation. This function will + * perform the same checks, but can inherently only do so AFTER the read + * operation has occurred. + * + * @param change + * The number of bytes that have been read or skipped, or -1 if the + * read-like operation has failed (and no bytes have been read). + * + * @return + * The provided number of bytes read/skipped. + * + * @throws APIException + * If the read-like operation that occurred has caused the stream + * length to exceed its maximum. + */ + private synchronized long limitedRead(long change) throws APIException { + + if (change != -1) { + bytesRead += change; + recheckLength(); + } + + return change; + + } + + @Override + public void close() throws IOException { + stream.close(); + } + + @Override + public int available() throws IOException { + return stream.available(); + } + + @Override + public long skip(long l) throws IOException { + recheckLength(); + return limitedRead(stream.skip(l)); + } + + @Override + public int read(byte[] bytes, int i, int i1) throws IOException { + recheckLength(); + return (int) limitedRead(stream.read(bytes, i, i1)); + } + + @Override + public int read(byte[] bytes) throws IOException { + recheckLength(); + return (int) limitedRead(stream.read(bytes)); + } + + @Override + public int read() throws IOException { + + recheckLength(); + + int value = stream.read(); + if (value != -1) + limitedRead(1); + + return value; + + } + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionMapper.java b/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionMapper.java index e6c9b4c5b..6d1d574d6 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionMapper.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/RESTExceptionMapper.java @@ -22,6 +22,7 @@ package org.apache.guacamole.rest; import com.google.inject.Inject; import com.google.inject.Singleton; import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -82,6 +83,10 @@ public class RESTExceptionMapper implements ExceptionMapper { @Override public Response toResponse(Throwable t) { + + // Pass WebApplicationException responses through untouched + if (t instanceof WebApplicationException) + return ((WebApplicationException) t).getResponse(); // Ensure any associated session is invalidated if unauthorized if (t instanceof GuacamoleUnauthorizedException) { diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/RESTServiceModule.java b/guacamole/src/main/java/org/apache/guacamole/rest/RESTServiceModule.java index 4efab0ff5..9fc1045b2 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/RESTServiceModule.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/RESTServiceModule.java @@ -24,10 +24,11 @@ import org.apache.guacamole.rest.session.UserContextResourceFactory; import org.apache.guacamole.rest.session.SessionRESTService; import com.google.inject.Scopes; import com.google.inject.assistedinject.FactoryModuleBuilder; -import com.google.inject.matcher.Matchers; import com.google.inject.servlet.ServletModule; +import com.sun.jersey.api.core.ResourceConfig; import com.sun.jersey.guice.spi.container.servlet.GuiceContainer; -import org.aopalliance.intercept.MethodInterceptor; +import java.util.HashMap; +import java.util.Map; import org.apache.guacamole.rest.activeconnection.ActiveConnectionModule; import org.codehaus.jackson.jaxrs.JacksonJsonProvider; import org.apache.guacamole.rest.auth.TokenRESTService; @@ -75,6 +76,8 @@ public class RESTServiceModule extends ServletModule { @Override protected void configureServlets() { + Map containerParams = new HashMap<>(); + // Bind session map bind(TokenSessionMap.class).toInstance(tokenSessionMap); @@ -87,6 +90,10 @@ public class RESTServiceModule extends ServletModule { // Automatically translate GuacamoleExceptions for REST methods bind(RESTExceptionMapper.class); + // Restrict API requests by entity size + containerParams.put(ResourceConfig.PROPERTY_CONTAINER_REQUEST_FILTERS, RequestSizeFilter.class.getName()); + bind(RequestSizeFilter.class).in(Scopes.SINGLETON); + // Set up the API endpoints bind(ExtensionRESTService.class); bind(LanguageRESTService.class); @@ -111,7 +118,7 @@ public class RESTServiceModule extends ServletModule { // Set up the servlet and JSON mappings bind(GuiceContainer.class); bind(JacksonJsonProvider.class).in(Scopes.SINGLETON); - serve("/api/*").with(GuiceContainer.class); + serve("/api/*").with(GuiceContainer.class, containerParams); // Serve Webjar JavaScript dependencies bind(WebjarsServlet.class).in(Scopes.SINGLETON); diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/RequestSizeFilter.java b/guacamole/src/main/java/org/apache/guacamole/rest/RequestSizeFilter.java new file mode 100644 index 000000000..74ac47275 --- /dev/null +++ b/guacamole/src/main/java/org/apache/guacamole/rest/RequestSizeFilter.java @@ -0,0 +1,89 @@ +/* + * 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.guacamole.rest; + +import com.google.inject.Inject; +import com.sun.jersey.spi.container.ContainerRequest; +import com.sun.jersey.spi.container.ContainerRequestFilter; +import com.sun.jersey.spi.resource.Singleton; +import java.io.InputStream; +import javax.ws.rs.ext.Provider; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.environment.Environment; +import org.apache.guacamole.properties.LongGuacamoleProperty; + +/** + * Filter which restricts REST API requests to a particular maximum size. + */ +@Singleton +@Provider +public class RequestSizeFilter implements ContainerRequestFilter { + + /** + * The default maximum number of bytes to accept within the entity body of + * any particular REST request. + */ + private final long DEFAULT_MAX_REQUEST_SIZE = 2097152; + + /** + * The maximum number of bytes to accept within the entity body of any + * particular REST request. If not specified, requests will be limited to + * 2 MB by default. Specifying 0 disables request size limitations. + */ + private final LongGuacamoleProperty API_MAX_REQUEST_SIZE = new LongGuacamoleProperty() { + + @Override + public String getName() { return "api-max-request-size"; } + + }; + + /** + * The Guacamole server environment. + */ + @Inject + private Environment environment; + + @Override + public ContainerRequest filter(ContainerRequest request) { + + // Retrieve configured request size limits + final long maxRequestSize; + try { + maxRequestSize = environment.getProperty(API_MAX_REQUEST_SIZE, DEFAULT_MAX_REQUEST_SIZE); + } + catch (GuacamoleException e) { + throw new APIException(e); + } + + // Ignore request size if limit is disabled + if (maxRequestSize == 0) + return request; + + // Restrict maximum size of requests which have an input stream + // available to be limited + InputStream stream = request.getEntityInputStream(); + if (stream != null) + request.setEntityInputStream(new LimitedRequestInputStream(stream, maxRequestSize)); + + return request; + + } + +} diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/history/APISortPredicate.java b/guacamole/src/main/java/org/apache/guacamole/rest/history/APISortPredicate.java index c45ae9a81..0fea582e0 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/history/APISortPredicate.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/history/APISortPredicate.java @@ -109,10 +109,7 @@ public class APISortPredicate { // Bail out if sort property is not valid catch (IllegalArgumentException e) { - throw new APIException( - Response.Status.BAD_REQUEST, - new GuacamoleClientException(String.format("Invalid sort property: \"%s\"", value)) - ); + throw new APIException(new GuacamoleClientException(String.format("Invalid sort property: \"%s\"", value))); } }