Skip to content

Commit

Permalink
[release/5.x] Cherry pick: Extend pathlen for CAs (#6662) (#6667)
Browse files Browse the repository at this point in the history
  • Loading branch information
achamayou authored Nov 20, 2024
1 parent 31764f4 commit b78fbba
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion src/crypto/openssl/key_pair.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions src/crypto/test/crypto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 1 addition & 1 deletion tests/e2e_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit b78fbba

Please sign in to comment.