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.

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

Adding -verify and expanding -x509 options for our OpenSSL tool #1951

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
3 changes: 3 additions & 0 deletions tool-openssl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ add_executable(
tool.cc
x509.cc
version.cc
verify.cc
)

target_include_directories(openssl PUBLIC ${PROJECT_SOURCE_DIR}/include)
Expand Down Expand Up @@ -59,6 +60,8 @@ if(BUILD_TESTING)
rsa_test.cc
x509.cc
x509_test.cc
verify.cc
verify_test.cc
)

target_link_libraries(tool_openssl_test boringssl_gtest_main ssl crypto)
Expand Down
1 change: 1 addition & 0 deletions tool-openssl/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@ bool md5Tool(const args_list_t &args);
bool rsaTool(const args_list_t &args);
bool X509Tool(const args_list_t &args);
bool VersionTool(const args_list_t &args);
bool VerifyTool(const args_list_t &args);

#endif //INTERNAL_H
3 changes: 2 additions & 1 deletion tool-openssl/tool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@

#include "./internal.h"

static const std::array<Tool, 5> kTools = {{
static const std::array<Tool, 6> kTools = {{
{"dgst", dgstTool},
{"md5", md5Tool},
{"rsa", rsaTool},
{"x509", X509Tool},
{"verify", VerifyTool},
{"version", VersionTool}
}};

Expand Down
198 changes: 198 additions & 0 deletions tool-openssl/verify.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR ISC

#include <openssl/base.h>
#include <openssl/x509.h>
#include <openssl/pem.h>
#include "internal.h"

static const argument_t kArguments[] = {
{ "-help", kBooleanArgument, "Display option summary" },
{ "-CAfile", kOptionalArgument, "A file of trusted certificates. The "
"file should contain one or more certificates in PEM format." },
{ "", kOptionalArgument, "" }
};

// setup_verification_store sets up an X509 certificate store for verification.
// It configures the store with file and directory lookups. It loads the
// specified CA file if provided and otherwise uses default locations.
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
static X509_STORE *setup_verification_store(std::string CAfile) {
X509_STORE *store = X509_STORE_new();
X509_LOOKUP *lookup;

if (!store) {
goto end;
}

lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file());
if (!lookup) {
goto end;
}

if (!CAfile.empty()) {
if (!X509_LOOKUP_load_file(lookup, CAfile.c_str(), X509_FILETYPE_PEM)) {
fprintf(stderr, "Error loading file %s\n", CAfile.c_str());
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
goto end;
}
} else {
X509_LOOKUP_load_file(lookup, NULL, X509_FILETYPE_DEFAULT);
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
}

lookup = X509_STORE_add_lookup(store, X509_LOOKUP_hash_dir());
if (lookup == NULL) {
goto end;
}
X509_LOOKUP_add_dir(lookup, NULL, X509_FILETYPE_DEFAULT);
smittals2 marked this conversation as resolved.
Show resolved Hide resolved

return store;

end:
X509_STORE_free(store);
return nullptr;
}

static int cb(int ok, X509_STORE_CTX *ctx) {
int cert_error = X509_STORE_CTX_get_error(ctx);
X509 *current_cert = X509_STORE_CTX_get_current_cert(ctx);

if (!ok) {
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
if (current_cert != NULL) {
X509_NAME_print_ex_fp(stderr,
X509_get_subject_name(current_cert),
0, XN_FLAG_ONELINE);
fprintf(stderr, "\n");
}
fprintf(stderr, "%serror %d at %d depth lookup: %s\n",
X509_STORE_CTX_get0_parent_ctx(ctx) ? "[CRL path] " : "",
cert_error,
X509_STORE_CTX_get_error_depth(ctx),
X509_verify_cert_error_string(cert_error));

/*
* Pretend that some errors are ok, so they don't stop further
* processing of the certificate chain. Setting ok = 1 does this.
* After X509_verify_cert() is done, we verify that there were
* no actual errors, even if the returned value was positive.
*/
switch (cert_error) {
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
case X509_V_ERR_NO_EXPLICIT_POLICY:
/* fall thru */
case X509_V_ERR_CERT_HAS_EXPIRED:
/* Continue even if the leaf is a self-signed cert */
case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:
/* Continue after extension errors too */
case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY:
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
case X509_V_ERR_INVALID_CA:
case X509_V_ERR_INVALID_NON_CA:
case X509_V_ERR_PATH_LENGTH_EXCEEDED:
case X509_V_ERR_CRL_HAS_EXPIRED:
case X509_V_ERR_CRL_NOT_YET_VALID:
case X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION:
/* errors due to strict conformance checking (-x509_strict) */
case X509_V_ERR_INVALID_PURPOSE:
ok = 1;
}
}
return ok;
}

static int check(X509_STORE *ctx, const char *file) {
bssl::UniquePtr<X509> x;
int i = 0, ret = 0;
X509_STORE_CTX *store_ctx;
smittals2 marked this conversation as resolved.
Show resolved Hide resolved

if (file) {
ScopedFILE cert_file(fopen(file, "rb"));
if (!cert_file) {
fprintf(stderr, "error %s: reading certificate failed\n", file);
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
}
x.reset(PEM_read_X509(cert_file.get(), nullptr, nullptr, nullptr));

} else {
bssl::UniquePtr<BIO> input(BIO_new_fp(stdin, BIO_CLOSE));
x.reset(PEM_read_bio_X509(input.get(), NULL, NULL, NULL));
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
}

if (x.get() == NULL) {
return 0;
}

store_ctx = X509_STORE_CTX_new();
if (store_ctx == NULL) {
fprintf(stderr, "error %s: X.509 store context allocation failed\n",
(file == NULL) ? "stdin" : file);
return 0;
}

if (!X509_STORE_CTX_init(store_ctx, ctx, x.get(), NULL)) {
X509_STORE_CTX_free(store_ctx);
fprintf(stderr,
"error %s: X.509 store context initialization failed\n",
(file == NULL) ? "stdin" : file);
return 0;
}

i = X509_verify_cert(store_ctx);
if (i > 0 && X509_STORE_CTX_get_error(store_ctx) == X509_V_OK) {
fprintf(stdout, "%s: OK\n", (file == NULL) ? "stdin" : file);
ret = 1;
} else {
fprintf(stderr,
"error %s: verification failed\n",
(file == NULL) ? "stdin" : file);
}
X509_STORE_CTX_free(store_ctx);

return ret;
}

bool VerifyTool(const args_list_t &args) {
std::string cafile;
bssl::UniquePtr<X509_STORE> store;
int ret;
size_t i = 0;

if (args.size() == 1 && args[0] == "-help") {
fprintf(stderr, "Usage: verify [options] cert.pem...\n"
"Certificates must be in PEM format and specified in a file.\n"
"We do not support reading from stdin yet. \n\n "
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
"Valid options are:\n");
PrintUsage(kArguments);
return false;
} else if (args.size() > 1 && args[0] == "-CAfile") {
cafile = args[1];
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
i += 2;
andrewhop marked this conversation as resolved.
Show resolved Hide resolved
}

store.reset(setup_verification_store(cafile));
smittals2 marked this conversation as resolved.
Show resolved Hide resolved

// Initialize certificate verification store
if (!store.get()) {
fprintf(stderr, "Error: Unable to setup certificate verification store.");
return false;
}
X509_STORE_set_verify_cb(store.get(), cb);

ERR_clear_error();

ret = 1;
smittals2 marked this conversation as resolved.
Show resolved Hide resolved

// No additional file or certs provided, read from stdin
if (args.size() == i) {
if (check(store.get(), NULL) != 1) {
ret = 0;
}
} else {
// Certs provided as files
for (; i < args.size(); i++) {
if (check(store.get(), args[i].c_str()) != 1) {
ret = 0;
}
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
}
}

if (!ret) {
return false;
}
return true;
}
157 changes: 157 additions & 0 deletions tool-openssl/verify_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
// 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 "internal.h"
#include "test_util.h"
#include "../crypto/test/test_util.h"


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

bssl::UniquePtr<EVP_PKEY> pkey(EVP_PKEY_new());
ASSERT_TRUE(pkey);
bssl::UniquePtr<RSA> rsa(RSA_new());
ASSERT_TRUE(rsa);
bssl::UniquePtr<BIGNUM> bn(BN_new());
ASSERT_TRUE(bn && rsa && BN_set_word(bn.get(), RSA_F4) && RSA_generate_key_ex(rsa.get(), 2048, bn.get(), nullptr));
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
ASSERT_TRUE(EVP_PKEY_assign_RSA(pkey.get(), rsa.release()));

ScopedFILE signkey_file(fopen(signkey_path, "wb"));
ASSERT_TRUE(signkey_file);
ASSERT_TRUE(PEM_write_PrivateKey(signkey_file.get(), pkey.get(), nullptr, nullptr, 0, nullptr, nullptr));

bssl::UniquePtr<X509> x509(CreateAndSignX509Certificate());
ASSERT_TRUE(x509);

ScopedFILE in_file(fopen(in_path, "wb"));
ASSERT_TRUE(in_file);
ASSERT_TRUE(PEM_write_X509(in_file.get(), x509.get()));

ScopedFILE ca_file(fopen(ca_path, "wb"));
ASSERT_TRUE(ca_file);
ASSERT_TRUE(PEM_write_X509(ca_file.get(), x509.get()));
}
void TearDown() override {
RemoveFile(ca_path);
RemoveFile(in_path);
RemoveFile(signkey_path);
}
char ca_path[PATH_MAX];
char in_path[PATH_MAX];
char signkey_path[PATH_MAX];
};


