Skip to content

Commit

Permalink
Change: set Scan ID of Scan Model to non-optional
Browse files Browse the repository at this point in the history
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.
The new implementation automatically fills in an UUID for the scan ID, when parsing a Scan Config.
  • Loading branch information
Kraemii committed Feb 15, 2024
1 parent 9b193b4 commit abf2686
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 52 deletions.
1 change: 1 addition & 0 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions rust/models/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ 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 }
uuid = { version = "1", features = ["v4"] }


[features]
default = ["serde_support", "bincode_support"]
Expand Down
8 changes: 6 additions & 2 deletions rust/models/src/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ use super::{scanner_preference::ScannerPreference, target::Target, vt::VT};
)]
#[cfg_attr(feature = "bincode_support", derive(bincode::Encode, bincode::Decode))]
pub struct Scan {
#[cfg_attr(feature = "serde_support", serde(default))]
#[cfg_attr(feature = "serde_support", serde(skip_deserializing, default = "uuid"))]
/// Unique ID of a scan
pub scan_id: Option<String>,
pub scan_id: String,
/// Information about the target to scan
pub target: Target,
#[cfg_attr(feature = "serde_support", serde(default))]
Expand All @@ -24,3 +24,7 @@ pub struct Scan {
/// List of VTs to execute for the target
pub vts: Vec<VT>,
}

fn uuid() -> String {
uuid::Uuid::new_v4().to_string()
}
8 changes: 2 additions & 6 deletions rust/openvasctl/src/ctl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(());
}

Expand Down
28 changes: 20 additions & 8 deletions rust/openvasd/src/controller/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,9 @@ where
(&Method::POST, Scans(None)) => {
match crate::request::json_request::<models::Scan, _>(&ctx.response, req).await
{
Ok(mut scan) => {
if scan.scan_id.is_some() {
return Ok(ctx
.response
.bad_request("field scan_id is not allowed to be set."));
}
let id = uuid::Uuid::new_v4().to_string();
Ok(scan) => {
let id = scan.scan_id.clone();
let resp = ctx.response.created(&id);
scan.scan_id = Some(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);
Expand Down Expand Up @@ -400,8 +394,26 @@ where
}
}

<<<<<<< HEAD
(&Method::GET, Vts(oid)) => {
let query = req.uri().query();
=======
(&Method::GET, Vts) => {
let oids = ctx.db.oids().await?;

Ok(ctx.response.ok_json_stream(oids).await)
}
(&Method::GET, GetVts(None)) => match &ctx.redis_cache {
Some(cache) => match cache.get_vts(None).await {
Ok(nvts) => Ok(ctx.response.ok(&nvts)),
Err(err) => Ok(ctx.response.internal_server_error(&err)),
},
None => Ok(ctx.response.empty(hyper::StatusCode::OK)),
},
(&Method::GET, GetVts(Some(vt_selection))) => {
let selection: Vec<String> =
vt_selection.split(',').map(|x| x.to_string()).collect();
>>>>>>> Change: set Scan ID of Scan Model to non-optional

let meta = match query {
Some("information=true") => true,
Expand Down
2 changes: 1 addition & 1 deletion rust/openvasd/src/controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: String::new(),
..Default::default()
};
let ctx = Arc::new(Context::default());
Expand Down
10 changes: 5 additions & 5 deletions rust/openvasd/src/storage/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
20 changes: 9 additions & 11 deletions rust/openvasd/src/storage/inmemory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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();
}

Expand All @@ -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<crypt::ChaCha20Crypt>) -> 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
}
Expand All @@ -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
Expand Down
27 changes: 10 additions & 17 deletions rust/osp/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,50 +28,43 @@ type Writer = quick_xml::Writer<Cursor<Vec<u8>>>;

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<Vec<u8>> {
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)
}

/// Returns the XML representation of the command.
pub fn try_to_xml(&self) -> Result<Vec<u8>> {
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(()))
}
}
}
Expand Down

0 comments on commit abf2686

Please sign in to comment.