Skip to content

Commit

Permalink
fix: TLS server certificate_sha_1 calculated incorrect hash (#902)
Browse files Browse the repository at this point in the history
certificate_sha_1() was originally used to calculate a unique value
representing a certificate. For trusted_ca_keys the certificate hash
needs to be over the whole certificate.

certificate_sha_1() updated to calculate the correct hash.
unit tests added to check the calculated hash
python test script to print trusted_ca_keys and certificate hashes

Signed-off-by: James Chapman <[email protected]>
  • Loading branch information
james-ctc authored and hikinggrass committed Oct 14, 2024
1 parent d1a151e commit b9ef35f
Show file tree
Hide file tree
Showing 4 changed files with 195 additions and 10 deletions.
39 changes: 29 additions & 10 deletions lib/staging/tls/openssl_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,20 @@ std::string certificate_to_pem(const X509* cert) {
return result;
}

certificate_ptr pem_to_certificate(const std::string& pem) {
certificate_ptr result{nullptr, nullptr};
auto* mem = BIO_new_mem_buf(pem.c_str(), static_cast<int>(pem.size()));
X509* cert = nullptr;

if (PEM_read_bio_X509(mem, &cert, nullptr, nullptr) == nullptr) {
log_error("PEM_read_bio_X509");
} else {
result = certificate_ptr{cert, &X509_free};
}
BIO_free(mem);
return result;
}

certificate_ptr der_to_certificate(const std::uint8_t* der, std::size_t len) {
certificate_ptr result{nullptr, nullptr};
const auto* ptr = der;
Expand All @@ -604,6 +618,18 @@ certificate_ptr der_to_certificate(const std::uint8_t* der, std::size_t len) {
return result;
}

DER certificate_to_der(const x509_st* cert) {
assert(cert != nullptr);

unsigned char* data{nullptr};

// DO NOT FREE - internal pointers to certificate
int len = i2d_X509(cert, &data);

// move data to DER
return {der_ptr{data, &DER::free}, static_cast<std::size_t>(len)};
}

verify_result_t verify_certificate(const X509* cert, const certificate_list& trust_anchors,
const certificate_list& untrusted) {
verify_result_t result = verify_result_t::Verified;
Expand Down Expand Up @@ -753,16 +779,9 @@ bool certificate_sha_1(openssl::sha_1_digest_t& digest, const X509* cert) {
assert(cert != nullptr);

bool bResult{false};
const ASN1_BIT_STRING* signature{nullptr};
const X509_ALGOR* alg{nullptr};
X509_get0_signature(&signature, &alg, cert);
if (signature != nullptr) {
unsigned char* data{nullptr};
const auto len = i2d_ASN1_BIT_STRING(signature, &data);
if (len > 0) {
bResult = openssl::sha_1(data, len, digest);
}
OPENSSL_free(data);
auto der = certificate_to_der(cert);
if (der) {
bResult = openssl::sha_1(der.get(), der.size(), digest);
}

return bResult;
Expand Down
15 changes: 15 additions & 0 deletions lib/staging/tls/openssl_util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,13 @@ bool use_certificate_and_key(ssl_st* ssl, const chain_t& chain);
*/
std::string certificate_to_pem(const x509_st* cert);

/**
* \brief convert a PEM string to a certificate
* \param[in] pem the PEM string
* \return the certificate or empty unique_ptr on error
*/
certificate_ptr pem_to_certificate(const std::string& pem);

/**
* \brief parse a DER (ASN.1) encoded certificate
* \param[in] der a pointer to the DER encoded certificate
Expand All @@ -423,6 +430,13 @@ std::string certificate_to_pem(const x509_st* cert);
*/
certificate_ptr der_to_certificate(const std::uint8_t* der, std::size_t len);

/**
* \brief encode a certificate to DER (ASN.1)
* \param[in] cert the certificate
* \return the DER encoded certificate or nullptr on error
*/
DER certificate_to_der(const x509_st* cert);

/**
* \brief verify a certificate against a certificate chain and trust anchors
* \param[in] cert the certificate to verify - when nullptr the certificate must
Expand Down Expand Up @@ -463,6 +477,7 @@ pkey_ptr certificate_public_key(x509_st* cert);
* \param[out] digest the SHA1 digest of the certificate
* \param[in] cert the certificate
* \return true on success
* \note this is the hash of the whole certificate including signature
*/
bool certificate_sha_1(openssl::sha_1_digest_t& digest, const x509_st* cert);

Expand Down
45 changes: 45 additions & 0 deletions lib/staging/tls/tests/openssl_util_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,33 @@ const char iso_exi_sig_b64[] =
const char iso_exi_sig_b64_nl[] =
"TI8gwUALpnYGqkgRVyovGtPBUInZVCA2NDC7JrSdsQTwjfqL+AVeY6S3Wo0xaSBv\nqNVDCLpY8FZrlrr2ks5ZUA==\n";

const char test_cert_pem[] = "-----BEGIN CERTIFICATE-----\n"
"MIICBDCCAaqgAwIBAgIUQnMkyWtvc/a5OG8dZr9ziA5uQqYwCgYIKoZIzj0EAwIw\n"
"TjELMAkGA1UEBhMCR0IxDzANBgNVBAcMBkxvbmRvbjEPMA0GA1UECgwGUGlvbml4\n"
"MR0wGwYDVQQDDBRDUyBSb290IFRydXN0IEFuY2hvcjAeFw0yNDA5MTkxMzQwMDBa\n"
"Fw0yNDEwMjExMzQwMDBaME4xCzAJBgNVBAYTAkdCMQ8wDQYDVQQHDAZMb25kb24x\n"
"DzANBgNVBAoMBlBpb25peDEdMBsGA1UEAwwUQ1MgUm9vdCBUcnVzdCBBbmNob3Iw\n"
"WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARLkawitst5NtPoYGpDCp8/GBTDrNRJ\n"
"pCzS3KHT2lZJDOwzegRn+Zhs0csqXIQgbkCqdSozg+d83QNKcpmJk4FYo2YwZDAO\n"
"BgNVHQ8BAf8EBAMCAQYwHQYDVR0OBBYEFB6Ytfi9uSF7NSYGXmyZEcKsWHwJMB8G\n"
"A1UdIwQYMBaAFB6Ytfi9uSF7NSYGXmyZEcKsWHwJMBIGA1UdEwEB/wQIMAYBAf8C\n"
"AQIwCgYIKoZIzj0EAwIDSAAwRQIge4+uxc2EFYD7AkHR+9d/NbULUnKFIBRLqYE+\n"
"Ib4h2CMCIQCtFWyvxwOUNidUTZGqyZXFmDyutJiNM0mi1iuFk8/8Mw==\n"
"-----END CERTIFICATE-----\n";

const char test_cert_hash[] = "082f891b26de97c8bdedb159f8d59113cfb55dc0";
const char test_cert_key_hash[] = "3b094e5f2594a3ae4511a9ff4285acd91fcd11c0";

inline const auto to_hex_string(const openssl::sha_1_digest_t& b) {
std::stringstream string_stream;
string_stream << std::hex;

for (int idx = 0; idx < sizeof(b); ++idx)
string_stream << std::setw(2) << std::setfill('0') << (int)b[idx];

return string_stream.str();
}

TEST(util, removeHyphen) {
const std::string expected{"UKSWI123456791A"};
std::string cert_emaid{"UKSWI123456791A"};
Expand All @@ -104,6 +131,24 @@ TEST(util, removeHyphen) {
EXPECT_EQ(cert_emaid, expected);
}

TEST(certificate_sha_1, hash) {
auto cert = openssl::pem_to_certificate(test_cert_pem);
EXPECT_TRUE(cert);
openssl::sha_1_digest_t digest;
auto res = openssl::certificate_sha_1(digest, cert.get());
EXPECT_TRUE(res);
EXPECT_EQ(to_hex_string(digest), test_cert_hash);
}

TEST(certificate_subject_public_key_sha_1, hash) {
auto cert = openssl::pem_to_certificate(test_cert_pem);
EXPECT_TRUE(cert);
openssl::sha_1_digest_t digest;
auto res = openssl::certificate_subject_public_key_sha_1(digest, cert.get());
EXPECT_TRUE(res);
EXPECT_EQ(to_hex_string(digest), test_cert_key_hash);
}

TEST(DER, equal) {
const std::uint8_t data[] = {1, 2, 3, 4, 5, 6, 7, 8, 9};
openssl::DER a;
Expand Down
106 changes: 106 additions & 0 deletions lib/staging/tls/tests/trusted_ca_keys_decode.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
#!/usr/bin/env python3

"""
trusted ca keys hex string
"0069011d484406bca8888997a7462416445e7db117114c017f204de30f1cd42c9e6dae91b6a8ac9b8d481ba601597be7013ad6fc397b78b01d90cea1b7f909f145011d484406bca8888997a7462416445e7db117114c0100fae3900795c888a4d4d7bd9fdffa60418ac19f"
length 0069
"01 1d484406bca8888997a7462416445e7db117114c"
"01 7f204de30f1cd42c9e6dae91b6a8ac9b8d481ba6"
"01 597be7013ad6fc397b78b01d90cea1b7f909f145"
"01 1d484406bca8888997a7462416445e7db117114c"
"01 00fae3900795c888a4d4d7bd9fdffa60418ac19f"
key hash from certificate
openssl x509 -in cert.pem -pubkey -noout | openssl enc -base64 -d | openssl dgst -sha1
"""

from cryptography import x509
from cryptography.hazmat.primitives import serialization
from cryptography.hazmat.primitives import hashes

import argparse


def certificate_key_hash(filename):
with open(filename, "rb") as fp:
cert = x509.load_pem_x509_certificate(fp.read())
pub = cert.public_key()
pub_der = pub.public_bytes(
encoding=serialization.Encoding.DER,
format=serialization.PublicFormat.SubjectPublicKeyInfo,
)
dgst = hashes.Hash(hashes.SHA1())
dgst.update(pub_der)
sha1 = dgst.finalize()
print(sha1.hex())


def certificate_hash(filename):
# note this is the hash of the whole certificate including signature
with open(filename, "rb") as fp:
cert = x509.load_pem_x509_certificate(fp.read())
pub_der = cert.public_bytes(encoding=serialization.Encoding.DER)
dgst = hashes.Hash(hashes.SHA1())
dgst.update(pub_der)
sha1 = dgst.finalize()
print(sha1.hex())


def trusted_ca_keys_decode(data):
data_len = int.from_bytes(data[:2], "big", signed=False)
data = data[2:]
assert len(data) == data_len
while data:
entry_type = data[0]
data = data[1:]
if entry_type == 0:
print("pre_agreed")
elif entry_type == 1:
sha1 = data[:20]
data = data[20:]
print("key_sha1_hash: %s" % sha1.hex())
elif entry_type == 2:
print("x509_name (not decoded yet)")
elif entry_type == 3:
sha1 = data[:20]
data = data[20:]
print("cert_sha1_hash: %s" % sha1.hex())


def trusted_ca_keys_decode_file(filename):
with open(filename, "rb") as fp:
trusted_ca_keys_decode(fp.read())


def trusted_ca_keys_decode_hex(hexstr):
trusted_ca_keys_decode(bytes.fromhex(hexstr))


# -----------------------------------------------------------------------------
if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument(
"--key",
action="store",
help="filename: Print sha1 hash of certificate public key",
)
parser.add_argument(
"--cert", action="store", help="filename: Print sha1 hash of certificate"
)
parser.add_argument(
"--file", action="store", help="filename: Parse trusted ca keys"
)
parser.add_argument(
"--hex", action="store", help="parse trusted ca keys hex string"
)

args = parser.parse_args()
if args.key:
certificate_key_hash(args.key)
if args.cert:
certificate_hash(args.cert)
if args.file:
trusted_ca_keys_decode_file(args.file)
if args.hex:
trusted_ca_keys_decode_hex(args.hex)

0 comments on commit b9ef35f

Please sign in to comment.