// ----------------------------- Verify Option Tests -----------------------------

// Test -CAfile with self-signed certificate
TEST_F(VerifyTest, VerifyTestSelfSignedCertWithCAfileTest) {
args_list_t args = {"-CAfile", ca_path, in_path};
bool result = VerifyTool(args);
ASSERT_TRUE(result);
}

// Test self-signed certificate without -CAfile
TEST_F(VerifyTest, VerifyTestSelfSignedCertWithoutCAfile) {
args_list_t args = {in_path};
bool result = VerifyTool(args);
ASSERT_FALSE(result);
}


// -------------------- Verify OpenSSL Comparison Tests --------------------------

// Comparison tests cannot run without set up of environment variables:
// AWSLC_TOOL_PATH and OPENSSL_TOOL_PATH.

class VerifyComparisonTest : public ::testing::Test {
protected:
void SetUp() override {
andrewhop marked this conversation as resolved.
Show resolved Hide resolved

// 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(ca_path), 0u);
ASSERT_GT(createTempFILEpath(signkey_path), 0u);
ASSERT_GT(createTempFILEpath(out_path_tool), 0u);
ASSERT_GT(createTempFILEpath(out_path_openssl), 0u);

x509.reset(CreateAndSignX509Certificate());
ASSERT_TRUE(x509);

ScopedFILE in_file(fopen(in_path, "wb"));
ASSERT_TRUE(in_file);
ASSERT_TRUE(PEM_write_X509(in_file.get(), x509.get()));

ScopedFILE ca_file(fopen(ca_path, "wb"));
ASSERT_TRUE(ca_file);
ASSERT_TRUE(PEM_write_X509(ca_file.get(), x509.get()));

bssl::UniquePtr<EVP_PKEY> pkey(EVP_PKEY_new());
ASSERT_TRUE(pkey);
bssl::UniquePtr<RSA> rsa(RSA_new());
bssl::UniquePtr<BIGNUM> bn(BN_new());
ASSERT_TRUE(bn && BN_set_word(bn.get(), RSA_F4) && RSA_generate_key_ex(rsa.get(), 2048, bn.get(), nullptr));
ASSERT_TRUE(EVP_PKEY_assign_RSA(pkey.get(), rsa.release()));

ScopedFILE signkey_file(fopen(signkey_path, "wb"));
ASSERT_TRUE(signkey_file);
ASSERT_TRUE(PEM_write_PrivateKey(signkey_file.get(), pkey.get(), nullptr, nullptr, 0, nullptr, nullptr));
}

void TearDown() override {
if (tool_executable_path != nullptr && openssl_executable_path != nullptr) {
RemoveFile(in_path);
RemoveFile(out_path_tool);
RemoveFile(out_path_openssl);
RemoveFile(signkey_path);
RemoveFile(ca_path);
}
}

char in_path[PATH_MAX];
char ca_path[PATH_MAX];
char out_path_tool[PATH_MAX];
char out_path_openssl[PATH_MAX];
char signkey_path[PATH_MAX];
bssl::UniquePtr<X509> x509;
const char* tool_executable_path;
const char* openssl_executable_path;
std::string tool_output_str;
std::string openssl_output_str;
};

