From fd9890a324150d7e9542e27f7a3661aabd3141ca Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Sun, 15 Dec 2024 17:25:16 -0500 Subject: [PATCH] Freshclam: download .sign files to verify CVDs and CDIFFs Also fix an issue where making a CLD would only include the CFG file for daily and not if patching any other database. Also verify CDIFF external digital signatures when applying CDIFFs with sigtool. --- Cargo.lock | 1 + libclamav_rust/Cargo.toml | 1 + libclamav_rust/cbindgen.toml | 1 + libclamav_rust/src/cdiff.rs | 170 +++++++++----- libclamav_rust/src/cvd.rs | 2 +- libclamav_rust/src/util.rs | 46 +++- libfreshclam/libfreshclam.c | 13 +- libfreshclam/libfreshclam_internal.c | 329 ++++++++++++++++++--------- sigtool/sigtool.c | 117 ++++++---- 9 files changed, 474 insertions(+), 206 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 16456d40b2..cb23358a50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -328,6 +328,7 @@ dependencies = [ "clam-sigutil", "delharc", "flate2", + "glob", "hex", "hex-literal", "image", diff --git a/libclamav_rust/Cargo.toml b/libclamav_rust/Cargo.toml index bf3ee1107b..768a65e6f6 100644 --- a/libclamav_rust/Cargo.toml +++ b/libclamav_rust/Cargo.toml @@ -30,6 +30,7 @@ clam-sigutil = { path = "../../clamav-signature-util" } tar = "0.4.43" md5 = "0.7.0" openssl = "0.10.68" +glob = "0.3.1" [features] not_ready = [] diff --git a/libclamav_rust/cbindgen.toml b/libclamav_rust/cbindgen.toml index 48f7f03c82..2815acf1e0 100644 --- a/libclamav_rust/cbindgen.toml +++ b/libclamav_rust/cbindgen.toml @@ -53,6 +53,7 @@ include = [ "evidence::IndicatorType", "scanners::scan_onenote", "scanners::cli_scanalz", + "util::glob_rm", ] # prefix = "CAPI_" diff --git a/libclamav_rust/src/cdiff.rs b/libclamav_rust/src/cdiff.rs index 894b666c8f..8c22df43bf 100644 --- a/libclamav_rust/src/cdiff.rs +++ b/libclamav_rust/src/cdiff.rs @@ -29,9 +29,7 @@ use std::{ str::{self, FromStr}, }; -use crate::sys; -use crate::util; -use crate::validate_str_param; +use crate::{codesign, ffi_error, ffi_util::FFIError, sys, validate_str_param}; use flate2::{read::GzDecoder, write::GzEncoder, Compression}; use log::{debug, error, warn}; @@ -146,6 +144,9 @@ pub enum Error { #[error("NUL found within CString")] CstringNulError(#[from] std::ffi::NulError), + + #[error("Can't verify: {0}")] + CannotVerify(String), } /// Errors particular to input handling (i.e., syntax, or side effects from @@ -469,7 +470,7 @@ pub fn script2cdiff(script_file_name: &str, builder: &str, server: &str) -> Resu // Make a copy of the script file name to use for the cdiff file let cdiff_file_name_string = script_file_name.to_string(); let mut cdiff_file_name = cdiff_file_name_string.as_str(); - debug!("script2cdiff() - script file name: {:?}", cdiff_file_name); + debug!("script2cdiff: script file name: {:?}", cdiff_file_name); // Remove the "".script" suffix if let Some(file_name) = cdiff_file_name.strip_suffix(".script") { @@ -494,7 +495,7 @@ pub fn script2cdiff(script_file_name: &str, builder: &str, server: &str) -> Resu // Add .cdiff suffix let cdiff_file_name = format!("{}.{}", cdiff_file_name, "cdiff"); - debug!("script2cdiff() - writing to: {:?}", &cdiff_file_name); + debug!("script2cdiff: writing to: {:?}", &cdiff_file_name); // Open cdiff_file_name for writing let mut cdiff_file: File = File::create(&cdiff_file_name) @@ -532,7 +533,7 @@ pub fn script2cdiff(script_file_name: &str, builder: &str, server: &str) -> Resu .map_err(|e| Error::FileMeta(cdiff_file_name.to_owned(), e))? .len(); debug!( - "script2cdiff() - wrote {} bytes to {}", + "script2cdiff: wrote {} bytes to {}", cdiff_file_len, cdiff_file_name ); @@ -575,13 +576,36 @@ pub fn script2cdiff(script_file_name: &str, builder: &str, server: &str) -> Resu Ok(()) } -/// This function is only meant to be called from sigtool.c +/// C interface for cdiff_apply() (below). +/// This function is for use in sigtool.c and libfreshclam_internal.c #[export_name = "cdiff_apply"] -pub extern "C" fn _cdiff_apply(fd: i32, mode: u16) -> i32 { - debug!( - "cdiff_apply() - called with file_descriptor={}, mode={}", - fd, mode - ); +pub unsafe extern "C" fn _cdiff_apply( + cdiff_file_path_str: *const c_char, + certs_directory_str: *const c_char, + mode: u16, + err: *mut *mut FFIError, +) -> bool { + let cdiff_file_path_str = validate_str_param!(cdiff_file_path_str); + let cdiff_file_path = match Path::new(cdiff_file_path_str).canonicalize() { + Ok(p) => p, + Err(e) => { + return ffi_error!( + err = err, + Error::CannotVerify(format!("Invalid cdiff file path: {}", e)) + ); + } + }; + + let certs_directory_str = validate_str_param!(certs_directory_str); + let certs_directory = match PathBuf::from_str(certs_directory_str) { + Ok(p) => p, + Err(e) => { + return ffi_error!( + err = err, + Error::CannotVerify(format!("Invalid certs directory path: {}", e)) + ); + } + }; let mode = if mode == 1 { ApplyMode::Cdiff @@ -589,13 +613,11 @@ pub extern "C" fn _cdiff_apply(fd: i32, mode: u16) -> i32 { ApplyMode::Script }; - let mut file = util::file_from_fd_or_handle(fd); - - if let Err(e) = cdiff_apply(&mut file, mode) { - error!("{}", e); - -1 + if let Err(e) = cdiff_apply(&cdiff_file_path, &certs_directory, mode) { + error!("Failed to apply {:?}: {}", cdiff_file_path, e); + ffi_error!(err = err, e) } else { - 0 + true } } @@ -612,58 +634,96 @@ pub extern "C" fn _cdiff_apply(fd: i32, mode: u16) -> i32 { /// A cdiff file contains a footer that is the signed signature of the sha256 /// file contains of the header and the body. The footer begins after the first /// ':' character to the left of EOF. -pub fn cdiff_apply(file: &mut File, mode: ApplyMode) -> Result<(), Error> { +pub fn cdiff_apply( + cdiff_file_path: &Path, + certs_directory: &Path, + mode: ApplyMode, +) -> Result<(), Error> { let path = std::env::current_dir().unwrap(); - debug!("cdiff_apply() - current directory is {}", path.display()); + debug!("cdiff_apply: applying {}", cdiff_file_path.display()); + debug!("cdiff_apply: current directory is {}", path.display()); + + // Open cdiff file for reading + let mut file = File::open(cdiff_file_path).map_err(Error::IoError)?; // Only read dsig, header, etc. if this is a cdiff file let header_length = match mode { ApplyMode::Script => 0, ApplyMode::Cdiff => { - 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()); - } - // Get file length let file_len = file.metadata()?.len() as usize; - let footer_offset = file_len - dsig.len() - 1; - - // The SHA is calculated from the contents of the beginning of the file - // up until the ':' before the dsig at the end of the file. - let sha256 = get_hash(file, footer_offset)?; - - debug!("cdiff_apply() - sha256: {}", hex::encode(sha256)); - - // cli_versig2 will expect dsig to be a null-terminated string - let dsig_cstring = CString::new(dsig)?; - - // Verify cdiff - let n = CString::new(PUBLIC_KEY_MODULUS).unwrap(); - let e = CString::new(PUBLIC_KEY_EXPONENT).unwrap(); - let versig_result = unsafe { - sys::cli_versig2( - sha256.to_vec().as_ptr(), - dsig_cstring.as_ptr(), - n.as_ptr() as *const c_char, - e.as_ptr() as *const c_char, - ) + + // Check if there is an external digital signature + // The filename would be the same as the cdiff file with an extra .sign extension + let sign_file_path = cdiff_file_path.with_extension("cdiff.sign"); + let verify_result = + codesign::verify_signed_file(cdiff_file_path, &sign_file_path, certs_directory); + let verified = match verify_result { + Ok(signer) => { + debug!( + "cdiff_apply: external signature verified. Signed by: {}", + signer + ); + true + } + Err(codesign::Error::InvalidDigitalSignature(m)) => { + debug!("cdiff_apply: invalid external signature: {}", m); + return Err(Error::InvalidDigitalSignature); + } + Err(e) => { + debug!("cdiff_apply: error validating external signature: {:?}", e); + + // If the external signature could not be validated (e.g. does not exist) + // then continue on and try to validate the internal signature. + false + } }; - debug!("cdiff_apply() - cli_versig2() result = {}", versig_result); - if versig_result != 0 { - return Err(Error::InvalidDigitalSignature); + + if !verified { + // try to verify the internal (legacy) digital signature + let dsig = read_dsig(&mut file)?; + debug!("cdiff_apply: final dsig length is {}", dsig.len()); + if is_debug_enabled() { + print_file_data(dsig.clone(), dsig.len()); + } + + let footer_offset = file_len - dsig.len() - 1; + + // The SHA is calculated from the contents of the beginning of the file + // up until the ':' before the dsig at the end of the file. + let sha256 = get_hash(&mut file, footer_offset)?; + + debug!("cdiff_apply: sha256: {}", hex::encode(sha256)); + + // cli_versig2 will expect dsig to be a null-terminated string + let dsig_cstring = CString::new(dsig)?; + + // Verify cdiff + let n = CString::new(PUBLIC_KEY_MODULUS).unwrap(); + let e = CString::new(PUBLIC_KEY_EXPONENT).unwrap(); + let versig_result = unsafe { + sys::cli_versig2( + sha256.to_vec().as_ptr(), + dsig_cstring.as_ptr(), + n.as_ptr() as *const c_char, + e.as_ptr() as *const c_char, + ) + }; + debug!("cdiff_apply: cli_versig2() result = {}", versig_result); + if versig_result != 0 { + return Err(Error::InvalidDigitalSignature); + } } // Read file length from header - let (header_len, header_offset) = read_size(file)?; + let (header_len, header_offset) = read_size(&mut file)?; debug!( - "cdiff_apply() - header len = {}, file len = {}, header offset = {}", + "cdiff_apply: header len = {}, file len = {}, header offset = {}", header_len, file_len, header_offset ); let current_pos = file.seek(SeekFrom::Start(header_offset as u64))?; - debug!("cdiff_apply() - current file offset = {}", current_pos); + debug!("cdiff_apply: current file offset = {}", current_pos); header_len as usize } }; @@ -1074,7 +1134,7 @@ fn cmd_close(ctx: &mut Context) -> Result<(), InputError> { ctx.additions.clear(); } - debug!("cmd_close() - finished"); + debug!("cmd_close: finished"); Ok(()) } @@ -1183,7 +1243,7 @@ fn read_dsig(file: &mut File) -> Result, SignatureError> { // Read from dsig_offset to EOF let mut dsig: Vec = vec![]; file.read_to_end(&mut dsig)?; - debug!("read_dsig() - dsig length is {}", dsig.len()); + debug!("read_dsig: dsig length is {}", dsig.len()); // Find the signature let offset: usize = SIG_SIZE + 1; diff --git a/libclamav_rust/src/cvd.rs b/libclamav_rust/src/cvd.rs index 24cf28db87..2192c2d4c3 100644 --- a/libclamav_rust/src/cvd.rs +++ b/libclamav_rust/src/cvd.rs @@ -186,7 +186,7 @@ impl CVD { let md5_str = std::str::from_utf8(md5_bytes) .map_err(|_| Error::Parse("MD5 hash string is not valid unicode".to_string()))?; let md5: Option = if md5_str.len() != 32 { - debug!("MD5 hash string is not 32 characters long. It may be empty"); + debug!("MD5 hash string not present."); None } else { Some(md5_str.to_string()) diff --git a/libclamav_rust/src/util.rs b/libclamav_rust/src/util.rs index 086fdf2786..d836d11b0f 100644 --- a/libclamav_rust/src/util.rs +++ b/libclamav_rust/src/util.rs @@ -20,11 +20,21 @@ * MA 02110-1301, USA. */ -use std::{ffi::CStr, fs::File}; +use std::{ffi::CStr, fs::File, os::raw::c_char}; -use log::error; +use glob::glob; +use log::{debug, error, warn}; -use crate::sys; +use crate::{ffi_error, ffi_util::FFIError, sys, validate_str_param}; + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("Glob error: {0}")] + GlobError(#[from] glob::GlobError), + + #[error("IO error: {0}")] + IoError(#[from] std::io::Error), +} /// Obtain a std::fs::File from an i32 in a platform-independent manner. /// @@ -128,3 +138,33 @@ pub unsafe fn scan_archive_metadata( ) } } + +/// C interface to delete files using a glob pattern. +/// +/// # Safety +/// +/// No parameters may be NULL. +#[export_name = "glob_rm"] +pub unsafe extern "C" fn glob_rm( + glob_str: *const c_char, + err: *mut *mut FFIError, +) -> bool { + let glob_str = validate_str_param!(glob_str); + + for entry in glob(glob_str).expect("Failed to read glob pattern") { + match entry { + Ok(path) => { + debug!("Deleting: {path:?}"); + if let Err(e) = std::fs::remove_file(&path) { + warn!("Failed to delete file: {path:?}"); + return ffi_error!(err = err, Error::IoError(e)); + } + } + Err(e) => { + return ffi_error!(err = err, Error::GlobError(e)); + } + } + } + + true +} diff --git a/libfreshclam/libfreshclam.c b/libfreshclam/libfreshclam.c index 6d6f6c19d3..9bd010cad7 100644 --- a/libfreshclam/libfreshclam.c +++ b/libfreshclam/libfreshclam.c @@ -374,13 +374,24 @@ fc_error_t fc_prune_database_directory(char **databaseList, uint32_t nDatabases) while ((dent = readdir(dir))) { if (dent->d_ino) { + // prune any CVD/CLD files that are not in the database list if ((NULL != (extension = strstr(dent->d_name, ".cld"))) || (NULL != (extension = strstr(dent->d_name, ".cvd")))) { + // find the first '-' or '.' in the filename + // Use this to determine the database name. + // We need this so we can ALSO prune the .sign files for unwanted databases. + // Will also be useful in case the database filename includes a hyphenated version number. + const char * first_dash_or_dot = strchr(dent->d_name, '-'); + if (NULL == first_dash_or_dot) { + first_dash_or_dot = extension; + } + uint32_t i; int bFound = 0; for (i = 0; i < nDatabases; i++) { - if (0 == strncmp(databaseList[i], dent->d_name, extension - dent->d_name)) { + // check that the database name is in the database list + if (0 == strncmp(databaseList[i], dent->d_name, first_dash_or_dot - dent->d_name)) { bFound = 1; } } diff --git a/libfreshclam/libfreshclam_internal.c b/libfreshclam/libfreshclam_internal.c index e32e37e4f1..b01f5115d5 100644 --- a/libfreshclam/libfreshclam_internal.c +++ b/libfreshclam/libfreshclam_internal.c @@ -1443,27 +1443,42 @@ static fc_error_t downloadFile( } static fc_error_t getcvd( + const char *database, const char *cvdfile, const char *tmpfile, char *server, uint32_t ifModifiedSince, - unsigned int remoteVersion, + uint32_t remoteVersion, + char **sign_file, + uint32_t *downloadedVersion, int logerr) { fc_error_t ret; cl_error_t cl_ret; fc_error_t status = FC_EARG; - struct cl_cvd *cvd = NULL; - char *tmpfile_with_extension = NULL; - char *url = NULL; - size_t urlLen = 0; + struct cl_cvd *cvd = NULL; + char extension[5] = {0}; + + char *tmpsignfile = NULL; + size_t tmpsignfileLen = 0; + char *url = NULL; + size_t urlLen = 0; + + char *sign_filename = NULL; + size_t sign_filenameLen = 0; + char *sign_file_url = NULL; + size_t sign_file_urlLen = 0; if ((NULL == cvdfile) || (NULL == tmpfile) || (NULL == server)) { logg(LOGG_ERROR, "getcvd: Invalid arguments.\n"); goto done; } + if (NULL != sign_file) { + *sign_file = NULL; + } + urlLen = strlen(server) + strlen("/") + strlen(cvdfile); url = malloc(urlLen + 1); snprintf(url, urlLen + 1, "%s/%s", server, cvdfile); @@ -1479,36 +1494,53 @@ static fc_error_t getcvd( goto done; } - /* Temporarily rename file to correct extension for verification. */ - tmpfile_with_extension = strdup(tmpfile); - if (!tmpfile_with_extension) { - logg(LOGG_ERROR, "Can't allocate memory for temp file with extension!\n"); - status = FC_EMEM; - goto done; - } - strncpy(tmpfile_with_extension + strlen(tmpfile_with_extension) - 4, cvdfile + strlen(cvdfile) - 4, 4); - if (rename(tmpfile, tmpfile_with_extension) == -1) { - logg(LOGG_ERROR, "Can't rename %s to %s: %s\n", tmpfile, tmpfile_with_extension, strerror(errno)); - status = FC_EDBDIRACCESS; - goto done; - } + // grab the extension from the cvdfile + strncpy(extension, cvdfile + strlen(cvdfile) - 4, 4); - if (CL_SUCCESS != (cl_ret = cl_cvdverify(tmpfile_with_extension))) { - logg(LOGG_ERROR, "Verification: %s\n", cl_strerror(cl_ret)); + if (NULL == (cvd = cl_cvdhead(tmpfile))) { + logg(LOGG_ERROR, "Can't read CVD header of new %s database.\n", cvdfile); status = FC_EBADCVD; goto done; } - if (NULL == (cvd = cl_cvdhead(tmpfile_with_extension))) { - logg(LOGG_ERROR, "Can't read CVD header of new %s database.\n", cvdfile); - status = FC_EBADCVD; - goto done; + // try to get the sign file before verifying the cvd + // use the cvd name + version to get the signature file + // sign-file = database + "-" + version + ".sign" + sign_filenameLen = strlen(database) + strlen("-") + 10 + strlen(".cvd") + strlen(".sign"); + sign_filename = malloc(sign_filenameLen + 1); + snprintf(sign_filename, sign_filenameLen + 1, "%s-%u%s.sign", database, cvd->version, extension); + + // sign-file-url = server + "/" + sign_filename + sign_file_urlLen = strlen(server) + strlen("/") + strlen(sign_filename); + sign_file_url = malloc(sign_file_urlLen + 1); + snprintf(sign_file_url, sign_file_urlLen + 1, "%s/%s", server, sign_filename); + + // sign-file-tempfilename = g_tempDirectory + sign_filename + tmpsignfileLen = strlen(g_tempDirectory) + strlen(PATHSEP) + strlen(sign_filename); + tmpsignfile = malloc(tmpsignfileLen + 1); + snprintf(tmpsignfile, tmpsignfileLen + 1, "%s" PATHSEP "%s", g_tempDirectory, sign_filename); + + ret = downloadFile(sign_file_url, tmpsignfile, 1, logerr, 0); + if (ret != FC_SUCCESS) { + logg(LOGG_DEBUG, "No external .sign digital signature file for %s-%u\n", database, cvd->version); + // It's not an error if the .sign file doesn't exist. + // Just continue with the cvd verification and hope we can use the legacy md5-based rsa method. + } else { + // Set the output variable to the sign file name so we can move it later. + logg(LOGG_DEBUG, "Downloaded digital signature file: %s\n", tmpsignfile); + if (NULL != sign_file) { + CLI_SAFER_STRDUP_OR_GOTO_DONE( + tmpsignfile, + *sign_file, + logg(LOGG_ERROR, "getcvd: Failed to duplicate sign file name.\n"); + status = FC_EMEM); + } } - /* Rename the file back to the original, since verification passed. */ - if (rename(tmpfile_with_extension, tmpfile) == -1) { - logg(LOGG_ERROR, "Can't rename %s to %s: %s\n", tmpfile_with_extension, tmpfile, strerror(errno)); - status = FC_EDBDIRACCESS; + // Now that we have the cvd and the sign file, we can verify the cvd. + if (CL_SUCCESS != (cl_ret = cl_cvdverify(tmpfile))) { + logg(LOGG_ERROR, "Verification: %s\n", cl_strerror(cl_ret)); + status = FC_EBADCVD; goto done; } @@ -1520,16 +1552,15 @@ static fc_error_t getcvd( goto done; } + if (NULL != downloadedVersion) { + *downloadedVersion = cvd->version; + } status = FC_SUCCESS; done: if (NULL != cvd) { cl_cvdfree(cvd); } - if (NULL != tmpfile_with_extension) { - unlink(tmpfile_with_extension); - free(tmpfile_with_extension); - } if (NULL != url) { free(url); } @@ -1541,6 +1572,15 @@ static fc_error_t getcvd( unlink(tmpfile); } } + if (NULL != sign_filename) { + free(sign_filename); + } + if (NULL != sign_file_url) { + free(sign_file_url); + } + if (NULL != tmpsignfile) { + free(tmpsignfile); + } return status; } @@ -1571,8 +1611,7 @@ static fc_error_t mkdir_and_chdir_for_cdiff_tmp(const char *database, const char if (-1 == access(tmpdir, R_OK | W_OK)) { /* - * Temp directory for incremental update (cdiff download) does not - * yet exist. + * Temp directory for incremental update (cdiff download) does not yet exist. */ int ret; bool is_cld = false; @@ -1631,7 +1670,7 @@ static fc_error_t mkdir_and_chdir_for_cdiff_tmp(const char *database, const char return status; } -static fc_error_t downloadPatch( +static fc_error_t downloadPatchAndApply( const char *database, const char *tmpdir, int version, @@ -1641,61 +1680,86 @@ static fc_error_t downloadPatch( fc_error_t ret; fc_error_t status = FC_EARG; - char *tempname = NULL; char patch[DB_FILENAME_MAX]; + char patch_sign_file[DB_FILENAME_MAX + 5]; char olddir[PATH_MAX]; char *url = NULL; size_t urlLen = 0; - int fd = -1; + char *sign_url = NULL; + size_t sign_urlLen = 0; + + FFIError *cdiff_apply_error = NULL; olddir[0] = '\0'; if ((NULL == database) || (NULL == tmpdir) || (NULL == server) || (0 == version)) { - logg(LOGG_ERROR, "downloadPatch: Invalid arguments.\n"); + logg(LOGG_ERROR, "downloadPatchAndApply: Invalid arguments.\n"); goto done; } if (NULL == getcwd(olddir, sizeof(olddir))) { - logg(LOGG_ERROR, "downloadPatch: Can't get path of current working directory\n"); + logg(LOGG_ERROR, "downloadPatchAndApply: Can't get path of current working directory\n"); status = FC_EDIRECTORY; goto done; } + /* + * Unpack the database into a new temp directory where we'll apply the patch, and chdir to it. + * If the directory already exists, we'll just chdir to it. + */ if (FC_SUCCESS != mkdir_and_chdir_for_cdiff_tmp(database, tmpdir)) { status = FC_EDIRECTORY; goto done; } - if (NULL == (tempname = cli_gentemp("."))) { - status = FC_EMEM; - goto done; - } - + /* + * Download the patch. + */ snprintf(patch, sizeof(patch), "%s-%d.cdiff", database, version); + urlLen = strlen(server) + strlen("/") + strlen(patch); url = malloc(urlLen + 1); snprintf(url, urlLen + 1, "%s/%s", server, patch); - if (FC_SUCCESS != (ret = downloadFile(url, tempname, 1, logerr, 0))) { + if (FC_SUCCESS != (ret = downloadFile(url, patch, 1, logerr, 0))) { if (ret == FC_EEMPTYFILE) { logg(LOGG_INFO, "Empty script %s, need to download entire database\n", patch); } else { - logg(logerr ? LOGG_ERROR : LOGG_WARNING, "downloadPatch: Can't download %s from %s\n", patch, url); + logg(logerr ? LOGG_ERROR : LOGG_WARNING, "downloadPatchAndApply: Can't download %s from %s\n", patch, url); } status = ret; goto done; } - if (-1 == (fd = open(tempname, O_RDONLY | O_BINARY))) { - logg(LOGG_ERROR, "downloadPatch: Can't open %s for reading\n", tempname); - status = FC_EFILE; - goto done; + /* + * Download the patch sign file. + */ + snprintf(patch_sign_file, sizeof(patch_sign_file), "%s.sign", patch); + + sign_urlLen = strlen(server) + strlen("/") + strlen(patch_sign_file); + sign_url = malloc(sign_urlLen + 1); + snprintf(sign_url, sign_urlLen + 1, "%s/%s", server, patch_sign_file); + + if (FC_SUCCESS != (ret = downloadFile(sign_url, patch_sign_file, 1, logerr, 0))) { + // No sign file is not an error. + // Just means we'll have to fall back to the legacy sha256-based rsa method for verifying CDIFFs. + logg(LOGG_DEBUG, "No external .sign digital signature file for %s\n", patch); + } else { + logg(LOGG_DEBUG, "Downloaded digital signature file: %s\n", patch_sign_file); } - if (-1 == cdiff_apply(fd, 1)) { - logg(LOGG_ERROR, "downloadPatch: Can't apply patch\n"); + /* + * Apply the patch. + */ + if (!cdiff_apply( + patch, + g_certsDirectory, + 1, + &cdiff_apply_error)) { + logg(LOGG_ERROR, "downloadPatchAndApply: Can't apply '%s': %s\n", + patch, ffierror_fmt(cdiff_apply_error)); status = FC_EFAILEDUPDATE; goto done; } @@ -1708,18 +1772,20 @@ static fc_error_t downloadPatch( free(url); } - if (-1 != fd) { - close(fd); + if (NULL != sign_url) { + free(sign_url); } - if (NULL != tempname) { - unlink(tempname); - free(tempname); + if (NULL != cdiff_apply_error) { + ffierror_free(cdiff_apply_error); } + /* + * Change back to the original directory. + */ if ('\0' != olddir[0]) { if (-1 == chdir(olddir)) { - logg(LOGG_ERROR, "downloadPatch: Can't chdir to %s\n", olddir); + logg(LOGG_ERROR, "downloadPatchAndApply: Can't chdir to %s\n", olddir); status = FC_EDIRECTORY; } } @@ -1781,6 +1847,7 @@ static fc_error_t buildcld( char olddir[PATH_MAX] = {0}; char info[DB_FILENAME_MAX]; + char cfg[DB_FILENAME_MAX]; char buff[CVD_HEADER_SIZE + 1]; char *pt; @@ -1872,9 +1939,11 @@ static fc_error_t buildcld( } } - if (-1 != access("daily.cfg", R_OK)) { - if (-1 == tar_addfile(fd, gzs, "daily.cfg")) { - logg(LOGG_ERROR, "buildcld: Can't add daily.cfg to new %s.cld - please check if there is enough disk space available\n", database); + snprintf(cfg, sizeof(cfg), "%s.cfg", database); + cfg[sizeof(cfg) - 1] = 0; + if (-1 != access(cfg, R_OK)) { + if (-1 == tar_addfile(fd, gzs, cfg)) { + logg(LOGG_ERROR, "buildcld: Can't add %s to new %s.cld - please check if there is enough disk space available\n", cfg, database); status = FC_EFAILEDUPDATE; goto done; } @@ -1888,7 +1957,7 @@ static fc_error_t buildcld( while (NULL != (dent = readdir(dir))) { if (dent->d_ino) { - if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, "..") || !strcmp(dent->d_name, "COPYING") || !strcmp(dent->d_name, "daily.cfg") || !strcmp(dent->d_name, info)) + if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, "..") || !strcmp(dent->d_name, "COPYING") || !strcmp(dent->d_name, cfg) || !strcmp(dent->d_name, info)) continue; if (tar_addfile(fd, gzs, dent->d_name) == -1) { @@ -2273,8 +2342,12 @@ fc_error_t updatedb( char *remoteFilename = NULL; char *newLocalFilename = NULL; - char *tmpdir = NULL; - char *tmpfile = NULL; + char *cld_build_dir = NULL; + char *tmpfile = NULL; + + char *signfile = NULL; + uint32_t downloadedVersion = 0; + FFIError *glob_rm_error = NULL; unsigned int flevel; @@ -2313,18 +2386,24 @@ fc_error_t updatedb( goto up_to_date; } - /* Download CVD or CLD to temp file */ - tmpfile = cli_gentemp(g_tempDirectory); + /* + * Download CVD or CLD to a file in the temp directory. + */ + + // Create a temp file for the new database. + tmpfile = calloc(1, strlen(g_tempDirectory) + strlen("/") + strlen(remoteFilename) + 1); if (!tmpfile) { status = FC_EMEM; goto done; } + snprintf(tmpfile, strlen(g_tempDirectory) + strlen("/") + strlen(remoteFilename) + 1, + "%s/%s", g_tempDirectory, remoteFilename); if ((localVersion == 0) || (!bScriptedUpdates)) { /* * Download entire file. */ - ret = getcvd(remoteFilename, tmpfile, server, localTimestamp, remoteVersion, logerr); + ret = getcvd(database, remoteFilename, tmpfile, server, localTimestamp, remoteVersion, &signfile, &downloadedVersion, logerr); if (FC_UPTODATE == ret) { logg(LOGG_WARNING, "Expected newer version of %s database but the server's copy is not newer than our local file (version %d).\n", database, localVersion); if (NULL != localFilename) { @@ -2343,6 +2422,8 @@ fc_error_t updatedb( goto done; } + // The file name won't change for a simple download. + // It will only change if we're doing a scripted update. newLocalFilename = cli_safer_strdup(remoteFilename); } else { /* @@ -2351,8 +2432,9 @@ fc_error_t updatedb( ret = FC_SUCCESS; uint32_t numPatchesReceived = 0; - tmpdir = cli_gentemp(g_tempDirectory); - if (!tmpdir) { + // Create a temp directory where we'll build the new CLD. + cld_build_dir = cli_gentemp_with_prefix(g_tempDirectory, "cld"); + if (!cld_build_dir) { status = FC_EMEM; goto done; } @@ -2383,7 +2465,10 @@ fc_error_t updatedb( { mprintf(LOGG_INFO, "Downloading database patch # %u...\n", i); } - ret = downloadPatch(database, tmpdir, i, server, llogerr); + + // If the build directory doesn't exist, we'll create it and unpack the database into it. + // Then we download and apply the patch. + ret = downloadPatchAndApply(database, cld_build_dir, i, server, llogerr); if (ret == FC_ECONNECTION || ret == FC_EFAILEDGET) { continue; } else { @@ -2413,7 +2498,7 @@ fc_error_t updatedb( logg(LOGG_WARNING, "Incremental update failed, trying to download %s\n", remoteFilename); } - ret = getcvd(remoteFilename, tmpfile, server, localTimestamp, remoteVersion, logerr); + ret = getcvd(database, remoteFilename, tmpfile, server, localTimestamp, remoteVersion, &signfile, &downloadedVersion, logerr); if (FC_SUCCESS != ret) { if (FC_EMIRRORNOTSYNC == ret) { /* Note: We can't retry with CDIFF's if FC_EMIRRORNOTSYNC happened here. @@ -2428,6 +2513,8 @@ fc_error_t updatedb( } } + // We gave up on patching, so it's back to a simple file download. + // The file name won't change for a simple download. newLocalFilename = cli_safer_strdup(remoteFilename); } else if (0 == numPatchesReceived) { logg(LOGG_INFO, "The database server doesn't have the latest patch for the %s database (version %u). The server will likely have updated if you check again in a few hours.\n", database, remoteVersion); @@ -2435,23 +2522,33 @@ fc_error_t updatedb( goto up_to_date; } else { /* - * CDIFFs downloaded; Use CDIFFs to turn old CVD/CLD into new updated CLD. + * CDIFFs downloaded and applied; Use CDIFFs to turn old CVD/CLD into new updated CLD. */ if (numPatchesReceived < remoteVersion - localVersion) { logg(LOGG_INFO, "Downloaded %u patches for %s, which is fewer than the %u expected patches.\n", numPatchesReceived, database, remoteVersion - localVersion); logg(LOGG_INFO, "We'll settle for this partial-update, at least for now.\n"); } - size_t newLocalFilenameLen = 0; - if (FC_SUCCESS != buildcld(tmpdir, database, tmpfile, g_bCompressLocalDatabase)) { + // For a scripted update, the new database will have + // a .cld extension. + // Overwrite the tmpfile's .cvd extension with a .cld extension + sprintf(tmpfile + strlen(tmpfile) - 3, "cld"); + + // And set the new filename that we'll used to copy to the DB directory + size_t newLocalFilenameLen = strlen(database) + strlen(".cld"); + newLocalFilename = malloc(newLocalFilenameLen + 1); + snprintf(newLocalFilename, newLocalFilenameLen + 1, "%s.cld", database); + + if (FC_SUCCESS != buildcld(cld_build_dir, database, tmpfile, g_bCompressLocalDatabase)) { logg(LOGG_ERROR, "updatedb: Incremental update failed. Failed to build CLD.\n"); status = FC_EBADCVD; goto done; } - newLocalFilenameLen = strlen(database) + strlen(".cld"); - newLocalFilename = malloc(newLocalFilenameLen + 1); - snprintf(newLocalFilename, newLocalFilenameLen + 1, "%s.cld", database); + // CLD's can't be signed, so we don't need to worry about the signature file. + // It's in the tmp directory so we don't need to manually delete it. + // Just free up the filename and we won't copy it into the DB directory later. + CLI_FREE_AND_SET_NULL(signfile); } } @@ -2460,26 +2557,6 @@ fc_error_t updatedb( * Test database before replacing original database with new database. */ if (NULL != g_cb_download_complete) { - char *tmpfile_with_extension = NULL; - size_t tmpfile_with_extension_len = strlen(tmpfile) + 1 + strlen(newLocalFilename); - - /* Suffix tmpfile with real database name & extension so it can be loaded. */ - tmpfile_with_extension = malloc(tmpfile_with_extension_len + 1); - if (!tmpfile_with_extension) { - status = FC_ETESTFAIL; - goto done; - } - snprintf(tmpfile_with_extension, tmpfile_with_extension_len + 1, "%s-%s", tmpfile, newLocalFilename); - if (rename(tmpfile, tmpfile_with_extension) == -1) { - logg(LOGG_ERROR, "updatedb: Can't rename %s to %s: %s\n", tmpfile, tmpfile_with_extension, strerror(errno)); - free(tmpfile_with_extension); - status = FC_EDBDIRACCESS; - goto done; - } - free(tmpfile); - tmpfile = tmpfile_with_extension; - tmpfile_with_extension = NULL; - /* Run callback to test it. */ logg(LOGG_DEBUG, "updatedb: Running g_cb_download_complete callback...\n"); if (FC_SUCCESS != (ret = g_cb_download_complete(tmpfile, context))) { @@ -2492,6 +2569,8 @@ fc_error_t updatedb( /* * Replace original database with new database. */ + logg(LOGG_DEBUG, "updatedb: Moving %s to %s" PATHSEP "%s\n", tmpfile, g_databaseDirectory, newLocalFilename); + #ifdef _WIN32 if (!access(newLocalFilename, R_OK) && unlink(newLocalFilename)) { logg(LOGG_ERROR, "Update failed. Can't delete the old database %s to replace it with a new database. Please fix the problem manually and try again.\n", newLocalFilename); @@ -2505,10 +2584,53 @@ fc_error_t updatedb( goto done; } + // If there are any old signature files for this database in the DB directory, delete them. + // We'll use a glob pattern to match the signature files + char *pattern = calloc(1, strlen(database) + strlen("-*.sign") + 1); + if (!pattern) { + logg(LOGG_ERROR, "updatedb: Failed to allocate memory for signature file pattern.\n"); + status = FC_EMEM; + goto done; + } + sprintf(pattern, "%s-*.sign", database); + + if (!glob_rm(pattern, &glob_rm_error)) { + cli_errmsg("updatedb: Failed to glob-delete old .sign files with pattern '%s': %s\n", + pattern, ffierror_fmt(glob_rm_error)); + ffierror_free(glob_rm_error); + free(pattern); + status = FC_ERROR; + goto done; + } + free(pattern); + + // If we have a signature file, move it from the temp directory to the database directory + if (NULL != signfile) { + char *newSignFilename = NULL; + + logg(LOGG_DEBUG, "updatedb: Moving signature file %s to database directory\n", signfile); + + // get the basename of the signfile + if (CL_SUCCESS != cli_basename(signfile, strlen(signfile), &newSignFilename)) { + logg(LOGG_ERROR, "updatedb: Failed to get basename of '%s'\n", signfile); + goto done; + } + + if (rename(signfile, newSignFilename) == -1) { + logg(LOGG_ERROR, "updatedb: Can't rename %s to %s: %s\n", signfile, newSignFilename, strerror(errno)); + free(newSignFilename); + status = FC_EDBDIRACCESS; + goto done; + } + free(newSignFilename); + } + /* If we just updated from a CVD to a CLD, delete the old CVD */ - if ((NULL != localFilename) && !access(localFilename, R_OK) && strcmp(newLocalFilename, localFilename)) - if (unlink(localFilename)) + if ((NULL != localFilename) && !access(localFilename, R_OK) && strcmp(newLocalFilename, localFilename)) { + if (unlink(localFilename)) { logg(LOGG_WARNING, "updatedb: Can't delete the old database file %s. Please remove it manually.\n", localFilename); + } + } /* Parse header to record number of sigs. */ if (NULL == (cvd = cl_cvdhead(newLocalFilename))) { @@ -2562,9 +2684,12 @@ fc_error_t updatedb( unlink(tmpfile); free(tmpfile); } - if (NULL != tmpdir) { - cli_rmdirs(tmpdir); - free(tmpdir); + if (NULL != cld_build_dir) { + cli_rmdirs(cld_build_dir); + free(cld_build_dir); + } + if (NULL != signfile) { + free(signfile); } return status; diff --git a/sigtool/sigtool.c b/sigtool/sigtool.c index 4a5d9e48b9..059b7e5f03 100644 --- a/sigtool/sigtool.c +++ b/sigtool/sigtool.c @@ -2239,9 +2239,12 @@ static int comparesha(const char *diff) static int rundiff(const struct optstruct *opts) { - int fd, ret; + int ret; unsigned short mode; const char *diff; + FFIError *cdiff_apply_error = NULL; + char *cvdcertsdir = NULL; + STATBUF statbuf; diff = optget(opts, "run-cdiff")->strarg; if (strstr(diff, ".cdiff")) { @@ -2253,16 +2256,41 @@ static int rundiff(const struct optstruct *opts) return -1; } - if ((fd = open(diff, O_RDONLY | O_BINARY)) == -1) { - mprintf(LOGG_ERROR, "rundiff: Can't open file %s\n", diff); - return -1; + cvdcertsdir = optget(opts, "cvdcertsdir")->strarg; + if (NULL == cvdcertsdir) { + // Check if the CVD_CERTS_DIR environment variable is set + cvdcertsdir = getenv("CVD_CERTS_DIR"); + + // If not, use the default value + if (NULL == cvdcertsdir) { + cvdcertsdir = CERTSDIR; + } } - ret = cdiff_apply(fd, mode); - close(fd); + if (LSTAT(cvdcertsdir, &statbuf) == -1) { + mprintf(LOGG_ERROR, + "ClamAV CA certificates directory is missing: %s\n" + "It should have been provided as a part of installation.", + cvdcertsdir); + return -1; + } - if (!ret) + if (!cdiff_apply( + diff, + cvdcertsdir, + mode, + &cdiff_apply_error)) { + mprintf(LOGG_ERROR, "rundiff: Error applying '%s': %s\n", + diff, ffierror_fmt(cdiff_apply_error)); + ret = -1; + } else { + // success. compare the SHA256 checksums of the files ret = comparesha(diff); + } + + if (NULL != cdiff_apply_error) { + ffierror_free(cdiff_apply_error); + } return ret; } @@ -2512,9 +2540,13 @@ static int dircopy(const char *src, const char *dest) static int verifydiff(const struct optstruct *opts, const char *diff, const char *cvd, const char *incdir) { - char *tempdir, cwd[512]; - int ret = 0, fd; + char *tempdir = NULL; + char cwd[512]; + int ret = -1; unsigned short mode; + FFIError *cdiff_apply_error = NULL; + char *cvdcertsdir = NULL; + bool created_temp_dir = false; if (strstr(diff, ".cdiff")) { mode = 1; @@ -2522,74 +2554,71 @@ static int verifydiff(const struct optstruct *opts, const char *diff, const char mode = 0; } else { mprintf(LOGG_ERROR, "verifydiff: Incorrect file name (no .cdiff/.script extension)\n"); - return -1; + goto done; } if (!(tempdir = createTempDir(opts))) { - return -1; + goto done; } + created_temp_dir = true; if (cvd) { if (CL_SUCCESS != cl_cvdunpack_ex(cvd, tempdir, true, NULL)) { mprintf(LOGG_ERROR, "verifydiff: Can't unpack CVD file %s\n", cvd); - removeTempDir(opts, tempdir); - free(tempdir); - return -1; + goto done; } } else { if (dircopy(incdir, tempdir) == -1) { mprintf(LOGG_ERROR, "verifydiff: Can't copy dir %s to %s\n", incdir, tempdir); - removeTempDir(opts, tempdir); - free(tempdir); - return -1; + goto done; } } if (!getcwd(cwd, sizeof(cwd))) { mprintf(LOGG_ERROR, "verifydiff: getcwd() failed\n"); - removeTempDir(opts, tempdir); - free(tempdir); - return -1; - } - - if ((fd = open(diff, O_RDONLY | O_BINARY)) == -1) { - mprintf(LOGG_ERROR, "verifydiff: Can't open diff file %s\n", diff); - removeTempDir(opts, tempdir); - free(tempdir); - return -1; + goto done; } if (chdir(tempdir) == -1) { mprintf(LOGG_ERROR, "verifydiff: Can't chdir to %s\n", tempdir); - removeTempDir(opts, tempdir); - free(tempdir); - close(fd); - return -1; + goto done; } - if (cdiff_apply(fd, mode) == -1) { + if (!cdiff_apply( + diff, + cvdcertsdir, + mode, + &cdiff_apply_error)) { mprintf(LOGG_ERROR, "verifydiff: Can't apply %s\n", diff); - if (chdir(cwd) == -1) + if (chdir(cwd) == -1) { mprintf(LOGG_WARNING, "verifydiff: Can't chdir to %s\n", cwd); - removeTempDir(opts, tempdir); - free(tempdir); - close(fd); - return -1; + } + goto done; } - close(fd); ret = comparesha(diff); - if (chdir(cwd) == -1) + if (chdir(cwd) == -1) { mprintf(LOGG_WARNING, "verifydiff: Can't chdir to %s\n", cwd); - removeTempDir(opts, tempdir); - free(tempdir); + } - if (!ret) { - if (cvd) + if (0 == ret) { + if (cvd) { mprintf(LOGG_INFO, "Verification: %s correctly applies to %s\n", diff, cvd); - else + } else { mprintf(LOGG_INFO, "Verification: %s correctly applies to the previous version\n", diff); + } + } + +done: + if (NULL != tempdir) { + free(tempdir); + } + if (NULL != cdiff_apply_error) { + ffierror_free(cdiff_apply_error); + } + if (created_temp_dir) { + removeTempDir(opts, tempdir); } return ret;