From b78fbba021b07b3c94744ff18cf878dbc8d46432 Mon Sep 17 00:00:00 2001
From: Amaury Chamayou <amchamay@microsoft.com>
Date: Wed, 20 Nov 2024 09:06:57 +0000
Subject: [PATCH] [release/5.x] Cherry pick: Extend pathlen for CAs (#6662)
 (#6667)

---
 CHANGELOG.md                    |  4 ++++
 src/crypto/openssl/key_pair.cpp |  4 +++-
 src/crypto/test/crypto.cpp      | 41 +++++++++++++++++++++++++++++++++
 tests/e2e_logging.py            |  2 +-
 4 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4ca6f0cbaeb9..849811ab7fc1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 - Added OpenAPI support for `std::unordered_set`.
 
+### Changed
+
+- Service certificates and endorsements used for historical receipts now have a pathlen constraint of 1 instead of 0, reflecting the fact that there can be a single intermediate in endorsement chains. Historically the value had been 0, which happened to work because of a quirk in OpenSSL when Issuer and Subject match on an element in the chain.
+
 ### Fixed
 
 - Services upgrading from 4.x to 5.x may accidentally change their service's subject name, resulting in cryptographic errors when verifying anything endorsed by the old subject name. The subject name field is now correctly populated and retained across joins, renewals, and disaster recoveries.
diff --git a/src/crypto/openssl/key_pair.cpp b/src/crypto/openssl/key_pair.cpp
index 7bbf3e7a3c2e..a992c76581bb 100644
--- a/src/crypto/openssl/key_pair.cpp
+++ b/src/crypto/openssl/key_pair.cpp
@@ -382,7 +382,9 @@ namespace ccf::crypto
     std::string constraints = "critical,CA:FALSE";
     if (ca)
     {
-      constraints = "critical,CA:TRUE,pathlen:0";
+      // 1 to allow for intermediate CAs with a different subject name,
+      // which can occur in service endorsements of some services.
+      constraints = "critical,CA:TRUE,pathlen:1";
     }
 
     // Add basic constraints
diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp
index c5c7b1c0afbf..4aeed621013e 100644
--- a/src/crypto/test/crypto.cpp
+++ b/src/crypto/test/crypto.cpp
@@ -1292,3 +1292,44 @@ TEST_CASE("COSE sign & verify")
 
   REQUIRE(cose_verifier->verify_detached(cose_sign, {}));
 }
+
+TEST_CASE("Sign and verify a chain with an intermediate and different subjects")
+{
+  auto root_kp = ccf::crypto::make_key_pair(CurveID::SECP384R1);
+  auto root_cert = generate_self_signed_cert(root_kp, "CN=root");
+
+  auto intermediate_kp = ccf::crypto::make_key_pair(CurveID::SECP384R1);
+  auto intermediate_csr = intermediate_kp->create_csr("CN=intermediate", {});
+
+  std::string valid_from = "20210311000000Z";
+  std::string valid_to = "20230611235959Z";
+  auto intermediate_cert =
+    root_kp->sign_csr(root_cert, intermediate_csr, valid_from, valid_to, true);
+
+  auto leaf_kp = ccf::crypto::make_key_pair(CurveID::SECP384R1);
+  auto leaf_csr = leaf_kp->create_csr("CN=leaf", {});
+  auto leaf_cert = intermediate_kp->sign_csr(
+    intermediate_cert, leaf_csr, valid_from, valid_to, true);
+
+  auto verifier = ccf::crypto::make_verifier(leaf_cert.raw());
+  auto rc = verifier->verify_certificate(
+    {&root_cert}, {&intermediate_cert}, true /* ignore time */
+  );
+
+  // Failed with pathlen: 0
+  REQUIRE(rc);
+
+  // Missing intermediate
+  rc = verifier->verify_certificate(
+    {&root_cert}, {}, true /* ignore time */
+  );
+
+  REQUIRE(!rc);
+
+  // Invalid root
+  rc = verifier->verify_certificate(
+    {&leaf_cert}, {}, true /* ignore time */
+  );
+
+  REQUIRE(!rc);
+}
\ No newline at end of file
diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py
index 64758d9821c3..fb5b48ecdb00 100644
--- a/tests/e2e_logging.py
+++ b/tests/e2e_logging.py
@@ -1797,7 +1797,7 @@ def test_basic_constraints(network, args):
     )
     assert basic_constraints.critical is True
     assert basic_constraints.value.ca is True
-    assert basic_constraints.value.path_length == 0
+    assert basic_constraints.value.path_length == 1
 
     node_pem = primary.get_tls_certificate_pem()
     node_cert = load_pem_x509_certificate(node_pem.encode(), default_backend())