Validate per-slot DIGESTS consistency - #3717
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds responder-side validation to ensure multi-key DIGESTS per-slot fields (SupportedSlotMask, KeyPairID, CertificateInfo, KeyUsageMask) are internally consistent and (when available) consistent with KEY_PAIR_INFO, addressing the class of misconfigurations described in issue #3638.
Changes:
- Added
libspdm_validate_supported_slot_mask()andlibspdm_validate_multi_key_slot_info()and invoked them from the responder DIGESTS path prior to emitting the response. - Implemented validation checks for CertificateInfo/KeyUsageMask bit validity and slot-0 KeyUsageMask constraints; added KEY_PAIR_INFO cross-checks when enabled.
- Expanded responder unit tests with new cases covering supported-slot-mask and multi-key slot-info validation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| library/spdm_responder_lib/libspdm_rsp_digests.c | Adds validation helpers and enforces them in the DIGESTS responder before response emission. |
| include/internal/libspdm_responder_lib.h | Declares new internal validation APIs for use within responder code. |
| unit_test/test_spdm_responder/digests.c | Extends DIGESTS responder tests to cover the new validation rules and error paths. |
Comments suppressed due to low confidence (1)
unit_test/test_spdm_responder/digests.c:507
- This test provisions all slots but does not set
local_supported_slot_mask, so DIGESTS will advertise SupportedSlotMask=0 even though every slot is populated; it will also fail if SupportedSlotMask validation is made strict (SupportedSlotMask covers ProvisionedSlotMask).
for (uint8_t index = 0; index < SPDM_MAX_SLOT_COUNT; index++) {
spdm_context->local_context.local_cert_chain_provision[index] =
&m_libspdm_local_certificate_chain[hash_size *index];
spdm_context->local_context.local_cert_chain_provision_size[index] = hash_size;
/* A populated multi-key slot shall report a non-zero CertModel. */
spdm_context->local_context.local_cert_info[index] =
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT;
}
/* Slot 0 shall set at least one usage bit. */
spdm_context->local_context.local_key_usage_bit_mask[0] =
SPDM_KEY_USAGE_BIT_MASK_KEY_EX_USE;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6273bea to
f3c54ce
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
library/spdm_responder_lib/libspdm_rsp_digests.c:306
- libspdm_get_response_digests() validates SupportedSlotMask and multi-key per-slot fields even when the negotiated/request SPDM version is < 1.3, but those fields are only emitted for SPDM 1.3+. This changes behavior for 1.0–1.2 sessions (and can cause an UNSPECIFIED error due to a missing local_supported_slot_mask / cert_info / key_usage provisioning that would never be sent on the wire). Gate these validations on spdm_request->header.spdm_version >= SPDM_MESSAGE_VERSION_13 so the checks run only when the fields can be emitted.
/* The reported per-slot values are provisioned by the integrator; verify they are internally
* consistent before emitting them. A failure here is a local misconfiguration. */
status = libspdm_validate_supported_slot_mask(spdm_context);
LIBSPDM_ASSERT(!LIBSPDM_STATUS_IS_ERROR(status));
if (LIBSPDM_STATUS_IS_ERROR(status)) {
return libspdm_generate_error_response(
spdm_context, SPDM_ERROR_CODE_UNSPECIFIED, 0, response_size, response);
}
/* The per-slot KeyPairID, CertificateInfo and KeyUsageMask fields are only emitted for a
* multi-key connection, so there is nothing to validate otherwise. */
if (spdm_context->connection_info.multi_key_conn_rsp) {
status = libspdm_validate_multi_key_slot_info(spdm_context);
LIBSPDM_ASSERT(!LIBSPDM_STATUS_IS_ERROR(status));
if (LIBSPDM_STATUS_IS_ERROR(status)) {
return libspdm_generate_error_response(
spdm_context, SPDM_ERROR_CODE_UNSPECIFIED, 0, response_size, response);
}
}
library/spdm_responder_lib/libspdm_rsp_digests.c:158
- In libspdm_validate_multi_key_slot_info(), several outputs from libspdm_read_key_pair_info() (capabilities/asym_algo_capabilities/current_asym_algo/pqc_* and total_key_pairs) are never used. With common warning settings (e.g., -Wunused-but-set-variable / -Wunused-variable) this can fail the build. Either explicitly mark them unused after a successful read, or refactor the HAL API to allow NULL for unneeded outputs (multi-file).
uint32_t asym_algo_capabilities;
uint32_t current_asym_algo;
uint32_t pqc_asym_algo_capabilities;
uint32_t current_pqc_asym_algo;
uint8_t assoc_cert_slot_mask;
f3c54ce to
07b8e0d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
library/spdm_responder_lib/libspdm_rsp_digests.c:33
- The SupportedSlotMask validation is gated on the responder’s supported version list, but SupportedSlotMask is emitted based on the negotiated/request SPDM version (see later check
if (spdm_request->header.spdm_version >= SPDM_MESSAGE_VERSION_13)). If the responder supports 1.3+ but negotiates 1.2 (or receives a <1.3 request), this can incorrectly treat a configuration as invalid and fail GET_DIGESTS even though SupportedSlotMask won’t be emitted.
/* SupportedSlotMask is only emitted for SPDM 1.3 and later. If the responder is not
* provisioned to support any such version there is nothing to validate. */
support_slot_mask_version = false;
for (index = 0; index < spdm_context->local_context.version.spdm_version_count; index++) {
if (libspdm_get_version_from_version_number(
unit_test/test_spdm_responder/digests.c:516
local_supported_slot_maskis built with|=in the loop, but it isn’t reset in this test case. If any prior test/setup leaves stale bits set, this case becomes order-dependent and can hide bugs in SupportedSlotMask handling.
spdm_context->local_context.local_supported_slot_mask |= (uint8_t)(1 << index);
The per-slot SupportedSlotMask, KeyPairID, CertificateInfo and KeyUsageMask in DIGESTS are integrator-provisioned. Verify them in the DIGESTS path via libspdm_validate_supported_slot_mask and libspdm_validate_multi_key_slot_info before emitting. Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: Claude Code:claude-opus-4-8
07b8e0d to
26c2a4a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
library/spdm_responder_lib/libspdm_rsp_digests.c:301
libspdm_validate_multi_key_slot_info()is called whenevermulti_key_conn_rspis true, but the per-slot KeyPairID/CertificateInfo/KeyUsageMask fields are only emitted when the negotiated SPDM version is >= 1.3 (see the later checks when computingadditional_sizeand writing the arrays). On SPDM < 1.3, this can reject a DIGESTS response due to multi-key slot info that would not be serialized.
/* The per-slot KeyPairID, CertificateInfo and KeyUsageMask fields are only emitted for a
* multi-key connection, so there is nothing to validate otherwise. */
if (spdm_context->connection_info.multi_key_conn_rsp) {
status = libspdm_validate_multi_key_slot_info(spdm_context);
LIBSPDM_ASSERT(!LIBSPDM_STATUS_IS_ERROR(status));
library/spdm_responder_lib/libspdm_rsp_digests.c:30
- The SupportedSlotMask validation is gated on the responder's supported versions list, but SupportedSlotMask is only emitted based on the negotiated connection version (see the later
if (spdm_request->header.spdm_version >= SPDM_MESSAGE_VERSION_13)when populatingparam1). As written, a SPDM 1.2 connection can incorrectly fail DIGESTS due to a missing/zerolocal_supported_slot_mask, even though the field would not be emitted.
This issue also appears on line 297 of the same file.
uint8_t index;
uint8_t supported_slot_mask;
bool support_slot_mask_version;
/* SupportedSlotMask is only emitted for SPDM 1.3 and later. If the responder is not
DIGESTS consistency
|
Will have review done before meeting on 3-Aug-2026. |
The per-slot SupportedSlotMask, KeyPairID, CertificateInfo and KeyUsageMask in DIGESTS are integrator-provisioned. Verify them in the DIGESTS path via libspdm_validate_supported_slot_mask and libspdm_validate_multi_key_slot_info before emitting.
Assisted-by: Claude Code:claude-opus-4-8
Ref: #3638