Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
SebastianCzapla opened this issue Sep 9, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@SebastianCzapla
Copy link
Contributor

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
Copy link
Contributor

philipandag commented Sep 12, 2024

@SebastianCzapla Do you still experience the issue on the newest develop branch after merging this PR #492? You can close the issue if so.

Sorry, that's unrelated.

@SebastianCzapla
Copy link
Contributor Author

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
Copy link
Contributor Author

Here's a preview of patch to remove redundant functionality: #507

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

@miczyg1
Copy link
Contributor

miczyg1 commented Sep 18, 2024

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

@SebastianCzapla
Copy link
Contributor Author

PR: #507 has been corrected with your comment taken into consideration. Main point is adding new variable to common config files "TPM_EXPECTED_VERSION", which we later check against.

@SebastianCzapla
Copy link
Contributor Author

dasharo-security.tar.gz

Here's a log from dasharo security suite on v1410 platform

@SebastianCzapla
Copy link
Contributor Author

Fixed in #507

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants