Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- Add external FIPS 140-2 / 140-3 compliant signatures for the Clam Databases. #1344

Closed
wants to merge 5 commits into from
Closed

- Add external FIPS 140-2 / 140-3 compliant signatures for the Clam Databases. #1344

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d233429
- Add external FIPS 140-2 / 140-3 compliant signatures for the Clam D…
mark-carey-sap Aug 19, 2024
d64161d
- Updated with clang-format to clean up pipeline issues.
mark-carey-sap Aug 20, 2024
e6d93ef
Added cryptographic method flexibility to external signatures. This …
mark-carey-sap Aug 22, 2024
bc057fe
Merge branch 'Cisco-Talos:main' into feature/fips-140-3-compliance
mark-carey-sap Aug 29, 2024
90ed3bb
Update libfreshclam/libfreshclam_internal.c
mark-carey-sap Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
252 changes: 245 additions & 7 deletions libclamav/crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@
#include <fcntl.h>

#include <openssl/evp.h>
#include <openssl/crypto.h>
#include <openssl/provider.h>
#include <openssl/opensslv.h>
#include <openssl/param_build.h>
#include <openssl/err.h>

// These are the public key components for the external signature
// Ideally these should be packaged in a way that allows individual entites
// to use their own keys if they should so desire.
#define CLI_NSTR_EXT_SIG "E32B3AC1D501EE975296A45BA65DD699100DADD340FF3BBD1F1030C66D6BB16DBFBD53DF4D97BBD42EF8FC777E7C114A6074A87DD8095A5C08B3DD7B85817713047647EF396C58358C5C22B5C3ADF85CE8D0ABC429F89E936EC917B64DD00E02A712E6666FAE1A71591092BCEE59E3141758C4719B4B08589117B0FF7CDBDBB261F8486A193E2E720AE0B16D40DD5E56E97346CBD8010DC81B35332F41C9E93E61490802DDCDFC823D581BA6888588968C68A3C95B93949AF411682E73323F7469473F668B0958F6966849FF03BDE808866D127A2C058B16F17C741A9EE50812A5C7841224E55BF7ADDB5AEAE8EB5476F9BC8740178AB35926D5DC375583C641"
#define CLI_ESTR_EXT_SIG "010001"

#if OPENSSL_VERSION_NUMBER < 0x10100000L
#define X509_CRL_get0_nextUpdate X509_CRL_get_nextUpdate
Expand Down Expand Up @@ -125,6 +136,74 @@ time_t timegm(struct tm *t)
}
#endif

/**
* This variable determines if we are operating in FIPS mode, default to no.
*/
int cli_fips_mode = 0;

/**
* @brief This function determines if we are running in FIPS mode and sets the cl_fips_mode variable
*
* This function is called by cl_init() and does not need to be called by the user
*
* @return int Returns 1 if we are running in FIPS mode, 0 otherwise
*
*/

void cli_setup_fips_configuration(void)
{
#if OPENSSL_VERSION_MAJOR == 1
// OpenSSL 1.x (1.0 or 1.1)
#ifdef OPENSSL_FIPS
if (FIPS_mode()) {
cli_infomsg_simple("cl_setup_fips_configuration: FIPS mode provider found.\n");
cl_fips_mode = 1;
} else {
cli_infomsg_simple("cl_setup_fips_configuration: FIPS mode provider was not found.\n");
cl_fips_mode = 0;
}
#else
cl_fips_mode = 0;
#endif

#elif OPENSSL_VERSION_MAJOR == 3
// OpenSSL 3.0.x
OSSL_LIB_CTX *libctx = OSSL_LIB_CTX_new();
if (libctx == NULL) {
cli_warnmsg("cl_setup_fips_configuration: Failed to create libctx.\n");
cli_fips_mode = 0;
return;
}

OSSL_PROVIDER *fips = OSSL_PROVIDER_load(libctx, "fips");
if (fips != NULL) {
cli_infomsg_simple("cl_setup_fips_configuration: FIPS mode provider found.\n");
cli_fips_mode = 1;
OSSL_PROVIDER_unload(fips);
OSSL_LIB_CTX_free(libctx);
} else {
cli_infomsg_simple("cl_setup_fips_configuration: FIPS mode provider was not found.\n");
cli_fips_mode = 0;
OSSL_LIB_CTX_free(libctx);
}
#else
#error "Unsupported OpenSSL version"
#endif
}

