Skip to content

Clean up CLI code #2927

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

Open
nhatnghiho wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Clean up CLI code #2927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
54fc549
Reformat with clang-format
nhatnghiho Jan 2, 2026
6483335
Remove redundant wording in unittest names
nhatnghiho Dec 24, 2025
81f1f1c
Remove redundant test setups
nhatnghiho Jan 2, 2026
8c4bbd3
Check return value of BIO functions
nhatnghiho Jan 2, 2026
ebb3455
Address remaining comments from #2898
nhatnghiho Jan 2, 2026
487d2b6
Address comments for enc cli from #2877
nhatnghiho Jan 2, 2026
b1f9a77
(cherry-pick) clean up pass_util namespace and _WIN32 macro
samuel40791765 Jan 2, 2026
80e4bf3
Fix windows and valgrind failures
nhatnghiho Jan 5, 2026
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
20 changes: 12 additions & 8 deletions tool-openssl/asn1parse.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ bool asn1parseTool(const args_list_t &args) {
using namespace ordered_args;
ordered_args_map_t parsed_args;
args_list_t extra_args;
if (!ParseOrderedKeyValueArguments(parsed_args, extra_args, args, kArguments)) {
if (!ParseOrderedKeyValueArguments(parsed_args, extra_args, args,
kArguments)) {
PrintUsage(kArguments);
return false;
}
Expand All @@ -31,7 +32,7 @@ bool asn1parseTool(const args_list_t &args) {
uint8_t *raw_input_bytes = nullptr;
ossl_uint8_ptr input_bytes(nullptr, &OPENSSL_free);
size_t input_bytes_len = 0;

bool help = false;

GetBoolArgument(&help, "-help", parsed_args);
Expand All @@ -49,7 +50,8 @@ bool asn1parseTool(const args_list_t &args) {
} else if (isStringUpperCaseEqual(inform_str, "PEM")) {
input_format = FORMAT_PEM;
} else {
fprintf(stderr, "Error: Invalid input format '%s'. Must be PEM or DER\n", inform_str.c_str());
fprintf(stderr, "Error: Invalid input format '%s'. Must be PEM or DER\n",
inform_str.c_str());
goto err;
}

Expand Down Expand Up @@ -80,8 +82,8 @@ bool asn1parseTool(const args_list_t &args) {

input_bytes_len = 0;
int i = 0;
// We could use BIO_read_asn1 here, but it does have some limitations. So match the OpenSSL behavior
// here instead.
// We could use BIO_read_asn1 here, but it does have some limitations. So
// match the OpenSSL behavior here instead.
for (;;) {
if (!BUF_MEM_grow(buf.get(), input_bytes_len + BUFSIZ)) {
goto err;
Expand All @@ -104,11 +106,13 @@ bool asn1parseTool(const args_list_t &args) {
ossl_char_ptr name(nullptr, &OPENSSL_free);
ossl_char_ptr header(nullptr, &OPENSSL_free);

// Technically this is the `-strictpem` behavior from OpenSSL in combination with `-inform PEM`. Otherwise OpenSSL
// would try to base64 decode outside of PEM blocks. This seems like a niche edge case so adopting the strict
// Technically this is the `-strictpem` behavior from OpenSSL in combination
// with `-inform PEM`. Otherwise OpenSSL would try to base64 decode outside
// of PEM blocks. This seems like a niche edge case so adopting the strict
// behavior for now.
long input_len = 0;
if (!PEM_read_bio(input_bio.get(), &raw_name, &raw_header, &raw_input_bytes, &input_len)) {
if (!PEM_read_bio(input_bio.get(), &raw_name, &raw_header, &raw_input_bytes,
&input_len)) {
fprintf(stderr, "Error reading PEM file\n");
goto err;
}
Expand Down
228 changes: 131 additions & 97 deletions tool-openssl/crl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR ISC

#include "openssl/x509.h"
#include <gtest/gtest.h>
#include <openssl/pem.h>
#include <cctype>
#include "../crypto/test/test_util.h"
#include "internal.h"
#include "openssl/x509.h"
#include "test_util.h"
#include "../crypto/test/test_util.h"
#include <cctype>

X509_CRL* createTestCRL() {
X509_CRL *createTestCRL() {
bssl::UniquePtr<X509_CRL> crl(X509_CRL_new());
if (!crl) {
ERR_print_errors_fp(stderr);
Expand All @@ -19,28 +19,31 @@ X509_CRL* createTestCRL() {
// Set issuer name
bssl::UniquePtr<X509_NAME> issuer(X509_NAME_new());
if (!issuer ||
!X509_NAME_add_entry_by_txt(issuer.get(), "CN", MBSTRING_ASC, (unsigned char *)"Test CA", -1, -1, 0) ||
!X509_CRL_set_issuer_name(crl.get(), issuer.get())) {
!X509_NAME_add_entry_by_txt(issuer.get(), "CN", MBSTRING_ASC,
(unsigned char *)"Test CA", -1, -1, 0) ||
!X509_CRL_set_issuer_name(crl.get(), issuer.get())) {
return nullptr;
}

// Set times
bssl::UniquePtr<ASN1_TIME> lastUpdate(ASN1_TIME_new());
bssl::UniquePtr<ASN1_TIME> nextUpdate(ASN1_TIME_new());
if (!lastUpdate || !nextUpdate || !X509_gmtime_adj(lastUpdate.get(), 0) ||
!X509_gmtime_adj(nextUpdate.get(), 86400L) || // 24 hours from now
!X509_CRL_set1_lastUpdate(crl.get(), lastUpdate.get()) ||
!X509_CRL_set1_nextUpdate(crl.get(), nextUpdate.get())) {
!X509_gmtime_adj(nextUpdate.get(), 86400L) || // 24 hours from now
!X509_CRL_set1_lastUpdate(crl.get(), lastUpdate.get()) ||
!X509_CRL_set1_nextUpdate(crl.get(), nextUpdate.get())) {
return nullptr;
}

// Add a revoked certificate
X509_REVOKED *revoked = X509_REVOKED_new();
bssl::UniquePtr<ASN1_INTEGER> serialNumber(ASN1_INTEGER_new());
if (!revoked || !serialNumber || !ASN1_INTEGER_set(serialNumber.get(), 1) || // Serial number of revoked cert
!X509_REVOKED_set_serialNumber(revoked, serialNumber.get()) ||
!X509_REVOKED_set_revocationDate(revoked, lastUpdate.get()) ||
!X509_CRL_add0_revoked(crl.get(), revoked)) {
if (!revoked || !serialNumber ||
!ASN1_INTEGER_set(serialNumber.get(),
1) || // Serial number of revoked cert
!X509_REVOKED_set_serialNumber(revoked, serialNumber.get()) ||
!X509_REVOKED_set_revocationDate(revoked, lastUpdate.get()) ||
!X509_CRL_add0_revoked(crl.get(), revoked)) {
return nullptr;
}

Expand All @@ -59,54 +62,52 @@ X509_CRL* createTestCRL() {
}

class CRLTest : public ::testing::Test {
protected:
void SetUp() override {
ASSERT_GT(createTempFILEpath(in_path), 0u);
protected:
void SetUp() override {
ASSERT_GT(createTempFILEpath(in_path), 0u);

// Create a test CRL
crl.reset(createTestCRL());
ASSERT_TRUE(crl);
// Create a test CRL
crl.reset(createTestCRL());
ASSERT_TRUE(crl);


ScopedFILE in_file(fopen(in_path, "wb"));
ASSERT_TRUE(in_file);
PEM_write_X509_CRL(in_file.get(), crl.get());
}
ScopedFILE in_file(fopen(in_path, "wb"));
ASSERT_TRUE(in_file);
PEM_write_X509_CRL(in_file.get(), crl.get());
}

void TearDown() override {
RemoveFile(in_path);
}
void TearDown() override { RemoveFile(in_path); }

char in_path[PATH_MAX];
bssl::UniquePtr<X509_CRL> crl;
char in_path[PATH_MAX];
bssl::UniquePtr<X509_CRL> crl;
};


// ----------------------------- CRL Option Tests -----------------------------

// Test -in
TEST_F(CRLTest, CRLTestIn) {
TEST_F(CRLTest, In) {
args_list_t args = {"-in", in_path};
bool result = CRLTool(args);
ASSERT_TRUE(result);
}

// Test -hash
TEST_F(CRLTest, CRLTestHash) {
TEST_F(CRLTest, Hash) {
args_list_t args = {"-in", in_path, "-hash"};
bool result = CRLTool(args);
ASSERT_TRUE(result);
}

// Test -fingerprint
TEST_F(CRLTest, CRLTestFingerprint) {
TEST_F(CRLTest, Fingerprint) {
args_list_t args = {"-in", in_path, "-fingerprint"};
bool result = CRLTool(args);
ASSERT_TRUE(result);
}

// Test -noout
TEST_F(CRLTest, CRLTestNoout) {
TEST_F(CRLTest, Noout) {
args_list_t args = {"-in", in_path, "-noout"};
bool result = CRLTool(args);
ASSERT_TRUE(result);
Expand All @@ -118,91 +119,124 @@ TEST_F(CRLTest, CRLTestNoout) {
// AWSLC_TOOL_PATH and OPENSSL_TOOL_PATH.

class CRLComparisonTest : 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);

// Create a test CRL
crl.reset(createTestCRL());
ASSERT_TRUE(crl);

ScopedFILE in_file(fopen(in_path, "wb"));
ASSERT_TRUE(in_file);
PEM_write_X509_CRL(in_file.get(), crl.get());
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";
}

void TearDown() override {
if (tool_executable_path != nullptr && openssl_executable_path != nullptr) {
RemoveFile(in_path);
RemoveFile(out_path_tool);
RemoveFile(out_path_openssl);
}
ASSERT_GT(createTempFILEpath(in_path), 0u);
ASSERT_GT(createTempFILEpath(out_path_tool), 0u);
ASSERT_GT(createTempFILEpath(out_path_openssl), 0u);

// Create a test CRL
crl.reset(createTestCRL());
ASSERT_TRUE(crl);

ScopedFILE in_file(fopen(in_path, "wb"));
ASSERT_TRUE(in_file);
PEM_write_X509_CRL(in_file.get(), crl.get());
}

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];
const char* tool_executable_path;
const char* openssl_executable_path;
std::string tool_output_str;
std::string openssl_output_str;
bssl::UniquePtr<X509_CRL> crl;
char in_path[PATH_MAX];
char out_path_tool[PATH_MAX];
char out_path_openssl[PATH_MAX];
const char *tool_executable_path;
const char *openssl_executable_path;
std::string tool_output_str;
std::string openssl_output_str;
bssl::UniquePtr<X509_CRL> crl;
};

// Test against OpenSSL output "openssl crl -in file"
TEST_F(CRLComparisonTest, CRLToolCompareOpenSSL) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " + in_path + " > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) + " crl -in " + in_path + " > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str);
TEST_F(CRLComparisonTest, Basic) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " +
in_path + " > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) +
" crl -in " + in_path + " > " +
out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool,
out_path_openssl, tool_output_str,
openssl_output_str);
}

// Test against OpenSSL output "openssl crl -in file -hash -fingerprint"
TEST_F(CRLComparisonTest, CRLToolCompareHashFingerprintOpenSSL) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " + in_path + " -hash -fingerprint > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) + " crl -in " + in_path + " -hash -fingerprint > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str);
TEST_F(CRLComparisonTest, HashFingerprint) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " +
in_path + " -hash -fingerprint > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) +
" crl -in " + in_path +
" -hash -fingerprint > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool,
out_path_openssl, tool_output_str,
openssl_output_str);
}

// Test against OpenSSL output "openssl crl -in file -hash -fingerprint -noout"
TEST_F(CRLComparisonTest, CRLToolCompareHashFingerprintNoOutOpenSSL) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " + in_path + " -hash -fingerprint -noout > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) + " crl -in " + in_path + " -hash -fingerprint -noout > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str);
TEST_F(CRLComparisonTest, HashFingerprintNoOut) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " +
in_path + " -hash -fingerprint -noout > " +
out_path_tool;
std::string openssl_command =
std::string(openssl_executable_path) + " crl -in " + in_path +
" -hash -fingerprint -noout > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool,
out_path_openssl, tool_output_str,
openssl_output_str);
}

// Test against OpenSSL output "openssl crl -in file -fingerprint -hash"
TEST_F(CRLComparisonTest, CRLToolCompareReorderedHashFingerprintOpenSSL) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " + in_path + " -fingerprint -hash > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) + " crl -in " + in_path + " -fingerprint -hash > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str);
TEST_F(CRLComparisonTest, ReorderedHashFingerprint) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " +
in_path + " -fingerprint -hash > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) +
" crl -in " + in_path +
" -fingerprint -hash > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool,
out_path_openssl, tool_output_str,
openssl_output_str);
}

// Test against OpenSSL output "openssl crl -in file -fingerprint -hash -noout"
TEST_F(CRLComparisonTest, CRLToolCompareReorderedHashFingerprintNoOutOpenSSL) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " + in_path + " -fingerprint -hash -noout > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) + " crl -in " + in_path + " -fingerprint -hash -noout > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str);
TEST_F(CRLComparisonTest, ReorderedHashFingerprintNoOut) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " +
in_path + " -fingerprint -hash -noout > " +
out_path_tool;
std::string openssl_command =
std::string(openssl_executable_path) + " crl -in " + in_path +
" -fingerprint -hash -noout > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool,
out_path_openssl, tool_output_str,
openssl_output_str);
}

// Test against OpenSSL output "openssl crl -in file -noout -fingerprint -hash"
TEST_F(CRLComparisonTest, CRLToolCompareReorderedNoOutHashFingerprintOpenSSL) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " + in_path + " -noout -fingerprint -hash > " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) + " crl -in " + in_path + " -noout -fingerprint -hash > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str);
TEST_F(CRLComparisonTest, ReorderedNoOutHashFingerprint) {
std::string tool_command = std::string(tool_executable_path) + " crl -in " +
in_path + " -noout -fingerprint -hash > " +
out_path_tool;
std::string openssl_command =
std::string(openssl_executable_path) + " crl -in " + in_path +
" -noout -fingerprint -hash > " + out_path_openssl;

RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool,
out_path_openssl, tool_output_str,
openssl_output_str);
}
Loading
Loading