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.

Sign up for GitHub

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

Jump to bottom

openssl speed fails in FIPS mode #17331

Closed
wants to merge 1 commit into from
Closed

openssl speed fails in FIPS mode #17331

wants to merge 1 commit into from

Conversation

beldmit
Copy link
Member

@beldmit beldmit commented Dec 21, 2021

...because it uses md5 for HMAC tests. Skip md5 in case of its
unavailability.

Checklist
  • documentation is added or updated
  • tests are added or updated

@beldmit beldmit added branch: 3.0 Merge to openssl-3.0 branch branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug approval: review pending This pull request needs review by a committer labels Dec 21, 2021
t8m
t8m reviewed Dec 21, 2021
View reviewed changes
apps/speed.c Outdated
if (!EVP_MAC_CTX_set_params(loopargs[i].mctx, params)) {
EVP_MAC_CTX_free(loopargs[i].mctx);
loopargs[i].mctx = NULL;
}
Copy link
Member

Choose a reason for hiding this comment

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

I'd just add a label after skip over to it instead of running the benchmark with NULL mctx. (Perhaps add some comment.)

Copy link
Member Author

Choose a reason for hiding this comment

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

@t8m sorry, I don't get your idea :(

Copy link
Member

Choose a reason for hiding this comment

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

I'd just add a label at if (doit[D_CBC_DES]) { and put goto here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm... Thanks, I will consider it.

@beldmit
Copy link
Member Author

beldmit commented Dec 21, 2021

Currently we have a code potentially allowing more than one loopargs

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/committers but the last update was 61 days ago

@paulidale
Copy link
Contributor

What is needed to progress this?

@beldmit
Copy link
Member Author

beldmit commented Feb 21, 2022

I need find some time to implement it according Tomaš's suggestion

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/committers but the last update was 61 days ago

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/committers but the last update was 92 days ago

@t8m t8m added stalled: awaiting contributor response This pull request is awaiting a response by the contributor and removed approval: review pending This pull request needs review by a committer labels May 25, 2022
@openssl-machine
Copy link
Collaborator

This PR is waiting for the creator to make requested changes but it has not been updated for 30 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

@DDvO
Copy link
Contributor

DDvO commented Jul 18, 2022

I need find some time to implement it according Tomaš's suggestion

@beldmit, are you still aware of this PR?

@beldmit
Copy link
Member Author

beldmit commented Jul 18, 2022

I'm still aware but it's relatively low in my priority list for now

@openssl-machine
Copy link
Collaborator

This PR is waiting for the creator to make requested changes but it has not been updated for 30 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

@beldmit
openssl speed fails in FIPS mode
b4d7ee4 
...because it uses md5 for HMAC tests. Skip md5 in case of its
unavailability.
@beldmit beldmit added approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member and removed stalled: awaiting contributor response This pull request is awaiting a response by the contributor labels Aug 20, 2022
@beldmit
Copy link
Member Author

beldmit commented Aug 20, 2022

I finally decided to fix it :)

@t-j-h t-j-h removed the approval: otc review pending This pull request needs review by an OTC member label Aug 20, 2022
@paulidale paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Aug 21, 2022
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Aug 23, 2022
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Aug 23, 2022
@beldmit @paulidale
openssl speed fails in FIPS mode
c63e863 
...because it uses md5 for HMAC tests. Skip md5 in case of its
unavailability.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17331)
@paulidale
Copy link
Contributor

Merged, thanks.

@paulidale paulidale closed this Aug 23, 2022
openssl-machine pushed a commit that referenced this pull request Aug 23, 2022
@beldmit @paulidale
openssl speed fails in FIPS mode
2686c5c 
...because it uses md5 for HMAC tests. Skip md5 in case of its
unavailability.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17331)

(cherry picked from commit c63e863)
sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
@beldmit @sftcd
openssl speed fails in FIPS mode
18e9e01 
...because it uses md5 for HMAC tests. Skip md5 in case of its
unavailability.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#17331)
beldmit added a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
@beldmit
openssl speed fails in FIPS mode
6334108 
...because it uses md5 for HMAC tests. Skip md5 in case of its
unavailability.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#17331)
@beldmit beldmit deleted the speed_fips branch January 16, 2023 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t8m t8m t8m approved these changes

@t-j-h t-j-h t-j-h approved these changes

@paulidale paulidale paulidale approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@beldmit @openssl-machine @paulidale @DDvO @t8m @t-j-h