// Test against OpenSSL with -CAfile & self-signed cert fed in as a file
smittals2 marked this conversation as resolved.
Show resolved Hide resolved
// "openssl verify -CAfile cert.pem cert.pem"
TEST_F(VerifyComparisonTest, VerifyToolOpenSSLCAFileSelfSignedComparison) {
std::string tool_command = std::string(tool_executable_path) + " verify -CAfile " + ca_path + " " + in_path + " &> " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) + " verify -CAfile " + ca_path + " " + in_path + " &> " + 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 with -CAfile & self-signed cert fed through stdin
// "cat cert.pem | openssl verify -CAfile cert.pem"
TEST_F(VerifyComparisonTest, VerifyToolOpenSSLCAFileSelfSignedStdinComparison) {
std::string tool_command = "cat " + std::string(ca_path) + " | " + std::string(tool_executable_path) + " verify -CAfile " + ca_path + " &> " + out_path_tool;
std::string openssl_command = "cat " + std::string(ca_path) + " | " + std::string(openssl_executable_path) + " verify -CAfile " + ca_path + " &> " + 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);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test that verify verifies a public cert (like amazon.com), this will verify that we are correctly loading the default system trust store certs. Traps to avoid:

  1. Don't create a time bomb
  2. Not everywhere that runs our CI has network access

Loading
Loading