diff --git a/libclamav_rust/build.rs b/libclamav_rust/build.rs index e2f90a50cb..9f16bb98b3 100644 --- a/libclamav_rust/build.rs +++ b/libclamav_rust/build.rs @@ -182,7 +182,7 @@ fn detect_clamav_build() -> Result<(), &'static str> { // LLVM is optional, and don't have a path to each library like we do with the other libs. let llvm_libs = env::var("LLVM_LIBS").unwrap_or("".into()); - if llvm_libs != "" { + if !llvm_libs.is_empty() { match env::var("LLVM_DIRS") { Err(env::VarError::NotPresent) => eprintln!("LLVM_DIRS not set"), Err(env::VarError::NotUnicode(_)) => return Err("environment value not unicode"), @@ -199,7 +199,7 @@ fn detect_clamav_build() -> Result<(), &'static str> { llvm_libs .split(',') - .for_each(|filepath_str| match parse_lib_path(&filepath_str) { + .for_each(|filepath_str| match parse_lib_path(filepath_str) { Ok(parsed_path) => { println!("cargo:rustc-link-search={}", parsed_path.dir); eprintln!(" - requesting that rustc link {:?}", &parsed_path.libname); @@ -278,7 +278,7 @@ struct ParsedLibraryPath { // Parse a library path, returning the portion expected after the `-l`, and the // directory containing the library -fn parse_lib_path<'a>(path: &'a str) -> Result { +fn parse_lib_path(path: &str) -> Result { let path = PathBuf::from(path); let file_name = path .file_name() diff --git a/libclamav_rust/src/cdiff.rs b/libclamav_rust/src/cdiff.rs index 57c0c12b59..dc369906c7 100644 --- a/libclamav_rust/src/cdiff.rs +++ b/libclamav_rust/src/cdiff.rs @@ -25,8 +25,8 @@ use std::{ io::{prelude::*, BufReader, BufWriter, Read, Seek, SeekFrom, Write}, iter::*, os::raw::c_char, - path::PathBuf, - str, + path::{Path, PathBuf}, + str::{self, FromStr}, }; use crate::sys; @@ -97,8 +97,12 @@ pub enum CdiffError { /// /// This error *may* wrap a processing error if the command has side effects /// (e.g., MOVE or CLOSE) - #[error("{1} on line {0}")] - Input(usize, InputError), + #[error("{err} on line {line}: {operation}")] + Input { + line: usize, + err: InputError, + operation: String, + }, /// An error encountered while handling a particular CDiff command #[error("processing {1} command on line {2}: {0}")] @@ -155,14 +159,20 @@ pub enum InputError { #[error("No DB open for action {0}")] NoDBForAction(&'static str), + #[error("Invalid DB \"{0}\" open for action {1}")] + InvalidDBForAction(String, &'static str), + #[error("File {0} not closed before opening {1}")] NotClosedBeforeOpening(String, String), #[error("{0} not Unicode")] NotUnicode(&'static str), - #[error("Forbidden characters found in database name {0}")] - ForbiddenCharactersInDB(String), + #[error("Invalid database name {0}. Characters must be alphanumeric or '.'")] + InvalidDBNameForbiddenCharacters(String), + + #[error("Invalid database name {0}. Must not specify parent directory.")] + InvalidDBNameNoParentDirectory(String), #[error("{0} missing for {1}")] MissingParameter(&'static str, &'static str), @@ -183,6 +193,9 @@ pub enum InputError { #[error("no final newline")] MissingNL, + + #[error("Database file is still open: {0}")] + DBStillOpen(String), } /// Errors encountered while processing @@ -286,6 +299,43 @@ impl<'a> DelOp<'a> { } } +#[derive(Debug)] +pub struct UnlinkOp<'a> { + db_name: &'a str, +} + +/// Method to parse the cdiff line describing an unlink operation +impl<'a> UnlinkOp<'a> { + pub fn new(data: &'a [u8]) -> Result { + let mut iter = data.split(|b| *b == b' '); + let db_name = str::from_utf8( + iter.next() + .ok_or(InputError::MissingParameter("UNLINK", "db_name"))?, + ) + .map_err(|_| InputError::NotUnicode("database name"))?; + + if !db_name + .chars() + .all(|x: char| x.is_alphanumeric() || x == '.') + { + // DB Name contains invalid characters. + return Err(InputError::InvalidDBNameForbiddenCharacters( + db_name.to_owned(), + )); + } + + let db_path = PathBuf::from_str(db_name).unwrap(); + if db_path.parent() != Some(Path::new("")) { + // DB Name must be not include a parent directory. + return Err(InputError::InvalidDBNameNoParentDirectory( + db_name.to_owned(), + )); + } + + Ok(UnlinkOp { db_name }) + } +} + #[derive(Debug)] pub struct MoveOp<'a> { src: PathBuf, @@ -448,7 +498,7 @@ pub fn script2cdiff(script_file_name: &str, builder: &str, server: &str) -> Resu .map_err(|e| CdiffError::FileCreate(cdiff_file_name.to_owned(), e))?; // Open the original script file for reading - let script_file: File = File::open(&script_file_name) + let script_file: File = File::open(script_file_name) .map_err(|e| CdiffError::FileOpen(script_file_name.to_owned(), e))?; // Get file length @@ -570,7 +620,7 @@ pub fn cdiff_apply(file: &mut File, mode: ApplyMode) -> Result<(), CdiffError> { let dsig = read_dsig(file)?; debug!("cdiff_apply() - final dsig length is {}", dsig.len()); if is_debug_enabled() { - print_file_data(dsig.clone(), dsig.len() as usize); + print_file_data(dsig.clone(), dsig.len()); } // Get file length @@ -644,10 +694,22 @@ fn cmd_open(ctx: &mut Context, db_name: Option<&[u8]>) -> Result<(), InputError> if !db_name .chars() - .all(|x: char| x.is_alphanumeric() || x == '\\' || x == '/' || x == '.') + .all(|x: char| x.is_alphanumeric() || x == '.') { - return Err(InputError::ForbiddenCharactersInDB(db_name.to_owned())); + // DB Name contains invalid characters. + return Err(InputError::InvalidDBNameForbiddenCharacters( + db_name.to_owned(), + )); } + + let db_path = PathBuf::from_str(db_name).unwrap(); + if db_path.parent() != Some(Path::new("")) { + // DB Name must be not include a parent directory. + return Err(InputError::InvalidDBNameNoParentDirectory( + db_name.to_owned(), + )); + } + ctx.open_db = Some(db_name.to_owned()); Ok(()) @@ -912,6 +974,7 @@ fn cmd_close(ctx: &mut Context) -> Result<(), InputError> { // Test for lines to add if !ctx.additions.is_empty() { let mut db_file = OpenOptions::new() + .create(true) .append(true) .open(&open_db) .map_err(ProcessingError::from)?; @@ -927,12 +990,16 @@ fn cmd_close(ctx: &mut Context) -> Result<(), InputError> { } /// Set up Context structure with data parsed from command unlink -fn cmd_unlink(ctx: &mut Context) -> Result<(), InputError> { +fn cmd_unlink(ctx: &mut Context, unlink_op: UnlinkOp) -> Result<(), InputError> { if let Some(open_db) = &ctx.open_db { - fs::remove_file(open_db).map_err(ProcessingError::from)?; - } else { - return Err(InputError::NoDBForAction("UNLINK")); + return Err(InputError::DBStillOpen(open_db.clone())); } + + // We checked that the db_name doesn't have any '/' or '\\' in it before + // adding to the UnlinkOp struct, so it's safe to say the path is just a local file and + // won't accidentally delete something in a different directory. + fs::remove_file(unlink_op.db_name).map_err(ProcessingError::from)?; + Ok(()) } @@ -960,9 +1027,12 @@ fn process_line(ctx: &mut Context, line: &[u8]) -> Result<(), InputError> { cmd_move(ctx, move_op) } b"CLOSE" => cmd_close(ctx), - b"UNLINK" => cmd_unlink(ctx), + b"UNLINK" => { + let unlink_op = UnlinkOp::new(remainder.unwrap())?; + cmd_unlink(ctx, unlink_op) + } _ => Err(InputError::UnknownCommand( - String::from_utf8(cmd.to_owned()).unwrap(), + String::from_utf8_lossy(&cmd).to_string(), )), } } @@ -986,10 +1056,14 @@ where 0 => break, n_read => { decompressed_bytes = decompressed_bytes + n_read + 1; - match linebuf.get(0) { + match linebuf.first() { // Skip comment lines Some(b'#') => continue, - _ => process_line(ctx, &linebuf).map_err(|e| CdiffError::Input(line_no, e))?, + _ => process_line(ctx, &linebuf).map_err(|e| CdiffError::Input { + line: line_no, + err: e, + operation: String::from_utf8_lossy(&linebuf).to_string(), + })?, } } } @@ -1035,7 +1109,7 @@ fn read_dsig(file: &mut File) -> Result, SignatureError> { // as the offset in the file that the header ends. fn read_size(file: &mut File) -> Result<(u32, usize), HeaderError> { // Seek to beginning of file. - file.seek(SeekFrom::Start(0))?; + file.rewind()?; // File should always start with "ClamAV-Diff". let prefix = b"ClamAV-Diff"; @@ -1080,7 +1154,7 @@ fn get_hash(file: &mut File, len: usize) -> Result<[u8; 32], CdiffError> { let mut hasher = Sha256::new(); // Seek to beginning of file - file.seek(SeekFrom::Start(0))?; + file.rewind()?; let mut sum: usize = 0; diff --git a/libclamav_rust/src/fuzzy_hash.rs b/libclamav_rust/src/fuzzy_hash.rs index fd4dd5aed3..8ac89e9fc2 100644 --- a/libclamav_rust/src/fuzzy_hash.rs +++ b/libclamav_rust/src/fuzzy_hash.rs @@ -123,7 +123,7 @@ pub struct FuzzyHashMeta { /// Initialize the hashmap #[no_mangle] pub extern "C" fn fuzzy_hashmap_new() -> sys::fuzzyhashmap_t { - Box::into_raw(Box::new(FuzzyHashMap::default())) as sys::fuzzyhashmap_t + Box::into_raw(Box::::default()) as sys::fuzzyhashmap_t } /// Free the hashmap diff --git a/libclamav_rust/src/logging.rs b/libclamav_rust/src/logging.rs index 7e70cf8eaf..7c0af7b863 100644 --- a/libclamav_rust/src/logging.rs +++ b/libclamav_rust/src/logging.rs @@ -88,8 +88,12 @@ mod tests { /// API exported for C code to log to standard error using Rust. /// This would be be an alternative to fputs, and reliably prints /// non-ASCII UTF8 characters on Windows, where fputs does not. +/// +/// # Safety +/// +/// This function dereferences the c_buff raw pointer. Pointer must be valid. #[no_mangle] -pub extern "C" fn clrs_eprint(c_buf: *const c_char) -> () { +pub unsafe extern "C" fn clrs_eprint(c_buf: *const c_char) { if c_buf.is_null() { return; } diff --git a/libfreshclam/dns.c b/libfreshclam/dns.c index 9536dd3c62..8f63abe6ba 100644 --- a/libfreshclam/dns.c +++ b/libfreshclam/dns.c @@ -71,12 +71,12 @@ dnsquery(const char *domain, int qtype, unsigned int *ttl) if (qtype == T_TXT) qtype = T_ANY; if ((len = res_query(domain, C_IN, qtype, answer, PACKETSZ)) < 0) { - logg(LOGG_INFO, "%cCan't query %s\n", - (qtype == T_TXT || qtype == T_ANY) ? '^' : '*', domain); + logg((qtype == T_TXT || qtype == T_ANY) ? LOGG_WARNING : LOGG_DEBUG, "Can't query %s\n", + domain); return NULL; } #else - logg(LOGG_INFO, "%cCan't query %s\n", (qtype == T_TXT) ? '^' : '*', domain); + logg((qtype == T_TXT) ? LOGG_WARNING : LOGG_DEBUG, "Can't query %s\n", domain); return NULL; #endif } diff --git a/libfreshclam/libfreshclam.c b/libfreshclam/libfreshclam.c index 73a44fc9d9..5783f4764f 100644 --- a/libfreshclam/libfreshclam.c +++ b/libfreshclam/libfreshclam.c @@ -56,6 +56,7 @@ // libclamav #include "clamav.h" +#include "clamav_rust.h" #include "others.h" #include "regex_list.h" #include "str.h" @@ -132,6 +133,12 @@ fc_error_t fc_initialize(fc_config *fcConfig) return status; } + /* Rust logging initialization */ + if (!clrs_log_init()) { + cli_dbgmsg("Unexpected problem occurred while setting up rust logging... continuing without rust logging. \ + Please submit an issue to https://github.com/Cisco-Talos/clamav"); + } + /* Initilize libcurl */ curl_global_init(CURL_GLOBAL_ALL); diff --git a/libfreshclam/libfreshclam_internal.c b/libfreshclam/libfreshclam_internal.c index 55b7c02c6e..6b10c2621b 100644 --- a/libfreshclam/libfreshclam_internal.c +++ b/libfreshclam/libfreshclam_internal.c @@ -987,11 +987,11 @@ static fc_error_t remote_cvdhead( * show the more generic information from curl_easy_strerror instead. */ size_t len = strlen(errbuf); - logg(LOGG_INFO, "%cremote_cvdhead: Download failed (%d) ", logerr ? '!' : '^', curl_ret); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "remote_cvdhead: Download failed (%d) ", curl_ret); if (len) - logg(LOGG_INFO, "%c Message: %s%s", logerr ? '!' : '^', errbuf, ((errbuf[len - 1] != '\n') ? "\n" : "")); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, " Message: %s%s", errbuf, ((errbuf[len - 1] != '\n') ? "\n" : "")); else - logg(LOGG_INFO, "%c Message: %s\n", logerr ? '!' : '^', curl_easy_strerror(curl_ret)); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, " Message: %s\n", curl_easy_strerror(curl_ret)); status = FC_ECONNECTION; goto done; } @@ -1057,11 +1057,11 @@ static fc_error_t remote_cvdhead( } default: { if (g_proxyServer) - logg(LOGG_INFO, "%cremote_cvdhead: Unexpected response (%li) from %s (Proxy: %s:%u)\n", - logerr ? '!' : '^', http_code, server, g_proxyServer, g_proxyPort); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "remote_cvdhead: Unexpected response (%li) from %s (Proxy: %s:%u)\n", + http_code, server, g_proxyServer, g_proxyPort); else - logg(LOGG_INFO, "%cremote_cvdhead: Unexpected response (%li) from %s\n", - logerr ? '!' : '^', http_code, server); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "remote_cvdhead: Unexpected response (%li) from %s\n", + http_code, server); status = FC_EFAILEDGET; goto done; } @@ -1071,7 +1071,7 @@ static fc_error_t remote_cvdhead( * Identify start of CVD header in response body. */ if (receivedData.size < CVD_HEADER_SIZE) { - logg(LOGG_INFO, "%cremote_cvdhead: Malformed CVD header (too short)\n", logerr ? '!' : '^'); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "remote_cvdhead: Malformed CVD header (too short)\n"); status = FC_EFAILEDGET; goto done; } @@ -1087,7 +1087,7 @@ static fc_error_t remote_cvdhead( (receivedData.buffer && !*receivedData.buffer) || (receivedData.buffer && !isprint(receivedData.buffer[i]))) { - logg(LOGG_INFO, "%cremote_cvdhead: Malformed CVD header (bad chars)\n", logerr ? '!' : '^'); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "remote_cvdhead: Malformed CVD header (bad chars)\n"); status = FC_EFAILEDGET; goto done; } @@ -1098,7 +1098,7 @@ static fc_error_t remote_cvdhead( * Parse CVD info into CVD info struct. */ if (!(cvdhead = cl_cvdparse(head))) { - logg(LOGG_INFO, "%cremote_cvdhead: Malformed CVD header (can't parse)\n", logerr ? '!' : '^'); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "remote_cvdhead: Malformed CVD header (can't parse)\n"); status = FC_EFAILEDGET; goto done; } else { @@ -1289,17 +1289,18 @@ static fc_error_t downloadFile( * show the more generic information from curl_easy_strerror instead. */ size_t len = strlen(errbuf); - logg(LOGG_INFO, "%cDownload failed (%d) ", logerr ? '!' : '^', curl_ret); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "Download failed (%d) ", curl_ret); if (len) - logg(LOGG_INFO, "%c Message: %s%s", logerr ? '!' : '^', errbuf, ((errbuf[len - 1] != '\n') ? "\n" : "")); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, " Message: %s%s", errbuf, ((errbuf[len - 1] != '\n') ? "\n" : "")); else - logg(LOGG_INFO, "%c Message: %s\n", logerr ? '!' : '^', curl_easy_strerror(curl_ret)); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, " Message: %s\n", curl_easy_strerror(curl_ret)); status = FC_ECONNECTION; goto done; } /* Check HTTP code */ curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code); + logg(LOGG_WARNING, " ******* RESULT %ld, SIZE: %zu ******* \n", http_code, receivedFile.size); switch (http_code) { case 200: case 206: { @@ -1363,11 +1364,11 @@ static fc_error_t downloadFile( } default: { if (g_proxyServer) - logg(LOGG_INFO, "%cdownloadFile: Unexpected response (%li) from %s (Proxy: %s:%u)\n", - logerr ? '!' : '^', http_code, url, g_proxyServer, g_proxyPort); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "downloadFile: Unexpected response (%li) from %s (Proxy: %s:%u)\n", + http_code, url, g_proxyServer, g_proxyPort); else - logg(LOGG_INFO, "%cdownloadFile: Unexpected response (%li) from %s\n", - logerr ? '!' : '^', http_code, url); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "downloadFile: Unexpected response (%li) from %s\n", + http_code, url); status = FC_EFAILEDGET; } } @@ -1426,7 +1427,7 @@ static fc_error_t getcvd( status = ret; goto done; } else if (ret > FC_UPTODATE) { - logg(LOGG_INFO, "%cCan't download %s from %s\n", logerr ? '!' : '^', cvdfile, url); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "Can't download %s from %s\n", cvdfile, url); status = ret; goto done; } @@ -1625,7 +1626,7 @@ static fc_error_t downloadPatch( if (ret == FC_EEMPTYFILE) { logg(LOGG_INFO, "Empty script %s, need to download entire database\n", patch); } else { - logg(LOGG_INFO, "%cdownloadPatch: Can't download %s from %s\n", logerr ? '!' : '^', patch, url); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "downloadPatch: Can't download %s from %s\n", patch, url); } status = ret; goto done; @@ -2343,6 +2344,7 @@ fc_error_t updatedb( if ( (FC_EEMPTYFILE == ret) || /* Request a new CVD if we got an empty CDIFF. */ + (FC_EFAILEDUPDATE == ret) || /* Request a new CVD if we failed to apply a CDIFF. */ (FC_SUCCESS != ret && ( /* Or if the incremental update failed: */ (0 == numPatchesReceived) && /* 1. Ask for the CVD if we didn't get any patches, */ (localVersion < remoteVersion - 1) /* 2. AND if we're more than 1 version out of date. */ @@ -2388,7 +2390,7 @@ fc_error_t updatedb( size_t newLocalFilenameLen = 0; if (FC_SUCCESS != buildcld(tmpdir, database, tmpfile, g_bCompressLocalDatabase)) { logg(LOGG_ERROR, "updatedb: Incremental update failed. Failed to build CLD.\n"); - status = FC_EFAILEDUPDATE; + status = FC_EBADCVD; goto done; } @@ -2602,7 +2604,7 @@ fc_error_t updatecustomdb( logg(LOGG_INFO, "%s is up-to-date (version: custom database)\n", databaseName); goto up_to_date; } else if (ret > FC_UPTODATE) { - logg(LOGG_INFO, "%cCan't download %s from %s\n", logerr ? '!' : '^', databaseName, url); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "Can't download %s from %s\n", databaseName, url); status = ret; goto done; } diff --git a/sigtool/sigtool.c b/sigtool/sigtool.c index da73ecdac9..ee723d23c5 100644 --- a/sigtool/sigtool.c +++ b/sigtool/sigtool.c @@ -3457,6 +3457,12 @@ int main(int argc, char **argv) } ret = 1; + /* Rust logging initialization */ + if (!clrs_log_init()) { + cli_dbgmsg("Unexpected problem occurred while setting up rust logging... continuing without rust logging. \ + Please submit an issue to https://github.com/Cisco-Talos/clamav"); + } + opts = optparse(NULL, argc, argv, 1, OPT_SIGTOOL, 0, NULL); if (!opts) { mprintf(LOGG_ERROR, "Can't parse command line options\n"); diff --git a/unit_tests/freshclam_test.py b/unit_tests/freshclam_test.py index 62c0a6daf0..ae5b9b8a24 100644 --- a/unit_tests/freshclam_test.py +++ b/unit_tests/freshclam_test.py @@ -87,7 +87,7 @@ def test_freshclam_00_version(self): port=TC.mock_mirror_port, )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} -V'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} -V'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -133,7 +133,7 @@ def test_freshclam_01_file_copy(self): user=getpass.getuser(), )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config}'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config}'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -174,7 +174,7 @@ def test_freshclam_02_http_403(self): port=TC.mock_mirror_port, user=getpass.getuser(), )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=daily'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=daily'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -189,7 +189,7 @@ def test_freshclam_02_http_403(self): # verify stderr self.verify_output(output.err, expected=expected_stderr) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=daily'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=daily'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -229,7 +229,7 @@ def test_freshclam_03_http_403_daemonized(self): port=TC.mock_mirror_port, user=getpass.getuser(), )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=daily --daemon -F'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=daily --daemon -F'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -269,7 +269,7 @@ def test_freshclam_04_http_429(self): port=TC.mock_mirror_port, user=getpass.getuser(), )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=daily'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=daily'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -322,7 +322,7 @@ def test_freshclam_05_cdiff_update(self): port=TC.mock_mirror_port, user=getpass.getuser(), )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -384,7 +384,7 @@ def test_freshclam_05_cdiff_update_UNC(self): port=TC.mock_mirror_port, user=getpass.getuser(), )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -440,7 +440,7 @@ def test_freshclam_06_cdiff_partial_minus_1(self): port=TC.mock_mirror_port, user=getpass.getuser(), )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -465,7 +465,7 @@ def test_freshclam_06_cdiff_partial_minus_1(self): # # Try again, we should be 1 behind which is tolerable and should not trigger a full CVD download # - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -526,7 +526,7 @@ def test_freshclam_07_cdiff_partial_minus_2(self): port=TC.mock_mirror_port, user=getpass.getuser(), )) - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -551,7 +551,7 @@ def test_freshclam_07_cdiff_partial_minus_2(self): # # Try again, we should be 2 behind which is NOT tolerable and SHOULD trigger a full CVD download # - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -617,7 +617,7 @@ def test_freshclam_07_no_cdiff_out_of_date_cvd(self): # # 1st attempt # - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -637,7 +637,7 @@ def test_freshclam_07_no_cdiff_out_of_date_cvd(self): # # 2nd attempt # - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -663,7 +663,7 @@ def test_freshclam_07_no_cdiff_out_of_date_cvd(self): # # 3rd attempt # - command = '{valgrind} {valgrind_args} {freshclam} --config-file={freshclam_config} --update-db=test'.format( + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config ) output = self.execute_command(command) @@ -685,6 +685,92 @@ def test_freshclam_07_no_cdiff_out_of_date_cvd(self): # verify stderr self.verify_output(output.err, unexpected=unexpected_results) + def test_freshclam_08_cdiff_update_twice(self): + self.step_name('Verify that freshclam can update from an older CVD to a newer with CDIFF patches, and then update from an older CLD to the latest with more CDIFF patches') + + # start with this CVD + shutil.copy(str(TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' /'test-1.cvd'), str(TC.path_db / 'test.cvd')) + + # advertise this CVD FIRST (by sending the header response to Range requests) + shutil.copy(str(TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' /'test-3.cvd'), str(TC.path_www / 'test.cvd.advertised')) + + # using these CDIFFs + shutil.copy(str(TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' /'test-2.cdiff'), str(TC.path_www)) + shutil.copy(str(TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' /'test-3.cdiff'), str(TC.path_www)) + + handler = partial(WebServerHandler_WWW, TC.path_www) + TC.mock_mirror = Process(target=mock_database_mirror, args=(handler, TC.mock_mirror_port)) + TC.mock_mirror.start() + + if TC.freshclam_config.exists(): + os.remove(str(TC.freshclam_config)) + + TC.freshclam_config.write_text(''' + DatabaseMirror http://localhost:{port} + DNSDatabaseInfo no + PidFile {freshclam_pid} + LogVerbose yes + LogFileMaxSize 0 + LogTime yes + DatabaseDirectory {path_db} + DatabaseOwner {user} + '''.format( + freshclam_pid=TC.freshclam_pid, + path_db=TC.path_db, + port=TC.mock_mirror_port, + user=getpass.getuser(), + )) + + # + # Now run the update for the first set up updates. + # + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( + valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config + ) + output = self.execute_command(command) + + assert output.ec == 0 # success + + expected_stdout = [ + 'test.cld updated', + ] + unexpected_results = [ + 'already up-to-date' + ] + + # verify stdout + self.verify_output(output.out, expected=expected_stdout, unexpected=unexpected_results) + + # verify stderr + self.verify_output(output.err, unexpected=unexpected_results) + + # advertise this newer CVD SECOND (by sending the header response to Range requests) + shutil.copy(str(TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' /'test-6.cvd'), str(TC.path_www / 'test.cvd.advertised')) + + # using these CDIFFs + shutil.copy(str(TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' /'test-4.cdiff'), str(TC.path_www)) + shutil.copy(str(TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' /'test-5.cdiff'), str(TC.path_www)) + shutil.copy(str(TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' /'test-6.cdiff'), str(TC.path_www)) + + # + # Now re-run the update for more updates. + # + command = '{valgrind} {valgrind_args} {freshclam} --no-dns --config-file={freshclam_config} --update-db=test'.format( + valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, freshclam=TC.freshclam, freshclam_config=TC.freshclam_config + ) + output = self.execute_command(command) + + assert output.ec == 0 # success + + expected_stdout = [ + 'test.cld updated', + ] + unexpected_results = [ + 'already up-to-date' + ] + + + def mock_database_mirror(handler, port=8001): ''' Process entry point for our HTTP Server to mock a database mirror. diff --git a/unit_tests/sigtool_test.py b/unit_tests/sigtool_test.py index 63a512537b..73c9ba4159 100644 --- a/unit_tests/sigtool_test.py +++ b/unit_tests/sigtool_test.py @@ -58,11 +58,13 @@ def tearDownClass(cls): super(TC, cls).tearDownClass() def setUp(self): + TC.original_cwd = os.getcwd() super(TC, self).setUp() def tearDown(self): super(TC, self).tearDown() self.verify_valgrind_log() + os.chdir(TC.original_cwd) def test_sigtool_00_version(self): self.step_name('sigtool version test') @@ -79,3 +81,79 @@ def test_sigtool_00_version(self): 'ClamAV {}'.format(TC.version), ] self.verify_output(output.out, expected=expected_results) + + def test_sigtool_01_run_cdiff(self): + self.step_name('sigtool run cdiff test') + # In addition to testing that running a cdiff works, this also tests a regression. + # Applying test-3.cdiff was failing because UNLINK wasn't properly implemented (since 0.105.0). + # We didn't notice it because logging wasn't enabled, and leniency in our freshclam cdiff process + # allowed the test to pass without noticing the bug. + + self.log.warning('VG: {}'.format(os.getenv("VG"))) + + (TC.path_tmp / 'run_cdiff').mkdir() + os.chdir(str(TC.path_tmp / 'run_cdiff')) + + command = '{valgrind} {valgrind_args} {sigtool} --unpack {cvd}'.format( + valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, sigtool=TC.sigtool, + cvd=TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' / 'test-1.cvd' + ) + output = self.execute_command(command) + + assert output.ec == 0 # success + + # Apply 1st cdiff. + + command = '{valgrind} {valgrind_args} {sigtool} --run-cdiff={cdiff}'.format( + valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, sigtool=TC.sigtool, + cdiff=TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' / 'test-2.cdiff' + ) + output = self.execute_command(command) + + # Apply 2nd cdiff. This one failed because the CLOSE operation should create a file + # if it didn't exist, and it was only appending but not creating. + + command = '{valgrind} {valgrind_args} {sigtool} --run-cdiff={cdiff}'.format( + valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, sigtool=TC.sigtool, + cdiff=TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' / 'test-3.cdiff' + ) + output = self.execute_command(command) + + assert output.ec == 0 # success + + def test_sigtool_02_rust_logs_messages_work(self): + self.step_name('sigtool test rust log macros work') + # In addition to testing that running a cdiff works, this also tests a regression. + # Applying test-3.cdiff was failing because UNLINK wasn't properly implemented (since 0.105.0). + # We didn't notice it because logging wasn't enabled, and leniency in our freshclam cdiff process + # allowed the test to pass without noticing the bug. + + self.log.warning('VG: {}'.format(os.getenv("VG"))) + + (TC.path_tmp / 'run_cdiff_log').mkdir() + os.chdir(str(TC.path_tmp / 'run_cdiff_log')) + + command = '{valgrind} {valgrind_args} {sigtool} --unpack {cvd}'.format( + valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, sigtool=TC.sigtool, + cvd=TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' / 'test-1.cvd' + ) + output = self.execute_command(command) + + assert output.ec == 0 # success + + # Apply cdiffs in wrong order. Should fail and print a message. + + command = '{valgrind} {valgrind_args} {sigtool} --run-cdiff={cdiff}'.format( + valgrind=TC.valgrind, valgrind_args=TC.valgrind_args, sigtool=TC.sigtool, + cdiff=TC.path_source / 'unit_tests' / 'input' / 'freshclam_testfiles' / 'test-3.cdiff' + ) + output = self.execute_command(command) + + assert output.ec == 1 # failure + + # Verify that the `error!()` message was printed to stderr. + # This didn't happen before when we didn't initialize the rust logging lib at the top of sigtool. + expected_results = [ + 'LibClamAV Error', + ] + self.verify_output(output.err, expected=expected_results)