[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Mon May 8 10:30:01 UTC 2017
The branch master has been updated
via 12635aa09d8dbf73fc29cd8e7bfe698d9c255ca3 (commit)
via de65f7b93a79ea952a7f08e0b8b3f36d1136ae7a (commit)
via 6af87546375953d5937fb4bdaac6a887765af615 (commit)
from 218712ffddfdf3c7574c1d945e094f6601500caa (commit)
- Log -----------------------------------------------------------------
commit 12635aa09d8dbf73fc29cd8e7bfe698d9c255ca3
Author: Matt Caswell <matt at openssl.org>
Date: Mon May 8 10:54:38 2017 +0100
Updates to supported_groups following review feedback
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3395)
commit de65f7b93a79ea952a7f08e0b8b3f36d1136ae7a
Author: Matt Caswell <matt at openssl.org>
Date: Fri May 5 10:30:07 2017 +0100
Add a test for supported_groups in the EE message
Check we send supported_groups in EE if there is a group we prefer instead
of the one sent in the key_share.
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3395)
commit 6af87546375953d5937fb4bdaac6a887765af615
Author: Matt Caswell <matt at openssl.org>
Date: Fri May 5 10:27:14 2017 +0100
Send the supported_groups extension in EE where applicable
The TLSv1.3 spec says that a server SHOULD send supported_groups in the
EE message if there is a group that it prefers to the one used in the
key_share. Clients MAY act on that. At the moment we don't do anything
with it on the client side, but that may change in the future.
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3395)
-----------------------------------------------------------------------
Summary of changes:
include/openssl/ssl.h | 1 +
ssl/ssl_err.c | 2 ++
ssl/statem/extensions.c | 2 +-
ssl/statem/extensions_srvr.c | 56 ++++++++++++++++++++++++++++++++++++
ssl/statem/statem_lib.c | 4 +--
ssl/statem/statem_locl.h | 6 ++++
test/recipes/70-test_tls13messages.t | 14 ++++++++-
test/testlib/checkhandshake.pm | 3 +-
8 files changed, 82 insertions(+), 6 deletions(-)
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 764ecea..e89e97f 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2496,6 +2496,7 @@ int ERR_load_SSL_strings(void);
# define SSL_F_TLS_CONSTRUCT_STOC_SERVER_NAME 459
# define SSL_F_TLS_CONSTRUCT_STOC_SESSION_TICKET 460
# define SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST 461
+# define SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS 544
# define SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP 462
# define SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO 521
# define SSL_F_TLS_GET_MESSAGE_BODY 351
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index a845dae..18a38df 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -398,6 +398,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
"tls_construct_stoc_session_ticket"},
{ERR_FUNC(SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST),
"tls_construct_stoc_status_request"},
+ {ERR_FUNC(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS),
+ "tls_construct_stoc_supported_groups"},
{ERR_FUNC(SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP),
"tls_construct_stoc_use_srtp"},
{ERR_FUNC(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO),
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 847ff13..8984577 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -151,7 +151,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
TLSEXT_TYPE_supported_groups,
SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
NULL, tls_parse_ctos_supported_groups, NULL,
- NULL /* TODO(TLS1.3): Need to add this */,
+ tls_construct_stoc_supported_groups,
tls_construct_ctos_supported_groups, NULL
},
#else
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 7ba1aac..22d2c4a 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -868,6 +868,62 @@ int tls_construct_stoc_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context,
}
#endif
+int tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt,
+ unsigned int context, X509 *x,
+ size_t chainidx, int *al)
+{
+ const unsigned char *groups;
+ size_t numgroups, i, first = 1;
+
+ /* s->s3->group_id is non zero if we accepted a key_share */
+ if (s->s3->group_id == 0)
+ return 1;
+
+ /* Get our list of supported groups */
+ if (!tls1_get_curvelist(s, 0, &groups, &numgroups) || numgroups == 0) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+
+ /* Copy group ID if supported */
+ for (i = 0; i < numgroups; i++, groups += 2) {
+ if (tls_curve_allowed(s, groups, SSL_SECOP_CURVE_SUPPORTED)) {
+ if (first) {
+ /*
+ * Check if the client is already using our preferred group. If
+ * so we don't need to add this extension
+ */
+ if (s->s3->group_id == GET_GROUP_ID(groups, 0))
+ return 1;
+
+ /* Add extension header */
+ if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups)
+ /* Sub-packet for supported_groups extension */
+ || !WPACKET_start_sub_packet_u16(pkt)
+ || !WPACKET_start_sub_packet_u16(pkt)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS,
+ ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+
+ first = 0;
+ }
+ if (!WPACKET_put_bytes_u16(pkt, GET_GROUP_ID(groups, 0))) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS,
+ ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+ }
+ }
+
+ if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, ERR_R_INTERNAL_ERROR);
+ return 0;
+ }
+
+ return 1;
+}
+
int tls_construct_stoc_session_ticket(SSL *s, WPACKET *pkt,
unsigned int context, X509 *x,
size_t chainidx, int *al)
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 36d5534..33206c6 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1957,9 +1957,7 @@ int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
return 0;
for (i = 0; i < num_groups; i++, groups += 2) {
- unsigned int share_id = (groups[0] << 8) | (groups[1]);
-
- if (group_id == share_id
+ if (group_id == GET_GROUP_ID(groups, 0)
&& (!checkallow
|| tls_curve_allowed(s, groups, SSL_SECOP_CURVE_CHECK))) {
return 1;
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 3b9311e..49a5ed5 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -55,6 +55,9 @@ int statem_flush(SSL *s);
typedef int (*confunc_f) (SSL *s, WPACKET *pkt);
+#define GET_GROUP_ID(group, idx) \
+ (unsigned int)(((group)[(idx) * 2] << 8) | (group)[((idx) * 2) + 1])
+
int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
size_t num_groups, int checkallow);
int create_synthetic_message_hash(SSL *s);
@@ -230,6 +233,9 @@ int tls_construct_stoc_early_data(SSL *s, WPACKET *pkt, unsigned int context,
int tls_construct_stoc_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context,
X509 *x, size_t chainidx, int *al);
#endif
+int tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt,
+ unsigned int context, X509 *x,
+ size_t chainidx, int *al);
int tls_construct_stoc_session_ticket(SSL *s, WPACKET *pkt,
unsigned int context, X509 *x,
size_t chainidx, int *al);
diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t
index c9603de..c211851 100644
--- a/test/recipes/70-test_tls13messages.t
+++ b/test/recipes/70-test_tls13messages.t
@@ -123,6 +123,8 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
checkhandshake::SERVER_NAME_SRV_EXTENSION],
[TLSProxy::Message::MT_ENCRYPTED_EXTENSIONS, TLSProxy::Message::EXT_ALPN,
checkhandshake::ALPN_SRV_EXTENSION],
+ [TLSProxy::Message::MT_ENCRYPTED_EXTENSIONS, TLSProxy::Message::EXT_SUPPORTED_GROUPS,
+ checkhandshake::SUPPORTED_GROUPS_SRV_EXTENSION],
[TLSProxy::Message::MT_CERTIFICATE, TLSProxy::Message::EXT_STATUS_REQUEST,
checkhandshake::STATUS_REQUEST_SRV_EXTENSION],
@@ -145,7 +147,7 @@ $proxy->serverconnects(2);
$proxy->clientflags("-sess_out ".$session);
$proxy->sessionfile($session);
$proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 15;
+plan tests => 16;
checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
checkhandshake::DEFAULT_EXTENSIONS,
"Default handshake test");
@@ -303,4 +305,14 @@ checkhandshake($proxy, checkhandshake::HRR_RESUME_HANDSHAKE,
| checkhandshake::PSK_CLI_EXTENSION
| checkhandshake::PSK_SRV_EXTENSION,
"Resumption handshake with HRR test");
+
+#Test 16: Acceptable but non preferred key_share
+$proxy->clear();
+$proxy->clientflags("-curves P-256");
+$proxy->start();
+checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
+ checkhandshake::DEFAULT_EXTENSIONS
+ | checkhandshake::SUPPORTED_GROUPS_SRV_EXTENSION,
+ "Default handshake test");
+
unlink $session;
diff --git a/test/testlib/checkhandshake.pm b/test/testlib/checkhandshake.pm
index d5d0e29..65c5135 100644
--- a/test/testlib/checkhandshake.pm
+++ b/test/testlib/checkhandshake.pm
@@ -52,7 +52,8 @@ use constant {
PSK_SRV_EXTENSION => 0x00010000,
KEY_SHARE_SRV_EXTENSION => 0x00020000,
PSK_KEX_MODES_EXTENSION => 0x00040000,
- KEY_SHARE_HRR_EXTENSION => 0x00080000
+ KEY_SHARE_HRR_EXTENSION => 0x00080000,
+ SUPPORTED_GROUPS_SRV_EXTENSION => 0x00100000
};
our @handmessages = ();
More information about the openssl-commits
mailing list