Skip to content

Comments

Make version check in s_server DTLS aware#16709

Closed
Sean-Der wants to merge 1 commit intoopenssl:masterfrom
Sean-Der:master
Closed

Make version check in s_server DTLS aware#16709
Sean-Der wants to merge 1 commit intoopenssl:masterfrom
Sean-Der:master

Conversation

@Sean-Der
Copy link

Resolves #16707

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Sep 29, 2021
@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Sep 29, 2021
@t8m t8m added approval: review pending This pull request needs review by a committer branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) branch: 3.0 Applies to openssl-3.0 branch branch: master Applies to master branch cla: trivial One of the commits is marked as 'CLA: trivial' triaged: bug The issue/pr is/fixes a bug labels Sep 30, 2021
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.

LGTM. Good for all branches. I am also OK with CLA: trivial.

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.

LGTM. Fine for all branched and agreed trivial.
@mattcaswell any thoughts?

@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 Sep 30, 2021
@t8m
Copy link
Member

t8m commented Oct 1, 2021

@paulidale Would you please formally approve?

@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.

@t8m
Copy link
Member

t8m commented Oct 5, 2021

@paulidale ping

BIO_printf(bio_s_out, "psk_server_cb\n");

if (SSL_version(ssl) >= TLS1_3_VERSION) {
if (SSL_version(ssl) >= TLS1_3_VERSION && SSL_version(ssl) < DTLS1_2_VERSION) {
Copy link
Member

Choose a reason for hiding this comment

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

This should instead be checking SSL_is_dtls()

@Sean-Der Sean-Der closed this Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) branch: 3.0 Applies to openssl-3.0 branch cla: trivial One of the commits is marked as 'CLA: trivial' triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

psk_server_cb in s_server returns 0 for DTLS because of invalid SSL version detection.

6 participants