From 9145a6ee1d346457dffe83872a236568c43ece3c Mon Sep 17 00:00:00 2001 From: Jens Reimann Date: Tue, 17 Sep 2024 14:49:44 +0200 Subject: [PATCH] fix: prevent uploading compressed files which might exhaust the memory --- Cargo.lock | 6 + Cargo.toml | 2 +- common/src/decompress.rs | 25 ++- common/src/model/bytesize.rs | 6 + etc/test-data/bomb.bz2 | Bin 0 -> 69778 bytes .../src/advisory/endpoints/config.rs | 5 + .../fundamental/src/advisory/endpoints/mod.rs | 8 +- .../src/advisory/endpoints/test.rs | 4 +- modules/fundamental/src/endpoints.rs | 36 ++--- modules/fundamental/src/error.rs | 3 + modules/fundamental/src/lib.rs | 2 +- .../fundamental/src/license/endpoints/test.rs | 4 +- .../src/organization/endpoints/test.rs | 4 +- .../fundamental/src/product/endpoints/test.rs | 4 +- .../fundamental/src/purl/endpoints/test.rs | 4 +- .../fundamental/src/sbom/endpoints/config.rs | 5 + modules/fundamental/src/sbom/endpoints/mod.rs | 8 +- .../fundamental/src/sbom/endpoints/test.rs | 4 +- modules/fundamental/src/test.rs | 48 ------ modules/fundamental/src/test/common.rs | 24 +++ modules/fundamental/src/test/mod.rs | 2 + .../src/vulnerability/endpoints/test.rs | 4 +- .../src/weakness/endpoints/test.rs | 3 +- modules/fundamental/tests/dataset.rs | 2 +- modules/fundamental/tests/limit.rs | 54 +++++++ modules/ingestor/Cargo.toml | 4 +- modules/ingestor/src/endpoints.rs | 19 ++- modules/ingestor/src/service/dataset/mod.rs | 28 +++- modules/ingestor/src/service/mod.rs | 10 +- modules/ingestor/tests/common.rs | 25 +++ modules/ingestor/tests/limit.rs | 41 +++++ server/src/lib.rs | 142 +++++++++++++++--- test-context/Cargo.toml | 4 + test-context/src/call.rs | 32 ++++ test-context/src/lib.rs | 3 +- 35 files changed, 454 insertions(+), 121 deletions(-) create mode 100644 etc/test-data/bomb.bz2 create mode 100644 modules/fundamental/src/advisory/endpoints/config.rs create mode 100644 modules/fundamental/src/sbom/endpoints/config.rs delete mode 100644 modules/fundamental/src/test.rs create mode 100644 modules/fundamental/src/test/common.rs create mode 100644 modules/fundamental/src/test/mod.rs create mode 100644 modules/fundamental/tests/limit.rs create mode 100644 modules/ingestor/tests/common.rs create mode 100644 modules/ingestor/tests/limit.rs create mode 100644 test-context/src/call.rs diff --git a/Cargo.lock b/Cargo.lock index 38e70fa63..1c8d186c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7716,6 +7716,7 @@ dependencies = [ name = "trustify-module-ingestor" version = "0.1.0-alpha.17" dependencies = [ + "actix-http", "actix-web", "anyhow", "bytes", @@ -7752,6 +7753,7 @@ dependencies = [ "tokio", "tokio-util", "tracing", + "trustify-auth", "trustify-common", "trustify-cvss", "trustify-entity", @@ -7832,7 +7834,10 @@ dependencies = [ name = "trustify-test-context" version = "0.1.0-alpha.17" dependencies = [ + "actix-http", + "actix-web", "anyhow", + "bytes", "futures", "hex", "liblzma", @@ -7848,6 +7853,7 @@ dependencies = [ "tokio", "tokio-util", "tracing", + "trustify-auth", "trustify-common", "trustify-migration", "trustify-module-ingestor", diff --git a/Cargo.toml b/Cargo.toml index f82b0f0b4..b1ca537c6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -135,7 +135,7 @@ utoipa-redoc = { version = "4.0.0", features = ["actix-web"] } utoipa-swagger-ui = "7.1.0" uuid = "1.7.0" walkdir = "2.5" -walker-common = "0.9.2" +walker-common = "0.9.3" walker-extras = "0.9.0" zip = "2.2.0" diff --git a/common/src/decompress.rs b/common/src/decompress.rs index 3e1c66503..54893445d 100644 --- a/common/src/decompress.rs +++ b/common/src/decompress.rs @@ -2,7 +2,7 @@ use actix_web::http::header; use anyhow::anyhow; use bytes::Bytes; use tokio::{runtime::Handle, task::JoinError}; -use walker_common::compression::{Compression, Detector}; +use walker_common::compression::{Compression, DecompressionOptions, Detector}; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -12,6 +12,8 @@ pub enum Error { Detector(anyhow::Error), #[error(transparent)] Io(#[from] std::io::Error), + #[error("payload too large")] + PayloadTooLarge, } /// Take some bytes, and an optional content-type header and decompress, if required. @@ -25,7 +27,11 @@ pub enum Error { /// **NOTE:** Depending on the size of the payload, this method might take some time. In an async /// context, it might be necessary to run this as a blocking function, or use [`decompress_async`] /// instead. -pub fn decompress(bytes: Bytes, content_type: Option) -> Result { +pub fn decompress( + bytes: Bytes, + content_type: Option, + limit: usize, +) -> Result { let content_type = content_type.as_ref().map(|ct| ct.as_ref()); // check what the user has declared @@ -56,16 +62,22 @@ pub fn decompress(bytes: Bytes, content_type: Option) -> Re // decompress (or not) - Ok(compression.decompress(bytes)?) + compression + .decompress_with(bytes, &DecompressionOptions::default().limit(limit)) + .map_err(|err| match err.kind() { + std::io::ErrorKind::WriteZero => Error::PayloadTooLarge, + _ => Error::from(err), + }) } /// An async version of [`decompress`]. pub async fn decompress_async( bytes: Bytes, content_type: Option, + limit: usize, ) -> Result, JoinError> { Handle::current() - .spawn_blocking(|| decompress(bytes, content_type)) + .spawn_blocking(move || decompress(bytes, content_type, limit)) .await } @@ -81,6 +93,7 @@ mod test { let bytes = decompress_async( document_bytes_raw("ubi9-9.2-755.1697625012.json").await?, None, + 0, ) .await??; @@ -98,6 +111,7 @@ mod test { let bytes = decompress_async( document_bytes_raw("openshift-container-storage-4.8.z.json.xz").await?, None, + 0, ) .await??; @@ -115,6 +129,7 @@ mod test { let bytes = decompress_async( document_bytes_raw("openshift-container-storage-4.8.z.json.xz").await?, Some(ContentType::json()), + 0, ) .await??; @@ -136,6 +151,7 @@ mod test { let result = decompress_async( document_bytes_raw("openshift-container-storage-4.8.z.json.xz").await?, Some(ContentType("application/json+bzip2".parse().unwrap())), + 0, ) .await?; @@ -153,6 +169,7 @@ mod test { let bytes = decompress_async( document_bytes_raw("openshift-container-storage-4.8.z.json.xz").await?, Some(ContentType("application/json+xz".parse().unwrap())), + 0, ) .await??; diff --git a/common/src/model/bytesize.rs b/common/src/model/bytesize.rs index 5bee422b4..c45ef0dd8 100644 --- a/common/src/model/bytesize.rs +++ b/common/src/model/bytesize.rs @@ -63,6 +63,12 @@ impl From for BinaryByteSize { } } +impl From for usize { + fn from(value: BinaryByteSize) -> Self { + value.0 .0 as usize + } +} + impl Deref for BinaryByteSize { type Target = ByteSize; diff --git a/etc/test-data/bomb.bz2 b/etc/test-data/bomb.bz2 new file mode 100644 index 0000000000000000000000000000000000000000..086e0b4d623c57524735ef7807b6b70ebc240a16 GIT binary patch literal 69778 zcmeI$zbk}s90&04Jzcq;>4tkIi{%s{Zenxil0_*}XJN#2Wx4Swk_?haE`veJkHI9| z#$>n{n2jcXfHL^;XYg8{<-0w7zn{J_=7afYKiH4*fqWz%%E$VFexx7j$9#Yf@Bu!+2lxOV-~)Vs5AXp# zzz6sMAK(LgfDiBiKEMa~03YB3e1H$|0Y1P7_y8Z^1AKrF@Bu!+2lxOV-~)Vs5AXp# zzz6sMAK(LgfDiBiKEMa~03YB3e1H$|0Y1P7_y8Z^1AKrF@Bu!+2lxOV-~)Vs5AXp# zzz6sMAK(LgfDiBiKEMa~03YB3e1H$|0Y1P7_y8Z^1AKrF@Bu!+2lxOV-~)Vs5AXp# zzz6sMAK(LgfDiBiKEMa~!2k7uj%HytG}b%Q?a-AE-wiRuKi~RT6z!foFSXjQD+5pY z^FmxK9Vf|KwYqyaTDd4B$;W2=P-?bIt^4cvv!2a{d?<&^TC`kBGW`=X*&FBnkNe?$ zm=ETo{a`=J2lA18C?D$w`jLL9AM*h|zz6sMAK(LgfDiBiKEMa~03YB3e1H$|0Y1P7 z_y8Z^1AKrF@Bu!+2lxOV-~)Vs5AXp#zz6sMAK(LgfDiBiKEMa~03YB3e1H$|0Y1P7 z_y8Z^1AKrF@Bu!+2lxOV-~)Vs5AXp#zz6sMAK(LgfDiBiKEMa~03YB3e1H$|0Y1P7 z_y8Z^1AKrF@Bu!+2lxOV-~)Vs5AXp#zz6sMAK(LgfDiBiKEMa~03YB3e1H$|0Y1P7 w_y8Z^1Ao>B9?uT;raCtI3fV9kejD)mLeJ#zOD4YRjgm%dr{``hT^(qrU)M*Vf&c&j literal 0 HcmV?d00001 diff --git a/modules/fundamental/src/advisory/endpoints/config.rs b/modules/fundamental/src/advisory/endpoints/config.rs new file mode 100644 index 000000000..e119a13c6 --- /dev/null +++ b/modules/fundamental/src/advisory/endpoints/config.rs @@ -0,0 +1,5 @@ +#[derive(Clone, Debug, Eq, PartialEq, Default)] +pub struct Config { + /// An upload limit in bytes. Zero meaning "unlimited". + pub upload_limit: usize, +} diff --git a/modules/fundamental/src/advisory/endpoints/mod.rs b/modules/fundamental/src/advisory/endpoints/mod.rs index ed1ce46f0..574bc8a99 100644 --- a/modules/fundamental/src/advisory/endpoints/mod.rs +++ b/modules/fundamental/src/advisory/endpoints/mod.rs @@ -1,3 +1,4 @@ +mod config; mod label; #[cfg(test)] mod test; @@ -6,6 +7,7 @@ use crate::{ advisory::service::AdvisoryService, purl::service::PurlService, Error, Error::Internal, }; use actix_web::{delete, get, http::header, post, web, HttpResponse, Responder}; +use config::Config; use futures_util::TryStreamExt; use std::str::FromStr; use trustify_common::{ @@ -19,11 +21,12 @@ use trustify_module_ingestor::service::{Format, IngestorService}; use trustify_module_storage::service::StorageBackend; use utoipa::{IntoParams, OpenApi}; -pub fn configure(config: &mut web::ServiceConfig, db: Database) { +pub fn configure(config: &mut web::ServiceConfig, db: Database, upload_limit: usize) { let advisory_service = AdvisoryService::new(db); config .app_data(web::Data::new(advisory_service)) + .app_data(web::Data::new(Config { upload_limit })) .service(all) .service(get) .service(delete) @@ -171,11 +174,12 @@ struct UploadParams { /// Upload a new advisory pub async fn upload( service: web::Data, + config: web::Data, web::Query(UploadParams { issuer, labels }): web::Query, content_type: Option>, bytes: web::Bytes, ) -> Result { - let bytes = decompress_async(bytes, content_type.map(|ct| ct.0)).await??; + let bytes = decompress_async(bytes, content_type.map(|ct| ct.0), config.upload_limit).await??; let result = service .ingest(&bytes, Format::Advisory, labels, issuer) .await?; diff --git a/modules/fundamental/src/advisory/endpoints/test.rs b/modules/fundamental/src/advisory/endpoints/test.rs index 20499dadb..44816d3c8 100644 --- a/modules/fundamental/src/advisory/endpoints/test.rs +++ b/modules/fundamental/src/advisory/endpoints/test.rs @@ -1,6 +1,6 @@ use crate::{ advisory::model::{AdvisoryDetails, AdvisorySummary}, - test::{caller, CallService}, + test::caller, }; use actix_http::StatusCode; use actix_web::test::TestRequest; @@ -18,7 +18,7 @@ use trustify_cvss::cvss3::{ }; use trustify_entity::labels::Labels; use trustify_module_ingestor::{graph::advisory::AdvisoryInformation, model::IngestResult}; -use trustify_test_context::{document_bytes, TrustifyContext}; +use trustify_test_context::{call::CallService, document_bytes, TrustifyContext}; use uuid::Uuid; #[test_context(TrustifyContext)] diff --git a/modules/fundamental/src/endpoints.rs b/modules/fundamental/src/endpoints.rs index a76013f3f..04c61fad8 100644 --- a/modules/fundamental/src/endpoints.rs +++ b/modules/fundamental/src/endpoints.rs @@ -4,27 +4,27 @@ use trustify_module_ingestor::graph::Graph; use trustify_module_ingestor::service::IngestorService; use trustify_module_storage::service::dispatch::DispatchBackend; +#[derive(Clone, Debug, Eq, PartialEq, Default)] +pub struct Config { + pub sbom_upload_limit: usize, + pub advisory_upload_limit: usize, +} + pub fn configure( - config: &mut web::ServiceConfig, + svc: &mut web::ServiceConfig, + config: Config, db: Database, storage: impl Into, ) { let ingestor_service = IngestorService::new(Graph::new(db.clone()), storage); - config.app_data(web::Data::new(ingestor_service)); - - crate::advisory::endpoints::configure(config, db.clone()); - - crate::license::endpoints::configure(config, db.clone()); - - crate::organization::endpoints::configure(config, db.clone()); - - crate::purl::endpoints::configure(config, db.clone()); - - crate::product::endpoints::configure(config, db.clone()); - - crate::sbom::endpoints::configure(config, db.clone()); - - crate::vulnerability::endpoints::configure(config, db.clone()); - - crate::weakness::endpoints::configure(config, db.clone()); + svc.app_data(web::Data::new(ingestor_service)); + + crate::advisory::endpoints::configure(svc, db.clone(), config.advisory_upload_limit); + crate::license::endpoints::configure(svc, db.clone()); + crate::organization::endpoints::configure(svc, db.clone()); + crate::purl::endpoints::configure(svc, db.clone()); + crate::product::endpoints::configure(svc, db.clone()); + crate::sbom::endpoints::configure(svc, db.clone(), config.sbom_upload_limit); + crate::vulnerability::endpoints::configure(svc, db.clone()); + crate::weakness::endpoints::configure(svc, db.clone()); } diff --git a/modules/fundamental/src/error.rs b/modules/fundamental/src/error.rs index a34766987..7d330d90c 100644 --- a/modules/fundamental/src/error.rs +++ b/modules/fundamental/src/error.rs @@ -65,6 +65,9 @@ impl ResponseError for Error { HttpResponse::UnsupportedMediaType() .json(ErrorInformation::new("UnsupportedCompression", self)) } + Error::Compression(decompress::Error::PayloadTooLarge) => { + HttpResponse::PayloadTooLarge().json(ErrorInformation::new("PayloadTooLarge", self)) + } Error::Compression(err) => { HttpResponse::BadRequest().json(ErrorInformation::new("CompressionError", err)) } diff --git a/modules/fundamental/src/lib.rs b/modules/fundamental/src/lib.rs index 28238928f..926aa6052 100644 --- a/modules/fundamental/src/lib.rs +++ b/modules/fundamental/src/lib.rs @@ -15,7 +15,7 @@ pub mod openapi; pub use openapi::openapi; pub mod endpoints; -pub use endpoints::configure; +pub use endpoints::{configure, Config}; pub mod error; diff --git a/modules/fundamental/src/license/endpoints/test.rs b/modules/fundamental/src/license/endpoints/test.rs index edb87d7ce..9501457c0 100644 --- a/modules/fundamental/src/license/endpoints/test.rs +++ b/modules/fundamental/src/license/endpoints/test.rs @@ -1,12 +1,12 @@ use crate::license::model::{ LicenseDetailsPurlSummary, LicenseSummary, SpdxLicenseDetails, SpdxLicenseSummary, }; -use crate::test::{caller, CallService}; +use crate::test::caller; use actix_web::test::TestRequest; use test_context::test_context; use test_log::test; use trustify_common::model::PaginatedResults; -use trustify_test_context::TrustifyContext; +use trustify_test_context::{call::CallService, TrustifyContext}; #[test_context(TrustifyContext)] #[test(actix_web::test)] diff --git a/modules/fundamental/src/organization/endpoints/test.rs b/modules/fundamental/src/organization/endpoints/test.rs index 67d9e15de..831d5ede5 100644 --- a/modules/fundamental/src/organization/endpoints/test.rs +++ b/modules/fundamental/src/organization/endpoints/test.rs @@ -1,4 +1,4 @@ -use crate::test::{caller, CallService}; +use crate::test::caller; use actix_web::cookie::time::OffsetDateTime; use actix_web::test::TestRequest; use jsonpath_rust::JsonPathQuery; @@ -10,7 +10,7 @@ use trustify_common::db::Transactional; use trustify_common::hashing::Digests; use trustify_common::model::Paginated; use trustify_module_ingestor::graph::advisory::AdvisoryInformation; -use trustify_test_context::TrustifyContext; +use trustify_test_context::{call::CallService, TrustifyContext}; #[test_context(TrustifyContext)] #[test(actix_web::test)] diff --git a/modules/fundamental/src/product/endpoints/test.rs b/modules/fundamental/src/product/endpoints/test.rs index 0145d6676..fc185cbe3 100644 --- a/modules/fundamental/src/product/endpoints/test.rs +++ b/modules/fundamental/src/product/endpoints/test.rs @@ -1,4 +1,4 @@ -use crate::test::{caller, CallService}; +use crate::test::caller; use actix_http::StatusCode; use actix_web::test::TestRequest; use jsonpath_rust::JsonPathQuery; @@ -8,7 +8,7 @@ use test_log::test; use trustify_common::db::query::Query; use trustify_common::model::Paginated; use trustify_module_ingestor::graph::product::ProductInformation; -use trustify_test_context::TrustifyContext; +use trustify_test_context::{call::CallService, TrustifyContext}; #[test_context(TrustifyContext)] #[test(actix_web::test)] diff --git a/modules/fundamental/src/purl/endpoints/test.rs b/modules/fundamental/src/purl/endpoints/test.rs index b5a44893a..3d27955c9 100644 --- a/modules/fundamental/src/purl/endpoints/test.rs +++ b/modules/fundamental/src/purl/endpoints/test.rs @@ -4,7 +4,7 @@ use crate::purl::model::details::versioned_purl::VersionedPurlDetails; use crate::purl::model::summary::base_purl::{BasePurlSummary, PaginatedBasePurlSummary}; use crate::purl::model::summary::purl::PaginatedPurlSummary; use crate::purl::model::summary::r#type::TypeSummary; -use crate::test::{caller, CallService}; +use crate::test::caller; use actix_web::test::TestRequest; use serde_json::Value; use std::str::FromStr; @@ -14,7 +14,7 @@ use trustify_common::db::Transactional; use trustify_common::model::PaginatedResults; use trustify_common::purl::Purl; use trustify_module_ingestor::graph::Graph; -use trustify_test_context::TrustifyContext; +use trustify_test_context::{call::CallService, TrustifyContext}; async fn setup(graph: &Graph) -> Result<(), anyhow::Error> { let log4j = graph diff --git a/modules/fundamental/src/sbom/endpoints/config.rs b/modules/fundamental/src/sbom/endpoints/config.rs new file mode 100644 index 000000000..e119a13c6 --- /dev/null +++ b/modules/fundamental/src/sbom/endpoints/config.rs @@ -0,0 +1,5 @@ +#[derive(Clone, Debug, Eq, PartialEq, Default)] +pub struct Config { + /// An upload limit in bytes. Zero meaning "unlimited". + pub upload_limit: usize, +} diff --git a/modules/fundamental/src/sbom/endpoints/mod.rs b/modules/fundamental/src/sbom/endpoints/mod.rs index 069d86fdc..8faf9f5a8 100644 --- a/modules/fundamental/src/sbom/endpoints/mod.rs +++ b/modules/fundamental/src/sbom/endpoints/mod.rs @@ -1,3 +1,4 @@ +mod config; mod label; #[cfg(test)] mod test; @@ -11,6 +12,7 @@ use crate::{ Error::{self, Internal}, }; use actix_web::{delete, get, http::header, post, web, HttpResponse, Responder}; +use config::Config; use futures_util::TryStreamExt; use sea_orm::prelude::Uuid; use std::str::FromStr; @@ -28,11 +30,12 @@ use trustify_module_ingestor::service::{Format, IngestorService}; use trustify_module_storage::service::StorageBackend; use utoipa::OpenApi; -pub fn configure(config: &mut web::ServiceConfig, db: Database) { +pub fn configure(config: &mut web::ServiceConfig, db: Database, upload_limit: usize) { let sbom_service = SbomService::new(db); config .app_data(web::Data::new(sbom_service)) + .app_data(web::Data::new(Config { upload_limit })) .service(all) .service(all_related) .service(get) @@ -386,11 +389,12 @@ struct UploadQuery { /// Upload a new SBOM pub async fn upload( service: web::Data, + config: web::Data, web::Query(UploadQuery { labels }): web::Query, content_type: Option>, bytes: web::Bytes, ) -> Result { - let bytes = decompress_async(bytes, content_type.map(|ct| ct.0)).await??; + let bytes = decompress_async(bytes, content_type.map(|ct| ct.0), config.upload_limit).await??; let result = service.ingest(&bytes, Format::SBOM, labels, None).await?; log::info!("Uploaded SBOM: {}", result.id); Ok(HttpResponse::Created().json(result)) diff --git a/modules/fundamental/src/sbom/endpoints/test.rs b/modules/fundamental/src/sbom/endpoints/test.rs index 07bf92fb2..6e34a538e 100644 --- a/modules/fundamental/src/sbom/endpoints/test.rs +++ b/modules/fundamental/src/sbom/endpoints/test.rs @@ -1,6 +1,6 @@ use crate::{ sbom::model::{SbomPackage, SbomSummary}, - test::{caller, CallService}, + test::caller, }; use actix_http::StatusCode; use actix_web::test::TestRequest; @@ -10,7 +10,7 @@ use test_log::test; use trustify_common::{id::Id, model::PaginatedResults}; use trustify_entity::labels::Labels; use trustify_module_ingestor::model::IngestResult; -use trustify_test_context::{document_bytes, TrustifyContext}; +use trustify_test_context::{call::CallService, document_bytes, TrustifyContext}; use uuid::Uuid; #[test_context(TrustifyContext)] diff --git a/modules/fundamental/src/test.rs b/modules/fundamental/src/test.rs deleted file mode 100644 index ecbcfeacc..000000000 --- a/modules/fundamental/src/test.rs +++ /dev/null @@ -1,48 +0,0 @@ -use crate::configure; -use actix_http::Request; -use actix_web::{ - dev::{Service, ServiceResponse}, - web, App, Error, -}; -use bytes::Bytes; -use sea_orm::prelude::async_trait::async_trait; -use serde::de::DeserializeOwned; -use trustify_auth::authorizer::Authorizer; -use trustify_test_context::TrustifyContext; - -/// A trait wrapping an `impl Service` in a way that we can pass it as a reference. -#[async_trait(?Send)] -pub trait CallService { - async fn call_service(&self, s: Request) -> ServiceResponse; - async fn call_and_read_body(&self, r: Request) -> Bytes; - async fn call_and_read_body_json(&self, r: Request) -> T; -} - -#[async_trait(?Send)] -impl CallService for S -where - S: Service, -{ - async fn call_service(&self, r: Request) -> ServiceResponse { - actix_web::test::call_service(self, r).await - } - async fn call_and_read_body(&self, r: Request) -> Bytes { - actix_web::test::call_and_read_body(self, r).await - } - async fn call_and_read_body_json(&self, r: Request) -> T { - actix_web::test::call_and_read_body_json(self, r).await - } -} - -pub async fn caller(ctx: &TrustifyContext) -> anyhow::Result { - Ok(actix_web::test::init_service( - App::new() - .app_data(web::PayloadConfig::default().limit(5 * 1024 * 1024)) - .app_data(web::Data::new(Authorizer::new(None))) - .service( - web::scope("/api") - .configure(|svc| configure(svc, ctx.db.clone(), ctx.storage.clone())), - ), - ) - .await) -} diff --git a/modules/fundamental/src/test/common.rs b/modules/fundamental/src/test/common.rs new file mode 100644 index 000000000..0a2f7a485 --- /dev/null +++ b/modules/fundamental/src/test/common.rs @@ -0,0 +1,24 @@ +use actix_web::{web, App}; +use trustify_auth::authorizer::Authorizer; +use trustify_test_context::{call::CallService, TrustifyContext}; + +#[allow(unused)] +pub async fn caller(ctx: &TrustifyContext) -> anyhow::Result { + caller_with(ctx, Config::default()).await +} + +pub async fn caller_with( + ctx: &TrustifyContext, + config: Config, +) -> anyhow::Result { + Ok(actix_web::test::init_service( + App::new() + .app_data(web::PayloadConfig::default().limit(5 * 1024 * 1024)) + .app_data(web::Data::new(Authorizer::new(None))) + .service( + web::scope("/api") + .configure(|svc| configure(svc, config, ctx.db.clone(), ctx.storage.clone())), + ), + ) + .await) +} diff --git a/modules/fundamental/src/test/mod.rs b/modules/fundamental/src/test/mod.rs new file mode 100644 index 000000000..b59efdeb0 --- /dev/null +++ b/modules/fundamental/src/test/mod.rs @@ -0,0 +1,2 @@ +use crate::endpoints::{configure, Config}; +include!("common.rs"); diff --git a/modules/fundamental/src/vulnerability/endpoints/test.rs b/modules/fundamental/src/vulnerability/endpoints/test.rs index 673ce1174..3ca78aba7 100644 --- a/modules/fundamental/src/vulnerability/endpoints/test.rs +++ b/modules/fundamental/src/vulnerability/endpoints/test.rs @@ -1,4 +1,4 @@ -use crate::test::{caller, CallService}; +use crate::test::caller; use crate::vulnerability::model::VulnerabilitySummary; use actix_http::StatusCode; use actix_web::test::TestRequest; @@ -16,7 +16,7 @@ use trustify_cvss::cvss3::{ }; use trustify_module_ingestor::graph::advisory::AdvisoryInformation; use trustify_module_ingestor::graph::vulnerability::VulnerabilityInformation; -use trustify_test_context::TrustifyContext; +use trustify_test_context::{call::CallService, TrustifyContext}; #[test_context(TrustifyContext)] #[test(actix_web::test)] diff --git a/modules/fundamental/src/weakness/endpoints/test.rs b/modules/fundamental/src/weakness/endpoints/test.rs index 2addd022b..7ca4423cd 100644 --- a/modules/fundamental/src/weakness/endpoints/test.rs +++ b/modules/fundamental/src/weakness/endpoints/test.rs @@ -1,11 +1,10 @@ use crate::test::caller; -use crate::test::CallService; use crate::weakness::model::{WeaknessDetails, WeaknessSummary}; use actix_web::test::TestRequest; use test_context::test_context; use test_log::test; use trustify_common::model::PaginatedResults; -use trustify_test_context::{document_read, TrustifyContext}; +use trustify_test_context::{call::CallService, document_read, TrustifyContext}; use zip::ZipArchive; #[test_context(TrustifyContext)] diff --git a/modules/fundamental/tests/dataset.rs b/modules/fundamental/tests/dataset.rs index 191d711a6..88268b73c 100644 --- a/modules/fundamental/tests/dataset.rs +++ b/modules/fundamental/tests/dataset.rs @@ -47,7 +47,7 @@ async fn ingest(ctx: TrustifyContext) -> anyhow::Result<()> { // ingest - let result = ctx.ingestor.ingest_dataset(&data, ()).await?; + let result = ctx.ingestor.ingest_dataset(&data, (), 0).await?; let ingest_time = start.elapsed(); diff --git a/modules/fundamental/tests/limit.rs b/modules/fundamental/tests/limit.rs new file mode 100644 index 000000000..c78b93116 --- /dev/null +++ b/modules/fundamental/tests/limit.rs @@ -0,0 +1,54 @@ +use actix_http::StatusCode; +use actix_web::test::TestRequest; +use test_context::test_context; +use test_log::test; +use trustify_module_fundamental::{configure, Config}; +use trustify_test_context::document_bytes_raw; + +include!("../src/test/common.rs"); + +#[test_context(TrustifyContext)] +#[test(actix_web::test)] +async fn upload_bomb_sbom(ctx: &TrustifyContext) -> anyhow::Result<()> { + let app = caller_with( + ctx, + Config { + sbom_upload_limit: 1024 * 1024, + advisory_upload_limit: 1024 * 1024, + }, + ) + .await?; + + let request = TestRequest::post() + .uri("/api/v1/sbom") + .set_payload(document_bytes_raw("bomb.bz2").await?) + .to_request(); + + let response = app.call_service(request).await; + assert_eq!(response.status(), StatusCode::PAYLOAD_TOO_LARGE); + + Ok(()) +} + +#[test_context(TrustifyContext)] +#[test(actix_web::test)] +async fn upload_bomb_advisory(ctx: &TrustifyContext) -> anyhow::Result<()> { + let app = caller_with( + ctx, + Config { + sbom_upload_limit: 1024 * 1024, + advisory_upload_limit: 1024 * 1024, + }, + ) + .await?; + + let request = TestRequest::post() + .uri("/api/v1/advisory") + .set_payload(document_bytes_raw("bomb.bz2").await?) + .to_request(); + + let response = app.call_service(request).await; + assert_eq!(response.status(), StatusCode::PAYLOAD_TOO_LARGE); + + Ok(()) +} diff --git a/modules/ingestor/Cargo.toml b/modules/ingestor/Cargo.toml index f60f8e4b3..ead775958 100644 --- a/modules/ingestor/Cargo.toml +++ b/modules/ingestor/Cargo.toml @@ -49,11 +49,13 @@ uuid = { workspace = true, features = ["v7"] } zip = { workspace = true } [dev-dependencies] +trustify-auth = { workspace = true } trustify-test-context = { workspace = true } -zip = { workspace = true } +actix-http = { workspace = true } rand = { workspace = true } rstest = { workspace = true } serde_yml = { workspace = true } test-context = { workspace = true } test-log = { workspace = true, features = ["log", "trace"] } +zip = { workspace = true } diff --git a/modules/ingestor/src/endpoints.rs b/modules/ingestor/src/endpoints.rs index 38ab11f05..1721dcec5 100644 --- a/modules/ingestor/src/endpoints.rs +++ b/modules/ingestor/src/endpoints.rs @@ -8,11 +8,23 @@ use trustify_entity::labels::Labels; use trustify_module_storage::service::dispatch::DispatchBackend; use utoipa::{IntoParams, OpenApi}; +#[derive(Clone, Debug, Eq, PartialEq, Default)] +pub struct Config { + /// Limit of a single content entry (after decompression). + pub dataset_entry_limit: usize, +} + /// mount the "ingestor" module -pub fn configure(svc: &mut web::ServiceConfig, db: Database, storage: impl Into) { +pub fn configure( + svc: &mut web::ServiceConfig, + config: Config, + db: Database, + storage: impl Into, +) { let ingestor_service = IngestorService::new(Graph::new(db.clone()), storage); svc.app_data(web::Data::new(ingestor_service)) + .app_data(web::Data::new(config)) .service(web::scope("/v1/dataset").service(upload)); } @@ -53,9 +65,12 @@ pub struct ApiDoc; /// Upload a new dataset pub async fn upload( service: web::Data, + config: web::Data, web::Query(UploadParams { labels }): web::Query, bytes: web::Bytes, ) -> Result { - let result = service.ingest_dataset(&bytes, labels).await?; + let result = service + .ingest_dataset(&bytes, labels, config.dataset_entry_limit) + .await?; Ok(HttpResponse::Created().json(result)) } diff --git a/modules/ingestor/src/service/dataset/mod.rs b/modules/ingestor/src/service/dataset/mod.rs index 039ac64b4..6399bbc4b 100644 --- a/modules/ingestor/src/service/dataset/mod.rs +++ b/modules/ingestor/src/service/dataset/mod.rs @@ -7,7 +7,8 @@ use crate::{ }; use anyhow::anyhow; use bytes::Bytes; -use sbom_walker::common::compression::decompress; +use sbom_walker::common::compression; +use sbom_walker::common::compression::{DecompressionOptions, Detector}; use std::{ collections::BTreeMap, io::{Cursor, Read}, @@ -23,11 +24,16 @@ use trustify_module_storage::{service::dispatch::DispatchBackend, service::Stora pub struct DatasetLoader<'g> { graph: &'g Graph, storage: &'g DispatchBackend, + limit: usize, } impl<'g> DatasetLoader<'g> { - pub fn new(graph: &'g Graph, storage: &'g DispatchBackend) -> Self { - Self { graph, storage } + pub fn new(graph: &'g Graph, storage: &'g DispatchBackend, limit: usize) -> Self { + Self { + graph, + storage, + limit, + } } #[instrument(skip(self, buffer), ret)] @@ -70,9 +76,23 @@ impl<'g> DatasetLoader<'g> { file.read_to_end(&mut data)?; let file_name = file_name.to_string(); + let opts = DecompressionOptions::new().limit(self.limit); let data = Handle::current() .spawn_blocking(move || { - decompress(Bytes::from(data), &file_name).map_err(Error::Generic) + let detector = Detector { + file_name: Some(&file_name), + ..Detector::default() + }; + detector + .decompress_with(Bytes::from(data), &opts) + .map_err(|err| match err { + compression::Error::Io(err) + if err.kind() == std::io::ErrorKind::WriteZero => + { + Error::PayloadTooLarge + } + _ => Error::Generic(anyhow!("{err}")), + }) }) .await??; diff --git a/modules/ingestor/src/service/mod.rs b/modules/ingestor/src/service/mod.rs index c5a9bc49c..d353d2f9d 100644 --- a/modules/ingestor/src/service/mod.rs +++ b/modules/ingestor/src/service/mod.rs @@ -51,6 +51,8 @@ pub enum Error { Join(#[from] JoinError), #[error(transparent)] Zip(#[from] zip::result::ZipError), + #[error("payload too large")] + PayloadTooLarge, } impl ResponseError for Error { @@ -121,6 +123,11 @@ impl ResponseError for Error { message: inner.to_string(), details: None, }), + Self::PayloadTooLarge => HttpResponse::PayloadTooLarge().json(ErrorInformation { + error: "PayloadTooLarge".into(), + message: self.to_string(), + details: None, + }), } } } @@ -218,8 +225,9 @@ impl IngestorService { &self, bytes: &[u8], labels: impl Into + Debug, + limit: usize, ) -> Result { - let loader = DatasetLoader::new(self.graph(), self.storage()); + let loader = DatasetLoader::new(self.graph(), self.storage(), limit); loader.load(labels.into(), bytes).await } } diff --git a/modules/ingestor/tests/common.rs b/modules/ingestor/tests/common.rs new file mode 100644 index 000000000..b23b06b65 --- /dev/null +++ b/modules/ingestor/tests/common.rs @@ -0,0 +1,25 @@ +use actix_web::{web, App}; +use trustify_auth::authorizer::Authorizer; +use trustify_module_ingestor::endpoints::{configure, Config}; +use trustify_test_context::{call::CallService, TrustifyContext}; + +#[allow(unused)] +pub async fn caller(ctx: &TrustifyContext) -> anyhow::Result { + caller_with(ctx, Config::default()).await +} + +pub async fn caller_with( + ctx: &TrustifyContext, + config: Config, +) -> anyhow::Result { + Ok(actix_web::test::init_service( + App::new() + .app_data(web::PayloadConfig::default().limit(5 * 1024 * 1024)) + .app_data(web::Data::new(Authorizer::new(None))) + .service( + web::scope("/api") + .configure(|svc| configure(svc, config, ctx.db.clone(), ctx.storage.clone())), + ), + ) + .await) +} diff --git a/modules/ingestor/tests/limit.rs b/modules/ingestor/tests/limit.rs new file mode 100644 index 000000000..87339b70d --- /dev/null +++ b/modules/ingestor/tests/limit.rs @@ -0,0 +1,41 @@ +#[path = "common.rs"] +mod common; + +use actix_http::StatusCode; +use actix_web::test::TestRequest; +use common::caller_with; +use std::io::{Cursor, Write}; +use test_context::test_context; +use test_log::test; +use trustify_module_ingestor::endpoints::Config; +use trustify_test_context::{call::CallService, document_bytes_raw, TrustifyContext}; +use zip::write::FileOptions; + +#[test_context(TrustifyContext)] +#[test(actix_web::test)] +async fn upload_bomb_dataset(ctx: &TrustifyContext) -> anyhow::Result<()> { + let app = caller_with( + ctx, + Config { + dataset_entry_limit: 1024 * 1024, + }, + ) + .await?; + + let mut data = vec![]; + let mut dataset = zip::write::ZipWriter::new(Cursor::new(&mut data)); + dataset.add_directory("spdx", FileOptions::<()>::default())?; + dataset.start_file("spdx/bomb.bz2", FileOptions::<()>::default())?; + dataset.write_all(&document_bytes_raw("bomb.bz2").await?)?; + dataset.finish()?; + + let request = TestRequest::post() + .uri("/api/v1/dataset") + .set_payload(data) + .to_request(); + + let response = app.call_service(request).await; + assert_eq!(response.status(), StatusCode::PAYLOAD_TOO_LARGE); + + Ok(()) +} diff --git a/server/src/lib.rs b/server/src/lib.rs index 605eb7fe7..f48eaea07 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -3,11 +3,12 @@ #[cfg(feature = "garage-door")] mod embedded_oidc; -pub mod openapi; mod sample_data; pub use sample_data::sample_data; +pub mod openapi; + use actix_web::{ body::MessageBody, dev::{ConnectionInfo, Url}, @@ -33,6 +34,7 @@ use trustify_auth::{ use trustify_common::{ config::{Database, StorageConfig, StorageStrategy}, db, + model::BinaryByteSize, }; use trustify_infrastructure::{ app::{ @@ -77,6 +79,30 @@ pub struct Run { #[arg(long, env)] pub working_dir: Option, + /// The size limit of SBOMs, uncompressed. + #[arg( + long, + env = "TRUSTD_SBOM_UPLOAD_LIMIT", + default_value_t = default::sbom_upload_limit() + )] + pub sbom_upload_limit: BinaryByteSize, + + /// The size limit of advisories, uncompressed. + #[arg( + long, + env = "TRUSTD_ADVISORY_UPLOAD_LIMIT", + default_value_t = default::advisory_upload_limit() + )] + pub advisory_upload_limit: BinaryByteSize, + + /// The size limit of documents in a dataset, uncompressed. + #[arg( + long, + env = "TRUSTD_DATASET_ENTRY_LIMIT", + default_value_t = default::dataset_entry_limit() + )] + pub dataset_entry_limit: BinaryByteSize, + // flattened commands must go last // /// Database configuration @@ -103,6 +129,23 @@ pub struct Run { pub ui: UiConfig, } +mod default { + use bytesize::ByteSize; + use trustify_common::model::BinaryByteSize; + + pub const fn sbom_upload_limit() -> BinaryByteSize { + BinaryByteSize(ByteSize::gib(1)) + } + + pub const fn advisory_upload_limit() -> BinaryByteSize { + BinaryByteSize(ByteSize::mib(128)) + } + + pub const fn dataset_entry_limit() -> BinaryByteSize { + BinaryByteSize(ByteSize::gib(1)) + } +} + #[derive(clap::Args, Debug, Clone)] #[command(next_help_heading = "UI")] #[group(id = "ui")] @@ -136,6 +179,14 @@ struct InitData { ui: UI, working_dir: Option, with_graphql: bool, + config: ModuleConfig, +} + +/// Groups all module configurations. +#[derive(Clone, Default)] +struct ModuleConfig { + fundamental: trustify_module_fundamental::endpoints::Config, + ingestor: trustify_module_ingestor::endpoints::Config, } impl Run { @@ -236,10 +287,21 @@ impl InitData { analytics_write_key: run.ui.analytics_write_key.unwrap_or_default(), }; + let config = ModuleConfig { + fundamental: trustify_module_fundamental::endpoints::Config { + sbom_upload_limit: run.sbom_upload_limit.into(), + advisory_upload_limit: run.advisory_upload_limit.into(), + }, + ingestor: trustify_module_ingestor::endpoints::Config { + dataset_entry_limit: run.dataset_entry_limit.into(), + }, + }; + Ok(InitData { authenticator, authorizer, db, + config, http: run.http, tracing: run.infra.tracing, swagger_oidc, @@ -266,12 +328,15 @@ impl InitData { .configure(move |svc| { configure( svc, - self.db.clone(), - self.storage.clone(), - self.swagger_oidc.clone(), - self.authenticator.clone(), - ui.clone(), - self.with_graphql, + Config { + config: self.config.clone(), + db: self.db.clone(), + storage: self.storage.clone(), + swagger_oidc: self.swagger_oidc.clone(), + auth: self.authenticator.clone(), + ui: ui.clone(), + with_graphql: self.with_graphql, + }, ); }) }; @@ -299,15 +364,30 @@ impl InitData { } } -fn configure( - svc: &mut web::ServiceConfig, +struct Config { + config: ModuleConfig, db: db::Database, - storage: impl Into + Clone, + storage: DispatchBackend, swagger_oidc: Option>, auth: Option>, ui: Arc, with_graphql: bool, -) { +} + +fn configure(svc: &mut web::ServiceConfig, config: Config) { + let Config { + config: ModuleConfig { + ingestor, + fundamental, + }, + db, + storage, + swagger_oidc, + auth, + ui, + with_graphql, + } = config; + let graph = Graph::new(db.clone()); // set global request limits @@ -365,8 +445,18 @@ fn configure( svc.app_data(graph) .service(web::scope("/api").wrap(new_auth(auth)).configure(|svc| { trustify_module_importer::endpoints::configure(svc, db.clone()); - trustify_module_ingestor::endpoints::configure(svc, db.clone(), storage.clone()); - trustify_module_fundamental::endpoints::configure(svc, db.clone(), storage); + trustify_module_ingestor::endpoints::configure( + svc, + ingestor, + db.clone(), + storage.clone(), + ); + trustify_module_fundamental::endpoints::configure( + svc, + fundamental, + db.clone(), + storage, + ); trustify_module_analysis::endpoints::configure(svc, db.clone()); })) .configure(|svc| { @@ -387,8 +477,8 @@ fn build_url(ci: &ConnectionInfo, path: impl Display) -> Option { #[cfg(test)] mod test { - use std::sync::Arc; - + use super::*; + use crate::Config; use actix_web::{ body::{to_bytes, to_bytes_limited}, http::{header, StatusCode}, @@ -396,9 +486,12 @@ mod test { web::{self, Bytes}, App, }; + use std::sync::Arc; use test_context::test_context; use test_log::test; - use trustify_module_storage::service::fs::FileSystemBackend; + use trustify_module_storage::{ + service::dispatch::DispatchBackend, service::fs::FileSystemBackend, + }; use trustify_module_ui::{endpoints::UiResources, UI}; use trustify_test_context::TrustifyContext; @@ -408,9 +501,20 @@ mod test { let db = ctx.db; let (storage, _) = FileSystemBackend::for_test().await?; let ui = Arc::new(UiResources::new(&UI::default())?); - let app = actix_web::test::init_service( - App::new().configure(|svc| super::configure(svc, db, storage, None, None, ui, true)), - ) + let app = actix_web::test::init_service(App::new().configure(|svc| { + configure( + svc, + Config { + config: ModuleConfig::default(), + db, + storage: DispatchBackend::Filesystem(storage), + swagger_oidc: None, + auth: None, + ui, + with_graphql: true, + }, + ) + })) .await; // main UI diff --git a/test-context/Cargo.toml b/test-context/Cargo.toml index 0db9d5dfe..a4a759c8f 100644 --- a/test-context/Cargo.toml +++ b/test-context/Cargo.toml @@ -6,12 +6,16 @@ publish.workspace = true license.workspace = true [dependencies] +trustify-auth = { workspace = true } trustify-common = { workspace = true } trustify-migration = { workspace = true } trustify-module-ingestor = { workspace = true } trustify-module-storage = { workspace = true } +actix-http = { workspace = true } +actix-web = { workspace = true } anyhow = { workspace = true } +bytes = { workspace = true } futures = { workspace = true } liblzma = { workspace = true } log = { workspace = true } diff --git a/test-context/src/call.rs b/test-context/src/call.rs new file mode 100644 index 000000000..b9dd4bc2a --- /dev/null +++ b/test-context/src/call.rs @@ -0,0 +1,32 @@ +use actix_http::Request; +use actix_web::{ + dev::{Service, ServiceResponse}, + Error, +}; +use bytes::Bytes; +use serde::de::DeserializeOwned; +use std::future::Future; + +/// A trait wrapping an `impl Service` in a way that we can pass it as a reference. +pub trait CallService { + fn call_service(&self, s: Request) -> impl Future; + fn call_and_read_body(&self, r: Request) -> impl Future; + fn call_and_read_body_json(&self, r: Request) -> impl Future; +} + +impl CallService for S +where + S: Service, +{ + async fn call_service(&self, r: Request) -> ServiceResponse { + actix_web::test::call_service(self, r).await + } + + async fn call_and_read_body(&self, r: Request) -> Bytes { + actix_web::test::call_and_read_body(self, r).await + } + + async fn call_and_read_body_json(&self, r: Request) -> T { + actix_web::test::call_and_read_body_json(self, r).await + } +} diff --git a/test-context/src/lib.rs b/test-context/src/lib.rs index aa598016f..9a4ba1e76 100644 --- a/test-context/src/lib.rs +++ b/test-context/src/lib.rs @@ -1,5 +1,6 @@ #![allow(clippy::expect_used)] +pub mod call; pub mod spdx; use futures::Stream; @@ -144,7 +145,7 @@ fn absolute(path: impl AsRef) -> Result { /// Load a test document and decompress it, if necessary. pub async fn document_bytes(path: &str) -> Result { let bytes = document_bytes_raw(path).await?; - let bytes = decompress_async(bytes, None).await??; + let bytes = decompress_async(bytes, None, 0).await??; Ok(bytes) }