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
Don't ask for an invalid group in an HRR #21163
Conversation
If the client sends us a group in a key_share that is in our supported_groups list but is otherwise not suitable (e.g. not compatible with TLSv1.3) we reject it. We should not ask for that same group again in a subsequent HRR. Fixes openssl#21157
Test that if the client sends a key share for a group in the server's supported_group list but is otherwise invalid, that we don't select it in the HRR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple minor things that don't block approval.
@@ -1449,7 +1449,11 @@ static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent) | |||
group_id = pgroups[i]; | |||
|
|||
if (check_in_list(s, group_id, clntgroups, clnt_num_groups, | |||
1)) | |||
1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does line-length allow for this to be put with the previous line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No - line-length prevents this
This pull request is ready to merge |
@mattcaswell do you want to change what @tmshort suggests? Or just merge please. |
Still good, though the |
This pull request is ready to merge |
If the client sends us a group in a key_share that is in our supported_groups list but is otherwise not suitable (e.g. not compatible with TLSv1.3) we reject it. We should not ask for that same group again in a subsequent HRR. Fixes #21157 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Todd Short <todd.short@me.com> (Merged from #21163)
Test that if the client sends a key share for a group in the server's supported_group list but is otherwise invalid, that we don't select it in the HRR. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Todd Short <todd.short@me.com> (Merged from #21163)
If the client sends us a group in a key_share that is in our supported_groups list but is otherwise not suitable (e.g. not compatible with TLSv1.3) we reject it. We should not ask for that same group again in a subsequent HRR. Fixes #21157 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Todd Short <todd.short@me.com> (Merged from #21163) (cherry picked from commit 7a949ae)
If the client sends us a group in a key_share that is in our supported_groups list but is otherwise not suitable (e.g. not compatible with TLSv1.3) we reject it. We should not ask for that same group again in a subsequent HRR. Fixes #21157 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Todd Short <todd.short@me.com> (Merged from #21163) (cherry picked from commit 7a949ae)
Pushed to master/3.1/3.0. Thanks. |
If the client sends us a group in a key_share that is in our
supported_groups list but is otherwise not suitable (e.g. not compatible
with TLSv1.3) we reject it. We should not ask for that same group again
in a subsequent HRR.
Fixes #21157