/**
* @brief Return the status of our FIPS condition.
*
* This function allows users of the library to determine if the library is running in FIPS mode.
*
* @return int Returns 1 if we are running in FIPS mode, 0 otherwise
*/

int cli_get_fips_mode(void)
{
return cli_fips_mode;
}

/**
* @brief This function initializes the openssl crypto system
*
Expand All @@ -145,6 +224,8 @@ int cl_initialize_crypto(void)
ERR_load_crypto_strings();
#endif

cli_setup_fips_configuration();

return 0;
}

Expand All @@ -170,7 +251,12 @@ unsigned char *cl_hash_data(const char *alg, const void *buf, size_t len, unsign
size_t cur;
int winres = 0;

md = EVP_get_digestbyname(alg);
// If OpenSSL is running in FIPS mode, we need to use the FIPS-compliant md5 lookup of the algorithm
if (cli_fips_mode && !strncasecmp(alg, "md5", 3))
md = EVP_MD_fetch(NULL, alg, "-fips");
else
md = EVP_get_digestbyname(alg);

if (!(md))
return NULL;

Expand Down Expand Up @@ -260,7 +346,12 @@ unsigned char *cl_hash_file_fd(int fd, const char *alg, unsigned int *olen)
const EVP_MD *md;
unsigned char *res;

md = EVP_get_digestbyname(alg);
// If OpenSSL is running in FIPS mode, we need to use the FIPS-compliant md5 lookup of the algorithm
if (cli_fips_mode && !strncasecmp(alg, "md5", 3))
md = EVP_MD_fetch(NULL, alg, "-fips");
Copy link

@dkontyko dkontyko Aug 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this PR is much bigger than #959, but this section of code appears to be the same as that PR. Is this introducing the same issues as that, and would this have the same potential memory leak mentioned on that PR? I didn't see any added calls to EVP_MD_free in this PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi David! So per the OpenSSL documentation, we don't really need to worry about freeing this. The first element of the OpenSSL documentation I want to cite is:

  • EVP_MD_free()

Decrements the reference count for the fetched EVP_MD structure. If the reference count drops to 0 then the structure is freed. If the argument is NULL, nothing is done.

So, we're really dealing with a reference count. While this means it's not really an issue, I also like to be tidy with reference counts, but that leads to the second section of documentation I wanted to highlight:

EVP_MD_fetch()

Fetches the digest implementation for the given algorithm from any provider offering it, within the criteria given by the properties. See "ALGORITHM FETCHING" in crypto(7) for further information.

The returned value must eventually be freed with EVP_MD_free().

Fetched EVP_MD structures are reference counted.

So, we should free when using EVP_MD_fetch. However we also have:

EVP_get_digestbyname(), EVP_get_digestbynid(), EVP_get_digestbyobj()

Returns an EVP_MD structure when passed a digest name, a digest NID or an ASN1_OBJECT structure respectively.

The EVP_get_digestbyname() function is present for backwards compatibility with OpenSSL prior to version 3 and is different to the EVP_MD_fetch() function since it does not attempt to "fetch" an implementation of the cipher. Additionally, it only knows about digests that are built-in to OpenSSL and have an associated NID. Similarly EVP_get_digestbynid() and EVP_get_digestbyobj() also return objects without an associated implementation.

When the digest objects returned by these functions are used (such as in a call to EVP_DigestInit_ex()) an implementation of the digest will be implicitly fetched from the loaded providers. This fetch could fail if no suitable implementation is available. Use EVP_MD_fetch() instead to explicitly fetch the algorithm and an associated implementation from a provider.

See "ALGORITHM FETCHING" in crypto(7) for more information about fetching.

The digest objects returned from these functions do not need to be freed with EVP_MD_free().

So, now we do not have to free when using EVP_get_digestbyname. Terribly confusing. That said, I've added the logic to free according to the documentation. Please see the updated commit.

I also added ECC secp521r1 support for NIST Approved Elliptical Curve signing, as well.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I appreciate the detailed explanation. I agree, that documentation seems inconsistent with itself.

else
md = EVP_get_digestbyname(alg);

if (!(md))
return NULL;

Expand Down Expand Up @@ -392,7 +483,12 @@ int cl_verify_signature_hash(EVP_PKEY *pkey, const char *alg, unsigned char *sig
const EVP_MD *md;
size_t mdsz;

md = EVP_get_digestbyname(alg);
// If OpenSSL is running in FIPS mode, we need to use the FIPS-compliant md5 lookup of the algorithm
if (cli_fips_mode && !strncasecmp(alg, "md5", 3))
md = EVP_MD_fetch(NULL, alg, "-fips");
else
md = EVP_get_digestbyname(alg);

if (!(md))
return -1;

Expand Down Expand Up @@ -437,7 +533,12 @@ int cl_verify_signature_fd(EVP_PKEY *pkey, const char *alg, unsigned char *sig,
if (!(digest))
return -1;

md = EVP_get_digestbyname(alg);
// If OpenSSL is running in FIPS mode, we need to use the FIPS-compliant md5 lookup of the algorithm
if (cli_fips_mode && !strncasecmp(alg, "md5", 3))
md = EVP_MD_fetch(NULL, alg, "-fips");
else
md = EVP_get_digestbyname(alg);

if (!(md)) {
free(digest);
return -1;
Expand Down Expand Up @@ -506,7 +607,12 @@ int cl_verify_signature(EVP_PKEY *pkey, const char *alg, unsigned char *sig, uns
return -1;
}

md = EVP_get_digestbyname(alg);
// If OpenSSL is running in FIPS mode, we need to use the FIPS-compliant md5 lookup of the algorithm
if (cli_fips_mode && !strncasecmp(alg, "md5", 3))
md = EVP_MD_fetch(NULL, alg, "-fips");
else
md = EVP_get_digestbyname(alg);

if (!(md)) {
free(digest);
if (decode)
Expand Down Expand Up @@ -725,7 +831,12 @@ unsigned char *cl_sign_data(EVP_PKEY *pkey, const char *alg, unsigned char *hash
unsigned int siglen;
unsigned char *sig;

md = EVP_get_digestbyname(alg);
// If OpenSSL is running in FIPS mode, we need to use the FIPS-compliant md5 lookup of the algorithm
if (cli_fips_mode && !strncasecmp(alg, "md5", 3))
md = EVP_MD_fetch(NULL, alg, "-fips");
else
md = EVP_get_digestbyname(alg);

if (!(md))
return NULL;

Expand Down Expand Up @@ -1148,7 +1259,12 @@ void *cl_hash_init(const char *alg)
EVP_MD_CTX *ctx;
const EVP_MD *md;

md = EVP_get_digestbyname(alg);
// If OpenSSL is running in FIPS mode, we need to use the FIPS-compliant md5 lookup of the algorithm
if (cli_fips_mode && !strncasecmp(alg, "md5", 3))
md = EVP_MD_fetch(NULL, alg, "-fips");
else
md = EVP_get_digestbyname(alg);

if (!(md))
return NULL;

Expand Down Expand Up @@ -1210,3 +1326,125 @@ void cl_hash_destroy(void *ctx)

EVP_MD_CTX_destroy((EVP_MD_CTX *)ctx);
}

#if OPENSSL_VERSION_MAJOR == 1
RSA *cli_build_ext_signing_key(void)
{
RSA *rsa = RSA_new();
BIGNUM *n = BN_new();
BIGNUM *e = BN_new();

if (!rsa || !n || !e) {
RSA_free(rsa);
BN_free(n);
BN_free(e);
return NULL;
}

if (!BN_hex2bn(&n, CLI_NSTR_EXT_SIG) || !BN_hex2bn(&e, CLI_ESTR_EXT_SIG)) {
RSA_free(rsa);
BN_free(n);
BN_free(e);
return NULL;
}
rsa->n = n;
rsa->e = e;

return rsa;
}
#elif OPENSSL_VERSION_MAJOR == 3
// Do this the OpenSSL 3 way, avoiding deprecation warnings
EVP_PKEY *cli_build_ext_signing_key(void)
{
EVP_PKEY *pkey = EVP_PKEY_new();
BIGNUM *n = BN_new();
BIGNUM *e = BN_new();
OSSL_PARAM_BLD *bld = OSSL_PARAM_BLD_new();
OSSL_PARAM *params = NULL;
int result = 0;

// Check bld and params
if (!pkey || !n || !e || !bld) {
EVP_PKEY_free(pkey);
BN_free(n);
BN_free(e);
OSSL_PARAM_BLD_free(bld);
return NULL;
}

// Set the public key components
if (!BN_hex2bn(&n, CLI_NSTR_EXT_SIG) || !BN_hex2bn(&e, CLI_ESTR_EXT_SIG)) {
EVP_PKEY_free(pkey);
BN_free(n);
BN_free(e);
OSSL_PARAM_BLD_free(bld);
return NULL;
}

result = OSSL_PARAM_BLD_push_BN(bld, "n", n);
if (!result) {
EVP_PKEY_free(pkey);
BN_free(n);
BN_free(e);
OSSL_PARAM_BLD_free(bld);
return NULL;
}

result = OSSL_PARAM_BLD_push_BN(bld, "e", e);
if (!result) {
EVP_PKEY_free(pkey);
BN_free(n);
BN_free(e);
OSSL_PARAM_BLD_free(bld);
return NULL;
}

result = OSSL_PARAM_BLD_push_BN(bld, "d", NULL);
if (!result) {
EVP_PKEY_free(pkey);
BN_free(n);
BN_free(e);
OSSL_PARAM_BLD_free(bld);
return NULL;
}

params = OSSL_PARAM_BLD_to_param(bld);
if (!params) {
EVP_PKEY_free(pkey);
BN_free(n);
BN_free(e);
OSSL_PARAM_BLD_free(bld);
return NULL;
}

OSSL_PARAM_BLD_free(bld);
BN_free(n);
BN_free(e);

EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_from_name(NULL, "RSA", NULL);
if (!ctx) {
EVP_PKEY_free(pkey);
return NULL;
}

if (EVP_PKEY_fromdata_init(ctx) <= 0) {
EVP_PKEY_free(pkey);
return NULL;
}

if (EVP_PKEY_fromdata(ctx, &pkey, EVP_PKEY_PUBLIC_KEY, params) <= 0) {
EVP_PKEY_free(pkey);
return NULL;
}

if (params)
OSSL_PARAM_free(params);

if (ctx)
EVP_PKEY_CTX_free(ctx);

return pkey;
}
#else
#error "Unsupported OpenSSL version"
#endif
37 changes: 37 additions & 0 deletions libclamav/crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (C) 2013-2024 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
* Copyright (C) 2011-2013 Sourcefire, Inc.
*
* Authors: aCaB
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
* MA 02110-1301, USA.
*/

