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

TLS check temp key type #2191

Closed
wants to merge 2 commits into from
Closed

Conversation

snhenson
Copy link
Contributor

@snhenson snhenson commented Jan 8, 2017

Checklist
  • documentation is added or updated
  • tests are added or updated
  • CLA is signed
Description of change

This adds a new test option so we can check the server temp key is of the expected type. It also extends the existing curve selection test to check the curve the server uses.

Add option ExpectedTmpKeyType to test the temporary key the server
sends is of the correct type.
@snhenson snhenson changed the title Tls check tmp key TLS check temp key type Jan 8, 2017
@@ -1038,6 +1039,19 @@ static HANDSHAKE_RESULT *do_handshake_internal(
if (session_out != NULL)
*session_out = SSL_get1_session(client.ssl);

if (SSL_get_server_tmp_key(client.ssl, &tmp_key)) {
int nid;
Copy link
Member

Choose a reason for hiding this comment

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

Some people seem to want a blank line here, but our style document doesn't seem to say anything about it.

Copy link
Member

Choose a reason for hiding this comment

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

We should add it to the style document as it has become a defacto standard style in recent reviews.

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Approved subject to the couple of minor style issues being corrected.

__owur static int parse_expected_tmp_key_type(SSL_TEST_CTX *test_ctx,
const char *value)
{
int nid;
Copy link
Member

Choose a reason for hiding this comment

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

Blank line here

@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label Jan 8, 2017
@snhenson snhenson added 1.1.0 branch: master Merge to master branch labels Jan 8, 2017
rhuijben pushed a commit to rhuijben/openssl that referenced this pull request Jan 8, 2017
Add option ExpectedTmpKeyType to test the temporary key the server
sends is of the correct type.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl/openssl#2191)
rhuijben pushed a commit to rhuijben/openssl that referenced this pull request Jan 8, 2017
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl/openssl#2191)
rhuijben pushed a commit to rhuijben/openssl that referenced this pull request Jan 8, 2017
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl/openssl#2191)
(cherry picked from commit 9c4319b)
rhuijben pushed a commit to rhuijben/openssl that referenced this pull request Jan 8, 2017
Add option ExpectedTmpKeyType to test the temporary key the server
sends is of the correct type.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl/openssl#2191)
(cherry picked from commit b93ad05)
@snhenson snhenson closed this Jan 9, 2017
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 Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants