Skip to content

Commit

Permalink
Merge pull request #5 from pkthapa/hashing-rev-comment
Browse files Browse the repository at this point in the history
Fixed: OpenSSL hashing review comments.
  • Loading branch information
pkthapa authored May 26, 2022
2 parents 16c6c50 + e37bb31 commit 2f9f332
Show file tree
Hide file tree
Showing 19 changed files with 21 additions and 331 deletions.
3 changes: 0 additions & 3 deletions .github/workflows/asan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ jobs:
script -q -e -c "make pull"
sudo df -h
script -q -e -c "CONCORD_BFT_CMAKE_OMIT_TEST_OUTPUT=TRUE CONCORD_BFT_CMAKE_KEEP_APOLLO_LOGS=FALSE CONCORD_BFT_CMAKE_ASAN=TRUE CONCORD_BFT_CMAKE_USE_FAKE_CLOCK_IN_TIME_SERVICE=TRUE \
CONCORD_BFT_CMAKE_USE_CRYPTOPP_HASH=TRUE \
CONCORD_BFT_CMAKE_USE_OPENSSL_SHA_256=FALSE \
CONCORD_BFT_CMAKE_USE_OPENSSL_SHA3_256=FALSE \
make build" \
&& script -q -e -c "make test"
- name: Check if ASAN passed
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/build_and_test_clang_debug.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ jobs:
-DUSE_OPENTRACING=ON \
-DOMIT_TEST_OUTPUT=OFF\
-DKEEP_APOLLO_LOGS=TRUE\
-DUSE_CRYPTOPP_HASH=TRUE\
-DUSE_OPENSSL_SHA_256=FALSE\
-DUSE_OPENSSL_SHA3_256=FALSE\
-DUSE_FAKE_CLOCK_IN_TIME_SERVICE=TRUE\" "\
&& script -q -e -c "make test"
- name: Prepare artifacts
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/build_and_test_clang_release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ jobs:
-DUSE_OPENTRACING=ON \
-DOMIT_TEST_OUTPUT=OFF\
-DKEEP_APOLLO_LOGS=TRUE\
-DUSE_CRYPTOPP_HASH=TRUE\
-DUSE_OPENSSL_SHA_256=FALSE\
-DUSE_OPENSSL_SHA3_256=FALSE\
-DUSE_FAKE_CLOCK_IN_TIME_SERVICE=TRUE\" "\
&& script -q -e -c "make test"
- name: Prepare artifacts
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/build_and_test_gcc_debug.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ jobs:
-DOMIT_TEST_OUTPUT=OFF\
-DKEEP_APOLLO_LOGS=TRUE\
-DRUN_APOLLO_TESTS=FALSE\
-DUSE_CRYPTOPP_HASH=TRUE\
-DUSE_OPENSSL_SHA_256=FALSE\
-DUSE_OPENSSL_SHA3_256=FALSE\
-DUSE_FAKE_CLOCK_IN_TIME_SERVICE=TRUE\" "\
&& script -q -e -c "make test"
- name: Prepare artifacts
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/build_and_test_gcc_release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ jobs:
-DOMIT_TEST_OUTPUT=OFF\
-DKEEP_APOLLO_LOGS=TRUE\
-DRUN_APOLLO_TESTS=FALSE\
-DUSE_CRYPTOPP_HASH=TRUE\
-DUSE_OPENSSL_SHA_256=FALSE\
-DUSE_OPENSSL_SHA3_256=FALSE\
-DUSE_FAKE_CLOCK_IN_TIME_SERVICE=TRUE\" "\
&& script -q -e -c "make test"
- name: Prepare artifacts
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/clang-tidy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ jobs:
-DUSE_S3_OBJECT_STORE=TRUE \
-DUSE_OPENTRACING=ON \
-DOMIT_TEST_OUTPUT=OFF\
-DUSE_CRYPTOPP_HASH=TRUE\
-DUSE_OPENSSL_SHA_256=FALSE\
-DUSE_OPENSSL_SHA3_256=FALSE\
-DUSE_FAKE_CLOCK_IN_TIME_SERVICE=TRUE\" "\
- name: Print failure info
if: failure()
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/codecoverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ jobs:
sudo df -h
script -q -e -c "CONCORD_BFT_CONTAINER_CC=clang CONCORD_BFT_CONTAINER_CXX=clang++ \
CONCORD_BFT_CMAKE_CODECOVERAGE=TRUE CONCORD_BFT_CMAKE_TRANSPORT=UDP \
CONCORD_BFT_CMAKE_USE_CRYPTOPP_HASH=TRUE \
CONCORD_BFT_CMAKE_USE_OPENSSL_SHA_256=FALSE \
CONCORD_BFT_CMAKE_USE_OPENSSL_SHA3_256=FALSE \
make build" && script -q -e -c "make test"
continue-on-error: true

Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/tsan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ jobs:
script -q -e -c "make pull"
sudo df -h
script -q -e -c "CONCORD_BFT_CMAKE_OMIT_TEST_OUTPUT=TRUE CONCORD_BFT_CMAKE_KEEP_APOLLO_LOGS=FALSE CONCORD_BFT_CMAKE_TSAN=TRUE CONCORD_BFT_CMAKE_USE_FAKE_CLOCK_IN_TIME_SERVICE=TRUE \
CONCORD_BFT_CMAKE_USE_CRYPTOPP_HASH=TRUE \
CONCORD_BFT_CMAKE_USE_OPENSSL_SHA_256=FALSE \
CONCORD_BFT_CMAKE_USE_OPENSSL_SHA3_256=FALSE \
make build" \
&& script -q -e -c "make test"
- name: Check if TSAN passed
Expand Down
13 changes: 0 additions & 13 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -114,19 +114,6 @@ if(CODECOVERAGE)
message( "-- Building with llvm Code Coverage Tools")
endif()

