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

Remove SHA1 from tests #2865

Merged
merged 8 commits into from
Jul 19, 2024
Merged

Remove SHA1 from tests #2865

merged 8 commits into from
Jul 19, 2024

Conversation

joholl
Copy link
Collaborator

@joholl joholl commented Jul 15, 2024

This has proven very tricky. Due to the sheer amount of SHA1 use, I could not understand every test case in depth.

  • Remove SHA1 (sometimes, I replaced SHA256 with SHA384 and SHA1 with SHA256 to keep code changes small)
  • Update hashes where necessary
  • Add comments where helpful
  • Add graceful handling of TPM2_RC_COMMAND_CODE for three tests

Fixes: #2818
Fixes: #2862

@JuergenReppSIT
Copy link
Member

To fix the failure in test/unit/tss2_policy.c in the function test_policy_instantiate the line
char hexdigest[32 + 32 + 1] = { 0 };
should be changed to:
char hexdigest[48 + 48 + 1] = { 0 };

Johannes Holland added 5 commits July 16, 2024 08:38
Signed-off-by: Johannes Holland <johannes.holland@infineon.de>
We forgot to input the old policyDigest for the hash calculation of the
new policyDigest.

Bug was not caught due to missing return code assignment in
policy-execute, see 80ffbf825f127.

Fixes: tpm2-software#2862

Signed-off-by: Johannes Holland <johannes.holland@infineon.de>
Also add some comments about hardcoded keys.

Signed-off-by: Johannes Holland <johannes.holland@infineon.de>
Signed-off-by: Johannes Holland <johannes.holland@infineon.de>
Signed-off-by: Johannes Holland <johannes.holland@infineon.de>
@joholl
Copy link
Collaborator Author

joholl commented Jul 16, 2024

Thanks, @JuergenReppSIT, fixed that.

Btw: In this PR, I also fixed the PolicyTemplate calculation bug #2862 here, please make sure to review this, too. We will want to backport that.

Signed-off-by: Johannes Holland <johannes.holland@infineon.de>
@joholl
Copy link
Collaborator Author

joholl commented Jul 17, 2024

Added a few commits for tpmclient. Now the test suite is SHA1-free for me.

Also saw a problem with tpmclient and libtpms. Fixed it, so now tpmclient can (actually) reset libtpms.

@AndreasFuchsTPM
Copy link
Member

../test/tpmclient/tpmclient.int.c:661:39: runtime error: index 20 out of bounds for type 'BYTE [20]'

Johannes Holland added 2 commits July 17, 2024 14:48
Signed-off-by: Johannes Holland <johannes.holland@infineon.de>
Signed-off-by: Johannes Holland <johannes.holland@infineon.de>
@JuergenReppSIT
Copy link
Member

I have tested the PR with a physical TPM with the following findings:

  • On pi3 (32bit) got the following compile error (was introduced with : 3161afd) ret which was declared as int was used instead of rc before.
src/tss2-tcti/tcti-mssim.c: In function ‘tcti_mssim_receive’:
src/tss2-tcti/tcti-mssim.c:408:12: error: comparison of integer expressions of different signedness: ‘TSS2_RC’ {aka ‘unsigned int’} and ‘int’ [-Werror=sign-compare]
 408 |     if (rc < (ssize_t)tcti_common->header.size) {
  • Fapi failed if sha384 is not available. sha384 should only be used in the profiles where it was used before. In the other cases the sha1 bank could be removed from the profile.
  • The test test/integration/policy-execute.c failed with:
WARNING:esys:src/tss2-esys/api/Esys_StartAuthSession.c:398:Esys_StartAuthSession_Finish() Received TPM Error
ERROR:esys:src/tss2-esys/api/Esys_StartAuthSession.c:143:Esys_StartAuthSession() Esys Finish ErrorCode (0x000005c3)
ERROR:test:test/integration/policy-execute.int.c:722:check_policy() Error: During initialization of session ErrorCode (0x000005c3)
ERROR:test:test/integration/policy-execute.int.c:939:test_policy_execute() Checking policy digest for sha384 failed ErrorCode (0x000005c3)
ERROR:test:test/integration/main-esys.c:49:main() Test returned 00000001

@joholl
Copy link
Collaborator Author

joholl commented Jul 18, 2024

CI is green now.

@AndreasFuchsTPM @JuergenReppSIT Feel free to review. Please note the digests which I just appended 0x00s, I'm not sure if those test should be green.

@AndreasFuchsTPM AndreasFuchsTPM merged commit 3f5a456 into tpm2-software:master Jul 19, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PolicyTemplate calculation incorrect Get rid of SHA1 in tests
3 participants