Skip to content

Fix benchmark failure on FIPS builds#6623

Merged
dgarske merged 3 commits intowolfSSL:masterfrom
bigbrett:FIPS-TLS-benchmark-CAST-fix
May 20, 2024
Merged

Fix benchmark failure on FIPS builds#6623
dgarske merged 3 commits intowolfSSL:masterfrom
bigbrett:FIPS-TLS-benchmark-CAST-fix

Conversation

@bigbrett
Copy link
Copy Markdown
Contributor

Description

Fixes bug where tls_bench can fail on FIPS builds in a multithreaded environment because of simultaneous attempts by client and server threads to run a CAST on-the-fly during the TLS handshake. This usually manifests itself as a KAT failure in ECC or AES, though I have seen a few other spurious error messages pop up depending on timing.

Bug reproduction

Try and run tls_bench on a multithreaded system on master and it should fail and report a KAT failure. Running on this branch should resolve the issue.

Comment thread examples/benchmark/tls_bench.c Outdated
@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 16, 2024

Retest this please

@dgarske dgarske requested a review from kaleb-himes May 16, 2024 18:53
@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 16, 2024

Retest this please

@dgarske dgarske force-pushed the FIPS-TLS-benchmark-CAST-fix branch from 64df3bb to e823da9 Compare May 17, 2024 14:09
@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 17, 2024

Can't seem to get it to re-test or pass this macOS check in GitHub CI. I went ahead and rebased and force pushed. Let's see if we still get a failure.

dgarske
dgarske previously approved these changes May 17, 2024
Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaleb-himes please finalize and merge if you are happy.

Comment thread examples/benchmark/tls_bench.c Outdated
int ret = 0;
int cast_idx = 0;

for (cast_idx=0; cast_idx<FIPS_CAST_COUNT; cast_idx++) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have spaces around '=' and '<'.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bigbrett please fix. Thanks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in latest commit. @dgarske thank you for bringing this PR back from the dead!!

@dgarske dgarske assigned bigbrett and unassigned kaleb-himes and wolfSSL-Bot May 20, 2024
@bigbrett bigbrett dismissed stale reviews from dgarske and JacobBarthelmeh via c6db51b May 20, 2024 18:47
@bigbrett bigbrett assigned bigbrett and wolfSSL-Bot and unassigned bigbrett May 20, 2024
@dgarske dgarske merged commit 7d4e601 into wolfSSL:master May 20, 2024
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.

6 participants