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

optimise ssl3_get_cipher_by_std_name() #13000

Closed

Conversation

hklaas
Copy link
Contributor

@hklaas hklaas commented Sep 26, 2020

Return immediately on matched cipher. Without this patch the code only breaks out of the inner for loop, meaning for a matched TLS13 cipher the code will still loop through 160ish SSL3 ciphers.

CLA: trivial

Checklist

Return immediately on matched cipher. Without this patch the code only breaks out of the inner for loop, meaning for a matched TLS13 cipher the code will still loop through 160ish SSL3 ciphers.

CLA: trivial
@paulidale paulidale added approval: review pending This pull request needs review by a committer branch: master Merge to master branch labels Sep 26, 2020
@kroeckx kroeckx added the cla: trivial One of the commits is marked as 'CLA: trivial' label Sep 26, 2020
Copy link
Member

@beldmit beldmit left a comment

Choose a reason for hiding this comment

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

LGTM

@beldmit beldmit 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 Sep 26, 2020
@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 Sep 27, 2020
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@beldmit
Copy link
Member

beldmit commented Sep 27, 2020

Merged. Thanks!

@beldmit beldmit closed this Sep 27, 2020
openssl-machine pushed a commit that referenced this pull request Sep 27, 2020
Return immediately on matched cipher. Without this patch the code only breaks out of the inner for loop, meaning for a matched TLS13 cipher the code will still loop through 160ish SSL3 ciphers.

CLA: trivial

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #13000)
@kaduk
Copy link
Contributor

kaduk commented Sep 27, 2020

Looking at this made me realize that the SCSVs don't need any special handling, thus #13010

@hklaas hklaas deleted the ssl3_get_cipher_by_std_name-optimisation branch September 28, 2020 05:40
kaduk pushed a commit to kaduk/openssl that referenced this pull request Oct 29, 2020
Return immediately on matched cipher. Without this patch the code only breaks out of the inner for loop, meaning for a matched TLS13 cipher the code will still loop through 160ish SSL3 ciphers.

CLA: trivial

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from openssl#13000)

(cherry picked from commit d93bded)
@kaduk kaduk mentioned this pull request Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch cla: trivial One of the commits is marked as 'CLA: trivial'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants