DMTF / SPDM-Responder-Validator

BSD 3-Clause "New" or "Revised" License
9 stars 13 forks source link

Multi-version test setup functions do not properly reset supported versions. #131

Open wmaroneAMD opened 4 months ago

wmaroneAMD commented 4 months ago

Looking to confirm behavior, as I believe this is a bug. Tested libspdm tag 3.2.0 + spdm responder validator tag 3.2.0

Tests 2, 3, 6, and 7 incorrectly omit tests depending what versions are specified. When version 1.0 is removed as a supported version, the negotiation step correctly identifies this and skips the 1.0 only tests, but fails to reset properly when negotiating for 1.0+ tests. The same pattern happens for tests if only 1.2 is supported and 1.1 checks are skipped.

Example, spdm_test_case_capabilities_version_mismatch: SPDM Version: 1.0+

SPDM 1.0 + 1.1:

test group 2 (spdm_test_group_capabilities) - pass: 38, fail: 0
  test case 2.1 (spdm_test_case_capabilities_success_10) - pass: 4, fail: 0
  test case 2.2 (spdm_test_case_capabilities_version_mismatch) - pass: 10, fail: 0
  test case 2.3 (spdm_test_case_capabilities_success_11) - pass: 9, fail: 0
  test case 2.4 (spdm_test_case_capabilities_invalid_request) - pass: 15, fail: 0
  test case 2.5 (spdm_test_case_capabilities_success_12) - pass: 0, fail: 0

SPDM 1.1 only:

test group 2 (spdm_test_group_capabilities) - pass: 24, fail: 0
  test case 2.1 (spdm_test_case_capabilities_success_10) - pass: 0, fail: 0
  test case 2.2 (spdm_test_case_capabilities_version_mismatch) - pass: 0, fail: 0
  test case 2.3 (spdm_test_case_capabilities_success_11) - pass: 9, fail: 0
  test case 2.4 (spdm_test_case_capabilities_invalid_request) - pass: 15, fail: 0
  test case 2.5 (spdm_test_case_capabilities_success_12) - pass: 0, fail: 0  

SPDM 1.1 + 1.2:

test group 2 (spdm_test_group_capabilities) - pass: 55, fail: 0
  test case 2.1 (spdm_test_case_capabilities_success_10) - pass: 0, fail: 0
  test case 2.2 (spdm_test_case_capabilities_version_mismatch) - pass: 0, fail: 0
  test case 2.3 (spdm_test_case_capabilities_success_11) - pass: 9, fail: 0
  test case 2.4 (spdm_test_case_capabilities_invalid_request) - pass: 20, fail: 0
  test case 2.5 (spdm_test_case_capabilities_success_12) - pass: 11, fail: 0

Modifying the tests to omit spdm_test_case_capabilities_success_10 from being considered:

test group 2 (spdm_test_group_capabilities) - pass: 65, fail: 0
  test case 2.2 (spdm_test_case_capabilities_version_mismatch) - pass: 10, fail: 0
  test case 2.3 (spdm_test_case_capabilities_success_11) - pass: 9, fail: 0
  test case 2.4 (spdm_test_case_capabilities_invalid_request) - pass: 20, fail: 0
  test case 2.5 (spdm_test_case_capabilities_success_12) - pass: 11, fail: 0

By resetting supported SPDM versions:

@@ -75,6 +75,13 @@ bool spdm_test_case_capabilities_setup_version_all (void *test_context)
     spdm_test_context = test_context;
     spdm_context = spdm_test_context->spdm_context;

+    uint16_t v_rec[] = { 0x1000, 0x1100, 0x1200 };
+    size_t v_size = sizeof(v_rec);
+
+    libspdm_zero_mem(&parameter, sizeof(parameter));
+    parameter.location = LIBSPDM_DATA_LOCATION_LOCAL;
+    libspdm_set_data(spdm_context, LIBSPDM_DATA_SPDM_VERSION, &parameter, (void*)&v_rec[0], v_size);

We get proper output for the 1.1 + 1.2 and 1.1 only cases:

1.1 only

test group 2 (spdm_test_group_capabilities) - pass: 44, fail: 0
  test case 2.1 (spdm_test_case_capabilities_success_10) - pass: 0, fail: 0
  test case 2.2 (spdm_test_case_capabilities_version_mismatch) - pass: 10, fail: 0
  test case 2.3 (spdm_test_case_capabilities_success_11) - pass: 9, fail: 0
  test case 2.4 (spdm_test_case_capabilities_invalid_request) - pass: 15, fail: 0
  test case 2.5 (spdm_test_case_capabilities_success_12) - pass: 0, fail: 0

1.1+1.2

test group 2 (spdm_test_group_capabilities) - pass: 65, fail: 0
  test case 2.1 (spdm_test_case_capabilities_success_10) - pass: 0, fail: 0
  test case 2.2 (spdm_test_case_capabilities_version_mismatch) - pass: 10, fail: 0
  test case 2.3 (spdm_test_case_capabilities_success_11) - pass: 9, fail: 0
  test case 2.4 (spdm_test_case_capabilities_invalid_request) - pass: 20, fail: 0
  test case 2.5 (spdm_test_case_capabilities_success_12) - pass: 11, fail: 0

Extended example: spdm_test_case_capabilities_invalid_request, SPDM Version: 1.1+

1.2 only

test group 2 (spdm_test_group_capabilities) - pass: 26, fail: 0
  test case 2.1 (spdm_test_case_capabilities_success_10) - pass: 0, fail: 0
  test case 2.2 (spdm_test_case_capabilities_version_mismatch) - pass: 0, fail: 0
  test case 2.3 (spdm_test_case_capabilities_success_11) - pass: 0, fail: 0
  test case 2.4 (spdm_test_case_capabilities_invalid_request) - pass: 0, fail: 0
  test case 2.5 (spdm_test_case_capabilities_success_12) - pass: 11, fail: 0

The teardown flow goes through libspdm_init_context_with_secured_context which should be resetting the version, but does not appear to be doing so correctly.

jyao1 commented 3 months ago

@wmaroneAMD , I agree with you.

It seems you already have patch, to make sure you own the credit, I will transfer owner to you. Do you mind submit PR directly?

wmaroneAMD commented 3 months ago

If you are OK with making the change in this manner, I can do that.