From 23f1c96e6bc9060b54ff08a6b4d6cf8b8e0c3b23 Mon Sep 17 00:00:00 2001
From: Ed Tanous <ed@tanous.net>
Date: Tue, 5 Dec 2023 15:57:46 -0800
Subject: 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>
---
 http/mutual_tls.hpp | 68 ++++-------------------------------------------------
 1 file changed, 4 insertions(+), 64 deletions(-)

(limited to 'http')

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');
-- 
cgit v1.2.3