Skip to content
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

Jump to bottom

Update x509 tool to write all output to common BIO which is a file or stdout #1800

Merged
merged 8 commits into from
Aug 29, 2024
Merged

Update x509 tool to write all output to common BIO which is a file or stdout #1800

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
6fe2c30
Update x509 tool to write all output to common BIO which is a file or…
andrewhop Aug 23, 2024
58afe58
Don't print the certificate if it is a checkend call
andrewhop Aug 23, 2024
8aa4c5a
Also don't print it for modulus
andrewhop Aug 23, 2024
83e7f39
Still print out the certificate if modulus is requested without noout
andrewhop Aug 27, 2024
b574247
Remove OpenSSL 1.0.2 testing, update x509 tool's out default value to…
andrewhop Aug 27, 2024
ecf1524
Install clang for asan
andrewhop Aug 27, 2024
18f933b
And configure clang to acutally be used
andrewhop Aug 27, 2024
0ba831d
Remove default argument so a user could pass in the file name 'stdout'
andrewhop Aug 27, 2024
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
4 changes: 3 additions & 1 deletion .github/workflows/opensslcomparison.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ jobs:
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
cmake clang ninja-build golang make autoconf pkg-config openssl
echo "CC=clang" >> $GITHUB_ENV
echo "CXX=clang++" >> $GITHUB_ENV
- name: Build AWS-LC & OpenSSL and Run Comparison Tests
run: |
./tests/ci/run_openssl_comparison_tests.sh
5 changes: 1 addition & 4 deletions tests/ci/run_openssl_comparison_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,22 @@ scratch_folder=${SYS_ROOT}/"openssl-scratch"
install_dir="${scratch_folder}/libcrypto_install_dir"
openssl_url='https://github.com/openssl/openssl.git'
openssl_1_1_1_branch='OpenSSL_1_1_1-stable'
openssl_1_0_2_branch='OpenSSL_1_0_2-stable'
openssl_3_1_branch='openssl-3.1'
openssl_3_2_branch='openssl-3.2'
openssl_master_branch='master'

