dasharo-security/tpm-support: duplicate tests covering the same functionality

SebastianCzapla commented 1 week ago

The problem you're addressing (if any)

Test case TPM001.001 TPM Support (firmware) and TPM002.001 Verify TPM version (firmware) cover exactly the same functionality:

TPM001.001 TPM Support (firmware)
    [Documentation]    This test aims to verify that the TPM is initialized
    ...    correctly and the PCRs can be accessed from the firmware.
    Skip If    not ${TESTS_IN_UBUNTU_SUPPORT}    TPM001.001 not supported
    Power On
    Boot System Or From Connected Disk    ubuntu
    Login To Linux
    Switch To Root User
    Get Cbmem From Cloud
    ${out}=    Execute Command In Terminal    cbmem -L
    Should Contain    ${out}    TPM2 log

TPM002.001 Verify TPM version (firmware)
    [Documentation]    This test aims to verify that the TPM version is
    ...    correctly recognized by the firmware.
    Skip If    not ${TESTS_IN_UBUNTU_SUPPORT}    TPM002.001 not supported
    Power On
    Boot System Or From Connected Disk    ubuntu
    Login To Linux
    Switch To Root User
    Get Cbmem From Cloud
    ${out}=    Execute Command In Terminal    cbmem -L
    Should Contain    ${out}    TPM2 log

Describe the solution you'd like

Both test cases could be merged into one, with descriptions updated to reflect the changes.

Where is the value to a user, and who might that user be?

No response

Describe alternatives you've considered

No response

Additional context

No response

philipandag commented 6 days ago

@SebastianCzapla Do you still experience the issue on the newest develop branch after merging this PR https://github.com/Dasharo/open-source-firmware-validation/pull/492? You can close the issue if so.

Sorry, that's unrelated.

SebastianCzapla commented 12 hours ago

I think that it is possible to merge TPM002 (TPM verify version) tests into TPM001(TPM Verify support). Version verification should happen as part of support testing, and resolving version earlier in test will be useful for extended coverage of test, where we need to verify that at least one of the TPM versions is available.

SebastianCzapla commented 11 hours ago

Here's a preview of patch to remove redundant functionality: https://github.com/Dasharo/open-source-firmware-validation/pull/507

@miczyg1 @filipleple please take a look and tell what do you think about it.

miczyg1 commented 11 hours ago

TPM002.001 Verify TPM version (firmware)
    [Documentation]    This test aims to verify that the TPM version is
    ...    correctly recognized by the firmware.
    Skip If    not ${TESTS_IN_UBUNTU_SUPPORT}    TPM002.001 not supported
    Power On
    Boot System Or From Connected Disk    ubuntu
    Login To Linux
    Switch To Root User
    Get Cbmem From Cloud
    ${out}=    Execute Command In Terminal    cbmem -L
    Should Contain    ${out}    TPM2 log

This test does something entirely different from what the name suggest. TPM version check I think should check whether the TPM is 1.2 or 2.0 compared to expected TPM version which should be on the board. E.g. Dell Optiplex and one of KGPE platforms have a TPM1.2, other platforms should have TPM2.0. THe test should check cbmem -1 not cbmem -L to check the version.

TPM001.001 is good, but it hardcodes that the event log will always be TPM2 log, but for some platforms it may not be true, e.g. Dell Optiplex where there is a TPM 1.2

Dasharo / open-source-firmware-validation