Split get_version/get_capabilities/negotiate_algorithms request. #2406

Wenxing-hou · 2023-10-13T03:39:22Z

Split get_version/get_capabilities/negotiate_algorithms request.

library/spdm_requester_lib/libspdm_req_get_capabilities.c

steven-bellock · 2023-10-19T00:13:15Z

include/library/spdm_requester_lib.h

+ * @retval LIBSPDM_STATUS_SUCCESS   The request message is created in the buffer.
+ **/
+libspdm_return_t
+libspdm_build_request_get_version(


libspdm_return_t libspdm_* should be on one line. There are many instances of this.

Thanks. I have updated the style.

steven-bellock · 2023-10-19T00:16:22Z

include/library/spdm_requester_lib.h

@@ -24,6 +24,128 @@
 **/
 libspdm_return_t libspdm_init_connection(void *spdm_context, bool get_version_only);

+/**


Since this is a new way to interact with libspdm messages these functions should go at, like, the bottom of this file. An Integrator will either use the classic way or this RESTful way and they shouldn't be mixed.

Thanks. I have put the declarations to the bottom.

steven-bellock · 2023-10-19T00:18:52Z

library/spdm_requester_lib/libspdm_req_get_capabilities.c

@@ -141,55 +141,27 @@ static bool validate_responder_capability(uint32_t capabilities_flag, uint8_t ve
 }

 /**
- * This function sends GET_CAPABILITIES and receives CAPABILITIES.
+ * This function builds GET_CAPABILITY request message.


GET_CAPABILITY -> GET_CAPABILITIES.

Thanks. I have fixed it.

Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>

Fix the issue: DMTF#2394 Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>

Fix the issue:DMTF#2394 Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>

PrithviAPai · 2023-12-16T03:16:15Z

@Wenxing-hou Is there possibility to have libspdm_process_response which can internally call handle_version_response, handle_caps_response or handle_algo_response ?
@steven-bellock please share your suggestion.

PrithviAPai · 2023-12-18T13:53:41Z

@Wenxing-hou I did test these APIs and here are some of my findings:
#2480
Please let me know if you need any logs.

Wenxing-hou · 2023-12-20T02:02:10Z

@PrithviAPai We talked about: you suspect that this issue may be related to the following logic, and I am checking it.

In libspdm_send_request we are taking backup of the last message 
We are missing to do that when we separate the APIs 
 
 /* backup it to last_spdm_request, because the caller wants to compare it with response */
    if (((const spdm_message_header_t *)request)->request_response_code != SPDM_RESPOND_IF_READY
        && ((const spdm_message_header_t *)request)->request_response_code != SPDM_CHUNK_GET
        && ((const spdm_message_header_t*) request)->request_response_code != SPDM_CHUNK_SEND) {
        libspdm_copy_mem (context->last_spdm_request,
                          libspdm_get_scratch_buffer_last_spdm_request_capacity(context),
                          request,
                          request_size
                          );
        context->last_spdm_request_size = request_size;
    }

PrithviAPai · 2023-12-20T03:50:51Z

@Wenxing-hou one more thing is with this split we would need to support encryption and decryption
So it would be better to have support for that also in split function. Ofcourse, it will not be supported for VCA but generally we might need to have some mechanism to have support for that. Please share your thoughts

jyao1 · 2023-12-20T06:11:01Z

@PrithviAPai , it is hard to debug an issue without real code.

Could you please share your test code, or do some basic debug to see where the problem is?

one more thing is with this split we would need to support encryption and decryption
So it would be better to have support for that also in split function. Ofcourse, it will not be supported for VCA but generally we might need to have some mechanism to have support for that. Please share your thoughts

I am not sure what you mean, can you share some code and let us know how you plan to use ?

PrithviAPai · 2023-12-20T06:41:38Z

@jyao1 , here is my sample code which I used to test the PR.

bool getVCA()
{
    std::array<uint8_t, 100> request;
    std::array<uint8_t, 500> response;
    size_t requestSize = request.size();

    //Get Version Send Part
    if (!validateSpdmRc(libspdm_build_request_get_version(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    std::vector<uint8_t> data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type 

    //Send the message in async manner and store response in std::vector<uint8_t> responseData;

    //Version Receive Part 
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_version(
            spdmContext, &response, responseData.size(),
            NULL, NULL)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }

    // Housekeeping part 
    std::fill_n(request.begin(), request.size(), 0);
    std::fill_n(response.begin(), response.size(), 0);
    data.clear();

    //Caps Send Part 
    if (!validateSpdmRc(libspdm_build_request_get_capabilities(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type 

    //Send the message in async manner and storeresponse in std::vector<uint8_t> responseData;
    
    //Caps Receive Part 
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_capabilities(
            spdmContext, &response, responseData.size())))
    {
        freeSpdmContext(spdmContext);
        return false;
    }

    // Housekeeping part 
    std::fill_n(request.begin(), request.size(), 0);
    std::fill_n(response.begin(), response.size(), 0);
    data.clear();

    //Negotiate Algo Send Part
    if (!validateSpdmRc(libspdm_build_request_negotiate_algorithms(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type

    //Send the message in async manner and store response in std::vector<uint8_t> responseData;

    //Algo Receive Part
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_algorithms(
            spdmContext, &response, responseData.size())))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    return true;
}

PrithviAPai · 2023-12-21T06:34:10Z

@jyao1 , here is my sample code which I used to test the PR.

bool getVCA()
{
    std::array<uint8_t, 100> request;
    std::array<uint8_t, 500> response;
    size_t requestSize = request.size();

    //Get Version Send Part
    if (!validateSpdmRc(libspdm_build_request_get_version(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    std::vector<uint8_t> data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type 

    //Send the message in async manner and store response in std::vector<uint8_t> responseData;

    //Version Receive Part 
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_version(
            spdmContext, &response, responseData.size(),
            NULL, NULL)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }

    // Housekeeping part 
    std::fill_n(request.begin(), request.size(), 0);
    std::fill_n(response.begin(), response.size(), 0);
    data.clear();

    //Caps Send Part 
    if (!validateSpdmRc(libspdm_build_request_get_capabilities(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type 

    //Send the message in async manner and storeresponse in std::vector<uint8_t> responseData;
    
    //Caps Receive Part 
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_capabilities(
            spdmContext, &response, responseData.size())))
    {
        freeSpdmContext(spdmContext);
        return false;
    }

    // Housekeeping part 
    std::fill_n(request.begin(), request.size(), 0);
    std::fill_n(response.begin(), response.size(), 0);
    data.clear();

    //Negotiate Algo Send Part
    if (!validateSpdmRc(libspdm_build_request_negotiate_algorithms(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type

    //Send the message in async manner and store response in std::vector<uint8_t> responseData;

    //Algo Receive Part
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_algorithms(
            spdmContext, &response, responseData.size())))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    return true;
}

Is my usage wrong here ?
The Integrator is looking for Transport encoded payload and payload size.
The intention is to send and receive the data asynchronously.

Wenxing-hou · 2023-12-25T03:33:16Z

@jyao1 , here is my sample code which I used to test the PR.

bool getVCA()
{
    std::array<uint8_t, 100> request;
    std::array<uint8_t, 500> response;
    size_t requestSize = request.size();

    //Get Version Send Part
    if (!validateSpdmRc(libspdm_build_request_get_version(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    std::vector<uint8_t> data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type 

    //Send the message in async manner and store response in std::vector<uint8_t> responseData;

    //Version Receive Part 
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_version(
            spdmContext, &response, responseData.size(),
            NULL, NULL)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }

    // Housekeeping part 
    std::fill_n(request.begin(), request.size(), 0);
    std::fill_n(response.begin(), response.size(), 0);
    data.clear();

    //Caps Send Part 
    if (!validateSpdmRc(libspdm_build_request_get_capabilities(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type 

    //Send the message in async manner and storeresponse in std::vector<uint8_t> responseData;
    
    //Caps Receive Part 
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_capabilities(
            spdmContext, &response, responseData.size())))
    {
        freeSpdmContext(spdmContext);
        return false;
    }

    // Housekeeping part 
    std::fill_n(request.begin(), request.size(), 0);
    std::fill_n(response.begin(), response.size(), 0);
    data.clear();

    //Negotiate Algo Send Part
    if (!validateSpdmRc(libspdm_build_request_negotiate_algorithms(
            spdmContext, &request, &requestSize)))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    data = formSendMessage(requestSize, request);
    data.insert(data.begin(), 5); // Transport layer Message Type

    //Send the message in async manner and store response in std::vector<uint8_t> responseData;

    //Algo Receive Part
    responseData.erase(responseData.begin()) // Remove value at index 0 which contains MCTP Message Type; the result contains only SPDM Resp Packet
    std::copy(responseData.begin(), responseData.end(),
              response.begin());
    if (!validateSpdmRc(libspdm_process_response_algorithms(
            spdmContext, &response, responseData.size())))
    {
        freeSpdmContext(spdmContext);
        return false;
    }
    return true;
}

Is my usage wrong here ? The Integrator is looking for Transport encoded payload and payload size. The intention is to send and receive the data asynchronously.

Wenxing-hou · 2023-12-25T03:36:35Z

@PrithviAPai
Hi. In libspdm, we first build the request and then call the libspdm_send_request, which will fill the context->last_spdm_request. In your test case, you use your own send function. I think you may need fill the context->last_spdm_request before send. Thanks!

PrithviAPai · 2023-12-25T14:48:57Z

@PrithviAPai Hi. In libspdm, we first build the request and then call the libspdm_send_request, which will fill the context->last_spdm_request. In your test case, you use your own send function. I think you may need fill the context->last_spdm_request before send. Thanks!

@Wenxing-hou thank you I will verify and let you know the results. Meanwhile how about the receive part ? How do we need to handle that ? Once we get response from device(SPDM endpoint) ?

PrithviAPai · 2024-01-04T04:35:54Z

@PrithviAPai Hi. In libspdm, we first build the request and then call the libspdm_send_request, which will fill the context->last_spdm_request. In your test case, you use your own send function. I think you may need fill the context->last_spdm_request before send. Thanks!

@Wenxing-hou thank you I will verify and let you know the results. Meanwhile how about the receive part ? How do we need to handle that ? Once we get response from device(SPDM endpoint) ?

@Wenxing-hou @steven-bellock @jyao1 any suggestions on this ?
calling libspdm_receive_response will trigger the callback which is NOT necessary in this case.
Is there any way to make this work without triggering send and receive callbacks ?

jyao1 · 2024-01-10T00:20:02Z

@PrithviAPai Hi. In libspdm, we first build the request and then call the libspdm_send_request, which will fill the context->last_spdm_request. In your test case, you use your own send function. I think you may need fill the context->last_spdm_request before send. Thanks!

@Wenxing-hou thank you I will verify and let you know the results. Meanwhile how about the receive part ? How do we need to handle that ? Once we get response from device(SPDM endpoint) ?

@Wenxing-hou @steven-bellock @jyao1 any suggestions on this ? calling libspdm_receive_response will trigger the callback which is NOT necessary in this case. Is there any way to make this work without triggering send and receive callbacks ?

The libspdm internal state should be managed correctly. Why you cannot call libspdm_receive_response ?

PrithviAPai · 2024-01-10T04:45:40Z

@PrithviAPai Hi. In libspdm, we first build the request and then call the libspdm_send_request, which will fill the context->last_spdm_request. In your test case, you use your own send function. I think you may need fill the context->last_spdm_request before send. Thanks!

@Wenxing-hou thank you I will verify and let you know the results. Meanwhile how about the receive part ? How do we need to handle that ? Once we get response from device(SPDM endpoint) ?

@Wenxing-hou @steven-bellock @jyao1 any suggestions on this ? calling libspdm_receive_response will trigger the callback which is NOT necessary in this case. Is there any way to make this work without triggering send and receive callbacks ?

The libspdm internal state should be managed correctly. Why you cannot call libspdm_receive_response ?

We are calling libspdm_handle_response with giving response buffer as one of the parameter. So I feel it would be unnecessary to call libspdm_receive_response. Please share your thoughts.

jyao1 · 2024-01-10T06:44:09Z

@PrithviAPai , maybe I am a little lost.

Here we are discussing how to support SPDM Requester.
Please clarify if you are talking SPDM Requester or SPDM Responder?

Also, there is no libspdm_handle_response API in libspdm. Which API you are talking?

PrithviAPai · 2024-01-10T08:34:24Z

@PrithviAPai , maybe I am a little lost.

Here we are discussing how to support SPDM Requester. Please clarify if you are talking SPDM Requester or SPDM Responder?

Also, there is no libspdm_handle_response API in libspdm. Which API you are talking?

Yes, this is about SPDM Requester. Sorry for the confusion
When we split encode and decode APIs we have libspdm_process_response_version , libspdm_process_response_capabilities and libspdm_process_response_algorithms. Integrator is supposed to call with response payload. So I did not understand the part when integrator needs to call libspdm_receive_response.

jyao1 · 2024-01-10T10:16:37Z

@PrithviAPai

libspdm_send_request() need to maintain the spdm context. For example, to backup the last_spdm_request.

See below:

    /* backup it to last_spdm_request, because the caller wants to compare it with response */
    if (((const spdm_message_header_t *)request)->request_response_code != SPDM_RESPOND_IF_READY
        && ((const spdm_message_header_t *)request)->request_response_code != SPDM_CHUNK_GET
        && ((const spdm_message_header_t*) request)->request_response_code != SPDM_CHUNK_SEND) {
        libspdm_copy_mem (context->last_spdm_request,
                          libspdm_get_scratch_buffer_last_spdm_request_capacity(context),
                          request,
                          request_size
                          );
        context->last_spdm_request_size = request_size;
    }

Because you do not call the function and your own code does not backup the last_spdm_request, that is the reason on the failure.

PrithviAPai · 2024-01-10T10:22:38Z

@PrithviAPai

libspdm_send_request() need to maintain the spdm context. For example, to backup the last_spdm_request.

See below:

    /* backup it to last_spdm_request, because the caller wants to compare it with response */
    if (((const spdm_message_header_t *)request)->request_response_code != SPDM_RESPOND_IF_READY
        && ((const spdm_message_header_t *)request)->request_response_code != SPDM_CHUNK_GET
        && ((const spdm_message_header_t*) request)->request_response_code != SPDM_CHUNK_SEND) {
        libspdm_copy_mem (context->last_spdm_request,
                          libspdm_get_scratch_buffer_last_spdm_request_capacity(context),
                          request,
                          request_size
                          );
        context->last_spdm_request_size = request_size;
    }

Because you do not call the function and your own code does not backup the last_spdm_request, that is the reason on the failure.

Okay I will try with calling libspdm_send_request once I have request to send.
I have question in response phase:
Once I have response from EndPoint I call libspdm_process_response_version which does not support transport decoding/decryption. So what is the suggestion on receive part when we split the Request and Response ?

Wenxing-hou force-pushed the split_get_cap branch 2 times, most recently from 59cc9eb to 46050be Compare October 13, 2023 06:42

Wenxing-hou changed the title ~~Split get_version/get_capabilities request.~~ Split get_version/get_capabilities/negotiate_algorithms request. Oct 13, 2023

jyao1 mentioned this pull request Oct 16, 2023

Feature request: Split requester API to send message and handle received response separately #2394

Open

mcsmonk reviewed Oct 16, 2023

View reviewed changes

library/spdm_requester_lib/libspdm_req_get_capabilities.c Show resolved Hide resolved

mcsmonk mentioned this pull request Oct 16, 2023

Improving for making GET_CAPABILITIES request message #2412

Closed

mcsmonk approved these changes Oct 17, 2023

View reviewed changes

steven-bellock requested changes Oct 19, 2023

View reviewed changes

jyao1 and others added 3 commits October 19, 2023 17:10

Split get_version request.

c05a387

Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>

Split get_capabilities request

541e682

Fix the issue: DMTF#2394 Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>

Split negotiate_algorithms request

34fc89f

Fix the issue:DMTF#2394 Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>

Wenxing-hou force-pushed the split_get_cap branch from 46050be to 34fc89f Compare October 19, 2023 09:14

steven-bellock mentioned this pull request Dec 15, 2023

FeatureRequest: Async support for send/receive payload #2480

Open

Wenxing-hou marked this pull request as draft December 20, 2023 01:58

Wenxing-hou closed this Dec 25, 2023

Wenxing-hou reopened this Dec 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split get_version/get_capabilities/negotiate_algorithms request. #2406

Split get_version/get_capabilities/negotiate_algorithms request. #2406

Wenxing-hou commented Oct 13, 2023 •

edited

steven-bellock Oct 19, 2023

Wenxing-hou Oct 19, 2023

steven-bellock Oct 19, 2023

Wenxing-hou Oct 19, 2023

steven-bellock Oct 19, 2023

Wenxing-hou Oct 19, 2023

PrithviAPai commented Dec 16, 2023

PrithviAPai commented Dec 18, 2023

Wenxing-hou commented Dec 20, 2023

PrithviAPai commented Dec 20, 2023

jyao1 commented Dec 20, 2023

PrithviAPai commented Dec 20, 2023 •

edited

PrithviAPai commented Dec 21, 2023

Wenxing-hou commented Dec 25, 2023

Wenxing-hou commented Dec 25, 2023

PrithviAPai commented Dec 25, 2023

PrithviAPai commented Jan 4, 2024

jyao1 commented Jan 10, 2024

PrithviAPai commented Jan 10, 2024

jyao1 commented Jan 10, 2024 •

edited

PrithviAPai commented Jan 10, 2024 •

edited

jyao1 commented Jan 10, 2024

PrithviAPai commented Jan 10, 2024 •

edited

Split get_version/get_capabilities/negotiate_algorithms request. #2406

Are you sure you want to change the base?

Split get_version/get_capabilities/negotiate_algorithms request. #2406

Conversation

Wenxing-hou commented Oct 13, 2023 • edited

steven-bellock Oct 19, 2023

Choose a reason for hiding this comment

Wenxing-hou Oct 19, 2023

Choose a reason for hiding this comment

steven-bellock Oct 19, 2023

Choose a reason for hiding this comment

Wenxing-hou Oct 19, 2023

Choose a reason for hiding this comment

steven-bellock Oct 19, 2023

Choose a reason for hiding this comment

Wenxing-hou Oct 19, 2023

Choose a reason for hiding this comment

PrithviAPai commented Dec 16, 2023

PrithviAPai commented Dec 18, 2023

Wenxing-hou commented Dec 20, 2023

PrithviAPai commented Dec 20, 2023

jyao1 commented Dec 20, 2023

PrithviAPai commented Dec 20, 2023 • edited

PrithviAPai commented Dec 21, 2023

Wenxing-hou commented Dec 25, 2023

Wenxing-hou commented Dec 25, 2023

PrithviAPai commented Dec 25, 2023

PrithviAPai commented Jan 4, 2024

jyao1 commented Jan 10, 2024

PrithviAPai commented Jan 10, 2024

jyao1 commented Jan 10, 2024 • edited

PrithviAPai commented Jan 10, 2024 • edited

jyao1 commented Jan 10, 2024

PrithviAPai commented Jan 10, 2024 • edited

Wenxing-hou commented Oct 13, 2023 •

edited

PrithviAPai commented Dec 20, 2023 •

edited

jyao1 commented Jan 10, 2024 •

edited

PrithviAPai commented Jan 10, 2024 •

edited

PrithviAPai commented Jan 10, 2024 •

edited