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

Modify is_tls13_capable() to take account of the servername cb (1.1.1) #13305

Closed
wants to merge 2 commits into from
Closed

Modify is_tls13_capable() to take account of the servername cb (1.1.1) #13305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
172b1e0
Modify is_tls13_capable() to take account of the servername cb
mattcaswell Nov 3, 2020
dc5b6ef
Test that we can negotiate TLSv1.3 if we have an SNI callback
mattcaswell Nov 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions ssl/statem/statem_lib.c
View file
Open in desktop
Expand Up @@ -1504,8 +1504,8 @@ static int ssl_method_error(const SSL *s, const SSL_METHOD *method)

/*
* Only called by servers. Returns 1 if the server has a TLSv1.3 capable
* certificate type, or has PSK or a certificate callback configured. Otherwise
* returns 0.
* certificate type, or has PSK or a certificate callback configured, or has
* a servername callback configure. Otherwise returns 0.
Copy link
Member

Choose a reason for hiding this comment

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

s/configure/configured/

*/
static int is_tls13_capable(const SSL *s)
{
Expand All @@ -1515,6 +1515,17 @@ static int is_tls13_capable(const SSL *s)
EC_KEY *eckey;
#endif

if (!ossl_assert(s->ctx != NULL) || !ossl_assert(s->session_ctx != NULL))
return 0;

/*
* A servername callback can change the available certs, so if a servername
* cb is set then we just assume TLSv1.3 will be ok
*/
if (s->ctx->ext.servername_cb != NULL
|| s->session_ctx->ext.servername_cb != NULL)
return 1;

#ifndef OPENSSL_NO_PSK
if (s->psk_server_callback != NULL)
return 1;
Expand Down
59 changes: 59 additions & 0 deletions test/sslapitest.c
View file
Open in desktop
Expand Up @@ -6658,6 +6658,62 @@ static int test_ssl_dup(void)
}
#endif

#ifndef OPENSSL_NO_TLS1_3
/*
* Test that setting an SNI callback works with TLSv1.3. Specifically we check
* that it works even without a certificate configured for the original
* SSL_CTX
*/
static int test_sni_tls13(void)
{
SSL_CTX *cctx = NULL, *sctx = NULL, *sctx2 = NULL;
SSL *clientssl = NULL, *serverssl = NULL;
int testresult = 0;

/* Reset callback counter */
snicb = 0;

/* Create an initial SSL_CTX with no certificate configured */
sctx = SSL_CTX_new(TLS_server_method());
if (!TEST_ptr(sctx))
goto end;
/* Require TLSv1.3 as a minimum */
if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), TLS_client_method(),
TLS1_3_VERSION, 0, &sctx2, &cctx, cert,
privkey)))
goto end;

/* Set up SNI */
if (!TEST_true(SSL_CTX_set_tlsext_servername_callback(sctx, sni_cb))
|| !TEST_true(SSL_CTX_set_tlsext_servername_arg(sctx, sctx2)))
goto end;

/*
* Connection should still succeed because the final SSL_CTX has the right
* certificates configured.
*/
if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
&clientssl, NULL, NULL))
|| !TEST_true(create_ssl_connection(serverssl, clientssl,
SSL_ERROR_NONE)))
goto end;

/* We should have had the SNI callback called exactly once */
if (!TEST_int_eq(snicb, 1))
goto end;

testresult = 1;

end:
SSL_free(serverssl);
SSL_free(clientssl);
SSL_CTX_free(sctx2);
SSL_CTX_free(sctx);
SSL_CTX_free(cctx);
return testresult;
}
#endif

int setup_tests(void)
{
if (!TEST_ptr(certsdir = test_get_argument(0))
Expand Down Expand Up @@ -6780,6 +6836,9 @@ int setup_tests(void)
ADD_ALL_TESTS(test_servername, 10);
#ifndef OPENSSL_NO_TLS1_2
ADD_TEST(test_ssl_dup);
#endif
#ifndef OPENSSL_NO_TLS1_3
ADD_TEST(test_sni_tls13);
#endif
return 1;
}
Expand Down