From f4d4970f4e5e6419e4531b90a27525e722a4f9d1 Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Wed, 14 Aug 2024 00:05:24 +0000 Subject: [PATCH 1/3] refactor md5 tool with dgst and fix stdin behavior --- .github/workflows/opensslcomparison.yml | 5 -- tests/ci/run_openssl_comparison_tests.sh | 0 tool-openssl/CMakeLists.txt | 3 - tool-openssl/dgst.cc | 101 +++++++++++++++++------ tool-openssl/dgst_test.cc | 73 +++++++++++++++- tool-openssl/internal.h | 5 ++ tool-openssl/md5.cc | 50 ----------- tool-openssl/md5_test.cc | 76 ----------------- tool-openssl/tool.cc | 8 +- 9 files changed, 155 insertions(+), 166 deletions(-) mode change 100644 => 100755 tests/ci/run_openssl_comparison_tests.sh delete mode 100644 tool-openssl/md5.cc delete mode 100644 tool-openssl/md5_test.cc diff --git a/.github/workflows/opensslcomparison.yml b/.github/workflows/opensslcomparison.yml index 6808bf6cc0..31f6ba021c 100644 --- a/.github/workflows/opensslcomparison.yml +++ b/.github/workflows/opensslcomparison.yml @@ -12,16 +12,11 @@ jobs: steps: - name: Checkout repository uses: actions/checkout@v3 - - name: Install OS Dependencies run: | sudo apt-get update -o Acquire::Languages=none -o Acquire::Translation=none sudo apt-get -y --no-install-recommends install \ cmake gcc ninja-build golang make autoconf pkg-config openssl - - - name: Make the script executable - run: chmod +x ./tests/ci/run_openssl_comparison_tests.sh - - name: Build AWS-LC & OpenSSL and Run Comparison Tests run: | ./tests/ci/run_openssl_comparison_tests.sh diff --git a/tests/ci/run_openssl_comparison_tests.sh b/tests/ci/run_openssl_comparison_tests.sh old mode 100644 new mode 100755 diff --git a/tool-openssl/CMakeLists.txt b/tool-openssl/CMakeLists.txt index 8f8335e5dd..9daac52077 100644 --- a/tool-openssl/CMakeLists.txt +++ b/tool-openssl/CMakeLists.txt @@ -6,7 +6,6 @@ add_executable( ../tool/fd.cc dgst.cc - md5.cc rsa.cc tool.cc x509.cc @@ -55,8 +54,6 @@ if(BUILD_TESTING) dgst.cc dgst_test.cc - md5.cc - md5_test.cc rsa.cc rsa_test.cc x509.cc diff --git a/tool-openssl/dgst.cc b/tool-openssl/dgst.cc index fdc9c58d79..30ad81b3c0 100644 --- a/tool-openssl/dgst.cc +++ b/tool-openssl/dgst.cc @@ -16,14 +16,13 @@ static const argument_t kArguments[] = { "Create a hashed MAC with the corresponding key"}, {"", kOptionalArgument, ""}}; -static bool dgst_file_op(const std::string &filename, const EVP_MD *digest, - const char *hmac_key, const size_t hmac_key_len) { - ScopedFD scoped_fd = OpenFD(filename.c_str(), O_RDONLY | O_BINARY); - int fd = scoped_fd.get(); - +static bool dgst_file_op(const std::string &filename, const int fd, + const EVP_MD *digest, const char *hmac_key, + const size_t hmac_key_len) { static const size_t kBufSize = 8192; std::unique_ptr buf(new uint8_t[kBufSize]); + // Run HMAC operations if |hmac_key| exists. if (hmac_key) { bssl::ScopedHMAC_CTX ctx; if (!HMAC_Init_ex(ctx.get(), hmac_key, hmac_key_len, digest, nullptr)) { @@ -58,27 +57,79 @@ static bool dgst_file_op(const std::string &filename, const EVP_MD *digest, return false; } - // Print HMAC output. - fprintf(stdout, "HMAC-%s(%s)= ", EVP_MD_get0_name(digest), - filename.c_str()); + // Print HMAC output. OpenSSL outputs the digest name with files, but not + // with stdin. + if (fd != 0) { + fprintf(stdout, "HMAC-%s(%s)= ", EVP_MD_get0_name(digest), + filename.c_str()); + } else { + fprintf(stdout, "(%s)= ", filename.c_str()); + }; for (size_t i = 0; i < expected_mac_len; i++) { fprintf(stdout, "%02x", mac[i]); } fprintf(stdout, "\n"); return true; } + // Run Hash operations. + else if (digest) { + bssl::ScopedEVP_MD_CTX ctx; + if (!EVP_DigestInit_ex(ctx.get(), digest, nullptr)) { + fprintf(stderr, "Failed to initialize EVP_MD_CTX.\n"); + return false; + } + + for (;;) { + size_t n; + if (!ReadFromFD(fd, &n, buf.get(), kBufSize)) { + fprintf(stderr, "Failed to read from %s: %s\n", filename.c_str(), + strerror(errno)); + return false; + } + + if (n == 0) { + break; + } + + if (!EVP_DigestUpdate(ctx.get(), buf.get(), n)) { + fprintf(stderr, "Failed to update hash.\n"); + return false; + } + } + + uint8_t hash[EVP_MAX_MD_SIZE]; + unsigned hash_len; + if (!EVP_DigestFinal_ex(ctx.get(), hash, &hash_len)) { + fprintf(stderr, "Failed to finish hash.\n"); + return false; + } + + // Print digest output. OpenSSL outputs the digest name with files, but not + // with stdin. + if (fd != 0) { + fprintf(stdout, "%s(%s)= ", EVP_MD_get0_name(digest), filename.c_str()); + } else { + fprintf(stdout, "(%s)= ", filename.c_str()); + }; + for (size_t i = 0; i < hash_len; i++) { + fprintf(stdout, "%02x", hash[i]); + } + fprintf(stdout, "\n"); + return true; + } return false; } -// Map arguments using tool/args.cc -bool dgstTool(const args_list_t &args) { +static bool dgst_tool_op(const args_list_t &args, const EVP_MD *digest) { std::vector file_inputs; // Default is SHA-256. // TODO: Make this customizable when "-digest" is introduced. - const EVP_MD *digest = EVP_sha256(); + if (digest == nullptr) { + digest = EVP_sha256(); + } // HMAC keys can be empty, but C++ strings have no way to differentiate // between null and empty. @@ -114,8 +165,8 @@ bool dgstTool(const args_list_t &args) { hmac_key_len = (*it).length(); } else { fprintf(stderr, - "dgst: Option -hmac needs a value\ndgst: Use -help for " - "summary.\n"); + "dgst: Option -hmac needs a value\n" + "dgst: Use -help for summary.\n"); return false; } } else { @@ -132,22 +183,24 @@ bool dgstTool(const args_list_t &args) { it++; } - if (hmac_key == nullptr) { - fprintf(stderr, "Only HMAC is supported as of now\n"); - return false; - } - if (file_inputs.empty()) { - fprintf(stderr, "stdin is not supported as of now\n"); - return false; - }; + // 0 denotes stdin. + if (!dgst_file_op("stdin", 0, digest, hmac_key, hmac_key_len)) { + return false; + } + } - // Do the dgst/hmac operation on all file inputs. - for (const auto &file_input : file_inputs) { - if (!dgst_file_op(file_input, digest, hmac_key, hmac_key_len)) { + // Do the dgst operation on all file inputs. + for (const auto &file_name : file_inputs) { + ScopedFD scoped_fd = OpenFD(file_name.c_str(), O_RDONLY | O_BINARY); + int fd = scoped_fd.get(); + if (!dgst_file_op(file_name, fd, digest, hmac_key, hmac_key_len)) { return false; } } return true; } + +bool dgstTool(const args_list_t &args) { return dgst_tool_op(args, nullptr); } +bool md5Tool(const args_list_t &args) { return dgst_tool_op(args, EVP_md5()); } diff --git a/tool-openssl/dgst_test.cc b/tool-openssl/dgst_test.cc index db34cd9201..1d144b6ebb 100644 --- a/tool-openssl/dgst_test.cc +++ b/tool-openssl/dgst_test.cc @@ -43,8 +43,17 @@ class DgstComparisonTest : public ::testing::Test { std::string openssl_output_str; }; +// OpenSSL versions 3.1.0 and later change from "(stdin)= " to "MD5(stdin) =" +std::string GetHash(const std::string& str) { + size_t pos = str.find('='); + if (pos == std::string::npos) { + return ""; + } + return str.substr(pos + 1); +} + // Test against OpenSSL output for "-hmac" -TEST_F(DgstComparisonTest, HmacToolCompareOpenSSL) { +TEST_F(DgstComparisonTest, HMAC_default_files) { std::string input_file = std::string(in_path); std::ofstream ofs(input_file); ofs << "AWS_LC_TEST_STRING_INPUT"; @@ -112,3 +121,65 @@ TEST_F(DgstComparisonTest, HmacToolCompareOpenSSL) { RemoveFile(input_file.c_str()); RemoveFile(input_file2.c_str()); } + + +TEST_F(DgstComparisonTest, HMAC_default_stdin) { + std::string tool_command = "echo hmac_this_string | " + + std::string(awslc_executable_path) + " dgst -hmac key > " + + out_path_awslc; + std::string openssl_command = "echo hmac_this_string | " + + std::string(openssl_executable_path) + " dgst -hmac key > " + + out_path_openssl; + + RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_awslc, + out_path_openssl, awslc_output_str, + openssl_output_str); + + std::string tool_hash = GetHash(awslc_output_str); + std::string openssl_hash = GetHash(openssl_output_str); + + ASSERT_EQ(tool_hash, openssl_hash); +} + +TEST_F(DgstComparisonTest, MD5_files) { + std::string input_file = std::string(in_path); + std::ofstream ofs(input_file); + ofs << "AWS_LC_TEST_STRING_INPUT"; + ofs.close(); + + std::string tool_command = std::string(awslc_executable_path) + " md5 < " + + input_file + " > " + out_path_awslc; + std::string openssl_command = std::string(openssl_executable_path) + + " md5 < " + input_file + " > " + + out_path_openssl; + + RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_awslc, + out_path_openssl, awslc_output_str, + openssl_output_str); + + std::string tool_hash = GetHash(awslc_output_str); + std::string openssl_hash = GetHash(openssl_output_str); + + ASSERT_EQ(tool_hash, openssl_hash); + + RemoveFile(input_file.c_str()); +} + +// Test against OpenSSL output with stdin. +TEST_F(DgstComparisonTest, MD5_stdin) { + std::string tool_command = "echo hash_this_string | " + + std::string(awslc_executable_path) + " md5 > " + + out_path_awslc; + std::string openssl_command = "echo hash_this_string | " + + std::string(openssl_executable_path) + + " md5 > " + out_path_openssl; + + RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_awslc, + out_path_openssl, awslc_output_str, + openssl_output_str); + + std::string tool_hash = GetHash(awslc_output_str); + std::string openssl_hash = GetHash(openssl_output_str); + + ASSERT_EQ(tool_hash, openssl_hash); +} diff --git a/tool-openssl/internal.h b/tool-openssl/internal.h index 3624472636..3d4e48c8ce 100644 --- a/tool-openssl/internal.h +++ b/tool-openssl/internal.h @@ -14,6 +14,11 @@ typedef bool (*tool_func_t)(const std::vector &args); +struct Tool { + const char *name; + tool_func_t func; +}; + bool IsNumeric(const std::string& str); X509* CreateAndSignX509Certificate(); diff --git a/tool-openssl/md5.cc b/tool-openssl/md5.cc deleted file mode 100644 index 74333f7144..0000000000 --- a/tool-openssl/md5.cc +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 OR ISC - -#include -#include -#include "internal.h" - -// MD5 command currently only supports stdin -static const argument_t kArguments[] = { - { "-help", kBooleanArgument, "Display option summary" }, - { "", kOptionalArgument, "" } -}; - -// Map arguments using tool/args.cc -bool md5Tool(const args_list_t &args) { - args_map_t parsed_args; - if (!ParseKeyValueArguments(&parsed_args, args, kArguments)) { - PrintUsage(kArguments); - return false; - } - - bool help = false; - GetBoolArgument(&help, "-help", parsed_args); - - if (help) { - PrintUsage(kArguments); - return false; - } - - // Read input from stdin - std::string input; - std::getline(std::cin, input); - - if (input.empty()) { - fprintf(stderr, "Error: no input provided\n"); - return false; - } - - unsigned char md5_digest[MD5_DIGEST_LENGTH]; - MD5(reinterpret_cast(input.c_str()), input.length(), md5_digest); - - printf("(stdin)= "); - for (int i = 0; i < MD5_DIGEST_LENGTH; ++i) { - printf("%02x", md5_digest[i]); - } - printf("\n"); - - return true; -} - diff --git a/tool-openssl/md5_test.cc b/tool-openssl/md5_test.cc deleted file mode 100644 index 96e0ed5956..0000000000 --- a/tool-openssl/md5_test.cc +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 OR ISC - -#include -#include -#include "internal.h" -#include "test_util.h" -#include "../crypto/test/test_util.h" -#include - -// -------------------- MD5 OpenSSL Comparison Test --------------------------- - -// Comparison tests cannot run without set up of environment variables: -// AWSLC_TOOL_PATH and OPENSSL_TOOL_PATH. - -class MD5ComparisonTest : public ::testing::Test { -protected: - void SetUp() override { - - // Skip gtests if env variables not set - tool_executable_path = getenv("AWSLC_TOOL_PATH"); - openssl_executable_path = getenv("OPENSSL_TOOL_PATH"); - if (tool_executable_path == nullptr || openssl_executable_path == nullptr) { - GTEST_SKIP() << "Skipping test: AWSLC_TOOL_PATH and/or OPENSSL_TOOL_PATH environment variables are not set"; - } - ASSERT_GT(createTempFILEpath(in_path), 0u); - ASSERT_GT(createTempFILEpath(out_path_tool), 0u); - ASSERT_GT(createTempFILEpath(out_path_openssl), 0u); - } - void TearDown() override { - if (tool_executable_path != nullptr && openssl_executable_path != nullptr) { - RemoveFile(in_path); - RemoveFile(out_path_tool); - RemoveFile(out_path_openssl); - } - } - char in_path[PATH_MAX]; - char out_path_tool[PATH_MAX]; - char out_path_openssl[PATH_MAX]; - bssl::UniquePtr rsa; - const char* tool_executable_path; - const char* openssl_executable_path; - std::string tool_output_str; - std::string openssl_output_str; -}; - -// OpenSSL versions 3.1.0 and later change from "(stdin)= " to "MD5(stdin) =" -std::string GetHash(const std::string& str) { - size_t pos = str.find('='); - if (pos == std::string::npos) { - return ""; - } - return str.substr(pos + 1); -} - -// Test against OpenSSL output -TEST_F(MD5ComparisonTest, MD5ToolCompareOpenSSL) { - std::string input_file = std::string(in_path); - std::ofstream ofs(input_file); - ofs << "AWS_LC_TEST_STRING_INPUT"; - ofs.close(); - - std::string tool_command = std::string(tool_executable_path) + " md5 < " + input_file + " > " + out_path_tool; - std::string openssl_command = std::string(openssl_executable_path) + " md5 < " + input_file + " > " + out_path_openssl; - - RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str); - - std::string tool_hash = GetHash(tool_output_str); - std::string openssl_hash = GetHash(openssl_output_str); - tool_hash = trim(tool_hash); - openssl_hash = trim(openssl_hash); - - ASSERT_EQ(tool_hash, openssl_hash); - - RemoveFile(input_file.c_str()); -} diff --git a/tool-openssl/tool.cc b/tool-openssl/tool.cc index 4288356635..469f1ea473 100644 --- a/tool-openssl/tool.cc +++ b/tool-openssl/tool.cc @@ -15,13 +15,6 @@ #include "./internal.h" -typedef bool (*tool_func_t)(const std::vector &args); - -struct Tool { - const char *name; - tool_func_t func; -}; - static const std::array kTools = {{ {"dgst", dgstTool}, {"md5", md5Tool}, @@ -97,6 +90,7 @@ int main(int argc, char **argv) { int starting_arg = 1; tool_func_t tool = FindTool(argc, argv, starting_arg); + // Print help option menu. if (tool == nullptr) { usage(argv[0]); return 1; From 2322a830b4b925d24ac2c5c384662e90958ff824 Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Wed, 14 Aug 2024 18:35:47 +0000 Subject: [PATCH 2/3] PR comments; separate dgst and hmac logic --- tool-openssl/dgst.cc | 184 ++++++++++++++++++++------------------ tool-openssl/dgst_test.cc | 23 +++-- 2 files changed, 110 insertions(+), 97 deletions(-) diff --git a/tool-openssl/dgst.cc b/tool-openssl/dgst.cc index 30ad81b3c0..ce4a5c5ba6 100644 --- a/tool-openssl/dgst.cc +++ b/tool-openssl/dgst.cc @@ -17,109 +17,107 @@ static const argument_t kArguments[] = { {"", kOptionalArgument, ""}}; static bool dgst_file_op(const std::string &filename, const int fd, - const EVP_MD *digest, const char *hmac_key, - const size_t hmac_key_len) { + const EVP_MD *digest) { static const size_t kBufSize = 8192; std::unique_ptr buf(new uint8_t[kBufSize]); - // Run HMAC operations if |hmac_key| exists. - if (hmac_key) { - bssl::ScopedHMAC_CTX ctx; - if (!HMAC_Init_ex(ctx.get(), hmac_key, hmac_key_len, digest, nullptr)) { - fprintf(stderr, "Failed to initialize HMAC_Init_ex.\n"); + bssl::ScopedEVP_MD_CTX ctx; + if (!EVP_DigestInit_ex(ctx.get(), digest, nullptr)) { + fprintf(stderr, "Failed to initialize EVP_MD_CTX.\n"); + return false; + } + + for (;;) { + size_t n; + if (!ReadFromFD(fd, &n, buf.get(), kBufSize)) { + fprintf(stderr, "Failed to read from %s: %s\n", filename.c_str(), + strerror(errno)); return false; } - // Update |buf| from file continuously. - for (;;) { - size_t n; - if (!ReadFromFD(fd, &n, buf.get(), kBufSize)) { - fprintf(stderr, "Failed to read from %s: %s\n", filename.c_str(), - strerror(errno)); - return false; - } - - if (n == 0) { - break; - } - - if (!HMAC_Update(ctx.get(), buf.get(), n)) { - fprintf(stderr, "Failed to update HMAC.\n"); - return false; - } + if (n == 0) { + break; } - const unsigned expected_mac_len = EVP_MD_size(digest); - std::unique_ptr mac(new uint8_t[expected_mac_len]); - unsigned mac_len; - if (!HMAC_Final(ctx.get(), mac.get(), &mac_len)) { - fprintf(stderr, "Failed to finalize HMAC.\n"); + if (!EVP_DigestUpdate(ctx.get(), buf.get(), n)) { + fprintf(stderr, "Failed to update hash.\n"); return false; } + } - // Print HMAC output. OpenSSL outputs the digest name with files, but not - // with stdin. - if (fd != 0) { - fprintf(stdout, "HMAC-%s(%s)= ", EVP_MD_get0_name(digest), - filename.c_str()); - } else { - fprintf(stdout, "(%s)= ", filename.c_str()); - }; - for (size_t i = 0; i < expected_mac_len; i++) { - fprintf(stdout, "%02x", mac[i]); - } - fprintf(stdout, "\n"); - return true; + uint8_t hash[EVP_MAX_MD_SIZE]; + unsigned hash_len; + if (!EVP_DigestFinal_ex(ctx.get(), hash, &hash_len)) { + fprintf(stderr, "Failed to finish hash.\n"); + return false; } - // Run Hash operations. - else if (digest) { - bssl::ScopedEVP_MD_CTX ctx; - if (!EVP_DigestInit_ex(ctx.get(), digest, nullptr)) { - fprintf(stderr, "Failed to initialize EVP_MD_CTX.\n"); - return false; - } - for (;;) { - size_t n; - if (!ReadFromFD(fd, &n, buf.get(), kBufSize)) { - fprintf(stderr, "Failed to read from %s: %s\n", filename.c_str(), - strerror(errno)); - return false; - } + // Print digest output. OpenSSL outputs the digest name with files, but not + // with stdin. + if (fd != 0) { + fprintf(stdout, "%s(%s)= ", EVP_MD_get0_name(digest), filename.c_str()); + } else { + fprintf(stdout, "(%s)= ", filename.c_str()); + }; + for (size_t i = 0; i < hash_len; i++) { + fprintf(stdout, "%02x", hash[i]); + } + fprintf(stdout, "\n"); + return true; +} - if (n == 0) { - break; - } +static bool hmac_file_op(const std::string &filename, const int fd, + const EVP_MD *digest, const char *hmac_key, + const size_t hmac_key_len) { + static const size_t kBufSize = 8192; + std::unique_ptr buf(new uint8_t[kBufSize]); - if (!EVP_DigestUpdate(ctx.get(), buf.get(), n)) { - fprintf(stderr, "Failed to update hash.\n"); - return false; - } - } + bssl::ScopedHMAC_CTX ctx; + if (!HMAC_Init_ex(ctx.get(), hmac_key, hmac_key_len, digest, nullptr)) { + fprintf(stderr, "Failed to initialize HMAC_Init_ex.\n"); + return false; + } - uint8_t hash[EVP_MAX_MD_SIZE]; - unsigned hash_len; - if (!EVP_DigestFinal_ex(ctx.get(), hash, &hash_len)) { - fprintf(stderr, "Failed to finish hash.\n"); + // Update |buf| from file continuously. + for (;;) { + size_t n; + if (!ReadFromFD(fd, &n, buf.get(), kBufSize)) { + fprintf(stderr, "Failed to read from %s: %s\n", filename.c_str(), + strerror(errno)); return false; } - // Print digest output. OpenSSL outputs the digest name with files, but not - // with stdin. - if (fd != 0) { - fprintf(stdout, "%s(%s)= ", EVP_MD_get0_name(digest), filename.c_str()); - } else { - fprintf(stdout, "(%s)= ", filename.c_str()); - }; - for (size_t i = 0; i < hash_len; i++) { - fprintf(stdout, "%02x", hash[i]); + if (n == 0) { + break; + } + + if (!HMAC_Update(ctx.get(), buf.get(), n)) { + fprintf(stderr, "Failed to update HMAC.\n"); + return false; } - fprintf(stdout, "\n"); - return true; } + const unsigned expected_mac_len = EVP_MD_size(digest); + std::unique_ptr mac(new uint8_t[expected_mac_len]); + unsigned mac_len; + if (!HMAC_Final(ctx.get(), mac.get(), &mac_len)) { + fprintf(stderr, "Failed to finalize HMAC.\n"); + return false; + } - return false; + // Print HMAC output. OpenSSL outputs the digest name with files, but not + // with stdin. + if (fd != 0) { + fprintf(stdout, "HMAC-%s(%s)= ", EVP_MD_get0_name(digest), + filename.c_str()); + } else { + fprintf(stdout, "(%s)= ", filename.c_str()); + }; + for (size_t i = 0; i < expected_mac_len; i++) { + fprintf(stdout, "%02x", mac[i]); + } + fprintf(stdout, "\n"); + return true; } static bool dgst_tool_op(const args_list_t &args, const EVP_MD *digest) { @@ -131,7 +129,7 @@ static bool dgst_tool_op(const args_list_t &args, const EVP_MD *digest) { digest = EVP_sha256(); } - // HMAC keys can be empty, but C++ strings have no way to differentiate + // HMAC keys can be empty, but C++ std::string has no way to differentiate // between null and empty. const char *hmac_key = nullptr; size_t hmac_key_len = 0; @@ -183,19 +181,35 @@ static bool dgst_tool_op(const args_list_t &args, const EVP_MD *digest) { it++; } + // Use stdin if no files are provided. if (file_inputs.empty()) { // 0 denotes stdin. - if (!dgst_file_op("stdin", 0, digest, hmac_key, hmac_key_len)) { - return false; + std::string file_name = "stdin"; + int fd = 0; + if (hmac_key) { + if (!hmac_file_op(file_name, fd, digest, hmac_key, hmac_key_len)) { + return false; + } + } else { + if (!dgst_file_op(file_name, fd, digest)) { + return false; + } } + return true; } // Do the dgst operation on all file inputs. for (const auto &file_name : file_inputs) { ScopedFD scoped_fd = OpenFD(file_name.c_str(), O_RDONLY | O_BINARY); int fd = scoped_fd.get(); - if (!dgst_file_op(file_name, fd, digest, hmac_key, hmac_key_len)) { - return false; + if (hmac_key) { + if (!hmac_file_op(file_name, fd, digest, hmac_key, hmac_key_len)) { + return false; + } + } else { + if (!dgst_file_op(file_name, fd, digest)) { + return false; + } } } diff --git a/tool-openssl/dgst_test.cc b/tool-openssl/dgst_test.cc index 1d144b6ebb..b4caee6058 100644 --- a/tool-openssl/dgst_test.cc +++ b/tool-openssl/dgst_test.cc @@ -44,7 +44,7 @@ class DgstComparisonTest : public ::testing::Test { }; // OpenSSL versions 3.1.0 and later change from "(stdin)= " to "MD5(stdin) =" -std::string GetHash(const std::string& str) { +std::string GetHash(const std::string &str) { size_t pos = str.find('='); if (pos == std::string::npos) { return ""; @@ -74,7 +74,7 @@ TEST_F(DgstComparisonTest, HMAC_default_files) { std::string awslc_hash = GetHash(awslc_output_str); std::string openssl_hash = GetHash(openssl_output_str); - ASSERT_EQ(awslc_hash, openssl_hash); + EXPECT_EQ(awslc_hash, openssl_hash); // Run -hmac again against multiple files. char in_path2[PATH_MAX]; @@ -98,7 +98,7 @@ TEST_F(DgstComparisonTest, HMAC_default_files) { awslc_hash = GetHash(awslc_output_str); openssl_hash = GetHash(openssl_output_str); - ASSERT_EQ(awslc_hash, openssl_hash); + EXPECT_EQ(awslc_hash, openssl_hash); // Run -hmac with empty key awslc_command = std::string(awslc_executable_path) + @@ -115,8 +115,7 @@ TEST_F(DgstComparisonTest, HMAC_default_files) { awslc_hash = GetHash(awslc_output_str); openssl_hash = GetHash(openssl_output_str); - ASSERT_EQ(awslc_hash, openssl_hash); - + EXPECT_EQ(awslc_hash, openssl_hash); RemoveFile(input_file.c_str()); RemoveFile(input_file2.c_str()); @@ -125,11 +124,11 @@ TEST_F(DgstComparisonTest, HMAC_default_files) { TEST_F(DgstComparisonTest, HMAC_default_stdin) { std::string tool_command = "echo hmac_this_string | " + - std::string(awslc_executable_path) + " dgst -hmac key > " + - out_path_awslc; + std::string(awslc_executable_path) + + " dgst -hmac key > " + out_path_awslc; std::string openssl_command = "echo hmac_this_string | " + - std::string(openssl_executable_path) + " dgst -hmac key > " + - out_path_openssl; + std::string(openssl_executable_path) + + " dgst -hmac key > " + out_path_openssl; RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_awslc, out_path_openssl, awslc_output_str, @@ -138,7 +137,7 @@ TEST_F(DgstComparisonTest, HMAC_default_stdin) { std::string tool_hash = GetHash(awslc_output_str); std::string openssl_hash = GetHash(openssl_output_str); - ASSERT_EQ(tool_hash, openssl_hash); + EXPECT_EQ(tool_hash, openssl_hash); } TEST_F(DgstComparisonTest, MD5_files) { @@ -160,7 +159,7 @@ TEST_F(DgstComparisonTest, MD5_files) { std::string tool_hash = GetHash(awslc_output_str); std::string openssl_hash = GetHash(openssl_output_str); - ASSERT_EQ(tool_hash, openssl_hash); + EXPECT_EQ(tool_hash, openssl_hash); RemoveFile(input_file.c_str()); } @@ -181,5 +180,5 @@ TEST_F(DgstComparisonTest, MD5_stdin) { std::string tool_hash = GetHash(awslc_output_str); std::string openssl_hash = GetHash(openssl_output_str); - ASSERT_EQ(tool_hash, openssl_hash); + EXPECT_EQ(tool_hash, openssl_hash); } From 93ad99fb0ea27031f115c7c17ab7fd2115124512 Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Wed, 14 Aug 2024 20:51:00 +0000 Subject: [PATCH 3/3] fix md5 file test to actually use files --- tool-openssl/dgst_test.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tool-openssl/dgst_test.cc b/tool-openssl/dgst_test.cc index b4caee6058..0ef6743fa5 100644 --- a/tool-openssl/dgst_test.cc +++ b/tool-openssl/dgst_test.cc @@ -146,6 +146,7 @@ TEST_F(DgstComparisonTest, MD5_files) { ofs << "AWS_LC_TEST_STRING_INPUT"; ofs.close(); + // Input file as pipe (stdin) std::string tool_command = std::string(awslc_executable_path) + " md5 < " + input_file + " > " + out_path_awslc; std::string openssl_command = std::string(openssl_executable_path) + @@ -161,6 +162,21 @@ TEST_F(DgstComparisonTest, MD5_files) { EXPECT_EQ(tool_hash, openssl_hash); + // Input file as regular command line option. + tool_command = std::string(awslc_executable_path) + " md5 " + input_file + + " > " + out_path_awslc; + openssl_command = std::string(openssl_executable_path) + " md5 " + + input_file + " > " + out_path_openssl; + + RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_awslc, + out_path_openssl, awslc_output_str, + openssl_output_str); + + tool_hash = GetHash(awslc_output_str); + openssl_hash = GetHash(openssl_output_str); + + EXPECT_EQ(tool_hash, openssl_hash); + RemoveFile(input_file.c_str()); }