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>
---
 test/http/mutual_tls.cpp | 86 +++++++++++++++++++-----------------------------
 1 file changed, 34 insertions(+), 52 deletions(-)

(limited to 'test')

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