You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2022/03/11 16:14:37 UTC
[trafficserver] branch master updated: Fixed memory leaks with CryptoContext (#8719)
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 40c3d57 Fixed memory leaks with CryptoContext (#8719)
40c3d57 is described below
commit 40c3d57629c719d1a85c1c3c13313b3cc4e19ccc
Author: Bryan Call <bc...@apache.org>
AuthorDate: Fri Mar 11 08:14:25 2022 -0800
Fixed memory leaks with CryptoContext (#8719)
---
include/tscore/CryptoHash.h | 20 ++++++++----
include/tscore/MD5.h | 3 --
include/tscore/SHA256.h | 13 +++-----
src/traffic_server/traffic_server.cc | 12 -------
src/tscore/CryptoHash.cc | 13 ++------
src/tscore/Makefile.am | 1 +
src/tscore/unit_tests/test_CryptoHash.cc | 55 ++++++++++++++++++++++++++++++++
7 files changed, 76 insertions(+), 41 deletions(-)
diff --git a/include/tscore/CryptoHash.h b/include/tscore/CryptoHash.h
index 767cb7b..4c2d97e 100644
--- a/include/tscore/CryptoHash.h
+++ b/include/tscore/CryptoHash.h
@@ -23,6 +23,7 @@
#pragma once
#include "tscore/BufferWriter.h"
+#include <openssl/evp.h>
#include <string_view>
/// Apache Traffic Server commons.
@@ -132,6 +133,9 @@ public:
/// @note This is just as fast as the previous style, as a new context must be initialized
/// every time this is done.
bool hash_immediate(CryptoHash &hash, void const *data, int length);
+
+protected:
+ EVP_MD_CTX *_ctx = nullptr;
};
inline bool
@@ -159,29 +163,31 @@ public:
UNSPECIFIED,
#if TS_ENABLE_FIPS == 0
MD5,
- MMH,
#endif
SHA256,
}; ///< What type of hash we really are.
static HashType Setting;
- /// Size of storage for placement @c new of hashing context.
- static size_t const OBJ_SIZE = 256;
+ ~CryptoContext()
+ {
+ delete _base;
+ _base = nullptr;
+ }
-protected:
- char _obj[OBJ_SIZE]; ///< Raw storage for instantiated context.
+private:
+ CryptoContextBase *_base = nullptr;
};
inline bool
CryptoContext::update(void const *data, int length)
{
- return reinterpret_cast<CryptoContextBase *>(_obj)->update(data, length);
+ return _base->update(data, length);
}
inline bool
CryptoContext::finalize(CryptoHash &hash)
{
- return reinterpret_cast<CryptoContextBase *>(_obj)->finalize(hash);
+ return _base->finalize(hash);
}
ts::BufferWriter &bwformat(ts::BufferWriter &w, ts::BWFSpec const &spec, ats::CryptoHash const &hash);
diff --git a/include/tscore/MD5.h b/include/tscore/MD5.h
index 3131197..8b1e5bb 100644
--- a/include/tscore/MD5.h
+++ b/include/tscore/MD5.h
@@ -29,9 +29,6 @@
class MD5Context : public ats::CryptoContextBase
{
-protected:
- EVP_MD_CTX *_ctx;
-
public:
MD5Context()
{
diff --git a/include/tscore/SHA256.h b/include/tscore/SHA256.h
index 9b3140a..446ae0c 100644
--- a/include/tscore/SHA256.h
+++ b/include/tscore/SHA256.h
@@ -29,26 +29,23 @@
class SHA256Context : public ats::CryptoContextBase
{
-protected:
- EVP_MD_CTX *ctx;
-
public:
SHA256Context()
{
- ctx = EVP_MD_CTX_new();
- EVP_DigestInit_ex(ctx, EVP_sha256(), nullptr);
+ _ctx = EVP_MD_CTX_new();
+ EVP_DigestInit_ex(_ctx, EVP_sha256(), nullptr);
}
- ~SHA256Context() { EVP_MD_CTX_free(ctx); }
+ ~SHA256Context() { EVP_MD_CTX_free(_ctx); }
/// Update the hash with @a data of @a length bytes.
bool
update(void const *data, int length) override
{
- return EVP_DigestUpdate(ctx, data, length);
+ return EVP_DigestUpdate(_ctx, data, length);
}
/// Finalize and extract the @a hash.
bool
finalize(CryptoHash &hash) override
{
- return EVP_DigestFinal_ex(ctx, hash.u8, nullptr);
+ return EVP_DigestFinal_ex(_ctx, hash.u8, nullptr);
}
};
diff --git a/src/traffic_server/traffic_server.cc b/src/traffic_server/traffic_server.cc
index 84c95aa..efc7dc4 100644
--- a/src/traffic_server/traffic_server.cc
+++ b/src/traffic_server/traffic_server.cc
@@ -768,18 +768,6 @@ CB_After_Cache_Init()
start = ink_atomic_swap(&delay_listen_for_cache, -1);
emit_fully_initialized_message();
-#if TS_ENABLE_FIPS == 0
- // Check for cache BC after the cache is initialized and before listen, if possible.
- if (cacheProcessor.min_stripe_version._major < CACHE_DB_MAJOR_VERSION) {
- // Versions before 23 need the MMH hash.
- if (cacheProcessor.min_stripe_version._major < 23) {
- Debug("cache_bc", "Pre 4.0 stripe (cache version %d.%d) found, forcing MMH hash for cache URLs",
- cacheProcessor.min_stripe_version._major, cacheProcessor.min_stripe_version._minor);
- URLHashContext::Setting = URLHashContext::MMH;
- }
- }
-#endif
-
if (1 == start) {
// The delay_listen_for_cache value was 1, therefore the main function
// delayed the call to start_HttpProxyServer until we got here. We must
diff --git a/src/tscore/CryptoHash.cc b/src/tscore/CryptoHash.cc
index 07bf2b6..35fcbef 100644
--- a/src/tscore/CryptoHash.cc
+++ b/src/tscore/CryptoHash.cc
@@ -45,25 +45,16 @@ CryptoContext::CryptoContext()
case UNSPECIFIED:
#if TS_ENABLE_FIPS == 0
case MD5:
- new (_obj) MD5Context;
- break;
- case MMH:
- new (_obj) MMHContext;
+ _base = new MD5Context;
break;
#else
case SHA256:
- new (_obj) SHA256Context;
+ _base = new SHA256Context;
break;
#endif
default:
ink_release_assert(!"Invalid global URL hash context");
};
-#if TS_ENABLE_FIPS == 0
- static_assert(CryptoContext::OBJ_SIZE >= sizeof(MD5Context), "bad OBJ_SIZE");
- static_assert(CryptoContext::OBJ_SIZE >= sizeof(MMHContext), "bad OBJ_SIZE");
-#else
- static_assert(CryptoContext::OBJ_SIZE >= sizeof(SHA256Context), "bad OBJ_SIZE");
-#endif
}
/**
diff --git a/src/tscore/Makefile.am b/src/tscore/Makefile.am
index c4110a7..e5b6ed6 100644
--- a/src/tscore/Makefile.am
+++ b/src/tscore/Makefile.am
@@ -170,6 +170,7 @@ test_tscore_SOURCES = \
unit_tests/test_ArgParser.cc \
unit_tests/test_BufferWriter.cc \
unit_tests/test_BufferWriterFormat.cc \
+ unit_tests/test_CryptoHash.cc \
unit_tests/test_Extendible.cc \
unit_tests/test_History.cc \
unit_tests/test_ink_inet.cc \
diff --git a/src/tscore/unit_tests/test_CryptoHash.cc b/src/tscore/unit_tests/test_CryptoHash.cc
new file mode 100644
index 0000000..1544cd1
--- /dev/null
+++ b/src/tscore/unit_tests/test_CryptoHash.cc
@@ -0,0 +1,55 @@
+/**
+ @file Test for CryptoHash
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+#include <array>
+#include <string_view>
+
+#include "tscore/ink_assert.h"
+#include "tscore/ink_defs.h"
+#include "tscore/CryptoHash.h"
+#include "catch.hpp"
+
+TEST_CASE("CrypoHash", "[libts][CrypoHash]")
+{
+ CryptoHash hash;
+ ats::CryptoContext ctx;
+ std::string_view test = "asdfsfsdfljhasdfkjasdkfuy239874kasjdf";
+ std::string_view sha256 = "2602CBA2CC0331EB7C455E9F36030B32CE9BB432A90759075F5A702772BE123B";
+ std::string_view md5 = "480AEF8C24AA94B80DC6214ECEC8CD1A";
+
+ // Hash the test data
+ ctx.update(test.data(), test.size());
+ ctx.finalize(hash);
+
+ // Write the output to a string
+ char buffer[(CRYPTO_HASH_SIZE * 2) + 1];
+ hash.toHexStr(buffer);
+
+ // Compair to a known hash value
+ if (CryptoContext::Setting == CryptoContext::SHA256) {
+ REQUIRE(strlen(buffer) == sha256.size());
+ REQUIRE(memcmp(sha256.data(), buffer, sha256.size()) == 0);
+ } else {
+ REQUIRE(strlen(buffer) == md5.size());
+ REQUIRE(memcmp(md5.data(), buffer, md5.size()) == 0);
+ }
+}