feat: Added result parser#6
Conversation
McPatate
left a comment
McPatate
left a comment
There was a problem hiding this comment.
Nice! I like what you did in the parsing :)
FYI in the case of ALLMATCHSCAN, clamav can return multiple results for a single file. That was why I initially had infection_types be a Vec.
It might interesting to implement Add for results
| /// Occurs when the response from clamd is not what the library | ||
| /// expects. Contains the invalid response. | ||
| #[error("invalid response from clamd: {0}")] | ||
| InvalidResponse(String), |
There was a problem hiding this comment.
Why was this block moved? It's triggering my OCD (it's not in alphabetical order anymore 😄)
There was a problem hiding this comment.
Well, it wasn't in the first place anyways ha
There was a problem hiding this comment.
I'd rename the file scan.rs rather than util.rs
There was a problem hiding this comment.
hmm or scan_result.rs
| } | ||
|
|
||
| #[derive(Debug, PartialEq)] | ||
| pub enum ScanCondition { |
There was a problem hiding this comment.
| pub enum ScanCondition { | |
| pub enum ScanStatus { |
?
| Benign, | ||
| } | ||
|
|
||
| enum ScanAnswer<'a> { |
There was a problem hiding this comment.
| enum ScanAnswer<'a> { | |
| enum ScanResponse<'a> { |
?
| pub scan_id: u64, | ||
| /// the scanned entity, can be a file path, the word "stream" | ||
| pub scanned_entity: String, | ||
| pub condition: ScanCondition, |
There was a problem hiding this comment.
| pub condition: ScanCondition, | |
| pub status: ScanStatus, |
| clamd_client.end_session().await?; | ||
| Ok(()) | ||
| } | ||
| // TODO 2023-07-17 to be replaced by a better interface |
There was a problem hiding this comment.
Wdym? Is this for another PR?
There was a problem hiding this comment.
Yeah I have an idea for how to do sessions in a nice, tokio/rust friendly way; but I guess I should've put that into that PR to not tangle it into here. I'll change that back for now.
| let mut bytes = BytesMut::new(); | ||
|
|
||
| let eicar_bytes = reqwest::get("https://secure.eicar.org/eicarcom2.zip") | ||
| .await? | ||
| .bytes() | ||
| .await?; | ||
|
|
||
| let result = clamd_client.scan_bytes(&eicar_bytes).await?; | ||
| match result { | ||
| ScanResult::Malignent { infection_types } => { | ||
| debug!("clamd found a virus(es):\n{}", infection_types.join("\n")) | ||
| bytes.extend(eicar_bytes); | ||
| bytes.extend( "exec('aW1wb3J0IHNvY2tldCxvcwpzbz1zb2NrZXQuc29ja2V0KHNvY2tldC5BRl')\n\nimport base64,sys;exec(base64.b64decode({2:str,3:lambda b:bytes()}))".as_bytes()); |
There was a problem hiding this comment.
What was the reasoning behind adding multiple viruses?
There was a problem hiding this comment.
Trying out whether scan returns multiple signatures. Forgot to remove it again :D
|
Yeah you are right, I forgot about that 😀 the issue with the old implementation is that the Framed Codec already splits the result based on the terminating |
|
Ah yes that makes sense, let's go with |
2 participants
This change adds a simple parser to be more strict about the response of clamd to expect. Each result contains now one single scan result again because I looked into the C code of clamd and tried out experimentally that clamd will only respond with multiple results when it also receives multiple streams or files. I want to add a nicer interface for sessions in another pull request, so this is preliminary work for this.