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

Fix BIO_get_ktls_send/recv to return 0 or 1 only #18178

Closed
wants to merge 3 commits into from
Closed

Fix BIO_get_ktls_send/recv to return 0 or 1 only #18178

wants to merge 3 commits into from

Conversation

t8m
Copy link
Member

@t8m t8m commented Apr 26, 2022

We use it as a boolean everywhere in the code and there is no reason to distinguish between the 0 and -1 return.

Fixes #18176

Checklist
  • documentation is added or updated

@t8m t8m 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 triaged: bug The issue/pr is/fixes a bug branch: 3.0 Merge to openssl-3.0 branch labels Apr 26, 2022
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Apr 26, 2022
@t8m t8m requested a review from mattcaswell April 27, 2022 08:28
mattcaswell
mattcaswell reviewed Apr 27, 2022
View reviewed changes
include/openssl/bio.h.in
# define BIO_get_ktls_recv(b) \
BIO_ctrl(b, BIO_CTRL_GET_KTLS_RECV, 0, NULL)
(BIO_ctrl(b, BIO_CTRL_GET_KTLS_RECV, 0, NULL) > 0)
Copy link
Member

Choose a reason for hiding this comment

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

This is actually slightly problematic. This changes the meaning of a public macro. Applications compiled against old headers may behave incorrectly even if they are linked against an up-to-date and fully patched library.

I'm not sure how we should handle that.

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO we can add documentation of that. The change is backwards compatible itself. I.e., if the app handles -1 return it won't be broken by this change.

@mattcaswell mattcaswell removed the approval: otc review pending This pull request needs review by an OTC member label Apr 28, 2022
akarl10
akarl10 reviewed May 9, 2022
View reviewed changes
doc/man3/BIO_ctrl.pod Outdated
@@ -167,7 +167,7 @@ the case of BIO_seek() on a file BIO for a successful operation.
=head1 HISTORY

The BIO_get_ktls_send() and BIO_get_ktls_recv() macros were added in
OpenSSL 3.0.
OpenSSL 3.0. They were modified to never return -1 in OpenSSL 3.0.3.
Copy link

Choose a reason for hiding this comment

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

3.0.3 in already out without this, so I think it will be 3.0.4

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! Thanks for the heads up. I'll update the PR.

@t8m
Copy link
Member Author

t8m commented May 9, 2022

@mattcaswell assuming still OK.

Ping for the second review.

@bernd-edlinger bernd-edlinger 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 May 13, 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 May 14, 2022
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@t8m
Copy link
Member Author

t8m commented May 16, 2022

Merged to master and openssl-3.0 branches. Thank you for the reviews.

@t8m t8m closed this May 16, 2022
openssl-machine pushed a commit that referenced this pull request May 16, 2022
@t8m
Fix BIO_get_ktls_send/recv to return 0 or 1 only
524bac5 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #18178)
openssl-machine pushed a commit that referenced this pull request May 16, 2022
@t8m
Fix BIO_get_ktls_send/recv to return 0 or 1 only
1417e2b 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #18178)

(cherry picked from commit 524bac5)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 17, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
b0c4cef 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 17, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
251e68b 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 17, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
790aea5 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 17, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
c0e2978 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
613bb63 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
6e39544 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
83c7565 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
332ca29 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
16238cd 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
0c3dae6 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
4abcb44 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
412e836 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
f5906aa 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
aa230c7 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
80dc01c 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 18, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
d1e3d66 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
723adf6 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
831dc57 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
919d7f0 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
6982266 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
01e60c2 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
5a6c63f 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
46e4cf2 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
11b9d23 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
48c2080 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
a42d0ea 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
4a071be 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
0dc9014 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
642b4fd 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
a118dc9 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
e94e3f3 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
JeffroMF pushed a commit to JeffroMF/openssl555555555555 that referenced this pull request May 19, 2022
@t8m @JeffroMF
Fix BIO_get_ktls_send/recv to return 0 or 1 only
57d7e86 
Fixes #18176

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from openssl/openssl#18178)
@freddy77 freddy77 mentioned this pull request Nov 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akarl10 akarl10 akarl10 left review comments

@mattcaswell mattcaswell mattcaswell approved these changes

@bernd-edlinger bernd-edlinger bernd-edlinger 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 severity: fips change The pull request changes FIPS provider sources 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.

OpenSSL 3.0 requires impementation of BIO_CTRL_GET_KTLS_SEND in custom BIO
5 participants
@t8m @openssl-machine @akarl10 @mattcaswell @bernd-edlinger