Merge pull request #2644 from nkumar04/nkumar/BC-20423

Added utility to parse subject field from tls cert bundle

Author: nkumar04

client/clientservice/src/configuration.cpp

@@ -20,6 +20,7 @@
 #include "secrets_manager_enc.h"
 #include "secrets_manager_plain.h"
 #include "client/clientservice/client_service.hpp"
+#include "crypto_utils.hpp"
 
 using concord::client::concordclient::ConcordClientConfig;
 using concord::client::concordclient::StateSnapshotConfig;
@@ -293,32 +294,8 @@ void readCert(const std::string& input_filename, std::string& out_data) {
 }
 
 std::string getClientIdFromClientCert(const std::string& client_cert_path, bool use_unified_certs) {
-  std::array<char, 128> buffer;
-  std::string client_id;
-  std::string delimiter;
-
-  // check if client cert can be opened
-  std::ifstream input_file(client_cert_path.c_str(), std::ios::in);
-
-  if (!input_file.is_open()) {
-    throw std::runtime_error("Could not open the input file (" + client_cert_path + ") at the concord client.");
-  }
-
-  // The cmd string is used to get the subject in the client cert.
-  std::string cmd =
-      "openssl crl2pkcs7 -nocrl -certfile " + client_cert_path + " | openssl pkcs7 -print_certs -noout | grep .";
-  std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd.c_str(), "r"), pclose);
-  if (!pipe) {
-    throw std::runtime_error("Failed to read subject fields from client cert - popen() failed!");
-  }
-
-  // parse the O field i.e., the client id from the subject field when
-  // unified certificates are used, else parse OU field.
-  delimiter = (use_unified_certs) ? "O = " : "OU = ";
-  if (fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr) {
-    client_id = parseClientIdFromSubject(buffer.data(), delimiter);
-  }
-  return client_id;
+  auto field_name = (use_unified_certs ? "O" : "OU");
+  return util::crypto::CertificateUtils::getSubjectFieldByName(client_cert_path, field_name);
 }
 
 // Parses the value of the OU field i.e., the client id from the subject

thin-replica-server/include/thin-replica-server/thin_replica_impl.hpp

@@ -36,6 +36,8 @@
 #include "kv_types.hpp"
 #include "kvbc_app_filter/kvbc_app_filter.h"
 #include "kvbc_app_filter/kvbc_key_types.h"
+#include "crypto_utils.hpp"
+
 #include "thin_replica.grpc.pb.h"
 #include "subscription_buffer.hpp"
 #include "trs_metrics.hpp"
