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.

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

An error response with any error code other than ResponseNotReady shall re-initialize L1/L2 to null. #2114

Closed
Zhiqiang520 opened this issue Jun 10, 2023 · 5 comments · Fixed by #2126
Assignees
Labels
bug Something isn't working

Comments

@Zhiqiang520
Copy link
Contributor

An error response with any error code other than ResponseNotReady shall re-initialize L1/L2 to null.

  1. Refer to the paragraph 432 in DSP0274_1.2.1.pdf.
    image
  2. So I think in libspdm_get_response_measurements function, should call libspdm_reset_message_m(spdm_context, session_info); before return any libspdm_generate_error_response other than ResponseNotReady error.
  3. the Requester issues a number of requests without requiring signatures followed by a final request requiring a signature over the entire set of request-response messages exchanged.

GET_MEASUREMENTS_REQUEST1
MEASUREMENTS_RESPONSE1
GET_MEASUREMENTS_REQUEST2
MEASUREMENTS_RESPONSE2
... ...
GET_MEASUREMENTS_REQUESTn
MEASUREMENTS_RESPONSEn

  1. In the above scenario, if MEASUREMENTS_RESPONSE1 successful, but MEASUREMENTS_RESPONSE2 error at the following line 364 or 372, then should call libspdm_reset_message_m(spdm_context, session_info); before return libspdm_generate_error_response.
    status = libspdm_append_message_m(spdm_context, session_info, spdm_request, request_size);
    if (LIBSPDM_STATUS_IS_ERROR(status)) {
    return libspdm_generate_error_response(spdm_context,
    SPDM_ERROR_CODE_UNSPECIFIED, 0,
    response_size, response);
    }
    status = libspdm_append_message_m(spdm_context, session_info,
    spdm_response, *response_size - signature_size);
    if (LIBSPDM_STATUS_IS_ERROR(status)) {
    return libspdm_generate_error_response(spdm_context,
    SPDM_ERROR_CODE_UNSPECIFIED, 0,
    response_size, response);
    }
  2. there are many same issues is in libspdm_rsp_measurements.c, for example,
    if (!libspdm_is_capabilities_flag_supported(
    spdm_context, false, 0,
    SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_CAP)) {
    return libspdm_generate_error_response(
    spdm_context, SPDM_ERROR_CODE_UNSUPPORTED_REQUEST,
    SPDM_GET_MEASUREMENTS, response_size, response);
    }
    if ((spdm_context->connection_info.algorithm.measurement_spec == 0) ||
    (spdm_context->connection_info.algorithm.measurement_hash_algo == 0) ) {
    return libspdm_generate_error_response(
    spdm_context, SPDM_ERROR_CODE_UNEXPECTED_REQUEST,
    0, response_size, response);
    }
  3. there are also many similar issues in libspdm_req_get_measurements.c, for example, the line 414~415,
    status = libspdm_append_message_m(spdm_context, session_info, spdm_request,
    spdm_request_size);
    if (LIBSPDM_STATUS_IS_ERROR(status)) {
    status = LIBSPDM_STATUS_BUFFER_FULL;
    goto receive_done;
    }
    status = libspdm_append_message_m(spdm_context, session_info, spdm_response,
    spdm_response_size - signature_size);
    if (LIBSPDM_STATUS_IS_ERROR(status)) {
    libspdm_reset_message_m(spdm_context, session_info);
    status = LIBSPDM_STATUS_BUFFER_FULL;
    goto receive_done;
    }
@jyao1 jyao1 added the bug Something isn't working label Jun 12, 2023
@jyao1
Copy link
Member

jyao1 commented Jun 12, 2023

Please help check if M1/M2 has similar issue.

@Zhiqiang520
Copy link
Contributor Author

Please help check if M1/M2 has similar issue.

a. After checking I found one new issue about set M1/M2 to null : #2118
b. Unlike L1/L2, for M1/M2:
Refer to the paragraph 362 ~ 363 in DSP0274_1.2.1.pdf.

If a response contains an ErrorCode other than ResponseNotReady :
No concatenation function is performed on the contents of both the original request and response.

@steven-bellock
Copy link
Contributor

Busy should not affect the transcript as well, but that's an issue with the specification. I will file an issue against the specification.

@jyao1 jyao1 self-assigned this Jun 13, 2023
@steven-bellock
Copy link
Contributor

After talking with the SPDM Working Group, Busy is still being discussed, but LargeResponse should definitely not reset L1/L2.

@jyao1
Copy link
Member

jyao1 commented Jun 14, 2023

I dropped BUSY in this PR.

If SPDM decides to add BUSY later, we can add a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants