commit 23f1c96e6bc9060b54ff08a6b4d6cf8b8e0c3b23
author Ed Tanous <ed@tanous.net> Tue Dec 05 15:57:46 2023 -0800
committer Ed Tanous <ed@tanous.net> Fri Dec 08 22:59:39 2023 +0000
tree 5772649c8a86823447d59083abf62fe09fdaad0d
parent e79239970c3701f12903e8ac1574b9210b69aebc

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

diff --git a/test/http/mutual_tls.cpp b/test/http/mutual_tls.cpp
index b1b7878..7b5cb25 100644
--- a/test/http/mutual_tls.cpp
+++ b/test/http/mutual_tls.cpp
@@ -25,6 +25,32 @@
     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 @@
 {
     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 @@
     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 @@
     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 @@
         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 @@
         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;