From a957dadd58c93ec9492bb839c2435cda1378d5f4 Mon Sep 17 00:00:00 2001 From: Kraemii Date: Thu, 15 Feb 2024 11:20:39 +0100 Subject: [PATCH] Change: set Scan ID of Scan Model to non-optional As the scan ID was set by the server and not by the client, it was set to Optional. This lead to many unnecessary checks, even though it was not possible, that this value is None. --- rust/models/Cargo.toml | 4 ++-- rust/models/src/scan.rs | 2 +- rust/openvasctl/src/ctl.rs | 8 ++------ rust/openvasd/Cargo.toml | 8 ++++---- rust/openvasd/src/controller/entry.rs | 4 ++-- rust/openvasd/src/controller/mod.rs | 2 +- rust/openvasd/src/storage/file.rs | 10 +++++----- rust/openvasd/src/storage/inmemory.rs | 20 +++++++++----------- rust/osp/src/commands.rs | 27 ++++++++++----------------- 9 files changed, 36 insertions(+), 49 deletions(-) diff --git a/rust/models/Cargo.toml b/rust/models/Cargo.toml index ca8d8908ef..1f0853c118 100644 --- a/rust/models/Cargo.toml +++ b/rust/models/Cargo.toml @@ -7,8 +7,8 @@ license = "GPL-2.0-or-later" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -serde = {version = "1", features = ["derive"], optional = true} -bincode = {version = "2.0.0-rc.3", optional = true } +serde = { version = "1", features = ["derive"], optional = true } +bincode = { version = "2.0.0-rc.3", optional = true } [features] default = ["serde_support", "bincode_support"] diff --git a/rust/models/src/scan.rs b/rust/models/src/scan.rs index aac965aa65..2491c2e52e 100644 --- a/rust/models/src/scan.rs +++ b/rust/models/src/scan.rs @@ -15,7 +15,7 @@ use super::{scanner_preference::ScannerPreference, target::Target, vt::VT}; pub struct Scan { #[cfg_attr(feature = "serde_support", serde(default))] /// Unique ID of a scan - pub scan_id: Option, + pub scan_id: String, /// Information about the target to scan pub target: Target, #[cfg_attr(feature = "serde_support", serde(default))] diff --git a/rust/openvasctl/src/ctl.rs b/rust/openvasctl/src/ctl.rs index 869621c9c0..2c9ee0b19b 100644 --- a/rust/openvasctl/src/ctl.rs +++ b/rust/openvasctl/src/ctl.rs @@ -91,11 +91,7 @@ impl OpenvasController { /// Prepare scan and start it pub fn start_scan(&mut self, scan: models::Scan) -> Result<(), OpenvasError> { - let scan_id = match scan.scan_id { - Some(id) => id, - None => return Err(OpenvasError::MissingID), - }; - self.add_init(scan_id.clone()); + self.add_init(scan.scan_id.clone()); // TODO: Create new DB for Scan // TODO: Add Scan ID to DB @@ -107,7 +103,7 @@ impl OpenvasController { // TODO: Prepare reverse lookup option (maybe part of target) // TODO: Prepare alive test option (maybe part of target) - if !self.add_running(scan_id)? { + if !self.add_running(scan.scan_id)? { return Ok(()); } diff --git a/rust/openvasd/Cargo.toml b/rust/openvasd/Cargo.toml index c03adc6746..b446871339 100644 --- a/rust/openvasd/Cargo.toml +++ b/rust/openvasd/Cargo.toml @@ -19,11 +19,11 @@ hyper-rustls = "0" tokio = { version = "1.28.1", features = ["full"] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } -serde_json = "1.0.96" -serde = { version = "1.0.163", features = ["derive"] } +serde_json = "1" +serde = { version = "1", features = ["derive"] } bincode = "1" uuid = { version = "1", features = ["v4", "fast-rng", "serde"] } -rustls = {version = "0.22"} +rustls = { version = "0.22" } tokio-rustls = "0.25" futures-util = "0.3.28" rustls-pemfile = "1.0.2" @@ -36,7 +36,7 @@ pbkdf2 = { version = "0.12.2", features = ["password-hash"] } sha2 = "0.10.7" generic-array = "0.14.7" base64 = "0.21.2" -hyper-util = { version = "0", features = [ "tokio" ]} +hyper-util = { version = "0", features = ["tokio"] } http-body-util = "0.1.0" http-body = "1" diff --git a/rust/openvasd/src/controller/entry.rs b/rust/openvasd/src/controller/entry.rs index 20b74191a8..b8cb9f4d00 100644 --- a/rust/openvasd/src/controller/entry.rs +++ b/rust/openvasd/src/controller/entry.rs @@ -257,14 +257,14 @@ where match crate::request::json_request::(&ctx.response, req).await { Ok(mut scan) => { - if scan.scan_id.is_some() { + if !scan.scan_id.is_empty() { return Ok(ctx .response .bad_request("field scan_id is not allowed to be set.")); } let id = uuid::Uuid::new_v4().to_string(); let resp = ctx.response.created(&id); - scan.scan_id = Some(id.clone()); + scan.scan_id = id.clone(); ctx.db.insert_scan(scan).await?; ctx.db.add_scan_client_id(id.clone(), cid).await?; tracing::debug!("Scan with ID {} created", &id); diff --git a/rust/openvasd/src/controller/mod.rs b/rust/openvasd/src/controller/mod.rs index 87992cf22b..35ad40584d 100644 --- a/rust/openvasd/src/controller/mod.rs +++ b/rust/openvasd/src/controller/mod.rs @@ -361,7 +361,7 @@ mod tests { #[tokio::test] async fn add_scan_with_id_fails() { let scan: models::Scan = models::Scan { - scan_id: Some(String::new()), + scan_id: "test".to_string(), ..Default::default() }; let ctx = Arc::new(Context::default()); diff --git a/rust/openvasd/src/storage/file.rs b/rust/openvasd/src/storage/file.rs index 6e3f1c7fa7..d16759e06a 100644 --- a/rust/openvasd/src/storage/file.rs +++ b/rust/openvasd/src/storage/file.rs @@ -212,7 +212,7 @@ where S: infisto::base::IndexedByteStorage + std::marker::Sync + std::marker::Send + Clone + 'static, { async fn insert_scan(&self, scan: models::Scan) -> Result<(), Error> { - let id = scan.scan_id.clone().unwrap_or_default(); + let id = scan.scan_id.clone(); let key = format!("scan_{id}"); let status_key = format!("status_{id}"); let storage = Arc::clone(&self.storage); @@ -562,7 +562,7 @@ mod tests { } "#; let mut scan: Scan = serde_json::from_str(jraw).unwrap(); - scan.scan_id = Some("aha".to_string()); + scan.scan_id = "aha".to_string(); let storage = infisto::base::CachedIndexFileStorer::init("/tmp/openvasd/credential").unwrap(); let nfp = "../../examples/feed/nasl"; @@ -615,7 +615,7 @@ mod tests { let mut scans = Vec::with_capacity(100); for i in 0..100 { let scan = Scan { - scan_id: Some(i.to_string()), + scan_id: i.to_string(), ..Default::default() }; scans.push(scan); @@ -631,7 +631,7 @@ mod tests { } for s in scans.clone().into_iter() { - storage.get_scan(&s.scan_id.unwrap()).await.unwrap(); + storage.get_scan(&s.scan_id).await.unwrap(); } storage.remove_scan("5").await.unwrap(); storage.insert_scan(scans[5].clone()).await.unwrap(); @@ -665,7 +665,7 @@ mod tests { .collect(); assert_eq!(2, range.len()); for s in scans { - let _ = storage.remove_scan(&s.scan_id.unwrap_or_default()).await; + let _ = storage.remove_scan(&s.scan_id).await; } let ids = storage.get_scan_ids().await.unwrap(); diff --git a/rust/openvasd/src/storage/inmemory.rs b/rust/openvasd/src/storage/inmemory.rs index cfa5b3798a..9f94391315 100644 --- a/rust/openvasd/src/storage/inmemory.rs +++ b/rust/openvasd/src/storage/inmemory.rs @@ -184,7 +184,7 @@ where E: crate::crypt::Crypt + Send + Sync + 'static, { async fn insert_scan(&self, sp: models::Scan) -> Result<(), Error> { - let id = sp.scan_id.clone().unwrap_or_default(); + let id = sp.scan_id.clone(); let mut scans = self.scans.write().await; if let Some(prgs) = scans.get_mut(&id) { prgs.scan = sp; @@ -243,9 +243,7 @@ where let scans = self.scans.read().await; let mut result = Vec::with_capacity(scans.len()); for (_, progress) in scans.iter() { - if let Some(id) = progress.scan.scan_id.as_ref() { - result.push(id.clone()); - } + result.push(progress.scan.scan_id.clone()); } Ok(result) } @@ -441,10 +439,10 @@ mod tests { async fn store_delete_scan() { let storage = Storage::default(); let scan = Scan::default(); - let id = scan.scan_id.clone().unwrap_or_default(); + let id = scan.scan_id.clone(); storage.insert_scan(scan).await.unwrap(); let (retrieved, _) = storage.get_scan(&id).await.unwrap(); - assert_eq!(retrieved.scan_id.unwrap_or_default(), id); + assert_eq!(retrieved.scan_id, id); storage.remove_scan(&id).await.unwrap(); } @@ -462,21 +460,21 @@ mod tests { scan.target.credentials = vec![pw]; - let id = scan.scan_id.clone().unwrap_or_default(); + let id = scan.scan_id.clone(); storage.insert_scan(scan).await.unwrap(); let (retrieved, _) = storage.get_scan(&id).await.unwrap(); - assert_eq!(retrieved.scan_id.unwrap_or_default(), id); + assert_eq!(retrieved.scan_id, id); assert_ne!(retrieved.target.credentials[0].password(), "test"); let (retrieved, _) = storage.get_decrypted_scan(&id).await.unwrap(); - assert_eq!(retrieved.scan_id.unwrap_or_default(), id); + assert_eq!(retrieved.scan_id, id); assert_eq!(retrieved.target.credentials[0].password(), "test"); } async fn store_scan(storage: &Storage) -> String { let mut scan = Scan::default(); let id = uuid::Uuid::new_v4().to_string(); - scan.scan_id = Some(id.clone()); + scan.scan_id = id.clone(); storage.insert_scan(scan).await.unwrap(); id } @@ -495,7 +493,7 @@ mod tests { async fn append_results() { let storage = Storage::default(); let scan = Scan::default(); - let id = scan.scan_id.clone().unwrap_or_default(); + let id = scan.scan_id.clone(); storage.insert_scan(scan).await.unwrap(); let fetch_result = (models::Status::default(), vec![models::Result::default()]); storage diff --git a/rust/osp/src/commands.rs b/rust/osp/src/commands.rs index 445be7ee33..e1e147051c 100644 --- a/rust/osp/src/commands.rs +++ b/rust/osp/src/commands.rs @@ -28,17 +28,13 @@ type Writer = quick_xml::Writer>>; impl<'a> ScanCommand<'a> { fn as_byte_response( - scan_id: Option<&str>, + scan_id: &str, element_name: &str, additional: &[(&str, &str)], f: &mut dyn FnMut(&mut Writer) -> Result<()>, ) -> Result> { let mut writer = Writer::new(Cursor::new(Vec::new())); - if let Some(scan_id) = &scan_id { - writer.within_id_element(IDAttribute::ScanID(scan_id), additional, element_name, f)?; - } else { - writer.within_element(element_name, f)?; - } + writer.within_id_element(IDAttribute::ScanID(scan_id), additional, element_name, f)?; let result = writer.into_inner().into_inner(); Ok(result) } @@ -46,32 +42,29 @@ impl<'a> ScanCommand<'a> { /// Returns the XML representation of the command. pub fn try_to_xml(&self) -> Result> { match self { - ScanCommand::Start(scan) => ScanCommand::as_byte_response( - scan.scan_id.as_deref(), - "start_scan", - &[], - &mut |writer| { + ScanCommand::Start(scan) => { + ScanCommand::as_byte_response(&scan.scan_id, "start_scan", &[], &mut |writer| { write_vts(scan, writer)?; write_target(scan, writer)?; write_scanner_prefs(scan, writer)?; Ok(()) - }, - ), + }) + } ScanCommand::Delete(scan_id) => { - ScanCommand::as_byte_response(Some(scan_id), "delete_scan", &[], &mut |_| Ok(())) + ScanCommand::as_byte_response(scan_id, "delete_scan", &[], &mut |_| Ok(())) } ScanCommand::Stop(scan_id) => { - ScanCommand::as_byte_response(Some(scan_id), "stop_scan", &[], &mut |_| Ok(())) + ScanCommand::as_byte_response(scan_id, "stop_scan", &[], &mut |_| Ok(())) } ScanCommand::GetDelete(scan_id) => ScanCommand::as_byte_response( - Some(scan_id), + scan_id, "get_scans", // removes results from ospd-openvas &[("pop_results", "1"), ("progress", "1")], &mut |_| Ok(()), ), ScanCommand::Get(scan_id) => { - ScanCommand::as_byte_response(Some(scan_id), "get_scans", &[], &mut |_| Ok(())) + ScanCommand::as_byte_response(scan_id, "get_scans", &[], &mut |_| Ok(())) } } }