Skip to content

Commit

Permalink
This fixes issue #95
Browse files Browse the repository at this point in the history 
- Certificate got another ctor which takes the flags to pass when
  formatting the X509_NAME values
- The default formatting changed to XN_FLAG_RFC2253 but can be overridden
  from the outside by defining UTHENTICODE_DEFAULT_XN_FLAGS
- This introduces an incompatibility _if_ the caller assumes that the
  issuer and subject can be compared in their string form
  • Loading branch information
@hugmyndakassi
hugmyndakassi committed Oct 28, 2024
1 parent f81d3a8 commit 8428b65
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 31 deletions.
11 changes: 10 additions & 1 deletion src/include/uthenticode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,17 @@ class Certificate {
/* TODO: Maybe add data_ and get_data(), with data_ populated from i2d_X509.
*/

#ifndef UTHENTICODE_DEFAULT_XN_FLAGS
static constexpr unsigned long const default_xn_flags =
XN_FLAG_RFC2253 | ASN1_STRFLGS_UTF8_CONVERT;
#else
static constexpr unsigned long const default_xn_flags = (UTHENTICODE_DEFAULT_XN_FLAGS);
#endif
static_assert((default_xn_flags & XN_FLAG_COMPAT) == 0,
"Logic is incompatible with XN_FLAG_COMPAT");

private:
Certificate(X509 *cert);
explicit Certificate(X509 *cert);
std::string subject_;
std::string issuer_;
std::string serial_number_;
Expand Down
22 changes: 16 additions & 6 deletions src/uthenticode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,26 @@ std::ostream &operator<<(std::ostream &os, checksum_kind kind) {
}
}

static inline bool name_to_string(std::string &strname, X509_NAME *name, unsigned long flags) {
std::unique_ptr<BIO, decltype(&BIO_free)> name_bio(BIO_new(BIO_s_mem()), BIO_free);
if (-1 != X509_NAME_print_ex(name_bio.get(), name, 0, (flags) & ~(ASN1_STRFLGS_ESC_MSB))) {
char *data = nullptr;
auto len = BIO_get_mem_data(name_bio.get(), &data);
if (data && len) {
strname = std::string(data, len);
return true;
}
}
return false;
}

Certificate::Certificate(X509 *cert) {
auto subject = impl::OpenSSL_ptr(X509_NAME_oneline(X509_get_subject_name(cert), nullptr, 0),
impl::OpenSSL_free);
auto issuer = impl::OpenSSL_ptr(X509_NAME_oneline(X509_get_issuer_name(cert), nullptr, 0),
impl::OpenSSL_free);
constexpr auto xn_flags = default_xn_flags;
std::ignore = name_to_string(issuer_, X509_get_issuer_name(cert), xn_flags);
std::ignore = name_to_string(subject_, X509_get_subject_name(cert), xn_flags);
auto serial_bn = impl::BN_ptr(ASN1_INTEGER_to_BN(X509_get_serialNumber(cert), nullptr), BN_free);
auto serial_number = impl::OpenSSL_ptr(BN_bn2hex(serial_bn.get()), impl::OpenSSL_free);

subject_ = std::string(subject.get());
issuer_ = std::string(issuer.get());
serial_number_ = std::string(serial_number.get());
}

Expand Down
48 changes: 24 additions & 24 deletions test/certificate-test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ TEST_F(Auth32Test, Certificate_properties) {
ASSERT_TRUE(signed_data.has_value());

auto signers = signed_data->get_signers();
ASSERT_EQ(signers[0].get_subject(), "/[email protected]");
ASSERT_EQ(signers[0].get_issuer(), "/[email protected]");
ASSERT_EQ(signers[0].get_subject(), "[email protected]");
ASSERT_EQ(signers[0].get_issuer(), "[email protected]");
ASSERT_EQ(signers[0].get_serial_number(), "4B2A0A5FE84F83BE4B5F587EC325FDA3");

auto certificates = signed_data->get_certificates();
ASSERT_EQ(certificates[0].get_subject(), "/[email protected]");
ASSERT_EQ(certificates[0].get_issuer(), "/[email protected]");
ASSERT_EQ(certificates[0].get_subject(), "[email protected]");
ASSERT_EQ(certificates[0].get_issuer(), "[email protected]");
ASSERT_EQ(certificates[0].get_serial_number(), "4B2A0A5FE84F83BE4B5F587EC325FDA3");
}

Expand All @@ -30,13 +30,13 @@ TEST_F(Auth32PlusTest, Certificate_properties) {
ASSERT_TRUE(signed_data.has_value());

auto signers = signed_data->get_signers();
ASSERT_EQ(signers[0].get_subject(), "/[email protected]");
ASSERT_EQ(signers[0].get_issuer(), "/[email protected]");
ASSERT_EQ(signers[0].get_subject(), "[email protected]");
ASSERT_EQ(signers[0].get_issuer(), "[email protected]");
ASSERT_EQ(signers[0].get_serial_number(), "5C01626BE30E6696475724EFA09135F3");

auto certificates = signed_data->get_certificates();
ASSERT_EQ(certificates[0].get_subject(), "/[email protected]");
ASSERT_EQ(certificates[0].get_issuer(), "/[email protected]");
ASSERT_EQ(certificates[0].get_subject(), "[email protected]");
ASSERT_EQ(certificates[0].get_issuer(), "[email protected]");
ASSERT_EQ(certificates[0].get_serial_number(), "5C01626BE30E6696475724EFA09135F3");
}