if(USE_CRYPTOPP_HASH)
message("-- USE_CRYPTOPP_HASH Enabled")
string(APPEND CMAKE_CXX_FLAGS " -DUSE_CRYPTOPP_HASH")
elseif(USE_OPENSSL_SHA_256)
message("-- USE_OPENSSL_SHA_256 Enabled")
string(APPEND CMAKE_CXX_FLAGS " -DUSE_OPENSSL_SHA_256")
elseif(USE_OPENSSL_SHA3_256)
message("-- USE_OPENSSL_SHA3_256 Enabled")
string(APPEND CMAKE_CXX_FLAGS " -DUSE_OPENSSL_SHA3_256")
else()
message(FATAL_ERROR "None of the cryptographic hashing libraries are enabled.")
endif()

if(USE_S3_OBJECT_STORE)
add_compile_definitions(USE_S3_OBJECT_STORE=1)
endif()
Expand Down
8 changes: 1 addition & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ CONCORD_BFT_CMAKE_TSAN?=FALSE
CONCORD_BFT_CMAKE_CODECOVERAGE?=FALSE
CONCORD_BFT_CMAKE_USE_FAKE_CLOCK_IN_TIME_SERVICE?=FALSE
ENABLE_RESTART_RECOVERY_TESTS?=FALSE
CONCORD_BFT_CMAKE_USE_CRYPTOPP_HASH?=TRUE
CONCORD_BFT_CMAKE_USE_OPENSSL_SHA_256?=FALSE
CONCORD_BFT_CMAKE_USE_OPENSSL_SHA3_256?=FALSE

ifeq (${CONCORD_BFT_CMAKE_ASAN},TRUE)
CONCORD_BFT_CMAKE_CXX_FLAGS_RELEASE='-O0 -g'
Expand Down Expand Up @@ -94,10 +91,7 @@ CONCORD_BFT_CMAKE_FLAGS?= \
-DTHREADCHECK=${CONCORD_BFT_CMAKE_TSAN} \
-DCODECOVERAGE=${CONCORD_BFT_CMAKE_CODECOVERAGE} \
-DTXN_SIGNING_ENABLED=${CONCORD_BFT_CMAKE_TRANSACTION_SIGNING_ENABLED} \
-DENABLE_RESTART_RECOVERY_TESTS=${ENABLE_RESTART_RECOVERY_TESTS} \
-DUSE_OPENSSL_SHA_256=${CONCORD_BFT_CMAKE_USE_OPENSSL_SHA_256} \
-DUSE_OPENSSL_SHA3_256=${CONCORD_BFT_CMAKE_USE_OPENSSL_SHA3_256} \
-DUSE_CRYPTOPP_HASH=${CONCORD_BFT_CMAKE_USE_CRYPTOPP_HASH}
-DENABLE_RESTART_RECOVERY_TESTS=${ENABLE_RESTART_RECOVERY_TESTS}