@@ -118,9 +120,6 @@ class ThinReplicaImpl {
   // last timestamp when subscription status for live updates was not ok
   std::optional<std::chrono::steady_clock::time_point> last_failed_subscribe_status_time;
 
-  // Max Subject length in certificates should be 555 bytes with ascii characters
-  static const uint16_t certSubjectLength{555};
-
  public:
   ThinReplicaImpl(std::unique_ptr<ThinReplicaServerConfig> config,
                   std::shared_ptr<concordMetrics::Aggregator> aggregator)
@@ -700,29 +699,10 @@ class ThinReplicaImpl {
                                       const std::string& root_cert_path,
                                       std::unordered_set<std::string>& cert_ou_field_set,
                                       bool use_unified_certs) {
-    std::array<char, 128> buffer;
-    std::string result;
-    std::string delimiter;
-    // Openssl doesn't provide a method to fetch all the x509 certificates
-    // directly from a bundled cert, due to the assumption of one certificate
-    // per file. But for some reason openssl supports displaying multiple certs
-    // from a pkcs7 file. So we generate an intermediate pkcs7 file using
-    // crl2pkcs7 openssl command to get the subject fields of all the certs from
-    // the bundled root cert.
-    std::string cmd =
-        "openssl crl2pkcs7 -nocrl -certfile " + root_cert_path + " | openssl pkcs7 -print_certs -noout | grep .";
-    std::unique_ptr<FILE, decltype(&pclose)> pipe_ptr(popen(cmd.c_str(), "r"), pclose);
-    if (!pipe_ptr) {
-      LOG_ERROR(logger_, "Failed to read from root cert - popen() failed, error: " << strerror(errno));
-      throw std::runtime_error("Failed to read from root cert - popen() failed!");
-    }
-
-    delimiter = (use_unified_certs) ? "O = " : "OU = ";
-
-    while (fgets(buffer.data(), buffer.size(), pipe_ptr.get()) != nullptr) {
-      result = buffer.data();
-      // parse the client id from the subject field
-      cert_ou_field_set.insert(parseClientIdFromSubject(result, delimiter));
+    auto field_name = (use_unified_certs ? "O" : "OU");
+    auto attribute_list = util::crypto::CertificateUtils::getSubjectFieldListByName(root_cert_path, field_name);
+    for (auto& val : attribute_list) {
+      cert_ou_field_set.emplace(std::move(val));
     }
   }
 
@@ -737,30 +717,8 @@ class ThinReplicaImpl {
   }
 
   static std::string getClientIdFromCertificate(const std::string& client_cert_path, bool use_unified_certs = false) {
-    std::array<char, certSubjectLength> buffer;
-    std::string client_id;
-    std::string delimiter;
-    // check if client cert can be opened
-    std::ifstream input_file(client_cert_path.c_str(), std::ios::in);
-
-    if (!input_file.is_open()) {
-      throw std::runtime_error("Could not open the input file at path (" + client_cert_path + ")");
-    }
-
-    // The cmd string is used to get the subject in the client cert.
-    const std::string cmd =
-        "openssl crl2pkcs7 -nocrl -certfile " + client_cert_path + " | openssl pkcs7 -print_certs -noout | grep .";
-    std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd.c_str(), "r"), pclose);
-    if (!pipe) {
-      throw std::runtime_error("Failed to read subject fields from client cert - popen() failed!");
-    }
-
-    delimiter = (use_unified_certs) ? "O = " : "OU = ";
-    if (fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr) {
-      // parse the client id from the subject field
-      client_id = parseClientIdFromSubject(buffer.data(), delimiter);
-    }
-    return client_id;
+    auto field_name = (use_unified_certs ? "O" : "OU");
+    return util::crypto::CertificateUtils::getSubjectFieldByName(client_cert_path, field_name);
   }
 
  private:

util/include/crypto_utils.hpp

@@ -15,6 +15,7 @@
 #include <utility>
 #include <string>
 #include <memory>
+#include <vector>
 
 #include <openssl/bio.h>
 #include <openssl/ec.h>
@@ -37,6 +38,11 @@ class CertificateUtils {
                                 uint32_t& remote_peer_id,
                                 std::string& conn_type,
                                 bool use_unified_certs);
