diff options
author | Ed Tanous <ed@tanous.net> | 2023-12-06 02:57:46 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2023-12-09 01:59:39 +0300 |
commit | 23f1c96e6bc9060b54ff08a6b4d6cf8b8e0c3b23 (patch) | |
tree | 5772649c8a86823447d59083abf62fe09fdaad0d /http | |
parent | e79239970c3701f12903e8ac1574b9210b69aebc (diff) | |
download | bmcweb-23f1c96e6bc9060b54ff08a6b4d6cf8b8e0c3b23.tar.xz |
Simplify mutual TLS checks
bmcweb should be using the openssl primitives for these checks. There
are examples where we've known to have gotten the behavior incorrect, so
given that OpenSSL clearly should know these things better than we do,
use it.
Tested: unit tests pass.
Change-Id: I0bcd381a9e3c9a1e8e6dc39534e81fa698570689
Signed-off-by: Ed Tanous <ed@tanous.net>
Diffstat (limited to 'http')
-rw-r--r-- | http/mutual_tls.hpp | 68 |
1 files changed, 4 insertions, 64 deletions
diff --git a/http/mutual_tls.hpp b/http/mutual_tls.hpp index 9cd4cde01c..64bcd490d4 100644 --- a/http/mutual_tls.hpp +++ b/http/mutual_tls.hpp @@ -47,10 +47,10 @@ inline std::shared_ptr<persistent_data::UserSession> BMCWEB_LOG_INFO("Last TLS error is: {}", ctxError); return nullptr; } + // Check that we have reached final certificate in chain int32_t depth = X509_STORE_CTX_get_error_depth(cts); if (depth != 0) - { BMCWEB_LOG_DEBUG( "Certificate verification in progress (depth {}), waiting to reach final depth", @@ -60,73 +60,13 @@ inline std::shared_ptr<persistent_data::UserSession> BMCWEB_LOG_DEBUG("Certificate verification of final depth"); - // Verify KeyUsage - bool isKeyUsageDigitalSignature = false; - bool isKeyUsageKeyAgreement = false; - - ASN1_BIT_STRING* usage = static_cast<ASN1_BIT_STRING*>( - X509_get_ext_d2i(peerCert, NID_key_usage, nullptr, nullptr)); - - if ((usage == nullptr) || (usage->data == nullptr)) - { - BMCWEB_LOG_DEBUG("TLS usage is null"); - return nullptr; - } - - for (auto usageChar : - std::span(usage->data, static_cast<size_t>(usage->length))) - { - if (KU_DIGITAL_SIGNATURE & usageChar) - { - isKeyUsageDigitalSignature = true; - } - if (KU_KEY_AGREEMENT & usageChar) - { - isKeyUsageKeyAgreement = true; - } - } - ASN1_BIT_STRING_free(usage); - - if (!isKeyUsageDigitalSignature || !isKeyUsageKeyAgreement) - { - BMCWEB_LOG_DEBUG("Certificate ExtendedKeyUsage does " - "not allow provided certificate to " - "be used for user authentication"); - return nullptr; - } - - // Determine that ExtendedKeyUsage includes Client Auth - - stack_st_ASN1_OBJECT* extUsage = static_cast<stack_st_ASN1_OBJECT*>( - X509_get_ext_d2i(peerCert, NID_ext_key_usage, nullptr, nullptr)); - - if (extUsage == nullptr) + if (X509_check_purpose(peerCert, X509_PURPOSE_SSL_CLIENT, 0) != 1) { - BMCWEB_LOG_DEBUG("TLS extUsage is null"); + BMCWEB_LOG_DEBUG( + "Chain does not allow certificate to be used for SSL client authentication"); return nullptr; } - bool isExKeyUsageClientAuth = false; - for (int i = 0; i < sk_ASN1_OBJECT_num(extUsage); i++) - { - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) - int nid = OBJ_obj2nid(sk_ASN1_OBJECT_value(extUsage, i)); - if (NID_client_auth == nid) - { - isExKeyUsageClientAuth = true; - break; - } - } - sk_ASN1_OBJECT_free(extUsage); - - // Certificate has to have proper key usages set - if (!isExKeyUsageClientAuth) - { - BMCWEB_LOG_DEBUG("Certificate ExtendedKeyUsage does " - "not allow provided certificate to " - "be used for user authentication"); - return nullptr; - } std::string sslUser; // Extract username contained in CommonName sslUser.resize(256, '\0'); |