Fix : vendor_response_get_id should not exist

Signed-off-by: Shital Jumbad <[email protected]>

Author: ShitalJumbad

doc/user_guide.md

@@ -453,42 +453,25 @@ Refer to spdm_server_init() in [spdm_responder.c](https://github.com/DMTF/spdm-e
 
     libspdm_register_get_response_func (spdm_context, libspdm_get_response);
     ```
-    3.2 This callbacks handle SPDM Vendor Defined Commands
-    ```C
-    libspdm_return_t libspdm_vendor_get_id_func(
-      void *spdm_context,
-      uint16_t *resp_standard_id,
-      uint8_t *resp_vendor_id_len,
-      void *resp_vendor_id)
-    {
-      // return responder vendor id
-      ...
-
-      return LIBSPDM_STATUS_SUCCESS;
-    }
-
-    vendor_response_get_id
-    libspdm_return_t libspdm_vendor_response_func(
-      void *spdm_context,
-      uint16_t req_standard_id,
-      uint8_t req_vendor_id_len,
-      const void *req_vendor_id,
-      uint16_t req_size,
-      const void *req_data,
-      uint16_t *resp_size,
-      void *resp_data)
-      {
-      // process request and create response
-      ...
-      // populate response header and payload
-      ...
-
-      return LIBSPDM_STATUS_SUCCESS;
-    }
+   3.2 This callback handles SPDM Vendor Defined Commands
+   ```C
+   libspdm_return_t libspdm_vendor_response_func(
+     void *spdm_context,
+     const uint32_t *session_id,
+     uint16_t req_standard_id,
+     uint8_t req_vendor_id_len,
+     const void *req_vendor_id,
+     uint32_t req_size,
+     const void *req_data,
+     uint32_t *resp_size,
+     void *resp_data)
+   {
+     // write payload to resp_data and set *resp_size to payload size
+     return LIBSPDM_STATUS_SUCCESS;
+   }
 
-    libspdm_register_vendor_get_id_callback_func(spdm_context, libspdm_vendor_get_id_func);
-    libspdm_register_vendor_callback_func(spdm_context, libspdm_vendor_response_func);
-    ```
+   libspdm_register_vendor_callback_func(spdm_context, libspdm_vendor_response_func);
+   ```
 
 4. Free the memory of contexts within the SPDM context when all flow is over.
    This function does not free the SPDM context itself.

include/internal/libspdm_common_lib.h

@@ -676,7 +676,6 @@ typedef struct {
 
 #if LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES
     libspdm_vendor_response_callback_func vendor_response_callback;
-    libspdm_vendor_get_id_callback_func vendor_response_get_id;
 #endif /* LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES */
 
 #if LIBSPDM_EVENT_RECIPIENT_SUPPORT

include/library/spdm_common_lib.h

@@ -945,24 +945,6 @@ bool libspdm_get_fips_mode(void);
 
 #if LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES
 
-/**
- * Vendor Response Get Vendor ID Callback Function Pointer.
- * Required to be able to compose the Vendor Defined Response correctly
- *
- * @param  spdm_context        A pointer to the SPDM context.
- * @param  session_id          If non-NULL then message is within a secure session.
- *                             If NULL then message is outside a secure session.
- * @param  resp_standard_id    Registry or Standards body used for response
- * @param  resp_vendor_id_len  Length in bytes of the vendor id field for the response
- * @param  resp_vendor_id      Vendor ID assigned by the Registry or Standards Body. Little-endian format
- **/
-typedef libspdm_return_t (*libspdm_vendor_get_id_callback_func)(
-    void *spdm_context,
-    const uint32_t *session_id,
-    uint16_t *resp_standard_id,
-    uint8_t *resp_vendor_id_len,
-    void *resp_vendor_id);
-
 /**
  * Vendor Response Callback Function Pointer.
  *
@@ -977,7 +959,7 @@ typedef libspdm_return_t (*libspdm_vendor_get_id_callback_func)(
  * @param  resp_size          Length of the response
  * @param  resp_data          The vendor defined response
  **/
-typedef libspdm_return_t (*libspdm_vendor_response_callback_func)(
+ typedef libspdm_return_t (*libspdm_vendor_response_callback_func)(
     void *spdm_context,
     const uint32_t *session_id,
     uint16_t req_standard_id,

include/library/spdm_responder_lib.h

@@ -285,26 +285,12 @@ void libspdm_register_cert_chain_buffer(
 #if LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES
 
 /**
- * This function registers the callback function for getting the Vendor ID for a VENDOR_DEFINED_RESPONSE to the device.
+ * Register the request-aware vendor-defined response callback.
  *
  * This is useful for creating unique responses to devices.
  *
  * @param  spdm_context     A pointer to the SPDM context.
- * @param  resp_callback_func   Response callback function
- *
- * @retval LIBSPDM_STATUS_SUCCESS Success
- * @retval LIBSPDM_STATUS_INVALID_PARAMETER Some parameters invalid or NULL
- **/
-libspdm_return_t libspdm_register_vendor_get_id_callback_func(void *spdm_context,
-                                                              libspdm_vendor_get_id_callback_func resp_callback);
-
-/**
- * This function registers the callback function for doing a VENDOR_DEFINED_RESPONSE to the device.
- *
- * This is useful for creating unique responses to devices.
- *
- * @param  spdm_context     A pointer to the SPDM context.
- * @param  resp_callback_func   Response callback function
+ * @param  resp_callback    Response callback function
  *
  * @retval LIBSPDM_STATUS_SUCCESS Success
  * @retval LIBSPDM_STATUS_INVALID_PARAMETER Some parameters invalid or NULL

library/spdm_responder_lib/libspdm_rsp_vendor_response.c

@@ -11,15 +11,6 @@
 /* expected number of bytes for VENDOR MESSAGE HEADERS */
 #define SPDM_VENDOR_DEFINED_FIXED_HEADER_LEN 7
 
-libspdm_return_t libspdm_register_vendor_get_id_callback_func(void *spdm_context,
-                                                              libspdm_vendor_get_id_callback_func resp_callback)
-{
-
-    libspdm_context_t *context = (libspdm_context_t *)spdm_context;
-    context->vendor_response_get_id = resp_callback;
-    return LIBSPDM_STATUS_SUCCESS;
-}
-
 libspdm_return_t libspdm_register_vendor_callback_func(void *spdm_context,
                                                        libspdm_vendor_response_callback_func resp_callback)
 {
@@ -79,9 +70,8 @@ libspdm_return_t libspdm_get_vendor_defined_response(libspdm_context_t *spdm_con
         session_id = &session_info->session_id;
     }
 
-    /* Check if caller is using the old Vendor Defined API. */
-    if ((spdm_context->vendor_response_callback == NULL ||
-         spdm_context->vendor_response_get_id == NULL)) {
+    /* Check if vendor callback is registered. */
+    if (spdm_context->vendor_response_callback == NULL) {
         if (spdm_context->get_response_func != NULL) {
             return ((libspdm_get_response_func)spdm_context->get_response_func)(
                 spdm_context,
@@ -221,9 +211,13 @@ libspdm_return_t libspdm_get_vendor_defined_response(libspdm_context_t *spdm_con
      *      Len2 bytes Response Payload
      */
 
-    /* replace capacity with size */
-    spdm_response->len = SPDM_MAX_VENDOR_ID_LENGTH;
-    resp_data = ((uint8_t *)response) + sizeof(spdm_vendor_defined_response_msg_t);
+    /* Set up pointers for the callback */
+    spdm_response->standard_id = spdm_request->standard_id;
+    spdm_response->len = spdm_request->len;
+    libspdm_copy_mem(((uint8_t *)response) + sizeof(spdm_vendor_defined_response_msg_t),
+                     spdm_request->len,
+                     req_vendor_id,
+                     spdm_request->len);
 
     if (use_large_payload) {
         req_data = ((const uint8_t *)request) +
@@ -237,31 +231,29 @@ libspdm_return_t libspdm_get_vendor_defined_response(libspdm_context_t *spdm_con
                    sizeof(uint16_t);
     }
 
-    status = spdm_context->vendor_response_get_id(
-        spdm_context,
-        session_id,
-        &spdm_response->standard_id,
-        &spdm_response->len,
-        resp_data);
-
-    /* move pointer and adjust buffer size */
+    /* move pointer */
+    resp_data = ((uint8_t *)response) + header_length;
+    /* adjust buffer size */
     if (use_large_payload) {
-        resp_data += spdm_response->len + sizeof(uint16_t) + sizeof(uint32_t);
-        response_capacity -= spdm_response->len + sizeof(uint16_t) + sizeof(uint32_t);
         resp_size = (uint32_t)response_capacity;
     } else {
-        resp_data += spdm_response->len + sizeof(uint16_t);
-        response_capacity -= spdm_response->len + sizeof(uint16_t);
         resp_size = (uint16_t)response_capacity;
     }
 
-    status = spdm_context->vendor_response_callback(spdm_context,
-                                                    session_id,
-                                                    spdm_request->standard_id,
-                                                    spdm_request->len,
-                                                    req_vendor_id, req_size, req_data,
-                                                    &resp_size,
-                                                    resp_data);
+    status = spdm_context->vendor_response_callback(
+        spdm_context,
+        session_id,
+        spdm_request->standard_id,
+        spdm_request->len,
+        req_vendor_id,
+        req_size,
+        req_data,
+        &resp_size,
+        resp_data);
+
+    if (LIBSPDM_STATUS_IS_ERROR(status)) {
+        return status;
+    }
 
     /* store back the response payload size */
     if (use_large_payload) {

unit_test/fuzzing/test_responder/test_spdm_responder_vendor_cmds/vendor_cmds.c

@@ -15,16 +15,6 @@ size_t libspdm_get_max_buffer_size(void)
     return LIBSPDM_MAX_SPDM_MSG_SIZE;
 }
 
-libspdm_return_t libspdm_vendor_get_id_func_test(
-    void *spdm_context,
-    const uint32_t *session_id,
-    uint16_t *resp_standard_id,
-    uint8_t *resp_vendor_id_len,
-    void *resp_vendor_id)
-{
-    return LIBSPDM_STATUS_SUCCESS;
-}
-
 libspdm_return_t libspdm_vendor_response_func_test(
     void *spdm_context,
     const uint32_t *session_id,
@@ -36,6 +26,12 @@ libspdm_return_t libspdm_vendor_response_func_test(
     uint32_t *resp_size,
     void *resp_data)
 {
+    /* Validate required parameters */
+    if (resp_size == NULL || resp_data == NULL)
+        return LIBSPDM_STATUS_INVALID_PARAMETER;
+
+    /* Set response payload */
+    *resp_size = 0;
     return LIBSPDM_STATUS_SUCCESS;
 }
 
@@ -59,8 +55,6 @@ void libspdm_test_responder_vendor_cmds_case1(void **State)
         LIBSPDM_CONNECTION_STATE_NEGOTIATED;
     spdm_context->local_context.is_requester = true;
 
-    libspdm_register_vendor_get_id_callback_func(spdm_context,
-                                                 libspdm_vendor_get_id_func_test);
     libspdm_register_vendor_callback_func(spdm_context,
                                           libspdm_vendor_response_func_test);
 

unit_test/test_spdm_responder/error_test/vendor_response_err.c

@@ -38,31 +38,6 @@ static void set_standard_state(libspdm_context_t *spdm_context)
     spdm_context->response_state = LIBSPDM_RESPONSE_STATE_NORMAL;
 }
 
-static libspdm_return_t libspdm_vendor_get_id_func_err_test(
-    void *spdm_context,
-    const uint32_t *session_id,
-    uint16_t *resp_standard_id,
-    uint8_t *resp_vendor_id_len,
-    void *resp_vendor_id)
-{
-    if (resp_standard_id == NULL ||
-        resp_vendor_id_len == NULL ||
-        resp_vendor_id == NULL)
-        return LIBSPDM_STATUS_INVALID_PARAMETER;
-
-    /* vendor id length in bytes */
-    if (*resp_vendor_id_len < 2)
-        return LIBSPDM_STATUS_INVALID_PARAMETER;
-
-    *resp_standard_id = 6;
-    /* vendor id length in bytes */
-    *resp_vendor_id_len = 2;
-    ((uint8_t*)resp_vendor_id)[0] = 0xAA;
-    ((uint8_t*)resp_vendor_id)[1] = 0xAA;
-
-    return LIBSPDM_STATUS_SUCCESS;
-}
-
 static libspdm_return_t libspdm_vendor_response_func_err_test(
     void *spdm_context,
     const uint32_t *session_id,
@@ -74,18 +49,15 @@ static libspdm_return_t libspdm_vendor_response_func_err_test(
     uint32_t *resp_size,
     void *resp_data)
 {
-    /* get pointer to response data payload and populate */
+    /* Validate required parameters */
+    if (resp_size == NULL || *resp_size == 0 || req_data == NULL || resp_data == NULL)
+        return LIBSPDM_STATUS_INVALID_PARAMETER;
+
+    /* Set response payload */
     uint8_t *resp_payload = (uint8_t *)resp_data;
-    /* get pointer to response length and populate */
     *resp_size = VENDOR_DEFINED_RESPONSE_PAYLOAD_SIZE;
-    /* store length of response */
     libspdm_set_mem(resp_payload, *resp_size, 0xFF);
 
-    if (resp_size == NULL || *resp_size == 0)
-        return LIBSPDM_STATUS_INVALID_PARAMETER;
-
-    /* TBD make an error here, like response len 65000, but different this time. */
-
     printf("Got request 0x%x, sent response 0x%x\n",
            ((const uint8_t*)req_data)[0], ((uint8_t*)resp_data)[0]);
 
@@ -174,7 +146,6 @@ static void libspdm_test_responder_vendor_cmds_err_case2(void **state)
     set_standard_state(spdm_context);
 
     status = libspdm_register_vendor_callback_func(spdm_context, NULL);
-    status = libspdm_register_vendor_get_id_callback_func(spdm_context, NULL);
 
     request.header.spdm_version = SPDM_MESSAGE_VERSION_10;
     request.header.request_response_code = SPDM_VENDOR_DEFINED_REQUEST;
@@ -197,7 +168,7 @@ static void libspdm_test_responder_vendor_cmds_err_case2(void **state)
     response_size = sizeof(response_buffer);
 
     status = libspdm_get_vendor_defined_response(spdm_context, sizeof(request),
-                                                 &request, &response_size, &response_buffer);
+                                                 request_buffer, &response_size, response_buffer);
 
     assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
     assert_int_equal(response_size, sizeof(spdm_error_response_t));
@@ -241,9 +212,6 @@ static void libspdm_test_responder_vendor_cmds_err_case3(void **state)
     spdm_context->local_context.capability.flags = 0; /* responder not support large payload */
     spdm_context->local_context.is_requester = false;
 
-    status = libspdm_register_vendor_get_id_callback_func(spdm_context,
-                                                          libspdm_vendor_get_id_func_err_test);
-    assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
     status = libspdm_register_vendor_callback_func(spdm_context,
                                                    libspdm_vendor_response_func_err_test);
     assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
@@ -313,9 +281,6 @@ static void libspdm_test_responder_vendor_cmds_err_case4(void **state)
     spdm_context->local_context.capability.flags = 0;
     spdm_context->local_context.is_requester = false;
 
-    status = libspdm_register_vendor_get_id_callback_func(spdm_context,
-                                                          libspdm_vendor_get_id_func_err_test);
-    assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
     status = libspdm_register_vendor_callback_func(spdm_context,
                                                    libspdm_vendor_response_func_err_test);
     assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
@@ -382,9 +347,6 @@ static void libspdm_test_responder_vendor_cmds_err_case5(void **state)
     spdm_context->local_context.capability.flags = 0;
     spdm_context->local_context.is_requester = false;
 
-    status = libspdm_register_vendor_get_id_callback_func(spdm_context,
-                                                          libspdm_vendor_get_id_func_err_test);
-    assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
     status = libspdm_register_vendor_callback_func(spdm_context,
                                                    libspdm_vendor_response_func_err_test);
     assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
@@ -451,9 +413,6 @@ static void libspdm_test_responder_vendor_cmds_err_case6(void **state)
     spdm_context->local_context.capability.flags |= SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_LARGE_RESP_CAP;
     spdm_context->local_context.is_requester = false;
 
-    status = libspdm_register_vendor_get_id_callback_func(spdm_context,
-                                                          libspdm_vendor_get_id_func_err_test);
-    assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
     status = libspdm_register_vendor_callback_func(spdm_context,
                                                    libspdm_vendor_response_func_err_test);
     assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
@@ -523,9 +482,6 @@ static void libspdm_test_responder_vendor_cmds_err_case7(void **state)
     spdm_context->local_context.capability.flags |= SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_LARGE_RESP_CAP;
     spdm_context->local_context.is_requester = false;
 
-    status = libspdm_register_vendor_get_id_callback_func(spdm_context,
-                                                          libspdm_vendor_get_id_func_err_test);
-    assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);
     status = libspdm_register_vendor_callback_func(spdm_context,
                                                    libspdm_vendor_response_func_err_test);
     assert_int_equal(status, LIBSPDM_STATUS_SUCCESS);