Expand All @@ -47,27 +47,27 @@ TEST_F(AuthNest32Test, Certificate_properties_nested) {
ASSERT_TRUE(signed_data.has_value());

auto signers = signed_data->get_signers();
ASSERT_EQ(signers[0].get_subject(), "/CN=A SHA-1 cert");
ASSERT_EQ(signers[0].get_issuer(), "/CN=A SHA-1 cert");
ASSERT_EQ(signers[0].get_subject(), "CN=A SHA-1 cert");
ASSERT_EQ(signers[0].get_issuer(), "CN=A SHA-1 cert");
ASSERT_EQ(signers[0].get_serial_number(), "103EE193544680954387616BB5ECA399");

auto certificates = signed_data->get_certificates();
ASSERT_EQ(certificates[0].get_subject(), "/CN=A SHA-1 cert");
ASSERT_EQ(certificates[0].get_issuer(), "/CN=A SHA-1 cert");
ASSERT_EQ(certificates[0].get_subject(), "CN=A SHA-1 cert");
ASSERT_EQ(certificates[0].get_issuer(), "CN=A SHA-1 cert");
ASSERT_EQ(certificates[0].get_serial_number(), "103EE193544680954387616BB5ECA399");

auto nested_signed_data = signed_data->get_nested_signed_data();

ASSERT_TRUE(nested_signed_data.has_value());

auto nested_signers = nested_signed_data->get_signers();
ASSERT_EQ(nested_signers[0].get_subject(), "/CN=A SHA-256 cert");
ASSERT_EQ(nested_signers[0].get_issuer(), "/CN=A SHA-256 cert");
ASSERT_EQ(nested_signers[0].get_subject(), "CN=A SHA-256 cert");
ASSERT_EQ(nested_signers[0].get_issuer(), "CN=A SHA-256 cert");
ASSERT_EQ(nested_signers[0].get_serial_number(), "2D96C54BA915F7B04781D80A799534B1");

auto nested_certificates = nested_signed_data->get_certificates();
ASSERT_EQ(nested_certificates[0].get_subject(), "/CN=A SHA-256 cert");
ASSERT_EQ(nested_certificates[0].get_issuer(), "/CN=A SHA-256 cert");
ASSERT_EQ(nested_certificates[0].get_subject(), "CN=A SHA-256 cert");
ASSERT_EQ(nested_certificates[0].get_issuer(), "CN=A SHA-256 cert");
ASSERT_EQ(nested_certificates[0].get_serial_number(), "2D96C54BA915F7B04781D80A799534B1");
}

Expand All @@ -78,26 +78,26 @@ TEST_F(AuthNest32PlusTest, Certificate_properties_nested) {
ASSERT_TRUE(signed_data.has_value());

auto signers = signed_data->get_signers();
ASSERT_EQ(signers[0].get_subject(), "/CN=A SHA-1 cert");
ASSERT_EQ(signers[0].get_issuer(), "/CN=A SHA-1 cert");
ASSERT_EQ(signers[0].get_subject(), "CN=A SHA-1 cert");
ASSERT_EQ(signers[0].get_issuer(), "CN=A SHA-1 cert");
ASSERT_EQ(signers[0].get_serial_number(), "1DFA12996D33C09D499BE8489BE35DE5");

auto certificates = signed_data->get_certificates();
ASSERT_EQ(certificates[0].get_subject(), "/CN=A SHA-1 cert");
ASSERT_EQ(certificates[0].get_issuer(), "/CN=A SHA-1 cert");
ASSERT_EQ(certificates[0].get_subject(), "CN=A SHA-1 cert");
ASSERT_EQ(certificates[0].get_issuer(), "CN=A SHA-1 cert");
ASSERT_EQ(certificates[0].get_serial_number(), "1DFA12996D33C09D499BE8489BE35DE5");

auto nested_signed_data = signed_data->get_nested_signed_data();

ASSERT_TRUE(nested_signed_data.has_value());

auto nested_signers = nested_signed_data->get_signers();
ASSERT_EQ(nested_signers[0].get_subject(), "/CN=A SHA-256 cert");
ASSERT_EQ(nested_signers[0].get_issuer(), "/CN=A SHA-256 cert");
ASSERT_EQ(nested_signers[0].get_subject(), "CN=A SHA-256 cert");
ASSERT_EQ(nested_signers[0].get_issuer(), "CN=A SHA-256 cert");
ASSERT_EQ(nested_signers[0].get_serial_number(), "24669C98D6ED318540F4953CD30250AB");

auto nested_certificates = nested_signed_data->get_certificates();
ASSERT_EQ(nested_certificates[0].get_subject(), "/CN=A SHA-256 cert");
ASSERT_EQ(nested_certificates[0].get_issuer(), "/CN=A SHA-256 cert");
ASSERT_EQ(nested_certificates[0].get_subject(), "CN=A SHA-256 cert");
ASSERT_EQ(nested_certificates[0].get_issuer(), "CN=A SHA-256 cert");
ASSERT_EQ(nested_certificates[0].get_serial_number(), "24669C98D6ED318540F4953CD30250AB");
}

0 comments on commit 8428b65

Please sign in to comment.