Why connection_state check is prior to version check at DIGESTS message

[bokyungseo1218]

I wanna know the reason why connection_state check is prior to version check at DIGESTS message

Case)
when responder receives GET_DIGESTS message before responder receive GET_VERSION message
I think rsponder returns SPDM_ERROR_CODE_UNEXPECTED_REQUEST(line 51) instead of SPDM_ERROR_CODE_VERSION_MISMATCH(line 33)
So I think that responder should check connection_state first.

https://github.com/DMTF/libspdm/blob/1c34043d8142ed285ead67eb07d4aa09c8d0a8d8/library/spdm_responder_lib/libspdm_rsp_digests.c#L33C1-L55C76

    if (spdm_request->header.spdm_version != libspdm_get_connection_version(spdm_context)) {
        return libspdm_generate_error_response(spdm_context,
                                               SPDM_ERROR_CODE_VERSION_MISMATCH, 0,
                                               response_size, response);
    }
    if (spdm_context->response_state != LIBSPDM_RESPONSE_STATE_NORMAL) {
        return libspdm_responder_handle_response_state(
            spdm_context,
            spdm_request->header.request_response_code,
            response_size, response);
    }
    if (!libspdm_is_capabilities_flag_supported(
            spdm_context, false, 0,
            SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_CERT_CAP)) {
        return libspdm_generate_error_response(
            spdm_context, SPDM_ERROR_CODE_UNSUPPORTED_REQUEST,
            SPDM_GET_DIGESTS, response_size, response);
    }
    if (spdm_context->connection_info.connection_state <
        LIBSPDM_CONNECTION_STATE_NEGOTIATED) {
        return libspdm_generate_error_response(spdm_context,
                                               SPDM_ERROR_CODE_UNEXPECTED_REQUEST,
                                               0, response_size, response);

[steven-bellock]

I wanna know the reason why connection_state check is prior to version check at DIGESTS message

I don't think there is a reason. But thinking about appropriate error codes.

1. After a reset, if the Requester has not sent GET_VERSION then, to any other request, the Responder should return RequestResynch.
2. After GET_CAPABILITIES is sent and the connection's version is established then we need to establish priority between a request that isn't correct for the current Responder state and a bad version field.

Regardless of the order of priority in 2, the Responder should be consistent throughout all its message handling.

[bokyungseo1218]

Thank @steven-bellock

1. After a reset, if the Requester has not sent GET_VERSION then, to any other request, the Responder should return RequestResynch.
   -> In this case, our developing Responder returns VersionMismatch instead of RequestResynch.
   -> I can not find a code that Responder returns RequestResynch (when the Requester has not sent GET_VERSION). Do you know about that?

[steven-bellock]

I can not find a code that Responder returns RequestResynch (when the Requester has not sent GET_VERSION). Do you know about that?

I'm not saying it does that right now, I'm just saying it should do that, as I think that's the correct error response. @jyao1 or @Zhiqiang520 may have a different opinion.

[jyao1]

I wanna know the reason why connection_state check is prior to version check at DIGESTS message

No special reason. If libspdm implements in a different way, you can ask the same question, e.g. "why connection_state check is after the version check".

My understanding is that: If a spec does not describe the order of checking, then any checking order should be allowed to return error immediately. Only after all checks pass, then the right response is returned.

[bokyungseo1218]

Thanks @jyao1
DSP0274 SPDM 1.2.1 defines out of order as below

17 General ordering rules
888 These general ordering rules apply to SPDM messages that form a transcript that eventually gets signed.
889 When requests are received out of order, the Responder can silently discard all requests or return an ERROR
message with ErrorCode=RequestResynch (with the exception of GET_VERSION ) until the Requester issues a
GET_VERSION . Out of order requests shall nullify the transcript.

So I expect libspdm returns ErroCode with RequestResynch not VersionMismatch at libspdm_get_response_digests (if the Requester has not sent GET_VERSION)

[jyao1]

I believe chapter 17 talks about the order of SPDM message in the transport level. It is not about the order of error checking.

[lazineer]

According to DSP0274, VersionMismatch is returned if the version is selected and requester uses a different version.

If the Requester uses a different version than the selected version in other Requests, the Responder should return an ERROR message with ErrorCode = VersionMismatch or the Responder can silently discard the Request.

If requester sends GET_DIGESTS before GET_VERSION, I think it shouldn't return VersionMismatch because no version was selected.

However, if the version is checked first as shown in the code below, VersionMismatch is returned even though no version is selected. So it seems that @bokyungseo1218 is saying which checking order is right.

libspdm/library/spdm_responder_lib/libspdm_rsp_digests.c

Lines 33 to 56 in 1c34043

	if (spdm_request->header.spdm_version != libspdm_get_connection_version(spdm_context)) {
	return libspdm_generate_error_response(spdm_context,
	SPDM_ERROR_CODE_VERSION_MISMATCH, 0,
	response_size, response);
	}
	if (spdm_context->response_state != LIBSPDM_RESPONSE_STATE_NORMAL) {
	return libspdm_responder_handle_response_state(
	spdm_context,
	spdm_request->header.request_response_code,
	response_size, response);
	}
	if (!libspdm_is_capabilities_flag_supported(
	spdm_context, false, 0,
	SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_CERT_CAP)) {
	return libspdm_generate_error_response(
	spdm_context, SPDM_ERROR_CODE_UNSUPPORTED_REQUEST,
	SPDM_GET_DIGESTS, response_size, response);
	}
	if (spdm_context->connection_info.connection_state <
	LIBSPDM_CONNECTION_STATE_NEGOTIATED) {
	return libspdm_generate_error_response(spdm_context,
	SPDM_ERROR_CODE_UNEXPECTED_REQUEST,
	0, response_size, response);
	}

Excluding VersionMismatch, RequestResynch seems appropriate according to 17 General ordering rules.

[steven-bellock]

When requests are received out of order, the Responder can either silently discard all requests (with the exception of GET_VERSION) or return an ERROR message of ErrorCode=RequestResynch until the Requester issues a GET_VERSION.

seems to be at odds with the definition of UnexpectedRequest:

The Responder received an unexpected request message. For example, CHALLENGE before NEGOTIATE_ALGORITHMS.

I'll file an issue against the specification.