mkdir -p "${scratch_folder}"
rm -rf "${scratch_folder:?}"/*

build_openssl $openssl_1_0_2_branch
build_openssl $openssl_1_1_1_branch
build_openssl $openssl_3_1_branch
build_openssl $openssl_3_2_branch
build_openssl $openssl_master_branch

run_build -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_C_STANDARD=11 -DENABLE_DILITHIUM=ON
run_build -DASAN=1

# OpenSSL 3.1.0 on switches from lib folder to lib64 folder
declare -A openssl_branches=(
["$openssl_1_0_2_branch"]="lib"
["$openssl_1_1_1_branch"]="lib"
["$openssl_3_1_branch"]="lib64"
["$openssl_3_2_branch"]="lib64"
Expand Down
2 changes: 0 additions & 2 deletions tool-openssl/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ bool IsNumeric(const std::string& str);

X509* CreateAndSignX509Certificate();

bool WriteSignedCertificate(X509 *x509, const std::string &out_path);

bool LoadPrivateKeyAndSignCertificate(X509 *x509, const std::string &signkey_path);

tool_func_t FindTool(const std::string &name);
Expand Down
64 changes: 25 additions & 39 deletions tool-openssl/x509.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ static const argument_t kArguments[] = {
{ "-in", kOptionalArgument, "Certificate input, or CSR input file with -req" },
{ "-req", kBooleanArgument, "Input is a CSR file (rather than a certificate)" },
{ "-signkey", kOptionalArgument, "Causes input file to be self signed using supplied private key" },
{ "-out", kOptionalArgument, "Output file to write to" },
{ "-out", kOptionalArgument, "Filepath to write all output to, if not set write to stdout" },
{ "-noout", kBooleanArgument, "Prevents output of the encoded version of the certificate" },
{ "-dates", kBooleanArgument, "Print the start and expiry dates of a certificate" },
{ "-modulus", kBooleanArgument, "Prints out value of the modulus of the public key contained in the certificate" },
Expand All @@ -22,13 +22,8 @@ static const argument_t kArguments[] = {
{ "", kOptionalArgument, "" }
};

bool WriteSignedCertificate(X509 *x509, const std::string &out_path) {
ScopedFILE out_file(fopen(out_path.c_str(), "wb"));
if (!out_file) {
fprintf(stderr, "Error: unable to open output file '%s'\n", out_path.c_str());
return false;
}
if (!PEM_write_X509(out_file.get(), x509)) {
static bool WriteSignedCertificate(X509 *x509, bssl::UniquePtr<BIO> &output_bio, const std::string &out_path) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can output_bio be const?

if (!PEM_write_bio_X509(output_bio.get(), x509)) {
fprintf(stderr, "Error: error writing certificate to '%s'\n", out_path.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, what's out_path.c_str() when it's stdout? /dev/stdout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I didn't think about this for this change. Turns out it is the empty string which we pass in as the default value in GetString(&out_path, "-out", "", parsed_args); I'll update that so it makes a bit more sense.

ERR_print_errors_fp(stderr);
return false;
Expand Down Expand Up @@ -88,6 +83,13 @@ bool X509Tool(const args_list_t &args) {
PrintUsage(kArguments);
return false;
}
bssl::UniquePtr<BIO> output_bio;
if (out_path.empty()) {
output_bio.reset(BIO_new_fp(stdout, BIO_NOCLOSE));
} else {
output_bio.reset(BIO_new(BIO_s_file()));
BIO_write_filename(output_bio.get(), out_path.c_str());
}

// Check for required option -in, and -req must include -signkey
if (in_path.empty()) {
Expand Down Expand Up @@ -188,11 +190,8 @@ bool X509Tool(const args_list_t &args) {
}
}

// Write the signed certificate to output file
if (!out_path.empty()) {
if (!WriteSignedCertificate(x509.get(), out_path)) {
return false;
}
if (!WriteSignedCertificate(x509.get(), output_bio, out_path)) {
return false;
}
} else {
// Parse x509 certificate from input PEM file
Expand All @@ -203,24 +202,17 @@ bool X509Tool(const args_list_t &args) {
return false;
}
if(text) {
bssl::UniquePtr<BIO> bio(BIO_new_fp(stdout, BIO_NOCLOSE));
X509_print(bio.get(), x509.get());
X509_print(output_bio.get(), x509.get());
}

if (dates) {
bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem()));
BIO_printf(output_bio.get(), "notBefore=");
ASN1_TIME_print(output_bio.get(), X509_get_notBefore(x509.get()));
BIO_printf(output_bio.get(), "\n");

if (ASN1_TIME_print(bio.get(), X509_get_notBefore(x509.get()))) {
char not_before[30] = {};
BIO_read(bio.get(), not_before, sizeof(not_before) - 1);
fprintf(stdout, "notBefore=%s\n", not_before);
}

if (ASN1_TIME_print(bio.get(), X509_get_notAfter(x509.get()))) {
char not_after[30] = {};
BIO_read(bio.get(), not_after, sizeof(not_after) - 1);
fprintf(stdout, "notAfter=%s\n", not_after);
}
BIO_printf(output_bio.get(), "notAfter=");
ASN1_TIME_print(output_bio.get(), X509_get_notAfter(x509.get()));
BIO_printf(output_bio.get(), "\n");
}

if (modulus) {
Expand Down Expand Up @@ -249,7 +241,8 @@ bool X509Tool(const args_list_t &args) {
for (char *p = hex_modulus; *p; ++p) {
*p = toupper(*p);
}
printf("Modulus=%s\n", hex_modulus);
BIO_printf(output_bio.get(), "Modulus=%s\n", hex_modulus);

OPENSSL_free(hex_modulus);
} else {
fprintf(stderr, "Error: public key is not an RSA key\n");
Expand All @@ -267,9 +260,9 @@ bool X509Tool(const args_list_t &args) {
}

if ((days_left * 86400 + seconds_left) < static_cast<int>(*checkend)) {
printf("Certificate will expire\n");
BIO_printf(output_bio.get(), "Certificate will expire\n");
} else {
printf("Certificate will not expire\n");
BIO_printf(output_bio.get(), "Certificate will not expire\n");
}
}

Expand All @@ -279,15 +272,8 @@ bool X509Tool(const args_list_t &args) {
}
}

if (!out_path.empty()) {
if (!WriteSignedCertificate(x509.get(), out_path)) {
return false;
}
}

if (!noout && !in_path.empty() && !checkend && parsed_args.count("-out")==0) {
bssl::UniquePtr<BIO> bio_out(BIO_new_fp(stdout, BIO_NOCLOSE));
if (!PEM_write_bio_X509(bio_out.get(), x509.get())) {
if (!noout && !checkend) {
if (!WriteSignedCertificate(x509.get(), output_bio, out_path)) {
return false;
}
}
Expand Down
14 changes: 14 additions & 0 deletions tool-openssl/x509_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,13 @@ TEST_F(X509ComparisonTest, X509ToolCompareModulusOpenSSL) {
RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str);

ASSERT_EQ(tool_output_str, openssl_output_str);

tool_command = std::string(tool_executable_path) + " x509 -in " + in_path + " -modulus -out " + out_path_tool;
openssl_command = std::string(openssl_executable_path) + " x509 -in " + in_path + " -modulus -out " + out_path_openssl;

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

ASSERT_EQ(tool_output_str, openssl_output_str);
}

// Test against OpenSSL output "openssl x509 -in in_file -checkend 0"
Expand Down Expand Up @@ -352,4 +359,11 @@ TEST_F(X509ComparisonTest, X509ToolCompareDatesNooutOpenSSL) {
RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool, out_path_openssl, tool_output_str, openssl_output_str);

ASSERT_EQ(tool_output_str, openssl_output_str);

tool_command = std::string(tool_executable_path) + " x509 -in " + in_path + " -dates -noout -out " + out_path_tool;
openssl_command = std::string(openssl_executable_path) + " x509 -in " + in_path + " -dates -noout -out " + out_path_openssl;

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

ASSERT_EQ(tool_output_str, openssl_output_str);
}
Loading