tyler
/
proxmox-backup
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

server: rest: implement max URI path and query length request limits 

Add a generous limit now and return the correct error (414 URI Too
Long). Otherwise we could to pretty larger GET requests, 64 KiB and
possible bigger (at 64 KiB my simple curl test failed due to
shell/curl limitations).

For now allow a 3072 characters as combined length of URI path and
query.

This is conform with the HTTP/1.1 RFCs (e.g., RFC 7231, 6.5.12 and
RFC 2616, 3.2.1) which do not specify any limits, upper or lower, but
require that all server accessible resources mus be reachable without
getting 414, which is normally fulfilled as we have various length
limits for stuff which could be in an URI, in place, e.g.:
 * user id: max. 64 chars
 * datastore: max. 32 chars

The only known problematic API endpoint is the catalog one, used in
the GUI's pxar file browser:
GET /api2/json/admin/datastore/<id>/catalog?..&filepath=<path>

The <path> is the encoded archive path, and can be arbitrary long.

But, this is a flawed design, as even without this new limit one can
easily generate archives which cannot be browsed anymore, as hyper
only accepts requests with max. 64 KiB in the URI.
So rather, we should move that to a GET-as-POST call, which has no
such limitations (and would not need to base32 encode the path).

Note: This change was inspired by adding a request access log, which
profits from such limits as we can then rely on certain atomicity
guarantees when writing requests to the log.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2020-10-15 17:49:16 +02:00 committed by Dietmar Maurer
parent 29633e2fe9
commit 4703ba81ce
1 changed files with 14 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/server/rest.rs
View File

@ -52,6 +52,8 @@ pub struct RestServer {
pub api_config: Arc<ApiConfig>,
}
const MAX_URI_QUERY_LENGTH: usize = 3072;
impl RestServer {
pub fn new(api_config: ApiConfig) -> Self {
@ -115,6 +117,10 @@ fn log_response(
if resp.extensions().get::<NoLogExtension>().is_some() { return; };
// we also log URL-to-long requests, so avoid message bigger than PIPE_BUF (4k on Linux)
// to profit from atomicty guarantees for O_APPEND opened logfiles
let path = &path[..MAX_URI_QUERY_LENGTH.min(path.len())];
let status = resp.status();
if !(status.is_success() || status.is_informational()) {
@ -532,12 +538,19 @@ async fn handle_request(
) -> Result<Response<Body>, Error> {
let (parts, body) = req.into_parts();
let method = parts.method.clone();
let (path, components) = tools::normalize_uri_path(parts.uri.path())?;
let comp_len = components.len();
let query = parts.uri.query().unwrap_or_default();
if path.len() + query.len() > MAX_URI_QUERY_LENGTH {
return Ok(Response::builder()
.status(StatusCode::URI_TOO_LONG)
.body("".into())
.unwrap());
}
let env_type = api.env_type();
let mut rpcenv = RestEnvironment::new(env_type);