Store groups as uint16_t
Instead of storing supported groups in on-the-wire format store
them as parsed uint16_t values. This simplifies handling of groups
as the values can be directly used instead of being converted.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4406)
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 61203ed..8b88b21 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1137,7 +1137,7 @@
&& (!s->hit
|| (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE)
!= 0)) {
- const unsigned char *pcurves, *pcurvestmp, *clntcurves;
+ const uint16_t *pcurves, *clntcurves;
size_t num_curves, clnt_num_curves, i;
unsigned int group_id = 0;
@@ -1158,9 +1158,8 @@
}
/* Find the first group we allow that is also in client's list */
- for (i = 0, pcurvestmp = pcurves; i < num_curves;
- i++, pcurvestmp += 2) {
- group_id = bytestogroup(pcurvestmp);
+ for (i = 0; i < num_curves; i++) {
+ group_id = pcurves[i];
if (check_in_list(s, group_id, clntcurves, clnt_num_curves, 1))
break;
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index bffe7ac..6975b94 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -139,7 +139,7 @@
unsigned int context, X509 *x,
size_t chainidx, int *al)
{
- const unsigned char *pcurves = NULL, *pcurvestmp;
+ const uint16_t *pcurves = NULL;
size_t num_curves = 0, i;
if (!use_ecc(s))
@@ -154,7 +154,6 @@
ERR_R_INTERNAL_ERROR);
return EXT_RETURN_FAIL;
}
- pcurvestmp = pcurves;
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups)
/* Sub-packet for supported_groups extension */
@@ -165,10 +164,11 @@
return EXT_RETURN_FAIL;
}
/* Copy curve ID if supported */
- for (i = 0; i < num_curves; i++, pcurvestmp += 2) {
- if (tls_curve_allowed(s, pcurvestmp, SSL_SECOP_CURVE_SUPPORTED)) {
- if (!WPACKET_put_bytes_u8(pkt, pcurvestmp[0])
- || !WPACKET_put_bytes_u8(pkt, pcurvestmp[1])) {
+ for (i = 0; i < num_curves; i++) {
+ uint16_t ctmp = pcurves[i];
+
+ if (tls_curve_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) {
+ if (!WPACKET_put_bytes_u16(pkt, ctmp)) {
SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
ERR_R_INTERNAL_ERROR);
return EXT_RETURN_FAIL;
@@ -595,8 +595,8 @@
{
#ifndef OPENSSL_NO_TLS1_3
size_t i, num_curves = 0;
- const unsigned char *pcurves = NULL;
- unsigned int curve_id = 0;
+ const uint16_t *pcurves = NULL;
+ uint16_t curve_id = 0;
/* key_share extension */
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
@@ -620,12 +620,12 @@
if (s->s3->group_id != 0) {
curve_id = s->s3->group_id;
} else {
- for (i = 0; i < num_curves; i++, pcurves += 2) {
+ for (i = 0; i < num_curves; i++) {
- if (!tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED))
+ if (!tls_curve_allowed(s, pcurves[i], SSL_SECOP_CURVE_SUPPORTED))
continue;
- curve_id = bytestogroup(pcurves);
+ curve_id = pcurves[i];
break;
}
}
@@ -1521,7 +1521,7 @@
}
if ((context & SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0) {
- unsigned const char *pcurves = NULL;
+ const uint16_t *pcurves = NULL;
size_t i, num_curves;
if (PACKET_remaining(pkt) != 0) {
@@ -1545,12 +1545,12 @@
SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR);
return 0;
}
- for (i = 0; i < num_curves; i++, pcurves += 2) {
- if (group_id == bytestogroup(pcurves))
+ for (i = 0; i < num_curves; i++) {
+ if (group_id == pcurves[i])
break;
}
if (i >= num_curves
- || !tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) {
+ || !tls_curve_allowed(s, group_id, SSL_SECOP_CURVE_SUPPORTED)) {
*al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_KEY_SHARE);
return 0;
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 0dbec91..583b8dd 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -499,7 +499,7 @@
#ifndef OPENSSL_NO_TLS1_3
unsigned int group_id;
PACKET key_share_list, encoded_pt;
- const unsigned char *clntcurves, *srvrcurves;
+ const uint16_t *clntcurves, *srvrcurves;
size_t clnt_num_curves, srvr_num_curves;
int group_nid, found = 0;
unsigned int curve_flags;
@@ -647,7 +647,7 @@
OPENSSL_free(s->session->ext.supportedgroups);
s->session->ext.supportedgroups = NULL;
s->session->ext.supportedgroups_len = 0;
- if (!PACKET_memdup(&supported_groups_list,
+ if (!tls1_save_u16(&supported_groups_list,
&s->session->ext.supportedgroups,
&s->session->ext.supportedgroups_len)) {
*al = SSL_AD_INTERNAL_ERROR;
@@ -917,7 +917,7 @@
unsigned int context, X509 *x,
size_t chainidx, int *al)
{
- const unsigned char *groups;
+ const uint16_t *groups;
size_t numgroups, i, first = 1;
/* s->s3->group_id is non zero if we accepted a key_share */
@@ -931,14 +931,16 @@
}
/* Copy group ID if supported */
- for (i = 0; i < numgroups; i++, groups += 2) {
- if (tls_curve_allowed(s, groups, SSL_SECOP_CURVE_SUPPORTED)) {
+ for (i = 0; i < numgroups; i++) {
+ uint16_t group = groups[i];
+
+ if (tls_curve_allowed(s, group, 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))
+ if (s->s3->group_id == group)
return EXT_RETURN_NOT_SENT;
/* Add extension header */
@@ -953,7 +955,7 @@
first = 0;
}
- if (!WPACKET_put_bytes_u16(pkt, GET_GROUP_ID(groups, 0))) {
+ if (!WPACKET_put_bytes_u16(pkt, group)) {
SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS,
ERR_R_INTERNAL_ERROR);
return EXT_RETURN_FAIL;
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index d296243..84ad2f6 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1964,7 +1964,7 @@
* 1) or 0 otherwise.
*/
#ifndef OPENSSL_NO_EC
-int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups,
+int check_in_list(SSL *s, uint16_t group_id, const uint16_t *groups,
size_t num_groups, int checkallow)
{
size_t i;
@@ -1972,10 +1972,12 @@
if (groups == NULL || num_groups == 0)
return 0;
- for (i = 0; i < num_groups; i++, groups += 2) {
- if (group_id == GET_GROUP_ID(groups, 0)
+ for (i = 0; i < num_groups; i++) {
+ uint16_t group = groups[i];
+
+ if (group_id == group
&& (!checkallow
- || tls_curve_allowed(s, groups, SSL_SECOP_CURVE_CHECK))) {
+ || tls_curve_allowed(s, group, SSL_SECOP_CURVE_CHECK))) {
return 1;
}
}
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index ae33fe5..9b76dc0 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -55,10 +55,7 @@
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,
+int check_in_list(SSL *s, uint16_t group_id, const uint16_t *groups,
size_t num_groups, int checkallow);
int create_synthetic_message_hash(SSL *s);
int parse_ca_names(SSL *s, PACKET *pkt, int *al);