# The consistency parameter makes sense only at MacOS.
# It is ignored at all other platforms.
Expand Down
6 changes: 6 additions & 0 deletions util/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ set(util_source_files
add_library(util STATIC ${util_source_files})
add_library(util_shared SHARED ${util_source_files})

# Use below macros to use CryptoPP's SHA_256 or OpenSSL's SHA_256 hashing.
# USE_CRYPTOPP_SHA_256=FALSE
# USE_OPENSSL_SHA_256=FALSE
target_compile_definitions(util PUBLIC USE_OPENSSL_SHA3_256)
target_compile_definitions(util_shared PUBLIC USE_OPENSSL_SHA3_256)

if(USE_OPENSSL)
if(NOT BUILD_THIRDPARTY)
find_package(OpenSSL REQUIRED)
Expand Down
144 changes: 0 additions & 144 deletions util/include/DigestImpl.ipp

This file was deleted.

16 changes: 6 additions & 10 deletions util/include/cryptopp_digest_creator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#pragma once

#include "digest_creator.hpp"
#include "digest_type.hpp"

#if defined MD5_DIGEST
Expand All @@ -25,8 +24,8 @@

namespace concord::util::digest {

// Implements digest creator using Crypto++ library.
class CryptoppDigestCreator : public DigestCreator {
// A class that generates SHA digest using CryptoPP library.
class CryptoppDigestCreator {
public:
CryptoppDigestCreator();
virtual ~CryptoppDigestCreator();
Expand All @@ -35,13 +34,10 @@ class CryptoppDigestCreator : public DigestCreator {
CryptoppDigestCreator(const CryptoppDigestCreator&) = delete;
CryptoppDigestCreator& operator=(const CryptoppDigestCreator&) = delete;

void update(const char* data, size_t len) override;
void writeDigest(char* outDigest) override;
size_t digestLength() const override;
bool compute(const char* input,
size_t inputLength,
char* outBufferForDigest,
size_t lengthOfBufferForDigest) override;
void update(const char* data, size_t len);
void writeDigest(char* outDigest);
size_t digestLength() const;
bool compute(const char* input, size_t inputLength, char* outBufferForDigest, size_t lengthOfBufferForDigest);

private:
void* internalState_;
Expand Down
4 changes: 2 additions & 2 deletions util/include/digest.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
#include "digest_type.hpp"
#include "digest_holder.hpp"
#include "cryptopp_digest_creator.hpp"
#include "openssl_digest_creator.ipp"
#include "openssl_digest_creator.hpp"

namespace concord::util::digest {

using BlockDigest = std::array<std::uint8_t, DIGEST_SIZE>;

#if defined USE_CRYPTOPP_HASH
#if defined USE_CRYPTOPP_SHA_256
using Digest = DigestHolder<CryptoppDigestCreator>;
using DigestGenerator = CryptoppDigestCreator;
#elif defined USE_OPENSSL_SHA_256
Expand Down
32 changes: 0 additions & 32 deletions util/include/digest_creator.hpp

This file was deleted.

6 changes: 3 additions & 3 deletions util/include/digest_holder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
#include <cstring>

#include "digest_type.hpp"
#include "digest_creator.hpp"
#include "hex_tools.h"

namespace concord::util::digest {

// It is responsible for holding the generated digest. The digest is generated by DigestCreator class.
template <typename CREATOR, typename = std::enable_if_t<std::is_base_of_v<DigestCreator, CREATOR>>>
// It is responsible for holding the generated digest. The digest is generated by digest generator classes such as
// OpenSSLDigestCreator and CryptoPPDigestCreator classes.
template <typename CREATOR>
class DigestHolder {
public:
DigestHolder() { std::memset(d, 0, DIGEST_SIZE); }
Expand Down
Loading

0 comments on commit 2f9f332

Please sign in to comment.