From 04acd3aa4a8ffa802b0bb09ff3f7dd9ef34c7041 Mon Sep 17 00:00:00 2001 From: Tim Bruijnzeels Date: Mon, 16 Mar 2020 13:04:29 -0300 Subject: [PATCH] Improve checking the size of post body. (#189) --- src/commons/error.rs | 10 ++++++++ src/daemon/config.rs | 12 ++++----- src/daemon/http/mod.rs | 53 ++++++++++++++++++++++++++------------- src/daemon/krillserver.rs | 20 +++++++-------- 4 files changed, 62 insertions(+), 33 deletions(-) diff --git a/src/commons/error.rs b/src/commons/error.rs index 8489e65e..acf2bb7f 100644 --- a/src/commons/error.rs +++ b/src/commons/error.rs @@ -62,6 +62,12 @@ pub enum Error { #[display(fmt = "Invalid path argument for seconds")] ApiInvalidSeconds, + #[display(fmt = "POST body exceeds configured limit")] + PostTooBig, + + #[display(fmt = "POST body cannot be read")] + PostCannotRead, + //----------------------------------------------------------------- // Repository Issues //----------------------------------------------------------------- @@ -343,6 +349,10 @@ impl Error { Error::ApiInvalidSeconds => ErrorResponse::new("api-invalid-path-seconds", &self), + Error::PostTooBig => ErrorResponse::new("api-post-body-exceeds-limit", &self), + + Error::PostCannotRead => ErrorResponse::new("api-post-body-cannot-read", &self), + //----------------------------------------------------------------- // Repository Issues (label: repo-*) //----------------------------------------------------------------- diff --git a/src/daemon/config.rs b/src/daemon/config.rs index 997be48b..3c5ce17f 100644 --- a/src/daemon/config.rs +++ b/src/daemon/config.rs @@ -82,15 +82,15 @@ impl ConfigDefaults { 600 } - fn post_limit_api() -> usize { + fn post_limit_api() -> u64 { 256 * 1024 // 256kB } - fn post_limit_rfc8181() -> usize { + fn post_limit_rfc8181() -> u64 { 32 * 1024 * 1024 // 32MB (roughly 8000 issued certificates, so a key roll for nicbr and 100% uptake should be okay) } - fn post_limit_rfc6492() -> usize { + fn post_limit_rfc6492() -> u64 { 1024 * 1024 // 1MB (for ref. the NIC br cert is about 200kB) } } @@ -155,13 +155,13 @@ pub struct Config { pub ca_refresh: u32, #[serde(default = "ConfigDefaults::post_limit_api")] - pub post_limit_api: usize, + pub post_limit_api: u64, #[serde(default = "ConfigDefaults::post_limit_rfc8181")] - pub post_limit_rfc8181: usize, + pub post_limit_rfc8181: u64, #[serde(default = "ConfigDefaults::post_limit_rfc6492")] - pub post_limit_rfc6492: usize, + pub post_limit_rfc6492: u64, } /// # Accessors diff --git a/src/daemon/http/mod.rs b/src/daemon/http/mod.rs index 00fcb6fc..0b52c352 100644 --- a/src/daemon/http/mod.rs +++ b/src/daemon/http/mod.rs @@ -14,6 +14,7 @@ use crate::commons::error::Error; use crate::commons::remote::{rfc6492, rfc8181}; use crate::daemon::auth::Auth; use crate::daemon::http::server::State; +use std::convert::TryInto; pub mod server; pub mod statics; @@ -232,45 +233,60 @@ impl Request { /// /// Here we want to limit the bytes consumed to a maximum. So, the /// code below is adapted from the method in the hyper crate. - pub async fn read_bytes(self, limit: usize) -> Result { + pub async fn read_bytes(self, limit: u64) -> Result { let body = self.request.into_body(); futures_util::pin_mut!(body); + if body.size_hint().lower() > limit { + return Err(Error::PostTooBig); + } + let mut size_processed = 0; - fn assert_body_size(size: usize, limit: usize) -> Result<(), io::Error> { - if size > limit { - Err(io::Error::new( - io::ErrorKind::Other, - "Post exceeds max length", - )) + fn assert_body_size( + size_processed: u64, + body_lower_hint: u64, + post_limit: u64, + ) -> Result<(), Error> { + if size_processed + body_lower_hint > post_limit { + Err(Error::PostTooBig) } else { Ok(()) } } + assert_body_size(size_processed, body.size_hint().lower(), limit)?; + // If there's only 1 chunk, we can just return Buf::to_bytes() let mut first = if let Some(buf) = body.data().await { - let buf = buf.map_err(|_| Error::custom("Error reading body"))?; - let size = buf.bytes().len(); + let buf = buf.map_err(|_| Error::PostCannotRead)?; + let size: u64 = buf + .bytes() + .len() + .try_into() + .map_err(|_| Error::PostTooBig)?; size_processed += size; - assert_body_size(size_processed, limit)?; buf } else { return Ok(Bytes::new()); }; + assert_body_size(size_processed, body.size_hint().lower(), limit)?; let second = if let Some(buf) = body.data().await { - let buf = buf.map_err(|_| Error::custom("Error reading body"))?; - let size = buf.bytes().len(); + let buf = buf.map_err(|_| Error::PostCannotRead)?; + let size: u64 = buf + .bytes() + .len() + .try_into() + .map_err(|_| Error::PostTooBig)?; size_processed += size; - assert_body_size(size_processed, limit)?; buf } else { return Ok(first.to_bytes()); }; + assert_body_size(size_processed, body.size_hint().lower(), limit)?; // With more than 1 buf, we gotta flatten into a Vec first. let cap = first.remaining() + second.remaining() + body.size_hint().lower() as usize; let mut vec = Vec::with_capacity(cap); @@ -278,11 +294,14 @@ impl Request { vec.put(second); while let Some(buf) = body.data().await { - let buf = buf.map_err(|_| Error::custom("Error reading body"))?; - let size = buf.bytes().len(); + let buf = buf.map_err(|_| Error::PostCannotRead)?; + let size: u64 = buf + .bytes() + .len() + .try_into() + .map_err(|_| Error::PostTooBig)?; size_processed += size; - assert_body_size(size_processed, limit)?; - + assert_body_size(size_processed, body.size_hint().lower(), limit)?; vec.put(buf); } diff --git a/src/daemon/krillserver.rs b/src/daemon/krillserver.rs index 9ecdda44..09f50d6a 100644 --- a/src/daemon/krillserver.rs +++ b/src/daemon/krillserver.rs @@ -62,13 +62,13 @@ pub struct KrillServer { } pub struct PostLimits { - api: usize, - rfc6492: usize, - rfc8181: usize, + api: u64, + rfc6492: u64, + rfc8181: u64, } impl PostLimits { - fn new(api: usize, rfc6492: usize, rfc8181: usize) -> Self { + fn new(api: u64, rfc6492: u64, rfc8181: u64) -> Self { PostLimits { api, rfc8181, @@ -76,13 +76,13 @@ impl PostLimits { } } - pub fn api(&self) -> usize { + pub fn api(&self) -> u64 { self.api } - pub fn rfc6492(&self) -> usize { + pub fn rfc6492(&self) -> u64 { self.rfc6492 } - pub fn rfc8181(&self) -> usize { + pub fn rfc8181(&self) -> u64 { self.rfc8181 } } @@ -203,15 +203,15 @@ impl KrillServer { self.authorizer.is_api_allowed(auth) } - pub fn limit_api(&self) -> usize { + pub fn limit_api(&self) -> u64 { self.post_limits.api() } - pub fn limit_rfc8181(&self) -> usize { + pub fn limit_rfc8181(&self) -> u64 { self.post_limits.rfc8181() } - pub fn limit_rfc6492(&self) -> usize { + pub fn limit_rfc6492(&self) -> u64 { self.post_limits.rfc6492() } }