Fold away EC point format negotiation.
The only point format that we ever support is uncompressed, which the
RFC says implementations MUST support. The TLS 1.3 and Curve25519
forecast is that point format negotiation is gone. Each curve has just
one point format and it's labeled, for historial reasons, as
"uncompressed".
Change-Id: I8ffc8556bed1127cf288d2a29671abe3c9b3c585
Reviewed-on: https://boringssl-review.googlesource.com/5542
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 7a3da45..bbd80a9 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1433,8 +1433,6 @@
STACK_OF(SRTP_PROTECTION_PROFILE) *srtp_profiles;
/* EC extension values inherited by SSL structure */
- size_t tlsext_ecpointformatlist_length;
- uint8_t *tlsext_ecpointformatlist;
size_t tlsext_ellipticcurvelist_length;
uint16_t *tlsext_ellipticcurvelist;
@@ -1785,8 +1783,6 @@
char *tlsext_hostname;
/* RFC4507 session ticket expected to be received or sent */
int tlsext_ticket_expected;
- size_t tlsext_ecpointformatlist_length;
- uint8_t *tlsext_ecpointformatlist; /* our list */
size_t tlsext_ellipticcurvelist_length;
uint16_t *tlsext_ellipticcurvelist; /* our list */
diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index 0694e56..8fa92f8 100644
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -502,11 +502,6 @@
* the server is expected to send a CertificateStatus message. */
char certificate_status_expected;
- /* peer_ecpointformatlist contains the EC point formats advertised by the
- * peer. */
- uint8_t *peer_ecpointformatlist;
- size_t peer_ecpointformatlist_length;
-
/* Server-only: peer_ellipticcurvelist contains the EC curve IDs advertised
* by the peer. This is only set on the server's end. The server does not
* advertise this extension to the client. */
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 86d1842..4d34498 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -228,7 +228,6 @@
sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);
OPENSSL_free(s->s3->tmp.certificate_types);
- OPENSSL_free(s->s3->tmp.peer_ecpointformatlist);
OPENSSL_free(s->s3->tmp.peer_ellipticcurvelist);
OPENSSL_free(s->s3->tmp.peer_psk_identity_hint);
BIO_free(s->s3->handshake_buffer);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 72b4c79..b837ad0 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -294,14 +294,6 @@
s->tlsext_ticket_expected = 0;
CRYPTO_refcount_inc(&ctx->references);
s->initial_ctx = ctx;
- if (ctx->tlsext_ecpointformatlist) {
- s->tlsext_ecpointformatlist = BUF_memdup(
- ctx->tlsext_ecpointformatlist, ctx->tlsext_ecpointformatlist_length);
- if (!s->tlsext_ecpointformatlist) {
- goto err;
- }
- s->tlsext_ecpointformatlist_length = ctx->tlsext_ecpointformatlist_length;
- }
if (ctx->tlsext_ellipticcurvelist) {
s->tlsext_ellipticcurvelist =
@@ -556,7 +548,6 @@
OPENSSL_free(ssl->tlsext_hostname);
SSL_CTX_free(ssl->initial_ctx);
- OPENSSL_free(ssl->tlsext_ecpointformatlist);
OPENSSL_free(ssl->tlsext_ellipticcurvelist);
OPENSSL_free(ssl->alpn_client_proto_list);
EVP_PKEY_free(ssl->tlsext_channel_id_private);
@@ -1774,7 +1765,6 @@
sk_X509_NAME_pop_free(ctx->client_CA, X509_NAME_free);
sk_SRTP_PROTECTION_PROFILE_free(ctx->srtp_profiles);
OPENSSL_free(ctx->psk_identity_hint);
- OPENSSL_free(ctx->tlsext_ecpointformatlist);
OPENSSL_free(ctx->tlsext_ellipticcurvelist);
OPENSSL_free(ctx->alpn_client_proto_list);
EVP_PKEY_free(ctx->tlsext_channel_id_private);
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 296ab80..eb47e19 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -345,10 +345,6 @@
{25, NID_secp521r1},
};
-static const uint8_t ecformats_default[] = {
- TLSEXT_ECPOINTFORMAT_uncompressed,
-};
-
static const uint16_t eccurves_default[] = {
23, /* X9_62_prime256v1 */
24, /* secp384r1 */
@@ -529,28 +525,6 @@
return 1;
}
-/* tls1_check_point_format returns one if |comp_id| is consistent with the
- * peer's point format preferences. */
-static int tls1_check_point_format(SSL *s, uint8_t comp_id) {
- uint8_t *p = s->s3->tmp.peer_ecpointformatlist;
- size_t plen = s->s3->tmp.peer_ecpointformatlist_length;
- size_t i;
-
- /* If point formats extension present check it, otherwise everything is
- * supported (see RFC4492). */
- if (p == NULL) {
- return 1;
- }
-
- for (i = 0; i < plen; i++) {
- if (comp_id == p[i]) {
- return 1;
- }
- }
-
- return 0;
-}
-
/* tls1_check_curve_id returns one if |curve_id| is consistent with both our
* and the peer's curve preferences. Note: if called as the client, only our
* preferences are checked; the peer (the server) does not send preferences. */
@@ -587,18 +561,6 @@
return 1;
}
-static void tls1_get_formatlist(SSL *s, const uint8_t **pformats,
- size_t *pformatslen) {
- /* If we have a custom point format list use it otherwise use default */
- if (s->tlsext_ecpointformatlist) {
- *pformats = s->tlsext_ecpointformatlist;
- *pformatslen = s->tlsext_ecpointformatlist_length;
- } else {
- *pformats = ecformats_default;
- *pformatslen = sizeof(ecformats_default);
- }
-}
-
int tls1_check_ec_cert(SSL *s, X509 *x) {
int ret = 0;
EVP_PKEY *pkey = X509_get_pubkey(x);
@@ -609,7 +571,7 @@
pkey->type != EVP_PKEY_EC ||
!tls1_curve_params_from_ec_key(&curve_id, &comp_id, pkey->pkey.ec) ||
!tls1_check_curve_id(s, curve_id) ||
- !tls1_check_point_format(s, comp_id)) {
+ comp_id != TLSEXT_ECPOINTFORMAT_uncompressed) {
goto done;
}
@@ -712,7 +674,7 @@
}
if (s->server && (!tls1_check_curve_id(s, curve_id) ||
- !tls1_check_point_format(s, comp_id))) {
+ comp_id != TLSEXT_ECPOINTFORMAT_uncompressed)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
*out_alert = SSL_AD_ILLEGAL_PARAMETER;
return 0;
@@ -1939,22 +1901,12 @@
return 0;
}
-static void ext_ec_point_init(SSL *ssl) {
- OPENSSL_free(ssl->s3->tmp.peer_ecpointformatlist);
- ssl->s3->tmp.peer_ecpointformatlist = NULL;
- ssl->s3->tmp.peer_ecpointformatlist_length = 0;
-}
-
static int ext_ec_point_add_extension(SSL *ssl, CBB *out) {
- const uint8_t *formats;
- size_t formats_len;
- tls1_get_formatlist(ssl, &formats, &formats_len);
-
- CBB contents, format_bytes;
+ CBB contents, formats;
if (!CBB_add_u16(out, TLSEXT_TYPE_ec_point_formats) ||
!CBB_add_u16_length_prefixed(out, &contents) ||
- !CBB_add_u8_length_prefixed(&contents, &format_bytes) ||
- !CBB_add_bytes(&format_bytes, formats, formats_len) ||
+ !CBB_add_u8_length_prefixed(&contents, &formats) ||
+ !CBB_add_u8(&formats, TLSEXT_ECPOINTFORMAT_uncompressed) ||
!CBB_flush(out)) {
return 0;
}
@@ -1982,9 +1934,11 @@
return 0;
}
- if (!CBS_stow(&ec_point_format_list, &ssl->s3->tmp.peer_ecpointformatlist,
- &ssl->s3->tmp.peer_ecpointformatlist_length)) {
- *out_alert = SSL_AD_INTERNAL_ERROR;
+ /* Per RFC 4492, section 5.1.2, implementations MUST support the uncompressed
+ * point format. */
+ if (memchr(CBS_data(&ec_point_format_list), TLSEXT_ECPOINTFORMAT_uncompressed,
+ CBS_len(&ec_point_format_list)) == NULL) {
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
return 0;
}
@@ -1999,8 +1953,7 @@
static int ext_ec_point_add_serverhello(SSL *ssl, CBB *out) {
const uint32_t alg_k = ssl->s3->tmp.new_cipher->algorithm_mkey;
const uint32_t alg_a = ssl->s3->tmp.new_cipher->algorithm_auth;
- const int using_ecc = ssl->s3->tmp.peer_ecpointformatlist_length != 0 &&
- ((alg_k & SSL_kECDHE) || (alg_a & SSL_aECDSA));
+ const int using_ecc = (alg_k & SSL_kECDHE) || (alg_a & SSL_aECDSA);
if (!using_ecc) {
return 1;
@@ -2196,7 +2149,7 @@
},
{
TLSEXT_TYPE_ec_point_formats,
- ext_ec_point_init,
+ NULL,
ext_ec_point_add_clienthello,
ext_ec_point_parse_serverhello,
ext_ec_point_parse_clienthello,
@@ -2590,21 +2543,9 @@
}
static int ssl_check_serverhello_tlsext(SSL *s) {
- int ret = SSL_TLSEXT_ERR_NOACK;
+ int ret = SSL_TLSEXT_ERR_OK;
int al = SSL_AD_UNRECOGNIZED_NAME;
- /* If we are client and using an elliptic curve cryptography cipher suite,
- * then if server returns an EC point formats lists extension it must contain
- * uncompressed. */
- uint32_t alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
- uint32_t alg_a = s->s3->tmp.new_cipher->algorithm_auth;
- if (((alg_k & SSL_kECDHE) || (alg_a & SSL_aECDSA)) &&
- !tls1_check_point_format(s, TLSEXT_ECPOINTFORMAT_uncompressed)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST);
- return -1;
- }
- ret = SSL_TLSEXT_ERR_OK;
-
if (s->ctx != NULL && s->ctx->tlsext_servername_callback != 0) {
ret = s->ctx->tlsext_servername_callback(s, &al,
s->ctx->tlsext_servername_arg);
