Skip to content

Commit

Permalink
addressed PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
smittals2 committed Nov 15, 2024
1 parent d1ce560 commit 9043a2d
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 146 deletions.
85 changes: 30 additions & 55 deletions tool-openssl/verify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,42 +17,28 @@ static const argument_t kArguments[] = {
// It configures the store with file and directory lookups. It loads the
// specified CA file if provided and otherwise uses default locations.
static X509_STORE *setup_verification_store(std::string CAfile) {
X509_STORE *store = X509_STORE_new();
bssl::UniquePtr<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;
return nullptr;
}

if (!CAfile.empty()) {
if (!X509_LOOKUP_load_file(lookup, CAfile.c_str(), X509_FILETYPE_PEM)) {
lookup = X509_STORE_add_lookup(store.get(), X509_LOOKUP_file());
if (!lookup || !X509_LOOKUP_load_file(lookup, CAfile.c_str(), X509_FILETYPE_PEM)) {
fprintf(stderr, "Error loading file %s\n", CAfile.c_str());
goto end;
}
} else {
if (!X509_LOOKUP_load_file(lookup, NULL, X509_FILETYPE_DEFAULT)) {
goto end;
return nullptr;
}
}

lookup = X509_STORE_add_lookup(store, X509_LOOKUP_hash_dir());
if (lookup == NULL) {
goto end;
}
if (!X509_LOOKUP_add_dir(lookup, NULL, X509_FILETYPE_DEFAULT)) {
goto end;
// Add default dir path
lookup = X509_STORE_add_lookup(store.get(), X509_LOOKUP_hash_dir());
if (!lookup || !X509_LOOKUP_add_dir(lookup, NULL, X509_FILETYPE_DEFAULT)) {
return nullptr;
}

return store;

end:
X509_STORE_free(store);
return nullptr;
return store.release();
}

static int cb(int ok, X509_STORE_CTX *ctx) {
Expand Down Expand Up @@ -101,75 +87,71 @@ static int cb(int ok, X509_STORE_CTX *ctx) {
}

static int check(X509_STORE *ctx, const char *file) {
bssl::UniquePtr<X509> x;
bssl::UniquePtr<X509> cert;
int i = 0, ret = 0;
X509_STORE_CTX *store_ctx;

if (file) {
ScopedFILE cert_file(fopen(file, "rb"));
if (!cert_file) {
fprintf(stderr, "error %s: reading certificate failed\n", file);
return 0;
}
x.reset(PEM_read_X509(cert_file.get(), nullptr, nullptr, nullptr));
cert.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(), nullptr, nullptr, nullptr));
cert.reset(PEM_read_bio_X509(input.get(), nullptr, nullptr, nullptr));
}

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

store_ctx = X509_STORE_CTX_new();
if (store_ctx == nullptr) {
bssl::UniquePtr<X509_STORE_CTX> store_ctx(X509_STORE_CTX_new());
if (store_ctx == nullptr || store_ctx.get() == nullptr) {
fprintf(stderr, "error %s: X.509 store context allocation failed\n",
(file == nullptr) ? "stdin" : file);
return 0;
}

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

i = X509_verify_cert(store_ctx);
if (i > 0 && X509_STORE_CTX_get_error(store_ctx) == X509_V_OK) {
i = X509_verify_cert(store_ctx.get());
if (i > 0 && X509_STORE_CTX_get_error(store_ctx.get()) == X509_V_OK) {
fprintf(stdout, "%s: OK\n", (file == nullptr) ? "stdin" : file);
ret = 1;
} else {
fprintf(stderr,
"error %s: verification failed\n",
(file == nullptr) ? "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 "
"Valid options are:\n");
fprintf(stderr,
"Usage: verify [options] [cert.pem...]\n"
"Certificates must be in PEM format. They can be specified in one or more files.\n"
"If no files are specified, the tool will read from stdin.\n\n"
"Valid options are:\n");
PrintUsage(kArguments);
return false;
} else if (args.size() > 1 && args[0] == "-CAfile") {
cafile = args[1];
i += 2;
}

store.reset(setup_verification_store(cafile));

bssl::UniquePtr<X509_STORE> store(setup_verification_store(cafile));
// Initialize certificate verification store
if (!store.get()) {
fprintf(stderr, "Error: Unable to setup certificate verification store.");
Expand All @@ -179,24 +161,17 @@ bool VerifyTool(const args_list_t &args) {

ERR_clear_error();

ret = 1;
int ret = 1;

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

if (!ret) {
return false;
}
return true;
return ret == 1;
}
40 changes: 11 additions & 29 deletions tool-openssl/verify_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,6 @@ class VerifyTest : public ::testing::Test {
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));
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);
Expand All @@ -42,11 +29,9 @@ class VerifyTest : public ::testing::Test {
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];
};


Expand Down Expand Up @@ -85,7 +70,6 @@ class VerifyComparisonTest : public ::testing::Test {

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);

Expand All @@ -99,25 +83,13 @@ class VerifyComparisonTest : public ::testing::Test {
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);
}
}
Expand All @@ -126,7 +98,6 @@ class VerifyComparisonTest : public ::testing::Test {
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;
Expand All @@ -145,6 +116,17 @@ TEST_F(VerifyComparisonTest, VerifyToolOpenSSLCAFileSelfSignedComparison) {
ASSERT_EQ(tool_output_str, openssl_output_str);
}

// Test against OpenSSL with -CAfile & 2 self-signed cert fed in as files
// "openssl verify -CAfile cert.pem cert.pem cert.pem"
TEST_F(VerifyComparisonTest, VerifyToolOpenSSLCAFileMultipleFilesComparison) {
std::string tool_command = std::string(tool_executable_path) + " verify -CAfile " + ca_path + " " + in_path + " " + in_path + " &> " + out_path_tool;
std::string openssl_command = std::string(openssl_executable_path) + " verify -CAfile " + ca_path + " " + in_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) {
Expand Down
9 changes: 4 additions & 5 deletions tool-openssl/x509.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,19 +294,18 @@ bool X509Tool(const args_list_t &args) {
}

if (fingerprint) {
int j;
unsigned int n;
unsigned int out_len;
unsigned char md[EVP_MAX_MD_SIZE];
const EVP_MD *digest = EVP_sha1();

if (!X509_digest(x509.get(), digest, md, &n)) {
if (!X509_digest(x509.get(), digest, md, &out_len)) {
fprintf(stderr, "Error: unable to obtain digest\n");
return false;
}
BIO_printf(output_bio.get(), "%s Fingerprint=",
OBJ_nid2sn(EVP_MD_type(digest)));
for (j = 0; j < (int)n; j++) {
BIO_printf(output_bio.get(), "%02X%c", md[j], (j + 1 == (int)n)
for (int j = 0; j < (int)out_len; j++) {
BIO_printf(output_bio.get(), "%02X%c", md[j], (j + 1 == (int)out_len)
? '\n' : ':');
}
}
Expand Down
Loading

0 comments on commit 9043a2d

Please sign in to comment.