#ifndef __CRYPTO_H
#define __CRYPTO_H
#include <openssl/opensslv.h>

void cli_setup_fips_configuration(void);
int cli_get_fips_mode(void);

#if OPENSSL_VERSION_MAJOR == 1
RSA *cli_build_ext_signing_key(void);
#elif OPENSSL_VERSION_MAJOR == 3
EVP_PKEY *cli_build_ext_signing_key(void);
#else
#error "Unsupported OpenSSL version"

Check failure on line 34 in libclamav/crypto.h

View workflow job for this annotation

GitHub Actions / build-windows 

 #error:  "Unsupported OpenSSL version"

Check failure on line 34 in libclamav/crypto.h

View workflow job for this annotation

GitHub Actions / build-windows 

 #error:  "Unsupported OpenSSL version"
#endif

#endif
11 changes: 11 additions & 0 deletions libclamav/cvd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@
#include "others.h"
#include "dsig.h"
#include "str.h"
#include "crypto.h"
#include "cvd.h"
#include "readdb.h"
#include "default.h"

#include <openssl/rsa.h>

#define TAR_BLOCKSIZE 512

static void cli_untgz_cleanup(char *path, gzFile infile, FILE *outfile, int fdd)
Expand Down Expand Up @@ -627,6 +630,14 @@ cl_error_t cli_cvdload(FILE *fs, struct cl_engine *engine, unsigned int *signo,

cli_dbgmsg("in cli_cvdload()\n");

// FIPS verification MUST preclude other verification if in FIPS mode.
if (cli_get_fips_mode()) {
ret = cli_sigver_external(filename);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call should not be guarded by cli_fips_mode, external signature file should be preferred if present.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I concur. If we're going to add a better way to validate signing, it should be preferred. We should only fallback to the legacy signature verification if A. the new-style signature is not present and B. not FIPS-mode.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than adding comments all over the code to this extent. Every place that guards the validation/fetching of the external signature file with cli_fips_mode should be removed so that the new format is the default and only trusted signature in versions that support it.

if (ret != CL_SUCCESS) {
return ret;
}
}

/* verify */
if ((ret = cli_cvdverify(fs, &cvd, dbtype)))
return ret;
Expand Down
Loading
Loading