From d687e50c61d3a06b99999942555803842f7c4526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Thu, 15 Jul 2021 09:39:45 +0200 Subject: [PATCH 1/5] Allow disablement of SHA-1 and MD5 This patch adds 2 new CMake options `OPENSCAP_ENABLE_SHA1` and `OPENSCAP_ENABLE_MD5`. By setting these CMake options to `OFF` we can disable SHA-1 and MD5 hashing algorithm in OpenSCAP. If they are set to `OFF`, the code using SHA-1 and MD5 won't be built. Linux distributions that wish to disable SHA-1 and MD5 can use these CMake options in their spec files from now on. Conditional compilation using C preprocessor macros is used to achieve it. The approach with conditional compilation has been preferred over the hard removal of the code because future versions of OpenSCAP can get into currently stable Linux distributions where a removal would have to be reverted by a downstream patch to avoid feature regression. The consequence of disabling MD5 and SHA-1 is that filehash probe will not be built but filehash_test is a deprecated OVAL test. Second consequence is that people won't be able to use SHA-1 and MD5 in their filehash58_test. That means people will have to rewrite their SCAP content to use SHA-256 or SHA-512 in their filehash58_objects. Resolves: rhbz#1936619 --- CMakeLists.txt | 7 +- config.h.in | 3 + src/OVAL/probes/crapi/digest.c | 8 ++ src/OVAL/probes/crapi/digest.h | 4 + src/OVAL/probes/crapi/md5.c | 4 + src/OVAL/probes/crapi/md5.h | 4 + src/OVAL/probes/crapi/sha1.c | 4 + src/OVAL/probes/crapi/sha1.h | 4 + .../probes/independent/filehash58_probe.c | 8 ++ tests/API/crypt/test_api_crypt.sh | 34 ++++-- tests/API/crypt/test_crapi_digest.c | 101 +++++++----------- tests/API/crypt/test_crapi_mdigest.c | 11 ++ tests/probes/filehash/CMakeLists.txt | 2 +- .../filehash58/check_filehash_simple.xml | 6 +- .../filehash58/test_probes_filehash58.sh | 4 +- tests/test_common.sh.in | 3 + 16 files changed, 130 insertions(+), 77 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cc7b5e0059..9a15d591cf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -251,11 +251,14 @@ if (WIN32) endif() cmake_dependent_option(ENABLE_PROBES_WINDOWS "build OVAL probes for the Windows OVAL tests" ${IS_WIN32} "ENABLE_PROBES" OFF) +option(OPENSCAP_ENABLE_SHA1 "Enable using the SHA-1 algorithm" ON) +option(OPENSCAP_ENABLE_MD5 "Enable using the MD5 algorithm" ON) + # INDEPENDENT PROBES cmake_dependent_option(OPENSCAP_PROBE_INDEPENDENT_ENVIRONMENTVARIABLE "Independent environmentvariable probe" ON "ENABLE_PROBES_INDEPENDENT; NOT WIN32" OFF) cmake_dependent_option(OPENSCAP_PROBE_INDEPENDENT_ENVIRONMENTVARIABLE58 "Independent environmentvariable58 probe" ON "ENABLE_PROBES_INDEPENDENT; NOT WIN32" OFF) cmake_dependent_option(OPENSCAP_PROBE_INDEPENDENT_FAMILY "Independent family probe" ON "ENABLE_PROBES_INDEPENDENT" OFF) -cmake_dependent_option(OPENSCAP_PROBE_INDEPENDENT_FILEHASH "Independent filehash probe" ON "ENABLE_PROBES_INDEPENDENT; CRYPTO_FOUND; NOT WIN32" OFF) +cmake_dependent_option(OPENSCAP_PROBE_INDEPENDENT_FILEHASH "Independent filehash probe" ON "ENABLE_PROBES_INDEPENDENT; CRYPTO_FOUND; OPENSCAP_ENABLE_SHA1; OPENSCAP_ENABLE_MD5; NOT WIN32" OFF) cmake_dependent_option(OPENSCAP_PROBE_INDEPENDENT_FILEHASH58 "Independent filehash58 probe" ON "ENABLE_PROBES_INDEPENDENT; CRYPTO_FOUND; NOT WIN32" OFF) cmake_dependent_option(OPENSCAP_PROBE_INDEPENDENT_SQL "Independent sql probe" ON "ENABLE_PROBES_INDEPENDENT; OPENDBX_FOUND; NOT WIN32" OFF) cmake_dependent_option(OPENSCAP_PROBE_INDEPENDENT_SQL57 "Independent sql57 probe" ON "ENABLE_PROBES_INDEPENDENT; OPENDBX_FOUND; NOT WIN32" OFF) @@ -371,6 +374,8 @@ message(STATUS " ") message(STATUS "OVAL:") message(STATUS "base probe support: ${ENABLE_PROBES}") message(STATUS "SEAP msgid bit-size: ${SEAP_MSGID_BITS}") +message(STATUS "SHA-1: ${OPENSCAP_ENABLE_SHA1}") +message(STATUS "MD5: ${OPENSCAP_ENABLE_MD5}") message(STATUS "") message(STATUS "Independent probes: ${ENABLE_PROBES_INDEPENDENT}") diff --git a/config.h.in b/config.h.in index 7fdbb0a752..1b72855822 100644 --- a/config.h.in +++ b/config.h.in @@ -133,6 +133,9 @@ #cmakedefine PYTHON2_PATH "@PYTHON2_PATH@" #cmakedefine PYTHON3_PATH "@PYTHON3_PATH@" +#cmakedefine OPENSCAP_ENABLE_SHA1 +#cmakedefine OPENSCAP_ENABLE_MD5 + #include "oscap_platforms.h" #include "compat.h" diff --git a/src/OVAL/probes/crapi/digest.c b/src/OVAL/probes/crapi/digest.c index c76963f8c4..fdd361813e 100644 --- a/src/OVAL/probes/crapi/digest.c +++ b/src/OVAL/probes/crapi/digest.c @@ -51,10 +51,14 @@ int crapi_digest_fd (int fd, crapi_alg_t alg, void *dst, size_t *size) } switch (alg) { +#ifdef OPENSCAP_ENABLE_MD5 case CRAPI_DIGEST_MD5: return crapi_md5_fd (fd, dst, size); +#endif +#ifdef OPENSCAP_ENABLE_SHA1 case CRAPI_DIGEST_SHA1: return crapi_sha1_fd (fd, dst, size); +#endif case CRAPI_DIGEST_SHA224: return crapi_sha224_fd (fd, dst, size); case CRAPI_DIGEST_SHA256: @@ -100,18 +104,22 @@ int crapi_mdigest_fd (int fd, int num, ... /* crapi_alg_t alg, void *dst, size_t size = va_arg (ap, size_t *); switch (alg) { +#ifdef OPENSCAP_ENABLE_MD5 case CRAPI_DIGEST_MD5: ctbl[i].init = &crapi_md5_init; ctbl[i].update = &crapi_md5_update; ctbl[i].fini = &crapi_md5_fini; ctbl[i].free = &crapi_md5_free; break; +#endif +#ifdef OPENSCAP_ENABLE_SHA1 case CRAPI_DIGEST_SHA1: ctbl[i].init = &crapi_sha1_init; ctbl[i].update = &crapi_sha1_update; ctbl[i].fini = &crapi_sha1_fini; ctbl[i].free = &crapi_sha1_free; break; +#endif case CRAPI_DIGEST_SHA224: ctbl[i].init = &crapi_sha224_init; ctbl[i].update = &crapi_sha224_update; diff --git a/src/OVAL/probes/crapi/digest.h b/src/OVAL/probes/crapi/digest.h index 1ffd4ebb9d..3de55e4f94 100644 --- a/src/OVAL/probes/crapi/digest.h +++ b/src/OVAL/probes/crapi/digest.h @@ -27,8 +27,12 @@ #include typedef enum { +#ifdef OPENSCAP_ENABLE_MD5 CRAPI_DIGEST_MD5 = 0x01, +#endif +#ifdef OPENSCAP_ENABLE_SHA1 CRAPI_DIGEST_SHA1 = 0x02, +#endif CRAPI_DIGEST_SHA256 = 0x04, CRAPI_DIGEST_SHA512 = 0x08, CRAPI_DIGEST_RMD160 = 0x10, diff --git a/src/OVAL/probes/crapi/md5.c b/src/OVAL/probes/crapi/md5.c index 688165e3b2..8fc32af027 100644 --- a/src/OVAL/probes/crapi/md5.c +++ b/src/OVAL/probes/crapi/md5.c @@ -33,6 +33,8 @@ #include "crapi.h" #include "md5.h" +#ifdef OPENSCAP_ENABLE_MD5 + #if defined(HAVE_NSS3) #include @@ -230,3 +232,5 @@ int crapi_md5_fd (int fd, void *dst, size_t *size) } return (0); } + +#endif /* OPENSCAP_ENABLE_MD5 */ diff --git a/src/OVAL/probes/crapi/md5.h b/src/OVAL/probes/crapi/md5.h index 6adf14a87b..15e82bb671 100644 --- a/src/OVAL/probes/crapi/md5.h +++ b/src/OVAL/probes/crapi/md5.h @@ -25,6 +25,8 @@ #include +#ifdef OPENSCAP_ENABLE_MD5 + void *crapi_md5_init (void *dst, void *size); int crapi_md5_update (void *ctxp, void *bptr, size_t blen); int crapi_md5_fini (void *ctxp); @@ -32,4 +34,6 @@ void crapi_md5_free (void *ctxp); int crapi_md5_fd (int fd, void *dst, size_t *size); +#endif /* OPENSCAP_ENABLE_MD5 */ + #endif /* CRAPI_MD5_H */ diff --git a/src/OVAL/probes/crapi/sha1.c b/src/OVAL/probes/crapi/sha1.c index aee7916510..9a757d0274 100644 --- a/src/OVAL/probes/crapi/sha1.c +++ b/src/OVAL/probes/crapi/sha1.c @@ -33,6 +33,8 @@ #include "crapi.h" #include "sha1.h" +#ifdef OPENSCAP_ENABLE_SHA1 + #if defined(HAVE_NSS3) #include @@ -226,3 +228,5 @@ int crapi_sha1_fd (int fd, void *dst, size_t *size) return (0); } + +#endif /* OPENSCAP_ENABLE_SHA1 */ diff --git a/src/OVAL/probes/crapi/sha1.h b/src/OVAL/probes/crapi/sha1.h index 5c31c7ce92..282046c6a9 100644 --- a/src/OVAL/probes/crapi/sha1.h +++ b/src/OVAL/probes/crapi/sha1.h @@ -25,6 +25,8 @@ #include +#ifdef OPENSCAP_ENABLE_SHA1 + void *crapi_sha1_init (void *dst, void *size); int crapi_sha1_update (void *ctxp, void *bptr, size_t blen); int crapi_sha1_fini (void *ctxp); @@ -32,4 +34,6 @@ void crapi_sha1_free (void *ctxp); int crapi_sha1_fd (int fd, void *dst, size_t *size); +#endif /* OPENSCAP_ENABLE_SHA1 */ + #endif /* CRAPI_SHA1_H */ diff --git a/src/OVAL/probes/independent/filehash58_probe.c b/src/OVAL/probes/independent/filehash58_probe.c index ff1e065746..895d8f92f4 100644 --- a/src/OVAL/probes/independent/filehash58_probe.c +++ b/src/OVAL/probes/independent/filehash58_probe.c @@ -60,8 +60,12 @@ #define CRAPI_INVALID -1 static const struct oscap_string_map CRAPI_ALG_MAP[] = { +#ifdef OPENSCAP_ENABLE_MD5 {CRAPI_DIGEST_MD5, "MD5"}, +#endif +#ifdef OPENSCAP_ENABLE_SHA1 {CRAPI_DIGEST_SHA1, "SHA-1"}, +#endif {CRAPI_DIGEST_SHA224, "SHA-224"}, {CRAPI_DIGEST_SHA256, "SHA-256"}, {CRAPI_DIGEST_SHA384, "SHA-384"}, @@ -71,8 +75,12 @@ static const struct oscap_string_map CRAPI_ALG_MAP[] = { }; static const struct oscap_string_map CRAPI_ALG_MAP_SIZE[] = { +#ifdef OPENSCAP_ENABLE_MD5 {16, "MD5"}, +#endif +#ifdef OPENSCAP_ENABLE_SHA1 {20, "SHA-1"}, +#endif {28, "SHA-224"}, {32, "SHA-256"}, {48, "SHA-384"}, diff --git a/tests/API/crypt/test_api_crypt.sh b/tests/API/crypt/test_api_crypt.sh index e5e9d361f0..937f7eaf91 100755 --- a/tests/API/crypt/test_api_crypt.sh +++ b/tests/API/crypt/test_api_crypt.sh @@ -21,9 +21,8 @@ function test_crapi_digest { local TEMPDIR="$(make_temp_dir /tmp tmp)" local ret_val=0; - local sum_md5=""; - local sum_sha1=""; - local sum_sha256=""; + local sum="" + local algo="$1" dd if=/dev/urandom of="${TEMPDIR}/a" count=1 bs=1k || return 2 dd if=/dev/urandom of="${TEMPDIR}/b" count=123 bs=1 || return 2 @@ -33,16 +32,21 @@ function test_crapi_digest { dd if=/dev/urandom of="${TEMPDIR}/f" count=312 bs=1 || return 2 for file in a b c d e f; do - sum_md5=$((md5sum "${TEMPDIR}/${file}" || openssl md5 "${TEMPDIR}/${file}") | sed -n 's|^.*\([0-9a-f]\{32\}\).*$|\1|p') - sum_sha1=$((sha1sum "${TEMPDIR}/${file}" || openssl sha1 "${TEMPDIR}/${file}") | sed -n 's|^.*\([0-9a-f]\{40\}\).*$|\1|p') - sum_sha256=$((sha256sum "${TEMPDIR}/${file}" || openssl sha256 "${TEMPDIR}/${file}") | sed -n 's|^.*\([0-9a-f]\{64\}\).*$|\1|p') + if [[ "$algo" == "md5" ]] ; then + sum=$((md5sum "${TEMPDIR}/${file}" || openssl md5 "${TEMPDIR}/${file}") | sed -n 's|^.*\([0-9a-f]\{32\}\).*$|\1|p') + elif [[ "$algo" == "sha1" ]] ; then + sum=$((sha1sum "${TEMPDIR}/${file}" || openssl sha1 "${TEMPDIR}/${file}") | sed -n 's|^.*\([0-9a-f]\{40\}\).*$|\1|p') + elif [[ "$algo" == "sha256" ]] ; then + sum=$((sha256sum "${TEMPDIR}/${file}" || openssl sha256 "${TEMPDIR}/${file}") | sed -n 's|^.*\([0-9a-f]\{64\}\).*$|\1|p') + else + return 2 + fi - if [[ "$sum_md5" == "" || "$sum_sha1" == "" || "$sum_sha256" == "" ]]; then + if [[ "$sum" == "" ]]; then return 2 fi - ./test_crapi_digest "${TEMPDIR}/${file}" "$sum_md5" "$sum_sha1" "$sum_sha256" || return 1 - #echo "$file: ret $?, 5: $sum_md5, 1: $sum_sha1, $sum_sha256" + ./test_crapi_digest "${TEMPDIR}/${file}" "$sum" "$algo" || return 1 done rm -rf "$TEMPDIR" @@ -88,8 +92,16 @@ function test_crapi_mdigest { test_init if [ -z ${CUSTOM_OSCAP+x} ] ; then - test_run "test_crapi_digest" test_crapi_digest - test_run "test_crapi_mdigest" test_crapi_mdigest + if [[ "$OPENSCAP_ENABLE_MD5" == "ON" ]] ; then + test_run "test_crapi_digest_md5" test_crapi_digest md5 + fi + if [[ "$OPENSCAP_ENABLE_SHA1" == "ON" ]] ; then + test_run "test_crapi_digest_sha1" test_crapi_digest sha1 + fi + test_run "test_crapi_digest_sha256" test_crapi_digest sha256 + if [[ "$OPENSCAP_ENABLE_MD5" == "ON" && "$OPENSCAP_ENABLE_SHA1" == "ON" ]] ; then + test_run "test_crapi_mdigest" test_crapi_mdigest + fi fi test_exit diff --git a/tests/API/crypt/test_crapi_digest.c b/tests/API/crypt/test_crapi_digest.c index 41ef8fbaa5..e0ccf5b2a2 100644 --- a/tests/API/crypt/test_crapi_digest.c +++ b/tests/API/crypt/test_crapi_digest.c @@ -58,88 +58,69 @@ static int mem2hex (uint8_t *mem, size_t mlen, char *str, size_t slen) int main (int argc, char *argv[]) { - uint8_t md5_dst[16]; - size_t md5_dstlen = sizeof md5_dst; - - uint8_t sha1_dst[20]; - size_t sha1_dstlen = sizeof sha1_dst; - - uint8_t sha256_dst[32]; - size_t sha256_dstlen = sizeof sha256_dst; - - char *orig_md5sum, comp_md5sum[(sizeof md5_dst * 2) + 1]; - char *orig_sha1sum, comp_sha1sum[(sizeof sha1_dst * 2) + 1]; - char *orig_sha256sum, comp_sha256sum[(sizeof sha256_dst * 2) + 1]; - char *filename; - int fd; - - if (argc != 5) { - fprintf (stderr, "Usage: %s \n", argv[0]); + if (argc != 4) { + fprintf (stderr, "Usage: %s \n", argv[0]); return (1); } - filename = argv[1]; - orig_md5sum = argv[2]; - orig_sha1sum = argv[3]; - orig_sha256sum = argv[4]; - - if (crapi_init (NULL) != 0) { - fprintf (stderr, "crapi_init() != 0\n"); - abort (); - } - - fd = open (filename, O_RDONLY); - - if (fd < 0) { - perror ("open"); - return (2); - } + char *filename = argv[1]; + char *orig_sum = argv[2]; + char *algorithm_str = argv[3]; - if (crapi_digest_fd (fd, CRAPI_DIGEST_MD5, &md5_dst, &md5_dstlen) != 0) { - fprintf (stderr, "crapi_digest() != 0\n"); - abort (); - } + crapi_alg_t algorithm; + size_t dstlen = 0; - mem2hex (md5_dst, md5_dstlen, comp_md5sum, sizeof comp_md5sum); + if (!strcmp(algorithm_str, "md5")) { +#ifdef OPENSCAP_ENABLE_MD5 + algorithm = CRAPI_DIGEST_MD5; + dstlen = 16; +#else + return 1; +#endif + } else if (!strcmp(algorithm_str, "sha1")) { +#ifdef OPENSCAP_ENABLE_SHA1 + algorithm = CRAPI_DIGEST_SHA1; + dstlen = 20; +#else + return 1; +#endif + } else if (!strcmp(algorithm_str, "sha256")) { + algorithm = CRAPI_DIGEST_SHA256; + dstlen = 32; + } else { + return 1; + } - if (strcmp (orig_md5sum, comp_md5sum) != 0) { - fprintf (stderr, "crapi_digest::MD5(%s) != %s (== %s)\n", filename, orig_md5sum, comp_md5sum); - abort (); - } + int fd = open(filename, O_RDONLY); - if (lseek (fd, 0, SEEK_SET) == (off_t)-1) { - perror ("lseek"); + if (fd < 0) { + perror ("open"); return (2); } - if (crapi_digest_fd (fd, CRAPI_DIGEST_SHA1, &sha1_dst, &sha1_dstlen) != 0) { - fprintf (stderr, "crapi_digest() != 0\n"); + if (crapi_init (NULL) != 0) { + fprintf (stderr, "crapi_init() != 0\n"); abort (); } - mem2hex (sha1_dst, sha1_dstlen, comp_sha1sum, sizeof comp_sha1sum); - - if (strcmp (orig_sha1sum, comp_sha1sum) != 0) { - fprintf (stderr, "crapi_digest::SHA1(%s) != %s (== %s)\n", filename, orig_sha1sum, comp_sha1sum); - abort (); - } + uint8_t *dst = malloc(dstlen); - if (lseek (fd, 0, SEEK_SET) == (off_t)-1) { - perror ("lseek"); - return (2); - } + size_t comp_sum_len = (dstlen * 2) + 1; + char *comp_sum = malloc(comp_sum_len); - if (crapi_digest_fd (fd, CRAPI_DIGEST_SHA256, &sha256_dst, &sha256_dstlen) != 0) { + if (crapi_digest_fd(fd, algorithm, dst, &dstlen) != 0) { fprintf (stderr, "crapi_digest() != 0\n"); abort (); } - mem2hex (sha256_dst, sha256_dstlen, comp_sha256sum, sizeof comp_sha256sum); + mem2hex (dst, dstlen, comp_sum, comp_sum_len); - if (strcmp (orig_sha256sum, comp_sha256sum) != 0) { - fprintf (stderr, "crapi_digest::SHA256(%s) != %s (== %s)\n", filename, orig_sha256sum, comp_sha256sum); + if (strcmp(orig_sum, comp_sum) != 0) { + fprintf (stderr, "crapi_digest::%s(%s) != %s (== %s)\n", algorithm_str, filename, orig_sum, comp_sum); abort (); } + free(dst); + free(comp_sum); close (fd); diff --git a/tests/API/crypt/test_crapi_mdigest.c b/tests/API/crypt/test_crapi_mdigest.c index 9c5d31dfb3..200a2bbd9e 100644 --- a/tests/API/crypt/test_crapi_mdigest.c +++ b/tests/API/crypt/test_crapi_mdigest.c @@ -24,6 +24,8 @@ #include #endif +#if (defined OPENSCAP_ENABLE_MD5 && defined OPENSCAP_ENABLE_SHA1) + #include #include #include @@ -127,3 +129,12 @@ int main (int argc, char *argv[]) return (0); } + +#else + +int main (int argc, char *argv[]) +{ + return 1; +} + +#endif diff --git a/tests/probes/filehash/CMakeLists.txt b/tests/probes/filehash/CMakeLists.txt index e9d579c44c..cd04df4f30 100644 --- a/tests/probes/filehash/CMakeLists.txt +++ b/tests/probes/filehash/CMakeLists.txt @@ -1,3 +1,3 @@ -if(ENABLE_PROBES_INDEPENDENT) +if(OPENSCAP_PROBE_INDEPENDENT_FILEHASH) add_oscap_test("test_probes_filehash.sh") endif() diff --git a/tests/probes/filehash58/check_filehash_simple.xml b/tests/probes/filehash58/check_filehash_simple.xml index 2f6fa877e6..33e6343c73 100644 --- a/tests/probes/filehash58/check_filehash_simple.xml +++ b/tests/probes/filehash58/check_filehash_simple.xml @@ -28,13 +28,13 @@ /oval-test - SHA-1 + SHA-256 - SHA-1 - f1d2d2f924e986ac86fdf7b36c94bcdf32beec15 + SHA-256 + b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c diff --git a/tests/probes/filehash58/test_probes_filehash58.sh b/tests/probes/filehash58/test_probes_filehash58.sh index 5d9c513f29..06ee64446d 100755 --- a/tests/probes/filehash58/test_probes_filehash58.sh +++ b/tests/probes/filehash58/test_probes_filehash58.sh @@ -96,7 +96,9 @@ function test_probes_filehash58_chroot_fail { test_init -test_run "test_probes_filehash58" test_probes_filehash58 +if [[ "$OPENSCAP_ENABLE_MD5" == "ON" && "$OPENSCAP_ENABLE_SHA1" == "ON" ]] ; then + test_run "test_probes_filehash58" test_probes_filehash58 +fi test_run "test_probes_filehash58_chroot_fail" test_probes_filehash58_chroot_fail diff --git a/tests/test_common.sh.in b/tests/test_common.sh.in index b562855271..ef3675c1bf 100755 --- a/tests/test_common.sh.in +++ b/tests/test_common.sh.in @@ -329,3 +329,6 @@ die() { } export -f assert_exists + +export OPENSCAP_ENABLE_MD5="@OPENSCAP_ENABLE_MD5@" +export OPENSCAP_ENABLE_SHA1="@OPENSCAP_ENABLE_SHA1@" From d7a800d3c9ffae2062a81e656e6013b8021b3d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Thu, 15 Jul 2021 13:30:28 +0200 Subject: [PATCH 2/5] Another test for filehash58 probe It renames the existing test case and also creates a new test case which is almost identical with the existing test case but it uses different hashing algorithms in the filehash58_test. The reason for creating it is that the existing test case uses the SHA-1 and MD5 alorithms. But we introduced CMake options OPENSCAP_ENABLE_SHA1 and OPENSCAP_ENABLE_MD5 which allow to not build support for these algorithms. If OpenSCAP will be built with these options the existing tests would be disabled and we wouldn't test the filehash58 probe. We can expect that some downstreams will turn the options OFF so they need an alternative test instead. --- .../filehash58/test_probes_filehash58.sh | 40 +- .../test_probes_filehash58_new_algos.xml.sh | 1064 +++++++++++++++++ ...> test_probes_filehash58_old_algos.xml.sh} | 0 3 files changed, 1099 insertions(+), 5 deletions(-) create mode 100644 tests/probes/filehash58/test_probes_filehash58_new_algos.xml.sh rename tests/probes/filehash58/{test_probes_filehash58.xml.sh => test_probes_filehash58_old_algos.xml.sh} (100%) diff --git a/tests/probes/filehash58/test_probes_filehash58.sh b/tests/probes/filehash58/test_probes_filehash58.sh index 06ee64446d..459654546a 100755 --- a/tests/probes/filehash58/test_probes_filehash58.sh +++ b/tests/probes/filehash58/test_probes_filehash58.sh @@ -16,19 +16,19 @@ # Test Cases. -function test_probes_filehash58 { +function test_probes_filehash58_old_algos { probecheck "filehash58" || return 255 require "md5sum" || return 255 require "sha1sum" || return 255 local ret_val=0; - local DF="test_probes_filehash58.xml" + local DF="test_probes_filehash58_old_algos.xml" local RF="results.xml" [ -f $RF ] && rm -f $RF - bash ${srcdir}/test_probes_filehash58.xml.sh > $DF + bash ${srcdir}/test_probes_filehash58_old_algos.xml.sh > $DF $OSCAP oval eval --results $RF $DF if [ -f $RF ]; then @@ -38,7 +38,35 @@ function test_probes_filehash58 { ret_val=1 fi - # The file was created as a side-effect of test_probes_filehash58.xml.sh + # The file was created as a side-effect of test_probes_filehash58_old_algos.xml.sh + [ $ret_val -eq 0 ] && rm -f /tmp/test_probes_filehash58.tmp + + return $ret_val +} + +function test_probes_filehash58_new_algos { + + probecheck "filehash58" || return 255 + require "sha256sum" || return 255 + require "sha512sum" || return 255 + + local ret_val=0 + local DF="test_probes_filehash58_new_algos.xml" + local RF="results.xml" + + [ -f $RF ] && rm -f $RF + + bash ${srcdir}/test_probes_filehash58_new_algos.xml.sh > $DF + $OSCAP oval eval --results $RF $DF + + if [ -f $RF ]; then + verify_results "def" $DF $RF 13 && verify_results "tst" $DF $RF 120 + ret_val=$? + else + ret_val=1 + fi + + # The file was created as a side-effect of test_probes_filehash58_new_algos.xml.sh [ $ret_val -eq 0 ] && rm -f /tmp/test_probes_filehash58.tmp return $ret_val @@ -97,9 +125,11 @@ function test_probes_filehash58_chroot_fail { test_init if [[ "$OPENSCAP_ENABLE_MD5" == "ON" && "$OPENSCAP_ENABLE_SHA1" == "ON" ]] ; then - test_run "test_probes_filehash58" test_probes_filehash58 + test_run "test_probes_filehash58_old_algos" test_probes_filehash58_old_algos fi +test_run "test_probes_filehash58_new_algos" test_probes_filehash58_new_algos + test_run "test_probes_filehash58_chroot_fail" test_probes_filehash58_chroot_fail test_run "test_probes_filehash58_chroot_pass" test_probes_filehash58_chroot_pass diff --git a/tests/probes/filehash58/test_probes_filehash58_new_algos.xml.sh b/tests/probes/filehash58/test_probes_filehash58_new_algos.xml.sh new file mode 100644 index 0000000000..d120a0d114 --- /dev/null +++ b/tests/probes/filehash58/test_probes_filehash58_new_algos.xml.sh @@ -0,0 +1,1064 @@ +#!/usr/bin/env bash + +echo "Test Probes: FILEHASH test" > /tmp/test_probes_filehash58.tmp + +cat < + + + + filehash58 + 1.0 + 5.11 + 2011-07-14T00:00:00-00:00 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + /tmp + test_probes_filehash58.tmp + SHA-512 + + + + + /tmp + test_probes_filehash58.invalid + SHA-512 + + + + + + + + + + /tmp/test_probes_filehash58.tmp + /tmp + test_probes_filehash58.tmp + SHA-512 + `sha512sum /tmp/test_probes_filehash58.tmp | awk '{print $1}'` + + + + + /tmp + test_probes_filehash58.tmp + SHA-512 + ce66ae981bfdcb0a5b63e296792662caf3e9c0414d1053593876f38fc1afdaffb89fdbe016a21e7357209fbaa611644307d589780cca97c04df0052bc903af22 + + + + + /tmp + test_probes_filehash58.tmp + + + + + + + + + SHA-256 + SHA-512 + + + `sha256sum /tmp/test_probes_filehash58.tmp | awk '{print $1}'` + 0bdf537d823dfb9194c25f2a444ac38fc177fc3ed2160acf7c15e3a07ccc600a8303e2a0e8ce50659395e0ab02910404e2134997b105794a167387de45f24768 + + + + + +EOF diff --git a/tests/probes/filehash58/test_probes_filehash58.xml.sh b/tests/probes/filehash58/test_probes_filehash58_old_algos.xml.sh similarity index 100% rename from tests/probes/filehash58/test_probes_filehash58.xml.sh rename to tests/probes/filehash58/test_probes_filehash58_old_algos.xml.sh From 02a404bd0663e9e8191af6ea89bcaff5833eaaec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Wed, 21 Jul 2021 10:49:06 +0200 Subject: [PATCH 3/5] Show supported crypto in oscap --version output It will allow users to discover which hash algorithms are supported by our probes. --- src/OVAL/probes/probe-table.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/OVAL/probes/probe-table.c b/src/OVAL/probes/probe-table.c index f7ca47702a..dd434ae6f2 100644 --- a/src/OVAL/probes/probe-table.c +++ b/src/OVAL/probes/probe-table.c @@ -391,6 +391,21 @@ void probe_table_list(FILE *output) fprintf(output, "%-14s", oval_family_get_text(oval_subtype_get_family(type))); fprintf(output, "%-29s", oval_subtype_get_text(type)); fprintf(output, "probe_%s", oval_subtype_get_text(type)); +#if (defined(OPENSCAP_ENABLE_MD5) && defined(OPENSCAP_ENABLE_SHA1)) + if (type == OVAL_INDEPENDENT_FILE_HASH) { + fprintf(output, " (MD5, SHA-1)"); + } +#endif + if (type == OVAL_INDEPENDENT_FILE_HASH58) { + fprintf(output, " ("); +#ifdef OPENSCAP_ENABLE_MD5 + fprintf(output, "MD5, "); +#endif +#ifdef OPENSCAP_ENABLE_SHA1 + fprintf(output, "SHA-1, "); +#endif + fprintf(output, "SHA-224, SHA-256, SHA-384, SHA-512)"); + } fprintf(output, "\n"); entry++; } From df30597efbf7caa47e7b143dae518da01246e512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Wed, 21 Jul 2021 14:34:16 +0200 Subject: [PATCH 4/5] Remove unused CRAPI_DIGEST_CNT This constant isn't used anywhere. Moerover, after introducing preprocessor variables OPENSCAP_ENABLE_MD5 and OPENSCAP_ENABLE_SHA1 which affect the count supported crypto algorithms this value could dynamically differ from 7. --- src/OVAL/probes/crapi/digest.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/OVAL/probes/crapi/digest.h b/src/OVAL/probes/crapi/digest.h index 3de55e4f94..0d66db1336 100644 --- a/src/OVAL/probes/crapi/digest.h +++ b/src/OVAL/probes/crapi/digest.h @@ -40,8 +40,6 @@ typedef enum { CRAPI_DIGEST_SHA384 = 0x40 } crapi_alg_t; -#define CRAPI_DIGEST_CNT 7 - #include "md5.h" #include "sha1.h" #include "sha2.h" From 741bffe331b9a1b737440d4b9dce9a1775cc8345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= Date: Wed, 21 Jul 2021 14:42:45 +0200 Subject: [PATCH 5/5] Raise an error when using unsupported hash types The OVAL content can have a hash algorithm in `hash_type` element that is allowed by the OVAL specification but isn't currently supported in OpenSCAP. At this moment, this can happen if OpenSCAP is compiled without SHA-1 or MD5 support by setting OPENSCAP_ENABLE_SHA1 or OPENSCAP_ENABLE_MD5 to OFF. In this situation we should warn the user, for example show a warning and add a message element to the OVAL results. To do that, we need to be able to iterate over all hash types specified in the OVAL specification, not only over the supported ones, because we need to distinguish between a no match, a match of supported algorithm and a match of an unsupported algorithm. Therefore we need to list them explicitly. The CRAPI_INVALID value has been replaced by 0 because it used to be a -1 but a negative value can't be compared with an enum. That would cause a compiler warning. --- .../probes/independent/filehash58_probe.c | 109 +++++++++++------- 1 file changed, 69 insertions(+), 40 deletions(-) diff --git a/src/OVAL/probes/independent/filehash58_probe.c b/src/OVAL/probes/independent/filehash58_probe.c index 895d8f92f4..53535d7069 100644 --- a/src/OVAL/probes/independent/filehash58_probe.c +++ b/src/OVAL/probes/independent/filehash58_probe.c @@ -54,11 +54,22 @@ #include "util.h" #include "probe/entcmp.h" #include "filehash58_probe.h" +#include "oscap_helpers.h" #define FILE_SEPARATOR '/' -#define CRAPI_INVALID -1 +/* List of hash types listed in OVAL specification */ +static const char *OVAL_FILEHASH58_HASH_TYPES[] = { + "MD5", + "SHA-1", + "SHA-224", + "SHA-256", + "SHA-384", + "SHA-512", + NULL +}; +/* List of hash types supported by OpenSCAP */ static const struct oscap_string_map CRAPI_ALG_MAP[] = { #ifdef OPENSCAP_ENABLE_MD5 {CRAPI_DIGEST_MD5, "MD5"}, @@ -71,7 +82,7 @@ static const struct oscap_string_map CRAPI_ALG_MAP[] = { {CRAPI_DIGEST_SHA384, "SHA-384"}, {CRAPI_DIGEST_SHA512, "SHA-512"}, /* {CRAPI_DIGEST_RMD160, "RMD-160"}, OVAL doesn't support this */ - {CRAPI_INVALID, NULL} + {0, NULL} }; static const struct oscap_string_map CRAPI_ALG_MAP_SIZE[] = { @@ -165,45 +176,64 @@ static int filehash58_cb(const char *prefix, const char *p, const char *f, const probe_item_add_msg(itm, OVAL_MESSAGE_LEVEL_ERROR, "Can't open \"%s\": errno=%d, %s.", pbuf, errno, strerror (errno)); probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR); - } else { - uint8_t hash_dst[1025]; - size_t hash_dstlen = sizeof hash_dst; - char hash_str[2051]; - crapi_alg_t hash_type; + probe_item_collect(ctx, itm); + return 0; + } - hash_type = oscap_string_to_enum(CRAPI_ALG_MAP, h); - hash_dstlen = oscap_string_to_enum(CRAPI_ALG_MAP_SIZE, h); + uint8_t hash_dst[1025]; + size_t hash_dstlen = sizeof(hash_dst); + char hash_str[2051]; + crapi_alg_t hash_type; - /* - * Compute hash value - */ - if (crapi_mdigest_fd (fd, 1, hash_type, hash_dst, &hash_dstlen) != 0) { - close (fd); - return (-1); - } + hash_type = oscap_string_to_enum(CRAPI_ALG_MAP, h); + if (hash_type == 0) { + char *msg = oscap_sprintf("This version of OpenSCAP doesn't support the '%s' hash algorithm.", h); + dW(msg); + itm = probe_item_create (OVAL_INDEPENDENT_FILE_HASH58, NULL, + "filepath", OVAL_DATATYPE_STRING, pbuf, + "path", OVAL_DATATYPE_STRING, p, + "filename", OVAL_DATATYPE_STRING, f, + "hash_type", OVAL_DATATYPE_STRING, h, + NULL); + probe_item_add_msg(itm, OVAL_MESSAGE_LEVEL_ERROR, msg); + free(msg); + probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR); + probe_item_collect(ctx, itm); + close(fd); + return 0; + } + + hash_dstlen = oscap_string_to_enum(CRAPI_ALG_MAP_SIZE, h); + /* + * Compute hash value + */ + if (crapi_mdigest_fd(fd, 1, hash_type, hash_dst, &hash_dstlen) != 0) { close (fd); + return (-1); + } - hash_str[0] = '\0'; - mem2hex (hash_dst, hash_dstlen, hash_str, sizeof hash_str); + close (fd); - /* - * Create and add the item - */ - itm = probe_item_create(OVAL_INDEPENDENT_FILE_HASH58, NULL, - "filepath", OVAL_DATATYPE_STRING, pbuf, - "path", OVAL_DATATYPE_STRING, p, - "filename", OVAL_DATATYPE_STRING, f, - "hash_type",OVAL_DATATYPE_STRING, h, - "hash", OVAL_DATATYPE_STRING, hash_str, - NULL); + hash_str[0] = '\0'; + mem2hex(hash_dst, hash_dstlen, hash_str, sizeof(hash_str)); - if (hash_dstlen == 0) { - probe_item_add_msg(itm, OVAL_MESSAGE_LEVEL_ERROR, - "Unable to compute %s hash value of \"%s\".", h, pbuf); - probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR); - } + /* + * Create and add the item + */ + itm = probe_item_create(OVAL_INDEPENDENT_FILE_HASH58, NULL, + "filepath", OVAL_DATATYPE_STRING, pbuf, + "path", OVAL_DATATYPE_STRING, p, + "filename", OVAL_DATATYPE_STRING, f, + "hash_type",OVAL_DATATYPE_STRING, h, + "hash", OVAL_DATATYPE_STRING, hash_str, + NULL); + + if (hash_dstlen == 0) { + probe_item_add_msg(itm, OVAL_MESSAGE_LEVEL_ERROR, + "Unable to compute %s hash value of \"%s\".", h, pbuf); + probe_item_setstatus(itm, SYSCHAR_STATUS_ERROR); } probe_item_collect(ctx, itm); @@ -300,15 +330,14 @@ int filehash58_probe_main(probe_ctx *ctx, void *arg) if ((ofts = oval_fts_open_prefixed(prefix, path, filename, filepath, behaviors, probe_ctx_getresult(ctx))) != NULL) { while ((ofts_ent = oval_fts_read(ofts)) != NULL) { /* find hash types to compare with entity, think "not satisfy" */ - const struct oscap_string_map *p = CRAPI_ALG_MAP; - while (p->value != CRAPI_INVALID) { - SEXP_t *crapi_hash_type_sexp = SEXP_string_new(p->string, strlen(p->string)); - if (probe_entobj_cmp(hash_type, crapi_hash_type_sexp) == OVAL_RESULT_TRUE) { - filehash58_cb(prefix, ofts_ent->path, ofts_ent->file, p->string, ctx); + for (int i = 0; OVAL_FILEHASH58_HASH_TYPES[i] != NULL; i++) { + const char *oval_filehash58_hash_type = OVAL_FILEHASH58_HASH_TYPES[i]; + SEXP_t *oval_filehash58_hash_type_sexp = SEXP_string_new(oval_filehash58_hash_type, strlen(oval_filehash58_hash_type)); + if (probe_entobj_cmp(hash_type, oval_filehash58_hash_type_sexp) == OVAL_RESULT_TRUE) { + filehash58_cb(prefix, ofts_ent->path, ofts_ent->file, oval_filehash58_hash_type, ctx); } - SEXP_free(crapi_hash_type_sexp); - p++; + SEXP_free(oval_filehash58_hash_type_sexp); } oval_ftsent_free(ofts_ent); }