+  // valid field_name: "C"/"L"/"ST"/"O"/"OU"/"CN"
+  static std::string getSubjectFieldByName(const std::string& cert_path, const std::string& attribute_name);
+  // This function accepts path to a cert bundle
+  static std::vector<std::string> getSubjectFieldListByName(const std::string& cert_bundle_path,
+                                                            const std::string& attribute_name);
 };
 class IVerifier {
  public:

util/src/crypto_utils.cpp

@@ -28,9 +28,17 @@
 #include <openssl/evp.h>
 #include <regex>
 #include "Logger.hpp"
+#include <unordered_map>
 
 using namespace CryptoPP;
 namespace concord::util::crypto {
+
+const std::unordered_map<std::string, int> name_to_id_map = {{"C", NID_countryName},
+                                                             {"L", NID_localityName},
+                                                             {"ST", NID_stateOrProvinceName},
+                                                             {"O", NID_organizationName},
+                                                             {"OU", NID_organizationalUnitName},
+                                                             {"CN", NID_commonName}};
 class ECDSAVerifier::Impl {
   std::unique_ptr<ECDSA<ECP, CryptoPP::SHA256>::Verifier> verifier_;
 
@@ -449,4 +457,76 @@ bool CertificateUtils::verifyCertificate(X509* cert, const std::string& public_k
   BIO_free(pub_bio);
   return (bool)r;
 }
+
+std::string CertificateUtils::getSubjectFieldByName(const std::string& cert_path, const std::string& attribute_name) {
+  if (name_to_id_map.find(attribute_name) == name_to_id_map.end()) {
+    LOG_ERROR(GL, "Invalid attribute name: " << attribute_name);
+    return std::string{};
+  }
+  const auto& nid = name_to_id_map.at(attribute_name);
+  auto deleter = [](FILE* fp) {
+    if (fp) fclose(fp);
+  };
+  char buf[1024];
+  std::unique_ptr<FILE, decltype(deleter)> fp(fopen(cert_path.c_str(), "r"), deleter);
+  if (!fp) {
+    LOG_ERROR(GL, "Certificate file not found, path: " << cert_path);
+    return std::string();
+  }
+
+  X509* cert = PEM_read_X509(fp.get(), NULL, NULL, NULL);
+  if (!cert) {
+    LOG_ERROR(GL, "Cannot parse certificate, path: " << cert_path);
+    return std::string();
+  }
+  // The returned value of X509_get_subject_name is an internal pointer
+  // which MUST NOT be freed.
+  X509_NAME* name = X509_get_subject_name(cert);
+  auto name_len = X509_NAME_get_text_by_NID(name, nid, buf, sizeof(buf));
+  if (name_len == -1 || name_len == -2) {
+    LOG_ERROR(GL, "name entry not found or invalid. error_code:" << name_len);
+    X509_free(cert);
+    return std::string{};
+  }
+  X509_free(cert);
+  return std::string(buf);
+}
+
+std::vector<std::string> CertificateUtils::getSubjectFieldListByName(const std::string& cert_bundle_path,
+                                                                     const std::string& attribute_name) {
+  auto attribute_list = std::vector<std::string>{};
+  X509_STORE* store = NULL;
+  if (name_to_id_map.find(attribute_name) == name_to_id_map.end()) {
+    LOG_ERROR(GL, "Invalid attribute name: " << attribute_name);
+    return {};
+  }
+  const auto& nid = name_to_id_map.at(attribute_name);
+  char buf[1024];
+  if (!(store = X509_STORE_new())) {
+    LOG_ERROR(GL, "Error creating X509_STORE_CTX object\n");
+    return {};
+  }
+  auto ret = X509_STORE_load_locations(store, cert_bundle_path.c_str(), NULL);
+  if (ret != 1) {
+    LOG_ERROR(GL, "Error loading CA cert or chain file\n");
+    return {};
+  }
+  auto objs = X509_STORE_get0_objects(store);
+  for (int i = 0; i < sk_X509_OBJECT_num(objs); i++) {
+    X509_OBJECT* x509_obj = sk_X509_OBJECT_value(objs, i);
+    if (x509_obj) {
+      X509* cert = X509_OBJECT_get0_X509(x509_obj);
+      if (cert) {
+        X509_NAME* name = X509_get_subject_name(cert);
+        auto name_len = X509_NAME_get_text_by_NID(name, nid, buf, sizeof(buf));
+        if (name_len == -1 || name_len == -2) {
+          continue;
+        }
+        attribute_list.emplace_back(std::string(buf));
+      }
+    }
+  }
+  X509_STORE_free(store);
+  return attribute_list;
+}
 }  // namespace concord::util::crypto

util/test/CMakeLists.txt

@@ -77,7 +77,7 @@ target_link_libraries(RawMemoryPool_test GTest::Main util)
 
 add_executable(crypto_utils_test crypto_utils_test.cpp )
 add_test(crypto_utils_test crypto_utils_test)
-target_link_libraries(crypto_utils_test GTest::Main util)
+target_link_libraries(crypto_utils_test stdc++fs GTest::Main util)
 
 add_executable(synchronized_value_test synchronized_value_test.cpp)
 add_test(synchronized_value_test synchronized_value_test)