Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enforce mandatory CA in SET_CERTIFICATE #2850

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Enforce mandatory CA in SET_CERTIFICATE #2850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions include/library/spdm_crypt_lib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,10 @@ 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had better add some reason for the ex/non-ex version difference.
For example, the basic constrain is not checked in 1.2, but will be checked in 1.3.

*
* This function differs from libspdm_x509_set_cert_certificate_check_ex in that in SPDM 1.2 the
* BasicConstraints CA field is optional while in SPDM 1.3 and later it is mandatory.
*
* @param[in] cert Pointer to the DER-encoded certificate data.
* @param[in] cert_size The size of certificate data in bytes.
Expand All @@ -935,7 +938,10 @@ bool libspdm_x509_set_cert_certificate_check(const uint8_t *cert, size_t cert_si
bool is_requester, bool is_device_cert_model);

/**
* Certificate Check for SPDM leaf cert when set_cert. It is used for SPDM 1.3.
* Certificate Check for SPDM leaf cert when set_cert. It is used for SPDM 1.3 and later.
*
* This function differs from libspdm_x509_set_cert_certificate_check in that in SPDM 1.2 the
* BasicConstraints CA field is optional while in SPDM 1.3 and later it is mandatory.
*
* @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
View file
Open in desktop
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.
*
* @retval true verify pass,two case: 1.basic constraints is not present in cert;
* 2. cert basic_constraints CA is false;
* 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.
*
* @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
70 changes: 46 additions & 24 deletions library/spdm_responder_lib/libspdm_rsp_set_certificate.c
View file
Open in desktop
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)) {
Comment on lines +48 to +56
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks weird to only support 1.3 in ex version and only support 1.2 in non-ex version.
I feel it will be easily mis-used, if it is designed in such way.

But I don't have good idea now. Need to think through.

return false;
}
}

return true;
Expand All @@ -58,8 +70,10 @@ 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;
uint8_t set_cert_model;

size_t root_cert_hash_size;
const spdm_cert_chain_t *cert_chain_header;
Expand All @@ -69,21 +83,21 @@ libspdm_return_t libspdm_get_response_set_certificate(libspdm_context_t *spdm_co
libspdm_session_info_t *session_info;
libspdm_session_state_t session_state;

bool is_device_cert_model;

spdm_request = request;

/* -=[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,
response_size, response);
}

if (spdm_request->header.spdm_version != libspdm_get_connection_version(spdm_context)) {
if (spdm_request->header.spdm_version != spdm_version) {
return libspdm_generate_error_response(spdm_context,
SPDM_ERROR_CODE_VERSION_MISMATCH, 0,
response_size, response);
Expand Down Expand Up @@ -153,13 +167,13 @@ libspdm_return_t libspdm_get_response_set_certificate(libspdm_context_t *spdm_co
root_cert_hash_size = libspdm_get_hash_size(
spdm_context->connection_info.algorithm.base_hash_algo);

if (libspdm_get_connection_version(spdm_context) >= SPDM_MESSAGE_VERSION_13) {
const uint8_t set_cert_model =
if (spdm_version >= SPDM_MESSAGE_VERSION_13) {
const uint8_t key_pair_id = spdm_request->header.param2;

set_cert_model =
(spdm_request->header.param1 &
SPDM_SET_CERTIFICATE_REQUEST_ATTRIBUTES_CERT_MODEL_MASK) >>
SPDM_SET_CERTIFICATE_REQUEST_ATTRIBUTES_CERT_MODEL_OFFSET;
const uint8_t key_pair_id = spdm_request->header.param2;

erase = (spdm_request->header.param1 & SPDM_SET_CERTIFICATE_REQUEST_ATTRIBUTES_ERASE) != 0;

if (spdm_context->connection_info.multi_key_conn_rsp) {
Expand All @@ -182,10 +196,23 @@ libspdm_return_t libspdm_get_response_set_certificate(libspdm_context_t *spdm_co
SPDM_ERROR_CODE_INVALID_REQUEST, 0,
response_size, response);
}
if ((spdm_context->local_context.capability.flags &
SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_ALIAS_CERT_CAP) != 0) {
set_cert_model = SPDM_CERTIFICATE_INFO_CERT_MODEL_ALIAS_CERT;
} else {
set_cert_model = SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT;
}
}
} else {
if ((spdm_context->local_context.capability.flags &
SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_ALIAS_CERT_CAP) != 0) {
set_cert_model = SPDM_CERTIFICATE_INFO_CERT_MODEL_ALIAS_CERT;
} else {
set_cert_model = SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT;
}
}

if ((libspdm_get_connection_version(spdm_context) >= SPDM_MESSAGE_VERSION_13) && erase) {
if ((spdm_version >= SPDM_MESSAGE_VERSION_13) && erase) {
/*the CertChain field shall be absent;the value of SetCertModel shall be zero*/
if ((request_size < sizeof(spdm_set_certificate_request_t)) ||
((spdm_request->header.param1 &
Expand Down Expand Up @@ -237,18 +264,13 @@ libspdm_return_t libspdm_get_response_set_certificate(libspdm_context_t *spdm_co
cert_chain = (const void*)((const uint8_t *)cert_chain
+ sizeof(spdm_cert_chain_t) + root_cert_hash_size);

is_device_cert_model = false;
if((spdm_context->local_context.capability.flags &
SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_ALIAS_CERT_CAP) == 0) {
is_device_cert_model = true;
}

#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);
set_cert_model);
if (!result) {
return libspdm_generate_error_response(spdm_context,
SPDM_ERROR_CODE_UNSPECIFIED, 0,
Expand Down
Loading