Port ClientHello extensions parsing to crypto/bytestring.
Change-Id: I673c929b78bcf6952db8dfb295dd79d455bcb2a0
Reviewed-on: https://boringssl-review.googlesource.com/1070
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/d1_srtp.c b/ssl/d1_srtp.c
index bcbae1a..59108c2 100644
--- a/ssl/d1_srtp.c
+++ b/ssl/d1_srtp.c
@@ -305,75 +305,45 @@
}
-int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al)
+int ssl_parse_clienthello_use_srtp_ext(SSL *s, CBS *cbs, int *out_alert)
{
- SRTP_PROTECTION_PROFILE *cprof,*sprof;
- STACK_OF(SRTP_PROTECTION_PROFILE) *clnt=0,*srvr;
- int ct;
- int mki_len;
+ CBS profile_ids, srtp_mki;
+ SRTP_PROTECTION_PROFILE *cprof, *sprof;
+ STACK_OF(SRTP_PROTECTION_PROFILE) *clnt = 0,*srvr;
int i,j;
- int id;
- int ret = 1;
+ int ret = 0;
- /* Length value + the MKI length */
- if(len < 3)
- {
+ if (!CBS_get_u16_length_prefixed(cbs, &profile_ids) ||
+ CBS_len(&profile_ids) < 2 ||
+ !CBS_get_u8_length_prefixed(cbs, &srtp_mki) ||
+ CBS_len(cbs) != 0)
+ {
OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_use_srtp_ext, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
- *al=SSL_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_DECODE_ERROR;
goto done;
}
- /* Pull off the length of the cipher suite list */
- n2s(d, ct);
- len -= 2;
-
- /* Check that it is even */
- if(ct%2)
- {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_use_srtp_ext, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
- *al=SSL_AD_DECODE_ERROR;
- goto done;
- }
-
- /* Check that lengths are consistent */
- if(len < (ct + 1))
- {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_use_srtp_ext, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
- *al=SSL_AD_DECODE_ERROR;
- goto done;
- }
+ clnt = sk_SRTP_PROTECTION_PROFILE_new_null();
-
- clnt=sk_SRTP_PROTECTION_PROFILE_new_null();
-
- while(ct)
+ while (CBS_len(&profile_ids) > 0)
{
- n2s(d,id);
- ct-=2;
- len-=2;
+ uint16_t profile_id;
- if(!find_profile_by_num(id,&cprof))
+ if (!CBS_get_u16(cbs, &profile_id))
{
- sk_SRTP_PROTECTION_PROFILE_push(clnt,cprof);
+ *out_alert = SSL_AD_DECODE_ERROR;
+ goto done;
}
- else
+
+ if (!find_profile_by_num(profile_id, &cprof))
{
- ; /* Ignore */
+ sk_SRTP_PROTECTION_PROFILE_push(clnt, cprof);
}
}
- /* Now extract the MKI value as a sanity check, but discard it for now */
- mki_len = *d;
- d++; len--;
+ /* Discard the MKI value for now. */
- if (mki_len != len)
- {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_use_srtp_ext, SSL_R_BAD_SRTP_MKI_VALUE);
- *al=SSL_AD_DECODE_ERROR;
- goto done;
- }
-
- srvr=SSL_get_srtp_profiles(s);
+ srvr = SSL_get_srtp_profiles(s);
/* Pick our most preferred profile. If no profiles have been
configured then the outer loop doesn't run
@@ -389,15 +359,14 @@
if(cprof->id==sprof->id)
{
- s->srtp_profile=sprof;
- *al=0;
- ret=0;
+ s->srtp_profile = sprof;
+ ret = 1;
goto done;
}
}
}
- ret=0;
+ ret = 1;
done:
if(clnt) sk_SRTP_PROTECTION_PROFILE_free(clnt);
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 9e5fdb5..e4968ea 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -154,6 +154,7 @@
#include <openssl/bn.h>
#include <openssl/buf.h>
+#include <openssl/bytestring.h>
#include <openssl/cipher.h>
#include <openssl/dh.h>
#include <openssl/ec.h>
@@ -886,6 +887,7 @@
SSL_CIPHER *c;
STACK_OF(SSL_CIPHER) *ciphers=NULL;
struct ssl_early_callback_ctx early_ctx;
+ CBS cbs;
/* We do this so that we will respond with our native type.
* If we are TLSv1 and we get SSLv3, we will respond with TLSv1,
@@ -1217,17 +1219,27 @@
goto f_err;
}
+ CBS_init(&cbs, p, d + n - p);
#ifndef OPENSSL_NO_TLSEXT
/* TLS extensions*/
if (s->version >= SSL3_VERSION)
{
- if (!ssl_parse_clienthello_tlsext(s,&p,d,n))
+ if (!ssl_parse_clienthello_tlsext(s, &cbs))
{
OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_PARSE_TLSEXT);
goto err;
}
}
+ /* There should be nothing left over in the record. */
+ if (CBS_len(&cbs) != 0)
+ {
+ /* wrong packet length */
+ al=SSL_AD_DECODE_ERROR;
+ OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_BAD_PACKET_LENGTH);
+ goto f_err;
+ }
+
/* Check if we want to use external pre-shared secret for this
* handshake for not reused session only. We need to generate
* server_random before calling tls_session_secret_cb in order to allow
@@ -1276,7 +1288,7 @@
s->cipher_list_by_id = sk_SSL_CIPHER_dup(s->session->ciphers);
}
}
-#endif
+#endif /* !OPENSSL_NO_TLSEXT */
/* Given s->session->ciphers and SSL_get_ciphers, we must
* pick a cipher */
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 8b99244..fb4aef9 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1271,7 +1271,7 @@
int nmatch);
unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned char *limit, size_t header_len);
unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned char *limit);
-int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n);
+int ssl_parse_clienthello_tlsext(SSL *s, CBS *cbs);
int ssl_check_clienthello_tlsext_late(SSL *s);
int ssl_parse_serverhello_tlsext(SSL *s, CBS *cbs);
int ssl_prepare_clienthello_tlsext(SSL *s);
@@ -1310,8 +1310,7 @@
int ssl_parse_serverhello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert);
int ssl_add_clienthello_renegotiate_ext(SSL *s, unsigned char *p, int *len,
int maxlen);
-int ssl_parse_clienthello_renegotiate_ext(SSL *s, unsigned char *d, int len,
- int *al);
+int ssl_parse_clienthello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert);
long ssl_get_algorithm2(SSL *s);
int tls1_process_sigalgs(SSL *s, const unsigned char *data, int dsize);
size_t tls12_get_psigalgs(SSL *s, const unsigned char **psigs);
@@ -1320,7 +1319,7 @@
void ssl_set_client_disabled(SSL *s);
int ssl_add_clienthello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int maxlen);
-int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al);
+int ssl_parse_clienthello_use_srtp_ext(SSL *s, CBS *cbs, int *out_alert);
int ssl_add_serverhello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int maxlen);
int ssl_parse_serverhello_use_srtp_ext(SSL *s, CBS *cbs, int *out_alert);
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 43f2c3f..135cf50 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1747,7 +1747,7 @@
#ifndef OPENSSL_NO_EC
/* ssl_check_for_safari attempts to fingerprint Safari using OS X
- * SecureTransport using the TLS extension block in |d|, of length |n|.
+ * SecureTransport using the TLS extension block in |cbs|.
* Safari, since 10.6, sends exactly these extensions, in this order:
* SNI,
* elliptic_curves
@@ -1758,8 +1758,8 @@
* Sadly we cannot differentiate 10.6, 10.7 and 10.8.4 (which work), from
* 10.8..10.8.3 (which don't work).
*/
-static void ssl_check_for_safari(SSL *s, const unsigned char *data, const unsigned char *d, int n) {
- unsigned short type, size;
+static void ssl_check_for_safari(SSL *s, const CBS *extensions)
+ {
static const unsigned char kSafariExtensionsBlock[] = {
0x00, 0x0a, /* elliptic_curves extension */
0x00, 0x08, /* 8 bytes */
@@ -1785,42 +1785,35 @@
0x04, 0x03, /* SHA-256/ECDSA */
0x02, 0x03, /* SHA-1/ECDSA */
};
+ CBS extensions_copy = *extensions, extension;
+ uint16_t type;
- if (data >= (d+n-2))
- return;
- data += 2;
-
- if (data > (d+n-4))
- return;
- n2s(data,type);
- n2s(data,size);
-
- if (type != TLSEXT_TYPE_server_name)
+ /* First extension is server_name. */
+ if (!CBS_get_u16(&extensions_copy, &type) ||
+ !CBS_get_u16_length_prefixed(&extensions_copy, &extension) ||
+ type != TLSEXT_TYPE_server_name)
return;
- if (data+size > d+n)
- return;
- data += size;
-
+ /* Compare the remainder of the extensions block. */
if (TLS1_get_client_version(s) >= TLS1_2_VERSION)
{
const size_t len1 = sizeof(kSafariExtensionsBlock);
const size_t len2 = sizeof(kSafariTLS12ExtensionsBlock);
- if (data + len1 + len2 != d+n)
+ if (len1 + len2 != CBS_len(&extensions_copy))
return;
- if (memcmp(data, kSafariExtensionsBlock, len1) != 0)
+ if (memcmp(CBS_data(&extensions_copy), kSafariExtensionsBlock, len1) != 0)
return;
- if (memcmp(data + len1, kSafariTLS12ExtensionsBlock, len2) != 0)
+ if (memcmp(CBS_data(&extensions_copy) + len1, kSafariTLS12ExtensionsBlock, len2) != 0)
return;
}
else
{
const size_t len = sizeof(kSafariExtensionsBlock);
- if (data + len != d+n)
+ if (len != CBS_len(&extensions_copy))
return;
- if (memcmp(data, kSafariExtensionsBlock, len) != 0)
+ if (memcmp(CBS_data(&extensions_copy), kSafariExtensionsBlock, len) != 0)
return;
}
@@ -1830,81 +1823,62 @@
/* tls1_alpn_handle_client_hello is called to process the ALPN extension in a
* ClientHello.
- * data: the contents of the extension, not including the type and length.
- * data_len: the number of bytes in |data|
- * al: a pointer to the alert value to send in the event of a non-zero
+ * cbs: the contents of the extension, not including the type and length.
+ * out_alert: a pointer to the alert value to send in the event of a zero
* return.
*
- * returns: 0 on success. */
-static int tls1_alpn_handle_client_hello(SSL *s, const unsigned char *data,
- unsigned data_len, int *al)
+ * returns: 1 on success. */
+static int tls1_alpn_handle_client_hello(SSL *s, CBS *cbs, int *out_alert)
{
- unsigned i;
- unsigned proto_len;
+ CBS protocol_name_list;
const unsigned char *selected;
unsigned char selected_len;
int r;
if (s->ctx->alpn_select_cb == NULL)
- return 0;
+ return 1;
- if (data_len < 2)
+ if (!CBS_get_u16_length_prefixed(cbs, &protocol_name_list) ||
+ CBS_len(cbs) != 0 ||
+ CBS_len(&protocol_name_list) < 2)
goto parse_error;
- /* data should contain a uint16 length followed by a series of 8-bit,
- * length-prefixed strings. */
- i = ((unsigned) data[0]) << 8 |
- ((unsigned) data[1]);
- data_len -= 2;
- data += 2;
- if (data_len != i)
- goto parse_error;
-
- if (data_len < 2)
- goto parse_error;
-
- for (i = 0; i < data_len;)
+ /* Validate the protocol list. */
+ CBS protocol_name_list_copy = protocol_name_list;
+ while (CBS_len(&protocol_name_list_copy) > 0)
{
- proto_len = data[i];
- i++;
+ CBS protocol_name;
- if (proto_len == 0)
+ if (!CBS_get_u8_length_prefixed(&protocol_name_list_copy, &protocol_name))
goto parse_error;
-
- if (i + proto_len < i || i + proto_len > data_len)
- goto parse_error;
-
- i += proto_len;
}
- r = s->ctx->alpn_select_cb(s, &selected, &selected_len, data, data_len,
- s->ctx->alpn_select_cb_arg);
+ r = s->ctx->alpn_select_cb(s, &selected, &selected_len,
+ CBS_data(&protocol_name_list), CBS_len(&protocol_name_list),
+ s->ctx->alpn_select_cb_arg);
if (r == SSL_TLSEXT_ERR_OK) {
if (s->s3->alpn_selected)
OPENSSL_free(s->s3->alpn_selected);
s->s3->alpn_selected = OPENSSL_malloc(selected_len);
if (!s->s3->alpn_selected)
{
- *al = SSL_AD_INTERNAL_ERROR;
- return -1;
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return 0;
}
memcpy(s->s3->alpn_selected, selected, selected_len);
s->s3->alpn_selected_len = selected_len;
}
- return 0;
+ return 1;
parse_error:
- *al = SSL_AD_DECODE_ERROR;
- return -1;
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
}
-static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n, int *al)
+static int ssl_scan_clienthello_tlsext(SSL *s, CBS *cbs, int *out_alert)
{
- unsigned short type;
- unsigned short size;
- unsigned short len;
- unsigned char *data = *p;
int renegotiate_seen = 0;
+ CBS extensions;
size_t i;
s->servername_done = 0;
@@ -1919,11 +1893,6 @@
s->s3->alpn_selected = NULL;
}
-#ifndef OPENSSL_NO_EC
- if (s->options & SSL_OP_SAFARI_ECDHE_ECDSA_BUG)
- ssl_check_for_safari(s, data, d, n);
-#endif /* !OPENSSL_NO_EC */
-
/* Clear any signature algorithms extension received */
if (s->cert->peer_sigalgs)
{
@@ -1943,26 +1912,42 @@
s->cert->pkeys[i].valid_flags = 0;
}
- if (data >= (d+n-2))
- goto ri_check;
- n2s(data,len);
-
- if (data > (d+n-len))
- goto ri_check;
-
- while (data <= (d+n-4))
+ /* There may be no extensions. */
+ if (CBS_len(cbs) == 0)
{
- n2s(data,type);
- n2s(data,size);
+ goto ri_check;
+ }
- if (data+size > (d+n))
- goto ri_check;
-#if 0
- fprintf(stderr,"Received extension type %d size %d\n",type,size);
-#endif
+ if (!CBS_get_u16_length_prefixed(cbs, &extensions))
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
+#ifndef OPENSSL_NO_EC
+ if (s->options & SSL_OP_SAFARI_ECDHE_ECDSA_BUG)
+ ssl_check_for_safari(s, &extensions);
+#endif /* !OPENSSL_NO_EC */
+
+ while (CBS_len(&extensions) != 0)
+ {
+ uint16_t type;
+ CBS extension;
+
+ /* Decode the next extension. */
+ if (!CBS_get_u16(&extensions, &type) ||
+ !CBS_get_u16_length_prefixed(&extensions, &extension))
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
if (s->tlsext_debug_cb)
- s->tlsext_debug_cb(s, 0, type, data, size,
- s->tlsext_debug_arg);
+ {
+ s->tlsext_debug_cb(s, 0, type, (unsigned char*)CBS_data(&extension),
+ CBS_len(&extension), s->tlsext_debug_arg);
+ }
+
/* The servername extension is treated as follows:
- Only the hostname type is supported with a maximum length of 255.
@@ -1988,194 +1973,188 @@
if (type == TLSEXT_TYPE_server_name)
{
- unsigned char *sdata;
- int servname_type;
- int dsize;
-
- if (size < 2)
+ CBS server_name_list;
+
+ if (!CBS_get_u16_length_prefixed(&extension, &server_name_list) ||
+ CBS_len(&server_name_list) < 1 ||
+ CBS_len(&extension) != 0)
{
- *al = SSL_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
- n2s(data,dsize);
- size -= 2;
- if (dsize > size )
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- sdata = data;
- while (dsize > 3)
+ /* Decode each ServerName in the extension. */
+ while (CBS_len(&server_name_list) > 0)
{
- servname_type = *(sdata++);
- n2s(sdata,len);
- dsize -= 3;
+ uint8_t name_type;
+ CBS host_name;
- if (len > dsize)
+ /* Decode the NameType. */
+ if (!CBS_get_u8(&server_name_list, &name_type))
{
- *al = SSL_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
- if (s->servername_done == 0)
- switch (servname_type)
+
+ if (s->servername_done)
+ continue;
+
+ /* Only host_name is supported. */
+ if (name_type != TLSEXT_NAMETYPE_host_name)
+ continue;
+
+ if (!s->hit)
{
- case TLSEXT_NAMETYPE_host_name:
- if (!s->hit)
+ if (s->session->tlsext_hostname)
{
- if(s->session->tlsext_hostname)
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- if (len > TLSEXT_MAXLEN_host_name)
- {
- *al = TLS1_AD_UNRECOGNIZED_NAME;
- return 0;
- }
- if ((s->session->tlsext_hostname = OPENSSL_malloc(len+1)) == NULL)
- {
- *al = TLS1_AD_INTERNAL_ERROR;
- return 0;
- }
- memcpy(s->session->tlsext_hostname, sdata, len);
- s->session->tlsext_hostname[len]='\0';
- if (strlen(s->session->tlsext_hostname) != len) {
- OPENSSL_free(s->session->tlsext_hostname);
- s->session->tlsext_hostname = NULL;
- *al = TLS1_AD_UNRECOGNIZED_NAME;
- return 0;
+ /* The ServerNameList MUST NOT
+ contain more than one name of
+ the same name_type. */
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
}
- s->servername_done = 1;
+ if (!CBS_get_u16_length_prefixed(&server_name_list, &host_name) ||
+ CBS_len(&host_name) < 1)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
}
- else
- s->servername_done = s->session->tlsext_hostname
- && strlen(s->session->tlsext_hostname) == len
- && strncmp(s->session->tlsext_hostname, (char *)sdata, len) == 0;
-
- break;
- default:
- break;
+ if (CBS_len(&host_name) > TLSEXT_MAXLEN_host_name)
+ {
+ *out_alert = SSL_AD_UNRECOGNIZED_NAME;
+ return 0;
+ }
+
+ /* host_name may not contain a NUL character. */
+ if (BUF_strnlen((const char*)CBS_data(&host_name),
+ CBS_len(&host_name)) != CBS_len(&host_name))
+ {
+ *out_alert = SSL_AD_UNRECOGNIZED_NAME;
+ return 0;
+ }
+
+ /* Copy the hostname as a string. */
+ s->session->tlsext_hostname = BUF_strndup(
+ (const char*)CBS_data(&host_name), CBS_len(&host_name));
+ if (s->session->tlsext_hostname == NULL)
+ {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return 0;
+ }
+ s->servername_done = 1;
}
-
- dsize -= len;
+ else
+ {
+ s->servername_done = s->session->tlsext_hostname
+ && strlen(s->session->tlsext_hostname) == CBS_len(&host_name)
+ && strncmp(s->session->tlsext_hostname,
+ (char *)CBS_data(&host_name), CBS_len(&host_name)) == 0;
+ }
}
- if (dsize != 0)
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
-
}
#ifndef OPENSSL_NO_EC
else if (type == TLSEXT_TYPE_ec_point_formats)
{
- unsigned char *sdata = data;
- int ecpointformatlist_length = *(sdata++);
+ CBS ec_point_format_list;
- if (ecpointformatlist_length != size - 1 ||
- ecpointformatlist_length < 1)
+ if (!CBS_get_u8_length_prefixed(&extension, &ec_point_format_list) ||
+ CBS_len(&extension) != 0)
{
- *al = TLS1_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
+
if (!s->hit)
{
- if(s->session->tlsext_ecpointformatlist)
+ if (!CBS_stow(&ec_point_format_list,
+ &s->session->tlsext_ecpointformatlist,
+ &s->session->tlsext_ecpointformatlist_length))
{
- OPENSSL_free(s->session->tlsext_ecpointformatlist);
- s->session->tlsext_ecpointformatlist = NULL;
- }
- s->session->tlsext_ecpointformatlist_length = 0;
- if ((s->session->tlsext_ecpointformatlist = OPENSSL_malloc(ecpointformatlist_length)) == NULL)
- {
- *al = TLS1_AD_INTERNAL_ERROR;
+ *out_alert = SSL_AD_INTERNAL_ERROR;
return 0;
}
- s->session->tlsext_ecpointformatlist_length = ecpointformatlist_length;
- memcpy(s->session->tlsext_ecpointformatlist, sdata, ecpointformatlist_length);
}
-#if 0
- fprintf(stderr,"ssl_parse_clienthello_tlsext s->session->tlsext_ecpointformatlist (length=%i) ", s->session->tlsext_ecpointformatlist_length);
- sdata = s->session->tlsext_ecpointformatlist;
- for (i = 0; i < s->session->tlsext_ecpointformatlist_length; i++)
- fprintf(stderr,"%i ",*(sdata++));
- fprintf(stderr,"\n");
-#endif
}
else if (type == TLSEXT_TYPE_elliptic_curves)
{
- unsigned char *sdata = data;
- int ellipticcurvelist_length = (*(sdata++) << 8);
- ellipticcurvelist_length += (*(sdata++));
+ CBS elliptic_curve_list;
- if (ellipticcurvelist_length != size - 2 ||
- ellipticcurvelist_length < 1)
+ if (!CBS_get_u16_length_prefixed(&extension, &elliptic_curve_list) ||
+ CBS_len(&extension) != 0)
{
- *al = TLS1_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
+
if (!s->hit)
{
- if(s->session->tlsext_ellipticcurvelist)
+ if (s->session->tlsext_ellipticcurvelist)
{
- *al = TLS1_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
- s->session->tlsext_ellipticcurvelist_length = 0;
- if ((s->session->tlsext_ellipticcurvelist = OPENSSL_malloc(ellipticcurvelist_length)) == NULL)
+
+ if (!CBS_stow(&elliptic_curve_list,
+ &s->session->tlsext_ellipticcurvelist,
+ &s->session->tlsext_ellipticcurvelist_length))
{
- *al = TLS1_AD_INTERNAL_ERROR;
+ *out_alert = SSL_AD_INTERNAL_ERROR;
return 0;
}
- s->session->tlsext_ellipticcurvelist_length = ellipticcurvelist_length;
- memcpy(s->session->tlsext_ellipticcurvelist, sdata, ellipticcurvelist_length);
}
-#if 0
- fprintf(stderr,"ssl_parse_clienthello_tlsext s->session->tlsext_ellipticcurvelist (length=%i) ", s->session->tlsext_ellipticcurvelist_length);
- sdata = s->session->tlsext_ellipticcurvelist;
- for (i = 0; i < s->session->tlsext_ellipticcurvelist_length; i++)
- fprintf(stderr,"%i ",*(sdata++));
- fprintf(stderr,"\n");
-#endif
}
#endif /* OPENSSL_NO_EC */
else if (type == TLSEXT_TYPE_session_ticket)
{
if (s->tls_session_ticket_ext_cb &&
- !s->tls_session_ticket_ext_cb(s, data, size, s->tls_session_ticket_ext_cb_arg))
+ !s->tls_session_ticket_ext_cb(s, CBS_data(&extension), CBS_len(&extension), s->tls_session_ticket_ext_cb_arg))
{
- *al = TLS1_AD_INTERNAL_ERROR;
+ *out_alert = SSL_AD_INTERNAL_ERROR;
return 0;
}
}
else if (type == TLSEXT_TYPE_renegotiate)
{
- if(!ssl_parse_clienthello_renegotiate_ext(s, data, size, al))
+ if (!ssl_parse_clienthello_renegotiate_ext(s, &extension, out_alert))
return 0;
renegotiate_seen = 1;
}
else if (type == TLSEXT_TYPE_signature_algorithms)
{
- int dsize;
- if (s->cert->peer_sigalgs || size < 2)
+ CBS supported_signature_algorithms;
+
+ /* The extension should not appear twice. */
+ if (s->cert->peer_sigalgs)
{
- *al = SSL_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
return 0;
}
- n2s(data,dsize);
- size -= 2;
- if (dsize != size || dsize & 1 || !dsize)
+
+ if (!CBS_get_u16_length_prefixed(&extension, &supported_signature_algorithms) ||
+ CBS_len(&extension) != 0)
{
- *al = SSL_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
- if (!tls1_process_sigalgs(s, data, dsize))
+
+ /* Ensure the signature algorithms are non-empty. It
+ * contains a list of SignatureAndHashAlgorithms
+ * which are two bytes each. */
+ if (CBS_len(&supported_signature_algorithms) == 0 ||
+ (CBS_len(&supported_signature_algorithms) % 2) != 0)
{
- *al = SSL_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
+ if (!tls1_process_sigalgs(s,
+ CBS_data(&supported_signature_algorithms),
+ CBS_len(&supported_signature_algorithms)))
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
return 0;
}
/* If sigalgs received and no shared algorithms fatal
@@ -2184,7 +2163,7 @@
if (s->cert->peer_sigalgs && !s->cert->shared_sigalgs)
{
OPENSSL_PUT_ERROR(SSL, ssl_add_serverhello_tlsext, SSL_R_NO_SHARED_SIGATURE_ALGORITHMS);
- *al = SSL_AD_ILLEGAL_PARAMETER;
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
return 0;
}
}
@@ -2193,114 +2172,121 @@
#if 0
else if (type == TLSEXT_TYPE_status_request)
{
-
- if (size < 5)
+ uint8_t status_type;
+ CBS responder_id_list;
+ CBS request_extensions;
+
+ /* Already seen the extension. */
+ if (s->tlsext_status_type != -1 ||
+ s->tlsext_ocsp_ids != NULL ||
+ s->tlsext_ocsp_exts != NULL)
{
- *al = SSL_AD_DECODE_ERROR;
+ *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
return 0;
}
- s->tlsext_status_type = *data++;
- size--;
- if (s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp)
+ if (!CBS_get_u8(&extension, &status_type))
{
- const unsigned char *sdata;
- int dsize;
- /* Read in responder_id_list */
- n2s(data,dsize);
- size -= 2;
- if (dsize > size )
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- while (dsize > 0)
- {
- OCSP_RESPID *id;
- int idsize;
- if (dsize < 4)
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- n2s(data, idsize);
- dsize -= 2 + idsize;
- size -= 2 + idsize;
- if (dsize < 0)
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- sdata = data;
- data += idsize;
- id = d2i_OCSP_RESPID(NULL,
- &sdata, idsize);
- if (!id)
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- if (data != sdata)
- {
- OCSP_RESPID_free(id);
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- if (!s->tlsext_ocsp_ids
- && !(s->tlsext_ocsp_ids =
- sk_OCSP_RESPID_new_null()))
- {
- OCSP_RESPID_free(id);
- *al = SSL_AD_INTERNAL_ERROR;
- return 0;
- }
- if (!sk_OCSP_RESPID_push(
- s->tlsext_ocsp_ids, id))
- {
- OCSP_RESPID_free(id);
- *al = SSL_AD_INTERNAL_ERROR;
- return 0;
- }
- }
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
- /* Read in request_extensions */
- if (size < 2)
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- n2s(data,dsize);
- size -= 2;
- if (dsize != size)
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- sdata = data;
- if (dsize > 0)
- {
- if (s->tlsext_ocsp_exts)
- {
- sk_X509_EXTENSION_pop_free(s->tlsext_ocsp_exts,
- X509_EXTENSION_free);
- }
+ /* Only OCSP is supported. */
+ if (status_type != TLSEXT_STATUSTYPE_ocsp)
+ continue;
- s->tlsext_ocsp_exts =
- d2i_X509_EXTENSIONS(NULL,
- &sdata, dsize);
- if (!s->tlsext_ocsp_exts
- || (data + dsize != sdata))
- {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
+ s->tlsext_status_type = status_type;
+
+ /* Extension consists of a responder_id_list and
+ * request_extensions. */
+ if (!CBS_get_u16_length_prefixed(&extension, &responder_id_list) ||
+ CBS_get_u16_length_prefixed(&extension, &request_extensions) ||
+ CBS_len(&extension) != 0)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
+ if (CBS_len(&responder_id_list) > 0)
+ {
+ s->tlsext_ocsp_ids = sk_OCSP_RESPID_new_null();
+ if (s->tlsext_ocsp_ids == NULL)
+ {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return 0;
}
}
- /* We don't know what to do with any other type
- * so ignore it.
- */
- else
- s->tlsext_status_type = -1;
+
+ /* Parse out the responder IDs. */
+ while (CBS_len(&responder_id_list) > 0)
+ {
+ CBS responder_id;
+ OCSP_RESPID *id;
+ const uint8_t *data;
+
+ /* Each ResponderID must have size at least 1. */
+ if (!CBS_get_u16_length_prefixed(&responder_id_list, &responder_id) ||
+ CBS_len(&responder_id) < 1)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
+ /* TODO(fork): Add CBS versions of d2i_FOO_BAR. */
+ data = CBS_data(&responder_id);
+ id = d2i_OCSP_RESPID(NULL, &data, CBS_len(&responder_id));
+ if (!id)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+ if (!CBS_skip(&responder_id, data - CBS_data(&responder_id)))
+ {
+ /* This should never happen. */
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ OCSP_RESPID_free(id);
+ return 0;
+ }
+ if (CBS_len(&responder_id) != 0)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ OCSP_RESPID_free(id);
+ return 0;
+ }
+
+ if (!sk_OCSP_RESPID_push(s->tlsext_ocsp_ids, id))
+ {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ OCSP_RESPID_free(id);
+ return 0;
+ }
+ }
+
+ /* Parse out request_extensions. */
+ if (CBS_len(&request_extensions) > 0)
+ {
+ const uint8_t *data;
+
+ data = CBS_data(&request_extensions);
+ s->tlsext_ocsp_exts = d2i_X509_EXTENSIONS(NULL,
+ &data, CBS_len(&request_extensions));
+ if (s->tlsext_ocsp_exts == NULL)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+ if (!CBS_skip(&request_extensions, data - CBS_data(&request_extensions)))
+ {
+ /* This should never happen. */
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return 0;
+ }
+ if (CBS_len(&request_extensions) != 0)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+ }
}
#endif
@@ -2309,6 +2295,13 @@
s->s3->tmp.finish_md_len == 0 &&
s->s3->alpn_selected == NULL)
{
+ /* The extension must be empty. */
+ if (CBS_len(&extension) != 0)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
/* We shouldn't accept this extension on a
* renegotiation.
*
@@ -2332,7 +2325,7 @@
s->ctx->alpn_select_cb &&
s->s3->tmp.finish_md_len == 0)
{
- if (tls1_alpn_handle_client_hello(s, data, size, al) != 0)
+ if (!tls1_alpn_handle_client_hello(s, &extension, out_alert))
return 0;
#ifndef OPENSSL_NO_NEXTPROTONEG
/* ALPN takes precedence over NPN. */
@@ -2342,11 +2335,27 @@
else if (type == TLSEXT_TYPE_channel_id &&
s->tlsext_channel_id_enabled)
+ {
+ /* The extension must be empty. */
+ if (CBS_len(&extension) != 0)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
s->s3->tlsext_channel_id_valid = 1;
+ }
else if (type == TLSEXT_TYPE_channel_id_new &&
s->tlsext_channel_id_enabled)
{
+ /* The extension must be empty. */
+ if (CBS_len(&extension) != 0)
+ {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
s->s3->tlsext_channel_id_valid = 1;
s->s3->tlsext_channel_id_new = 1;
}
@@ -2355,16 +2364,11 @@
/* session ticket processed earlier */
else if (type == TLSEXT_TYPE_use_srtp)
{
- if(ssl_parse_clienthello_use_srtp_ext(s, data, size,
- al))
+ if (!ssl_parse_clienthello_use_srtp_ext(s, &extension, out_alert))
return 0;
}
-
- data+=size;
}
- *p = data;
-
ri_check:
/* Need RI if renegotiating */
@@ -2372,7 +2376,7 @@
if (!renegotiate_seen && s->renegotiate &&
!(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION))
{
- *al = SSL_AD_HANDSHAKE_FAILURE;
+ *out_alert = SSL_AD_HANDSHAKE_FAILURE;
OPENSSL_PUT_ERROR(SSL, ssl_add_serverhello_tlsext, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
return 0;
}
@@ -2383,12 +2387,12 @@
return 1;
}
-int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n)
+int ssl_parse_clienthello_tlsext(SSL *s, CBS *cbs)
{
- int al = -1;
- if (ssl_scan_clienthello_tlsext(s, p, d, n, &al) <= 0)
+ int alert = -1;
+ if (ssl_scan_clienthello_tlsext(s, cbs, &alert) <= 0)
{
- ssl3_send_alert(s,SSL3_AL_FATAL,al);
+ ssl3_send_alert(s, SSL3_AL_FATAL, alert);
return 0;
}
diff --git a/ssl/t1_reneg.c b/ssl/t1_reneg.c
index 208fb47..877a7a6 100644
--- a/ssl/t1_reneg.c
+++ b/ssl/t1_reneg.c
@@ -147,50 +147,36 @@
/* Parse the client's renegotiation binding and abort if it's not
right */
-int ssl_parse_clienthello_renegotiate_ext(SSL *s, unsigned char *d, int len,
- int *al)
+int ssl_parse_clienthello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert)
{
- int ilen;
+ CBS renegotiated_connection;
- /* Parse the length byte */
- if(len < 1)
- {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext, SSL_R_RENEGOTIATION_ENCODING_ERR);
- *al=SSL_AD_ILLEGAL_PARAMETER;
- return 0;
- }
- ilen = *d;
- d++;
-
- /* Consistency check */
- if((ilen+1) != len)
- {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext, SSL_R_RENEGOTIATION_ENCODING_ERR);
- *al=SSL_AD_ILLEGAL_PARAMETER;
- return 0;
- }
+ if (!CBS_get_u8_length_prefixed(cbs, &renegotiated_connection) ||
+ CBS_len(cbs) != 0)
+ {
+ OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext, SSL_R_RENEGOTIATION_ENCODING_ERR);
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
/* Check that the extension matches */
- if(ilen != s->s3->previous_client_finished_len)
- {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext, SSL_R_RENEGOTIATION_MISMATCH);
- *al=SSL_AD_HANDSHAKE_FAILURE;
- return 0;
- }
+ if (CBS_len(&renegotiated_connection) != s->s3->previous_client_finished_len)
+ {
+ OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext, SSL_R_RENEGOTIATION_MISMATCH);
+ *out_alert = SSL_AD_HANDSHAKE_FAILURE;
+ return 0;
+ }
- if(memcmp(d, s->s3->previous_client_finished,
- s->s3->previous_client_finished_len))
- {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext, SSL_R_RENEGOTIATION_MISMATCH);
- *al=SSL_AD_HANDSHAKE_FAILURE;
- return 0;
- }
-#ifdef OPENSSL_RI_DEBUG
- fprintf(stderr, "%s RI extension received by server\n",
- ilen ? "Non-empty" : "Empty");
-#endif
+ if (memcmp(CBS_data(&renegotiated_connection),
+ s->s3->previous_client_finished,
+ s->s3->previous_client_finished_len))
+ {
+ OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext, SSL_R_RENEGOTIATION_MISMATCH);
+ *out_alert = SSL_AD_HANDSHAKE_FAILURE;
+ return 0;
+ }
- s->s3->send_connection_binding=1;
+ s->s3->send_connection_binding = 1;
return 1;
}
