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

speed: make hmac(sha256) the default hmac #22793

Closed
wants to merge 2 commits into from
Closed

speed: make hmac(sha256) the default hmac #22793

wants to merge 2 commits into from

Conversation

jamuir
Copy link
Member

@jamuir jamuir commented Nov 21, 2023

prefer hmac(sha256) rather than hmac(md5). Also, drop the "skip_hmac" label. If we are supposed to do hmac(hash_func) and hash_func cannot be found, then error out immediately.

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

speed: make hmac(sha256) the default hmac
1f6a1ce 
prefer hmac(sha256) rather than hmac(md5).  Also, drop the "skip_hmac"
label.  If we are supposed to do hmac(hash_func) and hash_func cannot
be found, then error out immediately.
@jamuir jamuir added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member tests: exempted The PR is exempt from requirements for testing branch: 3.2 Merge to openssl-3.2 labels Nov 21, 2023
@t8m t8m added triaged: feature The issue/pr requests/adds a feature and removed branch: 3.2 Merge to openssl-3.2 labels Nov 21, 2023
@t8m
Copy link
Member

t8m commented Nov 21, 2023

Too late for 3.2 - this is not a bug fix.

@t8m t8m removed the approval: otc review pending This pull request needs review by an OTC member label Nov 21, 2023
@jamuir
Copy link
Member Author

jamuir commented Nov 21, 2023

Too late for 3.2 - this is not a bug fix.

agreed that it does not need to be part of the first 3.2 release.

I applied the label thinking it could be applied to the 3.2 branch after the initial release.

@hlandau hlandau 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 Nov 21, 2023
@hlandau
Copy link
Member

hlandau commented Nov 21, 2023

Approved for master. Not for 3.2 branch.

@t8m
Copy link
Member

t8m commented Nov 21, 2023

I applied the label thinking it could be applied to the 3.2 branch after the initial release.

As it is not a bug fix it cannot.

@jamuir jamuir self-assigned this Nov 21, 2023
@jamuir
Copy link
Member Author

jamuir commented Nov 21, 2023

should I add a note to CHANGES.md about this? i.e. a note to explain that openssl speed hmac now exercises hmac-sha256 rather than hmac-md5?

@hlandau
Copy link
Member

hlandau commented Nov 22, 2023

should I add a note to CHANGES.md about this? i.e. a note to explain that openssl speed hmac now exercises hmac-sha256 rather than hmac-md5?

Yes, this would be nice.

@jamuir
Copy link
Member Author

jamuir commented Nov 22, 2023

I've pushed an update to CHANGES.md.

Please re-confirm.

t8m
t8m approved these changes Nov 22, 2023
View reviewed changes
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

Still OK

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@jamuir jamuir 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 Nov 23, 2023
@jamuir jamuir mentioned this pull request Nov 23, 2023
Closed
2 tasks
@hlandau
Copy link
Member

hlandau commented Nov 23, 2023

Merged to master. Thank you.

@hlandau hlandau closed this Nov 23, 2023
openssl-machine pushed a commit that referenced this pull request Nov 23, 2023
@hlandau
speed: make hmac(sha256) the default hmac
e580f06 
prefer hmac(sha256) rather than hmac(md5).  Also, drop the "skip_hmac"
label.  If we are supposed to do hmac(hash_func) and hash_func cannot
be found, then error out immediately.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #22793)
wanghao75 pushed a commit to openeuler-mirror/openssl that referenced this pull request Nov 25, 2023
speed: make hmac(sha256) the default hmac
1741c94 
prefer hmac(sha256) rather than hmac(md5).  Also, drop the "skip_hmac"
label.  If we are supposed to do hmac(hash_func) and hash_func cannot
be found, then error out immediately.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl/openssl#22793)

Signed-off-by: fly2x <fly2x@hitls.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hlandau hlandau hlandau approved these changes

@t8m t8m t8m approved these changes

Assignees

@jamuir jamuir

Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch tests: exempted The PR is exempt from requirements for testing triaged: feature The issue/pr requests/adds a feature
Projects
Project Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jamuir @t8m @hlandau @openssl-machine