Skip to content

Commit

Permalink
Fixes from code review
Browse files Browse the repository at this point in the history 
Create a sign-verifier once and pass it around.

Sign verifier can now load multiple certs from a single file.
Additionallly, it may load multiple certs and verify once.
No need to try each individually or check serial numbers.

Signer can also now load multiple certs from a bundle.

Fixed bug with sign --append option when file didn't exist yet.
  • Loading branch information
@micahsnyder
micahsnyder committed Dec 18, 2024
1 parent c84ae11 commit eb9140f
Show file tree
Hide file tree
Showing 14 changed files with 435 additions and 290 deletions.
103 changes: 89 additions & 14 deletions libclamav/cvd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static void cli_tgzload_cleanup(int comp, struct cli_dbio *dbio, int fdd)
}
}

static int cli_tgzload(cvd_t *cvd, struct cl_engine *engine, unsigned int *signo, unsigned int options, struct cli_dbio *dbio, struct cli_dbinfo *dbinfo)
static int cli_tgzload(cvd_t *cvd, struct cl_engine *engine, unsigned int *signo, unsigned int options, struct cli_dbio *dbio, struct cli_dbinfo *dbinfo, void *sign_verifier)
{
char osize[13], name[101];
char block[TAR_BLOCKSIZE];
Expand Down Expand Up @@ -210,7 +210,7 @@ static int cli_tgzload(cvd_t *cvd, struct cl_engine *engine, unsigned int *signo
off = ftell(dbio->fs);

if ((!dbinfo && cli_strbcasestr(name, ".info")) || (dbinfo && CLI_DBEXT(name))) {
ret = cli_load(name, engine, signo, options, dbio);
ret = cli_load(name, engine, signo, options, dbio, sign_verifier);
if (ret) {
cli_errmsg("cli_tgzload: Can't load %s\n", name);
cli_tgzload_cleanup(compr, dbio, fdd);
Expand Down Expand Up @@ -394,9 +394,11 @@ cl_error_t cl_cvdverify(const char *file)

cl_error_t cl_cvdverify_ex(const char *file, const char *certs_directory)
{
struct cl_engine *engine;
struct cl_engine *engine = NULL;
cl_error_t ret;
cvd_type dbtype = CVD_TYPE_UNKNOWN;
cvd_type dbtype = CVD_TYPE_UNKNOWN;
void *verifier = NULL;
FFIError *new_verifier_error = NULL;

if (!(engine = cl_engine_new())) {
cli_errmsg("cl_cvdverify: Can't create new engine\n");
Expand Down Expand Up @@ -425,15 +427,29 @@ cl_error_t cl_cvdverify_ex(const char *file, const char *certs_directory)
goto done;
}

ret = cli_cvdload(engine, NULL, CL_DB_STDOPT | CL_DB_PUA, dbtype, file, 1);
if (!codesign_verifier_new(engine->certs_directory, &verifier, &new_verifier_error)) {
cli_errmsg("cl_cvdverify: Failed to create a new code-signature verifier: %s\n", ffierror_fmt(new_verifier_error));
ret = CL_ECVD;
goto done;
}

ret = cli_cvdload(engine, NULL, CL_DB_STDOPT | CL_DB_PUA, dbtype, file, verifier, 1);

done:
cl_engine_free(engine);
if (NULL != engine) {
cl_engine_free(engine);
}
if (NULL != verifier) {
codesign_verifier_free(verifier);
}
if (NULL != new_verifier_error) {
ffierror_free(new_verifier_error);
}

return ret;
}

cl_error_t cli_cvdload(struct cl_engine *engine, unsigned int *signo, unsigned int options, cvd_type dbtype, const char *filename, unsigned int chkonly)
cl_error_t cli_cvdload(struct cl_engine *engine, unsigned int *signo, unsigned int options, cvd_type dbtype, const char *filename, void *sign_verifier, unsigned int chkonly)
{
cl_error_t status = CL_ECVD;
cl_error_t ret;
Expand Down Expand Up @@ -462,7 +478,7 @@ cl_error_t cli_cvdload(struct cl_engine *engine, unsigned int *signo, unsigned i
if (dbtype == CVD_TYPE_CVD) {
if (!cvd_verify(
cvd,
engine->certs_directory,
sign_verifier,
false,
&signer_name,
&cvd_verify_error)) {
Expand Down Expand Up @@ -534,9 +550,9 @@ cl_error_t cli_cvdload(struct cl_engine *engine, unsigned int *signo, unsigned i

dbio.chkonly = 0;
if (dbtype == CVD_TYPE_CUD) {
ret = cli_tgzload(cvd, engine, signo, options | CL_DB_UNSIGNED, &dbio, NULL);
ret = cli_tgzload(cvd, engine, signo, options | CL_DB_UNSIGNED, &dbio, NULL, sign_verifier);
} else {
ret = cli_tgzload(cvd, engine, signo, options | CL_DB_OFFICIAL, &dbio, NULL);
ret = cli_tgzload(cvd, engine, signo, options | CL_DB_OFFICIAL, &dbio, NULL, sign_verifier);
}
if (ret != CL_SUCCESS) {
status = ret;
Expand Down Expand Up @@ -569,7 +585,7 @@ cl_error_t cli_cvdload(struct cl_engine *engine, unsigned int *signo, unsigned i
options |= CL_DB_SIGNED | CL_DB_OFFICIAL;
}

status = cli_tgzload(cvd, engine, signo, options, &dbio, dbinfo);
status = cli_tgzload(cvd, engine, signo, options, &dbio, dbinfo, sign_verifier);

done:

Expand Down Expand Up @@ -603,7 +619,7 @@ cl_error_t cli_cvdload(struct cl_engine *engine, unsigned int *signo, unsigned i
return status;
}

cl_error_t cl_cvdunpack(const char *file, const char *dir, bool dont_verify)
cl_error_t cli_cvdunpack_and_verify(const char *file, const char *dir, bool dont_verify, void *verifier)
{
cl_error_t status = CL_SUCCESS;
cvd_t *cvd = NULL;
Expand All @@ -619,7 +635,7 @@ cl_error_t cl_cvdunpack(const char *file, const char *dir, bool dont_verify)
}

if (!dont_verify) {
if (!cvd_verify(cvd, NULL, false, &signer_name, &cvd_verify_error)) {
if (!cvd_verify(cvd, verifier, false, &signer_name, &cvd_verify_error)) {
cli_errmsg("CVD verification failed: %s\n", ffierror_fmt(cvd_verify_error));
status = CL_EVERIFY;
goto done;
Expand Down Expand Up @@ -654,6 +670,65 @@ cl_error_t cl_cvdunpack(const char *file, const char *dir, bool dont_verify)
}

cl_error_t cl_cvdunpack_ex(const char *file, const char *dir, bool dont_verify, const char *certs_directory)
{
cl_error_t status = CL_SUCCESS;
cvd_t *cvd = NULL;
FFIError *cvd_open_error = NULL;
FFIError *new_verifier_error = NULL;
FFIError *cvd_unpack_error = NULL;
char *signer_name = NULL;
void *verifier = NULL;

cvd = cvd_open(file, &cvd_open_error);
if (!cvd) {
cli_errmsg("Can't open CVD file %s: %s\n", file, ffierror_fmt(cvd_open_error));
return CL_EOPEN;
}

if (!dont_verify) {
if (!codesign_verifier_new(certs_directory, &verifier, &new_verifier_error)) {
cli_errmsg("Failed to create a new code-signature verifier: %s\n", ffierror_fmt(new_verifier_error));
status = CL_EUNPACK;
goto done;
}

status = cli_cvdunpack_and_verify(file, dir, dont_verify, verifier);
if (status != CL_SUCCESS) {
goto done;
}
}

if (!cvd_unpack(cvd, dir, &cvd_unpack_error)) {
cli_errmsg("CVD unpacking failed: %s\n", ffierror_fmt(cvd_unpack_error));
status = CL_EUNPACK;
goto done;
}

done:

if (NULL != signer_name) {
ffi_cstring_free(signer_name);
}
if (NULL != cvd) {
cvd_free(cvd);
}
if (NULL != cvd_open_error) {
ffierror_free(cvd_open_error);
}
if (NULL != new_verifier_error) {
ffierror_free(new_verifier_error);
}
if (NULL != cvd_unpack_error) {
ffierror_free(cvd_unpack_error);
}
if (NULL != verifier) {
codesign_verifier_free(verifier);
}

return status;
}

cl_error_t cl_cvdunpack(const char *file, const char *dir, bool dont_verify)
{
cl_error_t status = CL_SUCCESS;
cvd_t *cvd = NULL;
Expand All @@ -669,7 +744,7 @@ cl_error_t cl_cvdunpack_ex(const char *file, const char *dir, bool dont_verify,
}

if (!dont_verify) {
if (!cvd_verify(cvd, certs_directory, false, &signer_name, &cvd_verify_error)) {
if (!cvd_verify(cvd, NULL, false, &signer_name, &cvd_verify_error)) {
cli_errmsg("CVD verification failed: %s\n", ffierror_fmt(cvd_verify_error));
status = CL_EVERIFY;
goto done;
Expand Down
3 changes: 2 additions & 1 deletion libclamav/cvd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ typedef enum cvd_type {
CVD_TYPE_CUD,
} cvd_type;

cl_error_t cli_cvdload(struct cl_engine *engine, unsigned int *signo, unsigned int options, cvd_type dbtype, const char *filename, unsigned int chkonly);
cl_error_t cli_cvdload(struct cl_engine *engine, unsigned int *signo, unsigned int options, cvd_type dbtype, const char *filename, void *sign_verifier, unsigned int chkonly);
cl_error_t cli_cvdunpack_and_verify(const char *file, const char *dir, bool dont_verify, void *verifier);

#endif
1 change: 1 addition & 0 deletions libclamav/libclamav.map
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ CLAMAV_PRIVATE {
cli_magic_scan_buff;
cli_checklimits;
cli_matchmeta;
cli_cvdunpack_and_verify;

__cli_strcasestr;
__cli_strndup;
Expand Down
33 changes: 22 additions & 11 deletions libclamav/readdb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4690,9 +4690,9 @@ static int cli_loadpwdb(FILE *fs, struct cl_engine *engine, unsigned int options
return CL_SUCCESS;
}

static cl_error_t cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned int *signo, unsigned int options);
static cl_error_t cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned int *signo, unsigned int options, void *sign_verifier);

cl_error_t cli_load(const char *filename, struct cl_engine *engine, unsigned int *signo, unsigned int options, struct cli_dbio *dbio)
cl_error_t cli_load(const char *filename, struct cl_engine *engine, unsigned int *signo, unsigned int options, struct cli_dbio *dbio, void *sign_verifier)
{
cl_error_t ret = CL_SUCCESS;

Expand Down Expand Up @@ -4736,13 +4736,13 @@ cl_error_t cli_load(const char *filename, struct cl_engine *engine, unsigned int
ret = cli_loaddb(fs, engine, signo, options, dbio, dbname);

} else if (cli_strbcasestr(dbname, ".cvd")) {
ret = cli_cvdload(engine, signo, options, CVD_TYPE_CVD, filename, 0);
ret = cli_cvdload(engine, signo, options, CVD_TYPE_CVD, filename, sign_verifier, 0);

} else if (cli_strbcasestr(dbname, ".cld")) {
ret = cli_cvdload(engine, signo, options, CVD_TYPE_CLD, filename, 0);
ret = cli_cvdload(engine, signo, options, CVD_TYPE_CLD, filename, sign_verifier, 0);

} else if (cli_strbcasestr(dbname, ".cud")) {
ret = cli_cvdload(engine, signo, options, CVD_TYPE_CUD, filename, 0);
ret = cli_cvdload(engine, signo, options, CVD_TYPE_CUD, filename, sign_verifier, 0);

} else if (cli_strbcasestr(dbname, ".crb")) {
ret = cli_loadcrt(fs, engine, dbio);
Expand Down Expand Up @@ -5017,7 +5017,7 @@ static size_t count_signatures(const char *filepath, struct cl_engine *engine, u
return num_signatures;
}

static cl_error_t cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned int *signo, unsigned int options)
static cl_error_t cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned int *signo, unsigned int options, void *sign_verifier)
{
cl_error_t ret = CL_EOPEN;

Expand Down Expand Up @@ -5200,7 +5200,7 @@ static cl_error_t cli_loaddbdir(const char *dirname, struct cl_engine *engine, u
}
}

ret = cli_load(iter->path, engine, signo, options, NULL);
ret = cli_load(iter->path, engine, signo, options, NULL, sign_verifier);
if (ret) {
cli_errmsg("cli_loaddbdir: error loading database %s\n", iter->path);
goto done;
Expand Down Expand Up @@ -5239,7 +5239,9 @@ static cl_error_t cli_loaddbdir(const char *dirname, struct cl_engine *engine, u
cl_error_t cl_load(const char *path, struct cl_engine *engine, unsigned int *signo, unsigned int dboptions)
{
STATBUF sb;
int ret;
cl_error_t ret;
void *sign_verifier = NULL;
FFIError *new_verifier_error = NULL;

if (!engine) {
cli_errmsg("cl_load: engine == NULL\n");
Expand Down Expand Up @@ -5301,24 +5303,33 @@ cl_error_t cl_load(const char *path, struct cl_engine *engine, unsigned int *sig

engine->dboptions |= dboptions;

if (!codesign_verifier_new(engine->certs_directory, &sign_verifier, &new_verifier_error)) {
cli_errmsg("Failed to create a new code-signature verifier: %s\n", ffierror_fmt(new_verifier_error));
ffierror_free(new_verifier_error);
ret = CL_ECVD;
return ret;
}

switch (sb.st_mode & S_IFMT) {
case S_IFREG:
/* Count # of sigs in the database now */
engine->num_total_signatures += count_signatures(path, engine, dboptions);

ret = cli_load(path, engine, signo, dboptions, NULL);
ret = cli_load(path, engine, signo, dboptions, NULL, sign_verifier);
break;

case S_IFDIR:
/* Count # of signatures inside cli_loaddbdir(), before loading */
ret = cli_loaddbdir(path, engine, signo, dboptions | CL_DB_DIRECTORY);
ret = cli_loaddbdir(path, engine, signo, dboptions | CL_DB_DIRECTORY, sign_verifier);
break;

default:
cli_errmsg("cl_load(%s): Not supported database file type\n", path);
codesign_verifier_free(sign_verifier);
return CL_EOPEN;
}

codesign_verifier_free(sign_verifier);

if (engine->cb_sigload_progress) {
/* Let the progress callback function know we're done! */
(void)engine->cb_sigload_progress(*signo, *signo, engine->cb_sigload_progress_ctx);
Expand Down
2 changes: 1 addition & 1 deletion libclamav/readdb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ cl_error_t readdb_parse_ldb_subsignature(struct cli_matcher *root, const char *v
const char *offset, const uint32_t *lsigid, unsigned int options,
int current_subsig_index, int num_subsigs, struct cli_lsig_tdb *tdb);

cl_error_t cli_load(const char *filename, struct cl_engine *engine, unsigned int *signo, unsigned int options, struct cli_dbio *dbio);
cl_error_t cli_load(const char *filename, struct cl_engine *engine, unsigned int *signo, unsigned int options, struct cli_dbio *dbio, void *sign_verifier);

char *cli_dbgets(char *buff, unsigned int size, FILE *fs, struct cli_dbio *dbio);

Expand Down
2 changes: 2 additions & 0 deletions libclamav_rust/cbindgen.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ include = [
"cvd::cvd_get_file",
"codesign::codesign_sign_file",
"codesign::codesign_verify_file",
"codesign::codesign_verifier_new",
"codesign::codesign_verifier_free",
"fuzzy_hash::fuzzy_hash_calculate_image",
"fuzzy_hash::fuzzy_hash_load_subsignature",
"fuzzy_hash::fuzzy_hash_check",
Expand Down
35 changes: 12 additions & 23 deletions libclamav_rust/src/cdiff.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,15 @@
*/

use std::{
collections::BTreeMap,
ffi::{CStr, CString},
fs::{self, File, OpenOptions},
io::{prelude::*, BufReader, BufWriter, Read, Seek, SeekFrom, Write},
iter::*,
os::raw::c_char,
path::{Path, PathBuf},
str::{self, FromStr},
collections::BTreeMap, ffi::{c_void, CStr, CString}, fs::{self, File, OpenOptions}, io::{prelude::*, BufReader, BufWriter, Read, Seek, SeekFrom, Write}, iter::*, mem::ManuallyDrop, os::raw::c_char, path::{Path, PathBuf}, str::{self, FromStr}
};

use crate::{codesign, ffi_error, ffi_util::FFIError, sys, validate_str_param};
use crate::{
codesign::{self, Verifier},
ffi_error,
ffi_util::FFIError,
sys, validate_str_param,
};

use flate2::{read::GzDecoder, write::GzEncoder, Compression};
use log::{debug, error, warn};
Expand Down Expand Up @@ -585,7 +583,7 @@ pub fn script2cdiff(script_file_name: &str, builder: &str, server: &str) -> Resu
#[export_name = "cdiff_apply"]
pub unsafe extern "C" fn _cdiff_apply(
cdiff_file_path_str: *const c_char,
certs_directory_str: *const c_char,
verifier_ptr: *const c_void,
mode: u16,
err: *mut *mut FFIError,
) -> bool {
Expand All @@ -600,24 +598,15 @@ pub unsafe extern "C" fn _cdiff_apply(
}
};

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 verifier = ManuallyDrop::new(Box::from_raw(verifier_ptr as *mut Verifier));

let mode = if mode == 1 {
ApplyMode::Cdiff
} else {
ApplyMode::Script
};

if let Err(e) = cdiff_apply(&cdiff_file_path, &certs_directory, mode) {
if let Err(e) = cdiff_apply(&cdiff_file_path, &verifier, mode) {
error!("Failed to apply {:?}: {}", cdiff_file_path, e);
ffi_error!(err = err, e)
} else {
Expand All @@ -640,7 +629,7 @@ pub unsafe extern "C" fn _cdiff_apply(
/// ':' character to the left of EOF.
pub fn cdiff_apply(
cdiff_file_path: &Path,
certs_directory: &Path,
verifier: &Verifier,
mode: ApplyMode,
) -> Result<(), Error> {
let path = std::env::current_dir().unwrap();
Expand All @@ -661,7 +650,7 @@ pub fn cdiff_apply(
// 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);
codesign::verify_signed_file(cdiff_file_path, &sign_file_path, verifier);
let verified = match verify_result {
Ok(signer) => {
debug!(
Expand Down
Loading

0 comments on commit eb9140f

Please sign in to comment.