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

Fix the s_server -dtls option #12179

Closed
wants to merge 2 commits into from

Conversation

mattcaswell
Copy link
Member

DTLS in s_server was failing due to problems with the cookie generate callback which was recently converted to use EVP_MAC. The new code was failing to return the length of the generated cookie, and so libssl aborted the connection.

While I was at this fix I also corrected the value of the DTLS1_COOKIE_LENGTH value in the public header. This was known to be off-by-one, but could not be fixed due to ABI compat concerns. Since 3.0 is not ABI compatible we can fix it.

The DTLS1_COOKIE_LENGTH value was incorrect in the header files. We
couldn't change it before due to ABI concerns. However 3.0 is not ABI
compatible so we can now fix it.
The generate_cookie_callback was failing to pass back the generated
cookie length to the caller. This results in DTLS connection failures
from s_server.
@mattcaswell mattcaswell added the branch: master Merge to master branch label Jun 17, 2020
Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

Travis failure isn't due to this PR, but it would be nice to fix.

Line 845: unsigned int temp = 0;
And a blank line between 846 and 847.

Approved either way.

@paulidale paulidale added the approval: done This pull request has the required number of approvals label Jun 18, 2020
@paulidale paulidale added this to the 3.0.0 milestone Jun 18, 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 Jun 19, 2020
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@mattcaswell
Copy link
Member Author

Pushed, including the suggested tweak. Thanks.

openssl-machine pushed a commit that referenced this pull request Jun 19, 2020
The DTLS1_COOKIE_LENGTH value was incorrect in the header files. We
couldn't change it before due to ABI concerns. However 3.0 is not ABI
compatible so we can now fix it.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #12179)
openssl-machine pushed a commit that referenced this pull request Jun 19, 2020
The generate_cookie_callback was failing to pass back the generated
cookie length to the caller. This results in DTLS connection failures
from s_server.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #12179)
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants