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>

2 files changed, 38 insertions, 116 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');
diff --git a/test/http/mutual_tls.cpp b/test/http/mutual_tls.cpp
index b1b7878586..7b5cb25acd 100644
--- a/test/http/mutual_tls.cpp
+++ b/test/http/mutual_tls.cpp
@@ -25,6 +25,32 @@ class OSSLX509
     OSSLX509(OSSLX509&&) = delete;
 
     OSSLX509() = default;
+
+    void setSubjectName()
+    {
+        X509_NAME* name = X509_get_subject_name(ptr);
+        std::array<unsigned char, 5> user = {'u', 's', 'e', 'r', '\0'};
+        X509_NAME_add_entry_by_txt(name, "CN", MBSTRING_ASC, user.data(), -1,
+                                   -1, 0);
+    }
+    void sign()
+    {
+        // Generate test key
+        EVP_PKEY* pkey = nullptr;
+        EVP_PKEY_CTX* pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, nullptr);
+        ASSERT_EQ(EVP_PKEY_keygen_init(pctx), 1);
+        ASSERT_EQ(
+            EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx, NID_X9_62_prime256v1),
+            1);
+        ASSERT_EQ(EVP_PKEY_keygen(pctx, &pkey), 1);
+        EVP_PKEY_CTX_free(pctx);
+
+        // Sign cert with key
+        ASSERT_EQ(X509_set_pubkey(ptr, pkey), 1);
+        ASSERT_GT(X509_sign(ptr, pkey, EVP_sha256()), 0);
+        EVP_PKEY_free(pkey);
+    }
+
     X509* get()
     {
         return ptr;
@@ -61,11 +87,7 @@ TEST(MutualTLS, GoodCert)
 {
     OSSLX509 x509;
 
-    X509_NAME* name = X509_get_subject_name(x509.get());
-    std::array<unsigned char, 5> user = {'u', 's', 'e', 'r', '\0'};
-    X509_NAME_add_entry_by_txt(name, "CN", MBSTRING_ASC, user.data(), -1, -1,
-                               0);
-
+    x509.setSubjectName();
     X509_EXTENSION* ex = X509V3_EXT_conf_nid(nullptr, nullptr, NID_key_usage,
                                              "digitalSignature, keyAgreement");
     ASSERT_THAT(ex, NotNull());
@@ -76,6 +98,8 @@ TEST(MutualTLS, GoodCert)
     ASSERT_EQ(X509_add_ext(x509.get(), ex, -1), 1);
     X509_EXTENSION_free(ex);
 
+    x509.sign();
+
     OSSLX509StoreCTX x509Store;
     X509_STORE_CTX_set_current_cert(x509Store.get(), x509.get());
 
@@ -87,35 +111,13 @@ TEST(MutualTLS, GoodCert)
     EXPECT_THAT(session->username, "user");
 }
 
-TEST(MutualTLS, MissingSubject)
-{
-    OSSLX509 x509;
-
-    X509_EXTENSION* ex = X509V3_EXT_conf_nid(nullptr, nullptr, NID_key_usage,
-                                             "digitalSignature, keyAgreement");
-    ASSERT_THAT(ex, NotNull());
-    ASSERT_EQ(X509_add_ext(x509.get(), ex, -1), 1);
-    X509_EXTENSION_free(ex);
-    ex = X509V3_EXT_conf_nid(nullptr, nullptr, NID_ext_key_usage, "clientAuth");
-    ASSERT_THAT(ex, NotNull());
-    ASSERT_EQ(X509_add_ext(x509.get(), ex, -1), 1);
-    X509_EXTENSION_free(ex);
-
-    OSSLX509StoreCTX x509Store;
-    X509_STORE_CTX_set_current_cert(x509Store.get(), x509.get());
-
-    boost::asio::ip::address ip;
-    boost::asio::ssl::verify_context ctx(x509Store.get());
-    std::shared_ptr<persistent_data::UserSession> session = verifyMtlsUser(ip,
-                                                                           ctx);
-    ASSERT_THAT(session, IsNull());
-}
-
 TEST(MutualTLS, MissingKeyUsage)
 {
-    for (const char* usageString : {"digitalSignature", "keyAgreement"})
+    for (const char* usageString :
+         {"digitalSignature", "keyAgreement", "digitalSignature, keyAgreement"})
     {
         OSSLX509 x509;
+        x509.setSubjectName();
 
         X509_EXTENSION* ex = X509V3_EXT_conf_nid(nullptr, nullptr,
                                                  NID_key_usage, usageString);
@@ -128,6 +130,7 @@ TEST(MutualTLS, MissingKeyUsage)
         ASSERT_THAT(ex, NotNull());
         ASSERT_EQ(X509_add_ext(x509.get(), ex, -1), 1);
         X509_EXTENSION_free(ex);
+        x509.sign();
 
         OSSLX509StoreCTX x509Store;
         X509_STORE_CTX_set_current_cert(x509Store.get(), x509.get());
@@ -136,31 +139,10 @@ TEST(MutualTLS, MissingKeyUsage)
         boost::asio::ssl::verify_context ctx(x509Store.get());
         std::shared_ptr<persistent_data::UserSession> session =
             verifyMtlsUser(ip, ctx);
-        ASSERT_THAT(session, IsNull());
+        ASSERT_THAT(session, NotNull());
     }
 }
 
-TEST(MutualTLS, MissingExtKeyUsage)
-{
-    OSSLX509 x509;
-
-    X509_EXTENSION* ex = X509V3_EXT_conf_nid(nullptr, nullptr, NID_key_usage,
-                                             "digitalSignature, keyAgreement");
-
-    ASSERT_THAT(ex, NotNull());
-    ASSERT_EQ(X509_add_ext(x509.get(), ex, -1), 1);
-    X509_EXTENSION_free(ex);
-
-    OSSLX509StoreCTX x509Store;
-    X509_STORE_CTX_set_current_cert(x509Store.get(), x509.get());
-
-    boost::asio::ip::address ip;
-    boost::asio::ssl::verify_context ctx(x509Store.get());
-    std::shared_ptr<persistent_data::UserSession> session = verifyMtlsUser(ip,
-                                                                           ctx);
-    ASSERT_THAT(session, IsNull());
-}
-
 TEST(MutualTLS, MissingCert)
 {
     OSSLX509StoreCTX x509Store;