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

Fix chunk_seq_no wrap issue. #2952

Merged
merged 4 commits into from
Feb 7, 2025
Merged

Fix chunk_seq_no wrap issue. #2952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0995be1
Fix chunk_seq_no wrap in chunk send.
Li-Aaron Jan 7, 2025
63642f1
Fix chunk_seq_no wrap in chunk send ack.
Li-Aaron Jan 7, 2025
8d1e900
Fix chunk_seq_no wrap in chunk get.
Li-Aaron Jan 8, 2025
4c2e63d
Fix chunk_seq_no wrap in chunk get response.
Li-Aaron Jan 13, 2025
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
17 changes: 16 additions & 1 deletion library/spdm_requester_lib/libspdm_req_handle_error_response.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* Copyright Notice:
* Copyright 2021-2024 DMTF. All rights reserved.
* Copyright 2021-2025 DMTF. All rights reserved.
* License: BSD 3-Clause License. For full text see link: https://github.com/DMTF/libspdm/blob/main/LICENSE.md
**/

Expand Down Expand Up @@ -225,6 +225,7 @@ libspdm_return_t libspdm_handle_error_large_response(
size_t large_response_capacity;
size_t large_response_size;
size_t large_response_size_so_far;
uint64_t max_chunk_data_transfer_size;

if (libspdm_get_connection_version(spdm_context) < SPDM_MESSAGE_VERSION_12) {
return LIBSPDM_STATUS_UNSUPPORTED_CAP;
Expand Down Expand Up @@ -262,6 +263,10 @@ libspdm_return_t libspdm_handle_error_large_response(
message = scratch_buffer + libspdm_get_scratch_buffer_sender_receiver_offset(spdm_context);
message_size = libspdm_get_scratch_buffer_sender_receiver_capacity(spdm_context);

max_chunk_data_transfer_size =
Li-Aaron marked this conversation as resolved.
Show resolved Hide resolved
((size_t) spdm_context->local_context.capability.data_transfer_size
- sizeof(spdm_chunk_response_response_t)) * 65536 - sizeof(uint32_t);

libspdm_zero_mem(large_response, large_response_capacity);
large_response_size = 0;
large_response_size_so_far = 0;
Expand All @@ -280,6 +285,12 @@ libspdm_return_t libspdm_handle_error_large_response(
spdm_request->chunk_seq_no = chunk_seq_no;
spdm_request_size = sizeof(spdm_chunk_get_request_t);

if (chunk_seq_no == 0 && large_response_size_so_far != 0) {
Li-Aaron marked this conversation as resolved.
Show resolved Hide resolved
/* chunk_seq_no wrapped */
status = LIBSPDM_STATUS_INVALID_MSG_FIELD;
break;
}

LIBSPDM_DEBUG((LIBSPDM_DEBUG_INFO,
"CHUNK_GET Handle %d SeqNo %d\n", chunk_handle, chunk_seq_no));

Expand Down Expand Up @@ -346,6 +357,10 @@ libspdm_return_t libspdm_handle_error_large_response(
status = LIBSPDM_STATUS_INVALID_MSG_FIELD;
break;
}
if (large_response_size > max_chunk_data_transfer_size) {
status = LIBSPDM_STATUS_INVALID_MSG_FIELD;
break;
}
if (large_response_size <= SPDM_MIN_DATA_TRANSFER_SIZE_VERSION_12) {
status = LIBSPDM_STATUS_INVALID_MSG_FIELD;
break;
Expand Down
21 changes: 16 additions & 5 deletions library/spdm_requester_lib/libspdm_req_send_receive.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* Copyright Notice:
* Copyright 2021-2024 DMTF. All rights reserved.
* Copyright 2021-2025 DMTF. All rights reserved.
* License: BSD 3-Clause License. For full text see link: https://github.com/DMTF/libspdm/blob/main/LICENSE.md
**/

Expand Down Expand Up @@ -316,6 +316,7 @@ libspdm_return_t libspdm_handle_large_request(
size_t copy_size;
libspdm_chunk_info_t *send_info;
uint32_t min_data_transfer_size;
uint64_t max_chunk_data_transfer_size;
spdm_error_response_t *spdm_error;

if (libspdm_get_connection_version(spdm_context) < SPDM_MESSAGE_VERSION_12) {
Expand All @@ -330,6 +331,20 @@ libspdm_return_t libspdm_handle_large_request(
return LIBSPDM_STATUS_ERROR_PEER;
}

/* Fail if exceed max chunks */
min_data_transfer_size = LIBSPDM_MIN(
spdm_context->connection_info.capability.data_transfer_size,
spdm_context->local_context.capability.sender_data_transfer_size);

max_chunk_data_transfer_size =
Copy link
Member

Choose a reason for hiding this comment

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

we should also consult max_spdm_msg_size.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Responder's max_spdm_msg_size is already checked in libspdm_send_spdm_request. Checking the Requester's max_spdm_msg_size doesn't make sense, as the message already exists at this point and so must be small enough to fit.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. If it is already checked, just add a comment to say : it is already checked.

((size_t) min_data_transfer_size - sizeof(spdm_chunk_send_request_t)) * 65536 -
sizeof(uint32_t);
/* max_spdm_msg_size is already checked in caller */

if (request_size > max_chunk_data_transfer_size) {
Li-Aaron marked this conversation as resolved.
Show resolved Hide resolved
return LIBSPDM_STATUS_BUFFER_TOO_SMALL;
}

/* now we can get sender buffer */
transport_header_size = spdm_context->local_context.capability.transport_header_size;

Expand Down Expand Up @@ -359,10 +374,6 @@ libspdm_return_t libspdm_handle_large_request(
request = NULL; /* Invalidate to prevent accidental use. */
request_size = 0;

min_data_transfer_size = LIBSPDM_MIN(
spdm_context->connection_info.capability.data_transfer_size,
spdm_context->local_context.capability.sender_data_transfer_size);

do {
LIBSPDM_ASSERT(send_info->large_message_capacity >= transport_header_size);
spdm_request = (spdm_chunk_send_request_t *)((uint8_t *)message + transport_header_size);
Expand Down
20 changes: 17 additions & 3 deletions library/spdm_responder_lib/libspdm_rsp_chunk_get.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* Copyright Notice:
* Copyright 2021-2024 DMTF. All rights reserved.
* Copyright 2021-2025 DMTF. All rights reserved.
* License: BSD 3-Clause License. For full text see link: https://github.com/DMTF/libspdm/blob/main/LICENSE.md
**/

Expand All @@ -17,6 +17,7 @@ libspdm_return_t libspdm_get_response_chunk_get(
{
libspdm_chunk_info_t* get_info;
uint32_t min_data_transfer_size;
uint64_t max_chunk_data_transfer_size;

const spdm_chunk_get_request_t* spdm_request;
spdm_chunk_response_response_t* spdm_response;
Expand Down Expand Up @@ -103,12 +104,25 @@ libspdm_return_t libspdm_get_response_chunk_get(
response_size, response);
}

libspdm_zero_mem(response, *response_size);

min_data_transfer_size = LIBSPDM_MIN(
spdm_context->connection_info.capability.data_transfer_size,
spdm_context->local_context.capability.sender_data_transfer_size);

/* Fail if exceed max chunks */
max_chunk_data_transfer_size =
Copy link
Member

Choose a reason for hiding this comment

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

we should also consult max_spdm_msg_size.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same feedback as in https://github.com/DMTF/libspdm/pull/2952/files#r1943830275.

((size_t) min_data_transfer_size - sizeof(spdm_chunk_response_response_t)) * 65536 -
sizeof(uint32_t);
/* max_spdm_msg_size already checked in caller */

if (get_info->large_message_size > max_chunk_data_transfer_size) {
return libspdm_generate_error_response(
spdm_context,
SPDM_ERROR_CODE_RESPONSE_TOO_LARGE, 0,
response_size, response);
}

libspdm_zero_mem(response, *response_size);

/* Assert the data transfer size is smaller than the response size.
* Otherwise there is no reason to chunk this response. */
LIBSPDM_ASSERT(min_data_transfer_size < *response_size);
Expand Down
10 changes: 9 additions & 1 deletion library/spdm_responder_lib/libspdm_rsp_chunk_send_ack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* Copyright Notice:
* Copyright 2021-2024 DMTF. All rights reserved.
* Copyright 2021-2025 DMTF. All rights reserved.
* License: BSD 3-Clause License. For full text see link: https://github.com/DMTF/libspdm/blob/main/LICENSE.md
**/

Expand All @@ -25,6 +25,7 @@ libspdm_return_t libspdm_get_response_chunk_send(libspdm_context_t *spdm_context
size_t scratch_buffer_size;
uint8_t* chunk_response;
size_t chunk_response_size;
uint64_t max_chunk_data_transfer_size;

spdm_request = (const spdm_chunk_send_request_t*) request;

Expand Down Expand Up @@ -103,6 +104,9 @@ libspdm_return_t libspdm_get_response_chunk_send(libspdm_context_t *spdm_context
chunk = (((const uint8_t*) (spdm_request + 1)) + sizeof(uint32_t));
calc_max_chunk_size =
(uint32_t)request_size - (sizeof(spdm_chunk_send_request_t) + sizeof(uint32_t));
max_chunk_data_transfer_size =
((size_t) spdm_context->local_context.capability.data_transfer_size
- sizeof(spdm_chunk_send_request_t)) * 65536 - sizeof(uint32_t);

if (spdm_request->chunk_seq_no != 0
|| (spdm_request->chunk_size
Expand All @@ -112,6 +116,7 @@ libspdm_return_t libspdm_get_response_chunk_send(libspdm_context_t *spdm_context
|| spdm_request->chunk_size > calc_max_chunk_size
|| (uint32_t)request_size > spdm_context->local_context.capability.data_transfer_size
|| large_message_size > spdm_context->local_context.capability.max_spdm_msg_size
|| large_message_size > max_chunk_data_transfer_size
|| large_message_size <= SPDM_MIN_DATA_TRANSFER_SIZE_VERSION_12
|| (spdm_request->header.param1 & SPDM_CHUNK_SEND_REQUEST_ATTRIBUTE_LAST_CHUNK)
) {
Expand Down Expand Up @@ -160,6 +165,9 @@ libspdm_return_t libspdm_get_response_chunk_send(libspdm_context_t *spdm_context
|| ((uint32_t) request_size
> spdm_context->local_context.capability.data_transfer_size))) {
status = LIBSPDM_STATUS_INVALID_MSG_FIELD;
} else if (spdm_request->chunk_seq_no == 0) {
/* Chunk seq no wrapped */
status = LIBSPDM_STATUS_INVALID_MSG_FIELD;
} else {

libspdm_copy_mem(
Expand Down
93 changes: 92 additions & 1 deletion unit_test/test_spdm_requester/chunk_get.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* Copyright Notice:
* Copyright 2021-2024 DMTF. All rights reserved.
* Copyright 2021-2025 DMTF. All rights reserved.
* License: BSD 3-Clause License. For full text see link: https://github.com/DMTF/libspdm/blob/main/LICENSE.md
**/

Expand All @@ -15,6 +15,10 @@ static size_t m_libspdm_local_certificate_chain_size_test_case_1;

static uint8_t m_libspdm_local_large_response_buffer[LIBSPDM_MAX_SPDM_MSG_SIZE];

#define BUFFER_SIZE_FOR_CHUNK_SEQ_NO_WRAP_TEST 0x200000
static uint8_t m_libspdm_local_response_buffer_for_chunk_seq_no_wrap_test[
BUFFER_SIZE_FOR_CHUNK_SEQ_NO_WRAP_TEST];

static size_t m_libspdm_local_buffer_size;
static uint8_t m_libspdm_local_buffer[LIBSPDM_MAX_MESSAGE_M1M2_BUFFER_SIZE];

Expand Down Expand Up @@ -259,6 +263,29 @@ void libspdm_requester_chunk_get_test_case4_build_digest_response(
spdm_response->header.param2 |= (0xFF << 0);
}

void libspdm_requester_chunk_get_test_case5_build_vendor_response(
void* context, void* response, size_t* response_size)
{
spdm_vendor_defined_response_msg_t *spdm_response;

/* For exceed max chunk seq no */
*response_size =
(CHUNK_GET_REQUESTER_UNIT_TEST_DATA_TRANSFER_SIZE -
sizeof(spdm_chunk_response_response_t)) * 65536 - sizeof(uint32_t) + 0x10;

libspdm_set_mem(response, *response_size, 0xff);

spdm_response = response;

spdm_response->header.spdm_version = SPDM_MESSAGE_VERSION_12;
spdm_response->header.request_response_code = SPDM_VENDOR_DEFINED_RESPONSE;
spdm_response->header.param1 = 0;
spdm_response->header.param2 = 0;

spdm_response->standard_id = 6;
spdm_response->len = 2;
}

libspdm_return_t libspdm_requester_chunk_get_test_send_message(
void* spdm_context, size_t request_size, const void* request,
uint64_t timeout)
Expand Down Expand Up @@ -431,6 +458,9 @@ libspdm_return_t libspdm_requester_chunk_get_test_receive_message(
} else if (spdm_test_context->case_id == 0x4) {
build_response_func =
libspdm_requester_chunk_get_test_case4_build_digest_response;
} else if (spdm_test_context->case_id == 0x5) {
build_response_func =
libspdm_requester_chunk_get_test_case5_build_vendor_response;
} else {
LIBSPDM_ASSERT(0);
return LIBSPDM_STATUS_RECEIVE_FAIL;
Expand All @@ -453,6 +483,12 @@ libspdm_return_t libspdm_requester_chunk_get_test_receive_message(

sub_rsp = (spdm_message_header_t*) m_libspdm_local_large_response_buffer;
sub_rsp_size = sizeof(m_libspdm_local_large_response_buffer);
if (spdm_test_context->case_id == 0x5) {
sub_rsp =
(spdm_message_header_t*)
m_libspdm_local_response_buffer_for_chunk_seq_no_wrap_test;
sub_rsp_size = sizeof(m_libspdm_local_response_buffer_for_chunk_seq_no_wrap_test);
}
libspdm_zero_mem(sub_rsp, sub_rsp_size);

build_response_func(spdm_context, sub_rsp, &sub_rsp_size);
Expand Down Expand Up @@ -832,6 +868,57 @@ void libspdm_test_requester_chunk_get_case4(void** state)
}
#endif

#if LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES
static void libspdm_test_requester_chunk_get_case5(void **state)
{
/* Copied from Vendor Request case 1*/
libspdm_return_t status;
libspdm_test_context_t *spdm_test_context;
libspdm_context_t *spdm_context;

uint16_t standard_id = 6;
uint8_t vendor_id_len = 2;
uint8_t vendor_id[SPDM_MAX_VENDOR_ID_LENGTH] = {0xAA, 0xAA};
uint16_t data_len = 16;
uint8_t data[16];

spdm_test_context = *state;
spdm_context = spdm_test_context->spdm_context;
spdm_test_context->case_id = 0x5;
spdm_context->connection_info.version = SPDM_MESSAGE_VERSION_12 <<
SPDM_VERSION_NUMBER_SHIFT_BIT;
/* Large response need a large scratch buffer. */
spdm_context->connection_info.capability.max_spdm_msg_size =
BUFFER_SIZE_FOR_CHUNK_SEQ_NO_WRAP_TEST;
spdm_context->local_context.capability.max_spdm_msg_size =
BUFFER_SIZE_FOR_CHUNK_SEQ_NO_WRAP_TEST;
spdm_context->connection_info.connection_state =
LIBSPDM_CONNECTION_STATE_NEGOTIATED;
spdm_context->connection_info.capability.data_transfer_size =
CHUNK_GET_REQUESTER_UNIT_TEST_DATA_TRANSFER_SIZE;
spdm_context->local_context.capability.sender_data_transfer_size =
CHUNK_GET_REQUESTER_UNIT_TEST_DATA_TRANSFER_SIZE;
spdm_context->local_context.is_requester = true;

spdm_test_context->scratch_buffer_size =
libspdm_get_sizeof_required_scratch_buffer(spdm_context);
spdm_test_context->scratch_buffer = (void *)malloc(spdm_test_context->scratch_buffer_size);
libspdm_set_scratch_buffer (spdm_context,
spdm_test_context->scratch_buffer,
spdm_test_context->scratch_buffer_size);

libspdm_set_mem(data, sizeof(data), 0xAA);

status = libspdm_vendor_send_request_receive_response(spdm_context, NULL,
standard_id, vendor_id_len, vendor_id,
data_len, data,
&standard_id, &vendor_id_len, vendor_id,
&data_len, data);

assert_int_equal(status, LIBSPDM_STATUS_RECEIVE_FAIL);
}
#endif /* LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES */

int libspdm_requester_chunk_get_test_main(void)
{
/* Test the CHUNK_GET handlers in various requester handlers */
Expand All @@ -851,6 +938,10 @@ int libspdm_requester_chunk_get_test_main(void)
#if LIBSPDM_SEND_GET_CERTIFICATE_SUPPORT
/* Request Digests */
cmocka_unit_test(libspdm_test_requester_chunk_get_case4),
#endif
#if LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES
/* Request Vendor Specific Response and chunk seq no wrapped */
cmocka_unit_test(libspdm_test_requester_chunk_get_case5),
#endif
};

Expand Down
Loading