Skip to content

Commit c43e0c8

Browse files
Easton97-Jensjens
authored andcommitted
Refactor and improve SHA1 digest handling and mbedtls integration
- Migrate to TF-PSA-Crypto layout - Fix include and linkage issues - Harden runtime checks - Improve error and exception handling - Refactor digest helper and buffer usage
1 parent c0b60ea commit c43e0c8

File tree

7 files changed

+96
-52
lines changed

7 files changed

+96
-52
lines changed

Makefile.am

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ cppcheck:
6363
--enable=warning,style,performance,portability,unusedFunction,missingInclude \
6464
--inconclusive \
6565
--template="warning: {file},{line},{severity},{id},{message}" \
66-
-I headers -I . -I $(top_srcdir)/others -I $(top_srcdir)/src -I $(top_srcdir)/others/mbedtls/include \
66+
-I headers -I . -I $(top_srcdir)/others -I $(top_srcdir)/src -I $(top_srcdir)/others/mbedtls/include -I $(top_srcdir)/others/mbedtls/tf-psa-crypto/include -I $(top_srcdir)/others/mbedtls/tf-psa-crypto/drivers/builtin/include \
6767
--error-exitcode=1 \
6868
-i "src/parser/seclang-parser.cc" -i "src/parser/seclang-scanner.cc" \
6969
-i others \
@@ -99,4 +99,3 @@ pkgconfig_DATA = modsecurity.pc
9999
EXTRA_DIST = modsecurity.pc.in \
100100
modsecurity.conf-recommended \
101101
unicode.mapping
102-

