Skip to content

Commit

Permalink
Enforce mandatory CA in SET_CERTIFICATE
Browse files Browse the repository at this point in the history
Fix #2731.

Signed-off-by: Steven Bellock <sbellock@nvidia.com>
  • Loading branch information
steven-bellock committed Sep 24, 2024
1 parent 98d4a3a commit 5cab1dc
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 72 deletions.
2 changes: 1 addition & 1 deletion include/library/spdm_crypt_lib.h
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ bool libspdm_x509_certificate_check_ex(const uint8_t *cert, size_t cert_size,
uint32_t base_asym_algo, uint32_t base_hash_algo,
bool is_requester, uint8_t cert_model);
/**
* Certificate Check for SPDM leaf cert when set_cert.
* Certificate Check for SPDM leaf cert when set_cert. It is used for SPDM 1.2.
*
* @param[in] cert Pointer to the DER-encoded certificate data.
* @param[in] cert_size The size of certificate data in bytes.
Expand Down
111 changes: 49 additions & 62 deletions library/spdm_crypt_lib/libspdm_crypt_cert.c
Original file line number Diff line number Diff line change
Expand Up @@ -789,20 +789,31 @@ static bool libspdm_verify_leaf_cert_basic_constraints(const uint8_t *cert, size
}

/**
* Verify leaf cert basic_constraints CA is false
* Verify leaf certificate basic_constraints CA is correct for set certificate.
*
* @param[in] cert Pointer to the DER-encoded certificate data.
* @param[in] cert_size The size of certificate data in bytes.
* For SPDM 1.2
* - If certificate model is DeviceCert and CA is present then CA must be false.
* - If certificate model is AliasCert and CA is present then CA must be true.
* For SPDM 1.3 and up, CA must be present and
* - If certificate model is DeviceCert or GenericCert then CA must be false.
* - If certificate model is AliasCert then CA must be true.
*
* @retval true verify pass,two case: 1.basic constraints is not present in cert;
* 2. cert basic_constraints CA is false;
* @param[in] cert Pointer to the DER-encoded certificate data.
* @param[in] cert_size The size of certificate data in bytes.
* @param[in] cert_model The certificate model.
* @param[in] need_basic_constraints This value indicates whether basic_constraints must be present
* in the certificate.
*
* @retval true verify pass 1. basic_constraints is not present when allowed.
* 2. basic_constraints is present and correct.
* @retval false verify fail
**/
static bool libspdm_verify_set_cert_leaf_cert_basic_constraints(
const uint8_t *cert, size_t cert_size, uint8_t cert_model)
const uint8_t *cert, size_t cert_size, uint8_t cert_model, bool need_basic_constraints)
{
bool status;
/*basic_constraints from cert*/
/* basic_constraints from certificate. */
uint8_t cert_basic_constraints[LIBSPDM_MAX_BASIC_CONSTRAINTS_CA_LEN];
size_t len;

Expand All @@ -814,38 +825,40 @@ static bool libspdm_verify_set_cert_leaf_cert_basic_constraints(

status = libspdm_x509_get_extended_basic_constraints(cert, cert_size,
cert_basic_constraints, &len);
if (cert_model == SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT) {
/*device cert model*/
if (!status) {
return false;
} else if (len == 0) {
/* basic constraints is not present in cert */
return true;
}
if (!status) {
return false;
} else if (need_basic_constraints && (len == 0)) {
return false;
}

if ((len == sizeof(basic_constraints_false_case1)) &&
(libspdm_consttime_is_mem_equal(cert_basic_constraints,
basic_constraints_false_case1,
sizeof(basic_constraints_false_case1)))) {
return true;
}
if ((cert_model == SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT) ||
(cert_model == SPDM_CERTIFICATE_INFO_CERT_MODEL_GENERIC_CERT)) {
if (need_basic_constraints || (len != 0)) {
if ((len == sizeof(basic_constraints_false_case1)) &&
(libspdm_consttime_is_mem_equal(cert_basic_constraints,
basic_constraints_false_case1,
sizeof(basic_constraints_false_case1)))) {
return true;
}

if ((len == sizeof(basic_constraints_false_case2)) &&
(libspdm_consttime_is_mem_equal(cert_basic_constraints,
basic_constraints_false_case2,
sizeof(basic_constraints_false_case2)))) {
return true;
if ((len == sizeof(basic_constraints_false_case2)) &&
(libspdm_consttime_is_mem_equal(cert_basic_constraints,
basic_constraints_false_case2,
sizeof(basic_constraints_false_case2)))) {
return true;
}
}
} else {
/*alias cert model*/
if (status && (len == sizeof(basic_constraints_true_case)) &&
(libspdm_consttime_is_mem_equal(cert_basic_constraints,
basic_constraints_true_case,
sizeof(basic_constraints_true_case)))) {
return true;
/* Alias certificate model. */
if (need_basic_constraints || (len != 0)) {
if ((len == sizeof(basic_constraints_true_case)) &&
(libspdm_consttime_is_mem_equal(cert_basic_constraints,
basic_constraints_true_case,
sizeof(basic_constraints_true_case)))) {
return true;
}
}
}

return false;
}

Expand Down Expand Up @@ -1303,20 +1316,6 @@ bool libspdm_x509_certificate_check_ex(const uint8_t *cert, size_t cert_size,
return status;
}

/**
* Certificate Check for SPDM leaf cert when set_cert command.
*
* @param[in] cert Pointer to the DER-encoded certificate data.
* @param[in] cert_size The size of certificate data in bytes.
* @param[in] base_asym_algo SPDM base_asym_algo
* @param[in] base_hash_algo SPDM base_hash_algo
* @param[in] is_requester Is the function verifying a cert as a requester or responder.
* @param[in] is_device_cert_model If true, the local endpoint uses the DeviceCert model.
* If false, the local endpoint uses the AliasCert model.
*
* @retval true Success.
* @retval false Certificate is not valid.
**/
bool libspdm_x509_set_cert_certificate_check(const uint8_t *cert, size_t cert_size,
uint32_t base_asym_algo, uint32_t base_hash_algo,
bool is_requester, bool is_device_cert_model)
Expand All @@ -1339,23 +1338,10 @@ bool libspdm_x509_set_cert_certificate_check(const uint8_t *cert, size_t cert_si

/* verify basic constraints: need check with is_device_cert_model*/
status = libspdm_verify_set_cert_leaf_cert_basic_constraints(
cert, cert_size, cert_model);
cert, cert_size, cert_model, false);
return status;
}

/**
* Certificate Check for SPDM leaf cert when set_cert. It is used for SPDM 1.3.
*
* @param[in] cert Pointer to the DER-encoded certificate data.
* @param[in] cert_size The size of certificate data in bytes.
* @param[in] base_asym_algo SPDM base_asym_algo
* @param[in] base_hash_algo SPDM base_hash_algo
* @param[in] is_requester Is the function verifying a cert as a requester or responder.
* @param[in] cert_model One of the SPDM_CERTIFICATE_INFO_CERT_MODEL_* macros.
*
* @retval true Success.
* @retval false Certificate is not valid.
**/
bool libspdm_x509_set_cert_certificate_check_ex(const uint8_t *cert, size_t cert_size,
uint32_t base_asym_algo, uint32_t base_hash_algo,
bool is_requester, uint8_t cert_model)
Expand All @@ -1371,7 +1357,8 @@ bool libspdm_x509_set_cert_certificate_check_ex(const uint8_t *cert, size_t cert

/* verify basic constraints: need check with is_device_cert_model*/
status = libspdm_verify_set_cert_leaf_cert_basic_constraints(
cert, cert_size, cert_model);
cert, cert_size, cert_model, true);

return status;
}

Expand Down
34 changes: 25 additions & 9 deletions library/spdm_responder_lib/libspdm_rsp_set_certificate.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@

#if LIBSPDM_CERT_PARSE_SUPPORT
/*set_cert verify cert_chain*/
static bool libspdm_set_cert_verify_certchain(const uint8_t *cert_chain, size_t cert_chain_size,
static bool libspdm_set_cert_verify_certchain(uint8_t spdm_version,
const uint8_t *cert_chain, size_t cert_chain_size,
uint32_t base_asym_algo, uint32_t base_hash_algo,
bool is_device_cert_model)
uint8_t cert_model)
{
const uint8_t *root_cert_buffer;
size_t root_cert_buffer_size;
Expand All @@ -39,11 +40,22 @@ static bool libspdm_set_cert_verify_certchain(const uint8_t *cert_chain, size_t
return false;
}

/*verify leaf cert*/
if (!libspdm_x509_set_cert_certificate_check(leaf_cert_buffer, leaf_cert_buffer_size,
base_asym_algo, base_hash_algo,
false, is_device_cert_model)) {
return false;
if (spdm_version == SPDM_MESSAGE_VERSION_12) {
const bool is_device_cert_model =
(cert_model == SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);

/*verify leaf cert*/
if (!libspdm_x509_set_cert_certificate_check(leaf_cert_buffer, leaf_cert_buffer_size,
base_asym_algo, base_hash_algo,
false, is_device_cert_model)) {
return false;
}
} else {
if (!libspdm_x509_set_cert_certificate_check_ex(leaf_cert_buffer, leaf_cert_buffer_size,
base_asym_algo, base_hash_algo,
false, cert_model)) {
return false;
}
}

return true;
Expand All @@ -58,6 +70,7 @@ libspdm_return_t libspdm_get_response_set_certificate(libspdm_context_t *spdm_co
spdm_set_certificate_response_t *spdm_response;

bool result;
uint8_t spdm_version;
uint8_t slot_id;
bool erase;

Expand All @@ -76,7 +89,9 @@ libspdm_return_t libspdm_get_response_set_certificate(libspdm_context_t *spdm_co
/* -=[Check Parameters Phase]=- */
LIBSPDM_ASSERT(spdm_request->header.request_response_code == SPDM_SET_CERTIFICATE);

if (libspdm_get_connection_version(spdm_context) < SPDM_MESSAGE_VERSION_12) {
spdm_version = libspdm_get_connection_version(spdm_context);

if (spdm_version < SPDM_MESSAGE_VERSION_12) {
return libspdm_generate_error_response(spdm_context,
SPDM_ERROR_CODE_UNSUPPORTED_REQUEST,
SPDM_SET_CERTIFICATE,
Expand Down Expand Up @@ -245,7 +260,8 @@ libspdm_return_t libspdm_get_response_set_certificate(libspdm_context_t *spdm_co

#if LIBSPDM_CERT_PARSE_SUPPORT
/*check the cert_chain*/
result = libspdm_set_cert_verify_certchain(cert_chain, cert_chain_size,
result = libspdm_set_cert_verify_certchain(spdm_version,
cert_chain, cert_chain_size,
spdm_context->connection_info.algorithm.base_asym_algo,
spdm_context->connection_info.algorithm.base_hash_algo,
is_device_cert_model);
Expand Down

0 comments on commit 5cab1dc

Please sign in to comment.