Skip to content

Commit

Permalink
Support TLS1.3 extensions with DTLS1.3
Browse files Browse the repository at this point in the history
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #22261)
  • Loading branch information
fwh-dc authored and mattcaswell committed Mar 28, 2024
1 parent 1b71ffa commit e925a2b
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 77 deletions.
66 changes: 35 additions & 31 deletions ssl/statem/extensions.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
* to indicate to the client the complete list of groups supported
* by the server, with the server instead just indicating the
* selected group for this connection in the ServerKeyExchange
* message. TLS 1.3 adds a scheme for the server to indicate
* message. (D)TLS 1.3 adds a scheme for the server to indicate
* to the client its list of supported groups in the
* EncryptedExtensions message, but none of the relevant
* specifications permit sending supported_groups in the ServerHello.
Expand All @@ -198,7 +198,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
* ServerHello anyway. Up to and including the 1.1.0 release,
* we did not check for the presence of nonpermitted extensions,
* so to avoid a regression, we must permit this extension in the
* TLS 1.2 ServerHello as well.
* (D)TLS 1.2 ServerHello as well.
*
* Note that there is no tls_parse_stoc_supported_groups function,
* so we do not perform any additional parsing, validation, or
Expand Down Expand Up @@ -339,7 +339,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
{
TLSEXT_TYPE_supported_versions,
SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_SERVER_HELLO
| SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST | SSL_EXT_TLS_IMPLEMENTATION_ONLY,
| SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
NULL,
/* Processed inline as part of version selection */
NULL, tls_parse_stoc_supported_versions,
Expand All @@ -348,8 +348,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
},
{
TLSEXT_TYPE_psk_kex_modes,
SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS_IMPLEMENTATION_ONLY
| SSL_EXT_TLS1_3_ONLY,
SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_ONLY,
init_psk_kex_modes, tls_parse_ctos_psk_kex_modes, NULL, NULL,
tls_construct_ctos_psk_kex_modes, NULL
},
Expand All @@ -360,7 +359,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
*/
TLSEXT_TYPE_key_share,
SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_SERVER_HELLO
| SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST | SSL_EXT_TLS_IMPLEMENTATION_ONLY
| SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST
| SSL_EXT_TLS1_3_ONLY,
NULL, tls_parse_ctos_key_share, tls_parse_stoc_key_share,
tls_construct_stoc_key_share, tls_construct_ctos_key_share,
Expand All @@ -370,7 +369,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
/* Must be after key_share */
TLSEXT_TYPE_cookie,
SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST
| SSL_EXT_TLS_IMPLEMENTATION_ONLY | SSL_EXT_TLS1_3_ONLY,
| SSL_EXT_TLS1_3_ONLY,
NULL, tls_parse_ctos_cookie, tls_parse_stoc_cookie,
tls_construct_stoc_cookie, tls_construct_ctos_cookie, NULL
},
Expand All @@ -388,7 +387,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
{
TLSEXT_TYPE_compress_certificate,
SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_CERTIFICATE_REQUEST
| SSL_EXT_TLS_IMPLEMENTATION_ONLY | SSL_EXT_TLS1_3_ONLY,
| SSL_EXT_TLS1_3_ONLY,
tls_init_compress_certificate,
tls_parse_compress_certificate, tls_parse_compress_certificate,
tls_construct_compress_certificate, tls_construct_compress_certificate,
Expand Down Expand Up @@ -420,10 +419,10 @@ static const EXTENSION_DEFINITION ext_defs[] = {
NULL, NULL, NULL, tls_construct_ctos_padding, NULL
},
{
/* Required by the TLSv1.3 spec to always be the last extension */
/* Required by the (D)TLSv1.3 spec to always be the last extension */
TLSEXT_TYPE_psk,
SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_SERVER_HELLO
| SSL_EXT_TLS_IMPLEMENTATION_ONLY | SSL_EXT_TLS1_3_ONLY,
| SSL_EXT_TLS1_3_ONLY,
NULL, tls_parse_ctos_psk, tls_parse_stoc_psk, tls_construct_stoc_psk,
tls_construct_ctos_psk, final_psk
}
Expand Down Expand Up @@ -554,33 +553,33 @@ static int verify_extension(SSL_CONNECTION *s, unsigned int context,
int extension_is_relevant(SSL_CONNECTION *s, unsigned int extctx,
unsigned int thisctx)
{
int is_tls13;
int is_version13;

/*
* For HRR we haven't selected the version yet but we know it will be
* TLSv1.3
* (D)TLSv1.3
*/
if ((thisctx & SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0)
is_tls13 = 1;
is_version13 = 1;
else
is_tls13 = SSL_CONNECTION_IS_TLS13(s);
is_version13 = SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s);

if ((SSL_CONNECTION_IS_DTLS(s)
&& (extctx & SSL_EXT_TLS_IMPLEMENTATION_ONLY) != 0)
|| (s->version == SSL3_VERSION
&& (extctx & SSL_EXT_SSL3_ALLOWED) == 0)
/*
* Note that SSL_IS_TLS13() means "TLS 1.3 has been negotiated",
* Note that is_version13 means "(D)TLS 1.3 has been negotiated",
* which is never true when generating the ClientHello.
* However, version negotiation *has* occurred by the time the
* ClientHello extensions are being parsed.
* Be careful to allow TLS 1.3-only extensions when generating
* Be careful to allow (D)TLS 1.3-only extensions when generating
* the ClientHello.
*/
|| (is_tls13 && (extctx & SSL_EXT_TLS1_2_AND_BELOW_ONLY) != 0)
|| (!is_tls13 && (extctx & SSL_EXT_TLS1_3_ONLY) != 0
|| (is_version13 && (extctx & SSL_EXT_TLS1_2_AND_BELOW_ONLY) != 0)
|| (!is_version13 && (extctx & SSL_EXT_TLS1_3_ONLY) != 0
&& (thisctx & SSL_EXT_CLIENT_HELLO) == 0)
|| (s->server && !is_tls13 && (extctx & SSL_EXT_TLS1_3_ONLY) != 0)
|| (s->server && !is_version13 && (extctx & SSL_EXT_TLS1_3_ONLY) != 0)
|| (s->hit && (extctx & SSL_EXT_IGNORE_ON_RESUMPTION) != 0))
return 0;
return 1;
Expand Down Expand Up @@ -831,7 +830,8 @@ int should_add_extension(SSL_CONNECTION *s, unsigned int extctx,
if (!extension_is_relevant(s, extctx, thisctx)
|| ((extctx & SSL_EXT_TLS1_3_ONLY) != 0
&& (thisctx & SSL_EXT_CLIENT_HELLO) != 0
&& (SSL_CONNECTION_IS_DTLS(s) || max_version < TLS1_3_VERSION)))
&& (SSL_CONNECTION_IS_DTLS(s) ? DTLS_VERSION_LT(max_version, DTLS1_3_VERSION)
: max_version < TLS1_3_VERSION)))
return 0;

return 1;
Expand All @@ -858,7 +858,7 @@ int tls_construct_extensions(SSL_CONNECTION *s, WPACKET *pkt,
/*
* If extensions are of zero length then we don't even add the
* extensions length bytes to a ClientHello/ServerHello
* (for non-TLSv1.3).
* (for non-(D)TLSv1.3).
*/
|| ((context &
(SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO)) != 0
Expand Down Expand Up @@ -1069,8 +1069,8 @@ static int final_server_name(SSL_CONNECTION *s, unsigned int context, int sent)
return 0;

case SSL_TLSEXT_ERR_ALERT_WARNING:
/* TLSv1.3 doesn't have warning alerts so we suppress this */
if (!SSL_CONNECTION_IS_TLS13(s))
/* (D)TLSv1.3 doesn't have warning alerts so we suppress this */
if (!(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))
ssl3_send_alert(s, SSL3_AL_WARNING, altmp);
s->servername_done = 0;
return 1;
Expand Down Expand Up @@ -1177,15 +1177,15 @@ static int final_alpn(SSL_CONNECTION *s, unsigned int context, int sent)
if (!s->server && !sent && s->session->ext.alpn_selected != NULL)
s->ext.early_data_ok = 0;

if (!s->server || !SSL_CONNECTION_IS_TLS13(s))
if (!s->server || !(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))
return 1;

/*
* Call alpn_select callback if needed. Has to be done after SNI and
* cipher negotiation (HTTP/2 restricts permitted ciphers). In TLSv1.3
* cipher negotiation (HTTP/2 restricts permitted ciphers). In (D)TLSv1.3
* we also have to do this before we decide whether to accept early_data.
* In TLSv1.3 we've already negotiated our cipher so we do this call now.
* For < TLSv1.3 we defer it until after cipher negotiation.
* In (D)TLSv1.3 we've already negotiated our cipher so we do this call now.
* For < (D)TLSv1.3 we defer it until after cipher negotiation.
*
* On failure SSLfatal() already called.
*/
Expand Down Expand Up @@ -1337,7 +1337,7 @@ static int init_srtp(SSL_CONNECTION *s, unsigned int context)

static int final_sig_algs(SSL_CONNECTION *s, unsigned int context, int sent)
{
if (!sent && SSL_CONNECTION_IS_TLS13(s) && !s->hit) {
if (!sent && (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) && !s->hit) {
SSLfatal(s, TLS13_AD_MISSING_EXTENSION,
SSL_R_MISSING_SIGALGS_EXTENSION);
return 0;
Expand All @@ -1349,7 +1349,7 @@ static int final_sig_algs(SSL_CONNECTION *s, unsigned int context, int sent)
static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent)
{
#if !defined(OPENSSL_NO_TLS1_3)
if (!SSL_CONNECTION_IS_TLS13(s))
if (!(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))
return 1;

/* Nothing to do for key_share in an HRR */
Expand Down Expand Up @@ -1446,14 +1446,18 @@ static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent)
* Find the first group we allow that is also in client's list
*/
for (i = 0; i < num_groups; i++) {
int version;

group_id = pgroups[i];
version = SSL_CONNECTION_IS_DTLS(s) ?
DTLS1_3_VERSION : TLS1_3_VERSION;

if (check_in_list(s, group_id, clntgroups, clnt_num_groups,
1)
&& tls_group_allowed(s, group_id,
SSL_SECOP_CURVE_SUPPORTED)
&& tls_valid_group(s, group_id, TLS1_3_VERSION,
TLS1_3_VERSION, 0, NULL))
&& tls_valid_group(s, group_id, version,
version, 0, NULL))
break;
}

Expand Down

0 comments on commit e925a2b

Please sign in to comment.