build/win32/CMakeLists.txt

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,32 @@ target_compile_definitions(libinjection PRIVATE LIBINJECTION_VERSION="${LIBINJEC
5151
project(mbedcrypto C)
5252

5353
set(MBEDTLS_DIR ${BASE_DIR}/others/mbedtls)
54+
set(TF_PSA_CRYPTO_DIR ${MBEDTLS_DIR}/tf-psa-crypto)
55+
56+
add_library(mbedcrypto STATIC
57+
${TF_PSA_CRYPTO_DIR}/utilities/base64.c
58+
${TF_PSA_CRYPTO_DIR}/utilities/constant_time.c
59+
${TF_PSA_CRYPTO_DIR}/platform/platform_util.c
60+
${TF_PSA_CRYPTO_DIR}/extras/md.c
61+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/src/md5.c
62+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/src/sha1.c
63+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/src/sha256.c
64+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/src/sha512.c
65+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/src/sha3.c
66+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/src/ripemd160.c
67+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/src/psa_util_internal.c
68+
)
5469

55-
add_library(mbedcrypto STATIC ${MBEDTLS_DIR}/library/base64.c ${MBEDTLS_DIR}/library/sha1.c ${MBEDTLS_DIR}/library/md5.c ${MBEDTLS_DIR}/library/platform_util.c ${MBEDTLS_DIR}/library/constant_time.c)
56-
57-
target_include_directories(mbedcrypto PRIVATE ${MBEDTLS_DIR}/include)
70+
target_include_directories(mbedcrypto PRIVATE
71+
${MBEDTLS_DIR}/include
72+
${TF_PSA_CRYPTO_DIR}/include
73+
${TF_PSA_CRYPTO_DIR}/core
74+
${TF_PSA_CRYPTO_DIR}/extras
75+
${TF_PSA_CRYPTO_DIR}/library
76+
${TF_PSA_CRYPTO_DIR}/utilities
77+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/include
78+
${TF_PSA_CRYPTO_DIR}/drivers/builtin/src
79+
)
5880

5981
# get mbedtls version with git describe
6082
execute_process(
@@ -137,7 +159,7 @@ file(GLOB_RECURSE libModSecuritySources ${BASE_DIR}/src/*.cc)
137159
add_library(libModSecurity SHARED ${libModSecuritySources})
138160

139161
target_compile_definitions(libModSecurity PRIVATE WITH_PCRE2)
140-
target_include_directories(libModSecurity PRIVATE ${BASE_DIR} ${BASE_DIR}/headers ${BASE_DIR}/others ${MBEDTLS_DIR}/include)
162+
target_include_directories(libModSecurity PRIVATE ${BASE_DIR} ${BASE_DIR}/headers ${BASE_DIR}/others ${MBEDTLS_DIR}/include ${TF_PSA_CRYPTO_DIR}/include ${TF_PSA_CRYPTO_DIR}/drivers/builtin/include)
141163
target_link_libraries(libModSecurity PRIVATE pcre2::pcre2 libinjection mbedcrypto Poco::Poco Iphlpapi.lib)
142164

143165
macro(add_package_dependency project compile_definition link_library flag)

configure.ac

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ AC_DEFUN([LIBINJECTION_VERSION], m4_esyscmd_s(cd "others/libinjection" && git de
8484
AC_SUBST([LIBINJECTION_VERSION])
8585

8686
# Check for Mbed TLS
87-
if ! test -f "${srcdir}/others/mbedtls/library/base64.c"; then
87+
if ! test -f "${srcdir}/others/mbedtls/tf-psa-crypto/utilities/base64.c"; then
8888
AC_MSG_ERROR([\
8989
9090
@@ -532,4 +532,3 @@ if test "$aflFuzzer" = "true"; then
532532
echo " $ export CC=afl-clang-fast "
533533
echo " "
534534
fi
535-

others/Makefile.am

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,24 @@ noinst_HEADERS = \
1515
libinjection/src/libinjection_sqli.h \
1616
libinjection/src/libinjection_sqli_data.h \
1717
libinjection/src/libinjection_xss.h \
18-
mbedtls/include/mbedtls/base64.h \
19-
mbedtls/include/mbedtls/check_config.h \
18+
mbedtls/tf-psa-crypto/include/mbedtls/base64.h \
2019
mbedtls/include/mbedtls/mbedtls_config.h \
21-
mbedtls/include/mbedtls/md5.h \
22-
mbedtls/include/mbedtls/platform.h \
23-
mbedtls/include/mbedtls/sha1.h
20+
mbedtls/tf-psa-crypto/include/mbedtls/md.h \
21+
mbedtls/tf-psa-crypto/include/mbedtls/platform.h
2422

2523
libmbedtls_la_SOURCES = \
26-
mbedtls/library/base64.c \
27-
mbedtls/library/md5.c \
28-
mbedtls/library/sha1.c \
29-
mbedtls/library/platform_util.c
24+
mbedtls/tf-psa-crypto/utilities/base64.c \
25+
mbedtls/tf-psa-crypto/utilities/constant_time.c \
26+
mbedtls/tf-psa-crypto/platform/platform_util.c \
27+
mbedtls/tf-psa-crypto/extras/md.c \
28+
mbedtls/tf-psa-crypto/drivers/builtin/src/md5.c \
29+
mbedtls/tf-psa-crypto/drivers/builtin/src/sha1.c \
30+
mbedtls/tf-psa-crypto/drivers/builtin/src/sha256.c \
31+
mbedtls/tf-psa-crypto/drivers/builtin/src/sha512.c \
32+
mbedtls/tf-psa-crypto/drivers/builtin/src/sha3.c \
33+
mbedtls/tf-psa-crypto/drivers/builtin/src/ripemd160.c \
34+
mbedtls/tf-psa-crypto/drivers/builtin/src/psa_util_internal.c
3035

31-
libmbedtls_la_CFLAGS = -DMBEDTLS_CONFIG_FILE=\"mbedtls/mbedtls_config.h\" -I$(top_srcdir)/others/mbedtls/include
36+
libmbedtls_la_CFLAGS = -DMBEDTLS_CONFIG_FILE=\"mbedtls/mbedtls_config.h\" -I$(top_srcdir)/others/mbedtls/include -I$(top_srcdir)/others/mbedtls/tf-psa-crypto/include -I$(top_srcdir)/others/mbedtls/tf-psa-crypto/core -I$(top_srcdir)/others/mbedtls/tf-psa-crypto/extras -I$(top_srcdir)/others/mbedtls/tf-psa-crypto/library -I$(top_srcdir)/others/mbedtls/tf-psa-crypto/utilities -I$(top_srcdir)/others/mbedtls/tf-psa-crypto/drivers/builtin/include -I$(top_srcdir)/others/mbedtls/tf-psa-crypto/drivers/builtin/src
3237
libmbedtls_la_CPPFLAGS =
3338
libmbedtls_la_LIBADD =

src/Makefile.am

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,8 @@ libmodsecurity_la_CPPFLAGS = \
296296
-g \
297297
-I$(top_srcdir)/others \
298298
-I$(top_srcdir)/others/mbedtls/include \
299+
-I$(top_srcdir)/others/mbedtls/tf-psa-crypto/include \
300+
-I$(top_srcdir)/others/mbedtls/tf-psa-crypto/drivers/builtin/include \
299301
-fPIC \
300302
-O3 \
301303
-I$(top_srcdir)/headers \
@@ -343,4 +345,3 @@ libmodsecurity_la_LIBADD = \
343345
$(MAXMIND_LDADD) \
344346
$(SSDEEP_LDADD) \
345347
$(YAJL_LDADD)
346-

src/utils/md5.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,15 @@
1717
#define SRC_UTILS_MD5_H_
1818

1919
#include "src/utils/sha1.h"
20-
#include "mbedtls/md5.h"
2120
#include <string>
2221

2322
namespace modsecurity::Utils {
2423

2524

26-
class Md5 : public DigestImpl<&mbedtls_md5, 16> {
25+
class Md5 : public DigestImpl<MBEDTLS_MD_MD5, 16> {
2726
};
2827

2928

3029
} // namespace modsecurity::Utils
3130

32-
#endif // SRC_UTILS_MD5_H_
31+
#endif // SRC_UTILS_MD5_H_

src/utils/sha1.h

Lines changed: 49 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
44
*
55
* You may not use this file except in compliance with
6-
* the License. You may obtain a copy of the License at
6+
* the License. You may obtain a copy of the License at
77
*
88
* http://www.apache.org/licenses/LICENSE-2.0
99
*
@@ -16,60 +16,79 @@
1616
#ifndef SRC_UTILS_SHA1_H_
1717
#define SRC_UTILS_SHA1_H_
1818

19+
#include <array>
20+
#include <exception>
1921
#include <string>
20-
#include <cassert>
22+
#include <string_view>
2123

2224
#include "src/utils/string.h"
23-
#include "mbedtls/sha1.h"
25+
#include "mbedtls/md.h"
2426

2527
namespace modsecurity::Utils {
2628

29+
class DigestCalculationException : public std::exception {
30+
public:
31+
explicit DigestCalculationException(const char *message) noexcept
32+
: m_message(message) { }
2733

28-
using DigestOp = int (*)(const unsigned char *, size_t, unsigned char []);
34+
const char *what() const noexcept override {
35+
return m_message;
36+
}
2937

38+
private:
39+
const char *m_message;
40+
};
3041

31-
template<DigestOp digestOp, int DigestSize>
42+
43+
template<mbedtls_md_type_t DigestType, int DigestSize>
3244
class DigestImpl {
3345
public:
34-
3546
static std::string digest(const std::string& input) {
36-
return digestHelper(input, [](const auto digest) {
37-
return std::string(digest);
38-
});
47+
const auto digestBytes = calculateDigest(input);
48+
return std::string(digestBytes.begin(), digestBytes.end());
3949
}
4050

4151
static void digestReplace(std::string& value) {
42-
digestHelper(value, [&value](const auto digest) mutable {
43-
value = digest;
44-
});
52+
const auto digestBytes = calculateDigest(value);
53+
value.assign(digestBytes.begin(), digestBytes.end());
4554
}
4655

47-
static std::string hexdigest(const std::string &input) {
48-
return digestHelper(input, [](const auto digest) {
49-
return utils::string::string_to_hex(digest);
50-
});
56+
static std::string hexdigest(const std::string& input) {
57+
const auto digestBytes = calculateDigest(input);
58+
const std::string digestString(digestBytes.begin(), digestBytes.end());
59+
return utils::string::string_to_hex(digestString);
5160
}
5261

53-
private:
54-
55-
template<typename ConvertOp>
56-
static auto digestHelper(const std::string &input,
57-
ConvertOp convertOp) -> auto {
58-
char digest[DigestSize];
59-
60-
const auto ret = (*digestOp)(reinterpret_cast<const unsigned char *>(input.c_str()),
61-
input.size(), reinterpret_cast<unsigned char *>(digest));
62-
assert(ret == 0);
63-
64-
return convertOp(std::string_view(digest, DigestSize));
62+
private:
63+
static std::array<unsigned char, DigestSize> calculateDigest(
64+
std::string_view input) {
65+
std::array<unsigned char, DigestSize> digestBytes = {};
66+
67+
const mbedtls_md_info_t *mdInfo = mbedtls_md_info_from_type(DigestType);
68+
if (mdInfo == nullptr) {
69+
throw DigestCalculationException(
70+
"mbedtls_md_info_from_type() returned nullptr");
71+
}
72+
73+
const auto *inputBytes =
74+
static_cast<const unsigned char *>(static_cast<const void *>(input.data()));
75+
76+
if (const int ret = mbedtls_md(
77+
mdInfo,
78+
inputBytes,
79+
input.size(),
80+
digestBytes.data()); ret != 0) {
81+
throw DigestCalculationException("mbedtls_md() failed");
82+
}
83+
84+
return digestBytes;
6585
}
6686
};
6787

6888

69-
class Sha1 : public DigestImpl<&mbedtls_sha1, 20> {
89+
class Sha1 : public DigestImpl<MBEDTLS_MD_SHA1, 20> {
7090
};
7191

72-
7392
} // namespace modsecurity::Utils
7493

7594
#endif // SRC_UTILS_SHA1_H_

0 commit comments

Comments
 (0)