Port ServerHello extension parsing to CBS.

This gives us systematic bounds-checking on all the parses. Also adds a
convenience function, CBS_memdup, for saving the current contents of a CBS.

Change-Id: I17dad74575f03121aee3f771037b8806ff99d0c3
Reviewed-on: https://boringssl-review.googlesource.com/1031
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/bytestring/bytestring.h b/crypto/bytestring/bytestring.h
index 8f788a7..9c908a2 100644
--- a/crypto/bytestring/bytestring.h
+++ b/crypto/bytestring/bytestring.h
@@ -53,6 +53,13 @@
 /* CBS_len returns the number of bytes remaining in |cbs|. */
 size_t CBS_len(const CBS *cbs);
 
+/* CBS_stow copies the current contents of |cbs| into |*out_ptr| and
+ * |*out_len|. If |*out_ptr| is not NULL, the contents are freed with
+ * OPENSSL_free. It returns one on success and zero on allocation failure. On
+ * success, |*out_ptr| should be freed with OPENSSL_free. If |cbs| is empty,
+ * |*out_ptr| will be NULL. */
+int CBS_stow(const CBS *cbs, uint8_t **out_ptr, size_t *out_len);
+
 /* CBS_get_u8 sets |*out| to the next uint8_t from |cbs| and advances |cbs|. It
  * returns one on success and zero on error. */
 int CBS_get_u8(CBS *cbs, uint8_t *out);
diff --git a/crypto/bytestring/cbs.c b/crypto/bytestring/cbs.c
index 8ab8a74..639fdbb 100644
--- a/crypto/bytestring/cbs.c
+++ b/crypto/bytestring/cbs.c
@@ -12,6 +12,8 @@
  * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
 
+#include <openssl/buf.h>
+#include <openssl/mem.h>
 #include <openssl/bytestring.h>
 
 #include <assert.h>
@@ -46,6 +48,28 @@
   return cbs->len;
 }
 
+int CBS_stow(const CBS *cbs, uint8_t **out_ptr, size_t *out_len) {
+  if (*out_ptr != NULL) {
+    OPENSSL_free(*out_ptr);
+    *out_ptr = NULL;
+  }
+  *out_len = 0;
+
+  if (cbs->len == 0) {
+    return 1;
+  }
+  *out_ptr = BUF_memdup(cbs->data, cbs->len);
+  if (*out_ptr == NULL) {
+    return 0;
+  }
+  *out_len = cbs->len;
+  return 1;
+}
+
+void *CBS_memdup(const CBS *cbs) {
+  return BUF_memdup(cbs->data, cbs->len);
+}
+
 static int cbs_get_u(CBS *cbs, uint32_t *out, size_t len) {
   uint32_t result = 0;
   size_t i;
diff --git a/ssl/d1_srtp.c b/ssl/d1_srtp.c
index e98b512..c27d766 100644
--- a/ssl/d1_srtp.c
+++ b/ssl/d1_srtp.c
@@ -118,6 +118,7 @@
 
 #include <stdio.h>
 
+#include <openssl/bytestring.h>
 #include <openssl/obj.h>
 #include <openssl/err.h>
 
@@ -429,35 +430,37 @@
 	}
     
 
-int ssl_parse_serverhello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al)
+int ssl_parse_serverhello_use_srtp_ext(SSL *s, CBS *cbs, int *out_alert)
 	{
-	unsigned id;
+	CBS profile_ids, srtp_mki;
+	uint16_t profile_id;
 	int i;
-        int ct;
 
 	STACK_OF(SRTP_PROTECTION_PROFILE) *clnt;
 	SRTP_PROTECTION_PROFILE *prof;
 
-	if(len!=5)
+	/* The extension consists of a u16-prefixed profile ID list containing a
+	 * single uint16_t profile ID, then followed by a u8-prefixed srtp_mki
+	 * field.
+	 *
+	 * See https://tools.ietf.org/html/rfc5764#section-4.1.1
+	 */
+	if (!CBS_get_u16_length_prefixed(cbs, &profile_ids) ||
+		!CBS_get_u16(&profile_ids, &profile_id) ||
+		CBS_len(&profile_ids) != 0 ||
+		!CBS_get_u8_length_prefixed(cbs, &srtp_mki) ||
+		CBS_len(cbs) != 0)
 		{
 		OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_use_srtp_ext, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
-		*al=SSL_AD_DECODE_ERROR;
+		*out_alert = SSL_AD_DECODE_ERROR;
 		return 1;
 		}
 
-        n2s(d, ct);
-	if(ct!=2)
+	if (CBS_len(&srtp_mki) != 0)
 		{
-		OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_use_srtp_ext, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
-		*al=SSL_AD_DECODE_ERROR;
-		return 1;
-		}
-
-	n2s(d,id);
-        if (*d)  /* Must be no MKI, since we never offer one */
-		{
+		/* Must be no MKI, since we never offer one. */
 		OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_use_srtp_ext, SSL_R_BAD_SRTP_MKI_VALUE);
-		*al=SSL_AD_ILLEGAL_PARAMETER;
+		*out_alert = SSL_AD_ILLEGAL_PARAMETER;
 		return 1;
 		}
 
@@ -467,7 +470,7 @@
 	if (clnt == NULL)
 		{
 		OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_use_srtp_ext, SSL_R_NO_SRTP_PROFILES);
-		*al=SSL_AD_DECODE_ERROR;
+		*out_alert = SSL_AD_DECODE_ERROR;
 		return 1;
 		}
     
@@ -478,16 +481,16 @@
 		{
 		prof=sk_SRTP_PROTECTION_PROFILE_value(clnt,i);
 	    
-		if(prof->id == id)
+		if(prof->id == profile_id)
 			{
 			s->srtp_profile=prof;
-			*al=0;
+			*out_alert = 0;
 			return 0;
 			}
 		}
 
 	OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_use_srtp_ext, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
-	*al=SSL_AD_DECODE_ERROR;
+	*out_alert = SSL_AD_ILLEGAL_PARAMETER;
 	return 1;
 	}
 
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 8c2eeb9..b32a6ff 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -151,6 +151,7 @@
 #include <stdio.h>
 
 #include <openssl/buf.h>
+#include <openssl/bytestring.h>
 #include <openssl/rand.h>
 #include <openssl/obj.h>
 #include <openssl/evp.h>
@@ -929,6 +930,7 @@
 	int al=SSL_AD_INTERNAL_ERROR,ok;
 	unsigned int j;
 	long n;
+	CBS cbs;
 	/* Hello verify request and/or server hello version may not
 	 * match so set first packet if we're negotiating version.
 	 */
@@ -1134,16 +1136,19 @@
 		goto f_err;
 		}
 
+        /* TODO(fork): Port the rest of this function to CBS. */
+	CBS_init(&cbs, p, d + n - p);
 #ifndef OPENSSL_NO_TLSEXT
 	/* TLS extensions*/
-	if (!ssl_parse_serverhello_tlsext(s,&p,d,n))
+	if (!ssl_parse_serverhello_tlsext(s, &cbs))
 		{
 		OPENSSL_PUT_ERROR(SSL, ssl3_get_server_hello, SSL_R_PARSE_TLSEXT);
 		goto err; 
 		}
 #endif
 
-	if (p != (d+n))
+        /* There should be nothing left over in the record. */
+	if (CBS_len(&cbs) != 0)
 		{
 		/* wrong packet length */
 		al=SSL_AD_DECODE_ERROR;
diff --git a/ssl/ssl3.h b/ssl/ssl3.h
index 792f4dd..3363308 100644
--- a/ssl/ssl3.h
+++ b/ssl/ssl3.h
@@ -590,8 +590,8 @@
 	 * ClientHello has been processed. In a client these contain the
 	 * protocol that the server selected once the ServerHello has been
 	 * processed. */
-	unsigned char *alpn_selected;
-	unsigned alpn_selected_len;
+	uint8_t *alpn_selected;
+	size_t alpn_selected_len;
 #endif	/* OPENSSL_NO_TLSEXT */
 
 	/* In a client, this means that the server supported Channel ID and that
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index fc8c88e..195e958 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1289,7 +1289,7 @@
 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_check_clienthello_tlsext_late(SSL *s);
-int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n);
+int ssl_parse_serverhello_tlsext(SSL *s, CBS *cbs);
 int ssl_prepare_clienthello_tlsext(SSL *s);
 int ssl_prepare_serverhello_tlsext(SSL *s);
 
@@ -1335,8 +1335,7 @@
 void ssl_clear_hash_ctx(EVP_MD_CTX **hash);
 int ssl_add_serverhello_renegotiate_ext(SSL *s, unsigned char *p, int *len,
 					int maxlen);
-int ssl_parse_serverhello_renegotiate_ext(SSL *s, unsigned char *d, int len,
-					  int *al);
+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,
@@ -1351,7 +1350,7 @@
 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_add_serverhello_use_srtp_ext(SSL *s, unsigned char *p, int *len, int maxlen);
-int ssl_parse_serverhello_use_srtp_ext(SSL *s, unsigned char *d, int len,int *al);
+int ssl_parse_serverhello_use_srtp_ext(SSL *s, CBS *cbs, int *out_alert);
 
 /* s3_cbc.c */
 void ssl3_cbc_copy_mac(unsigned char* out,
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 4107bad..b3cb70b 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -109,6 +109,7 @@
 #include <stdio.h>
 #include <assert.h>
 
+#include <openssl/bytestring.h>
 #include <openssl/evp.h>
 #include <openssl/hmac.h>
 #include <openssl/mem.h>
@@ -1964,8 +1965,8 @@
 
 	if (s->s3->alpn_selected)
 		{
-		const unsigned char *selected = s->s3->alpn_selected;
-		unsigned len = s->s3->alpn_selected_len;
+		const uint8_t *selected = s->s3->alpn_selected;
+		size_t len = s->s3->alpn_selected_len;
 
 		if ((long)(limit - ret - 4 - 2 - 1 - len) < 0)
 			return NULL;
@@ -2823,30 +2824,28 @@
 /* ssl_next_proto_validate validates a Next Protocol Negotiation block. No
  * elements of zero length are allowed and the set of elements must exactly fill
  * the length of the block. */
-static char ssl_next_proto_validate(unsigned char *d, unsigned len)
+static char ssl_next_proto_validate(const CBS *cbs)
 	{
-	unsigned int off = 0;
+	CBS copy = *cbs;
 
-	while (off < len)
+	while (CBS_len(&copy) != 0)
 		{
-		if (d[off] == 0)
+		CBS proto;
+		if (!CBS_get_u8_length_prefixed(&copy, &proto) ||
+			CBS_len(&proto) == 0)
+			{
 			return 0;
-		off += d[off];
-		off++;
+			}
 		}
-
-	return off == len;
+	return 1;
 	}
 #endif
 
-static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n, int *al)
+static int ssl_scan_serverhello_tlsext(SSL *s, CBS *cbs, int *out_alert)
 	{
-	unsigned short length;
-	unsigned short type;
-	unsigned short size;
-	unsigned char *data = *p;
 	int tlsext_servername = 0;
 	int renegotiate_seen = 0;
+	CBS extensions;
 
 #ifndef OPENSSL_NO_NEXTPROTONEG
 	s->s3->next_proto_neg_seen = 0;
@@ -2860,231 +2859,239 @@
 
 #ifndef OPENSSL_NO_HEARTBEATS
 	s->tlsext_heartbeat &= ~(SSL_TLSEXT_HB_ENABLED |
-	                       SSL_TLSEXT_HB_DONT_SEND_REQUESTS);
+				SSL_TLSEXT_HB_DONT_SEND_REQUESTS);
 #endif
 
-	if (data >= (d+n-2))
-		goto ri_check;
-
-	n2s(data,length);
-	if (data+length != d+n)
+	/* There may be no extensions. */
+	if (CBS_len(cbs) == 0)
 		{
-		*al = SSL_AD_DECODE_ERROR;
+		goto ri_check;
+		}
+
+	if (!CBS_get_u16_length_prefixed(cbs, &extensions))
+		{
+		*out_alert = SSL_AD_DECODE_ERROR;
 		return 0;
 		}
 
-	while(data <= (d+n-4))
+	while (CBS_len(&extensions) != 0)
 		{
-		n2s(data,type);
-		n2s(data,size);
+		uint16_t type;
+		CBS extension;
 
-		if (data+size > (d+n))
-	   		goto ri_check;
+		/* 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, 1, type, data, size,
-						s->tlsext_debug_arg);
+			{
+			s->tlsext_debug_cb(s, 1, type, (unsigned char*)CBS_data(&extension),
+				CBS_len(&extension), s->tlsext_debug_arg);
+			}
 
 		if (type == TLSEXT_TYPE_server_name)
 			{
-			if (s->tlsext_hostname == NULL || size > 0)
+			/* The extension must be empty. */
+			if (CBS_len(&extension) != 0)
 				{
-				*al = TLS1_AD_UNRECOGNIZED_NAME;
+				*out_alert = SSL_AD_DECODE_ERROR;
 				return 0;
 				}
-			tlsext_servername = 1;   
+			/* We must have sent it in ClientHello. */
+			if (s->tlsext_hostname == NULL)
+				{
+				*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
+				return 0;
+				}
+			tlsext_servername = 1;
 			}
-
 #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)
+			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;
 				}
-			s->session->tlsext_ecpointformatlist_length = 0;
-			if (s->session->tlsext_ecpointformatlist != NULL) OPENSSL_free(s->session->tlsext_ecpointformatlist);
-			if ((s->session->tlsext_ecpointformatlist = OPENSSL_malloc(ecpointformatlist_length)) == NULL)
+
+			if (!CBS_stow(&ec_point_format_list,
+					&s->session->tlsext_ecpointformatlist,
+					&s->session->tlsext_ecpointformatlist_length))
 				{
-				*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_serverhello_tlsext s->session->tlsext_ecpointformatlist ");
-			sdata = s->session->tlsext_ecpointformatlist;
-			for (i = 0; i < s->session->tlsext_ecpointformatlist_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;
 				}
-			if ((SSL_get_options(s) & SSL_OP_NO_TICKET)
-				|| (size > 0))
+
+			if ((SSL_get_options(s) & SSL_OP_NO_TICKET) || CBS_len(&extension) > 0)
 				{
-				*al = TLS1_AD_UNSUPPORTED_EXTENSION;
+				*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
 				return 0;
 				}
+
 			s->tlsext_ticket_expected = 1;
 			}
 #ifdef TLSEXT_TYPE_opaque_prf_input
 		else if (type == TLSEXT_TYPE_opaque_prf_input)
 			{
-			unsigned char *sdata = data;
+			CBS opaque_prf_input_value;
 
-			if (size < 2)
+			if (!CBS_get_u16_length_prefixed(&extension, &opaque_prf_input_value) ||
+				CBS_len(&extension) != 0)
 				{
-				*al = SSL_AD_DECODE_ERROR;
+				*out_alert = SSL_AD_DECODE_ERROR;
 				return 0;
 				}
-			n2s(sdata, s->s3->server_opaque_prf_input_len);
-			if (s->s3->server_opaque_prf_input_len != size - 2)
-				{
-				*al = SSL_AD_DECODE_ERROR;
-				return 0;
-				}
-			
-			if (s->s3->server_opaque_prf_input != NULL) /* shouldn't really happen */
-				OPENSSL_free(s->s3->server_opaque_prf_input);
-			if (s->s3->server_opaque_prf_input_len == 0)
-				s->s3->server_opaque_prf_input = OPENSSL_malloc(1); /* dummy byte just to get non-NULL */
-			else
-				s->s3->server_opaque_prf_input = BUF_memdup(sdata, s->s3->server_opaque_prf_input_len);
 
-			if (s->s3->server_opaque_prf_input == NULL)
+			if (!CBS_stow(&opaque_prf_input_value,
+					&s->s3->server_opaque_prf_input,
+					&s->s3->server_opaque_prf_input_len))
 				{
-				*al = TLS1_AD_INTERNAL_ERROR;
+				*out_alert = SSL_AD_INTERNAL_ERROR;
 				return 0;
 				}
 			}
 #endif
 		else if (type == TLSEXT_TYPE_status_request)
 			{
-			/* MUST be empty and only sent if we've requested
-			 * a status request message.
-			 */ 
-			if ((s->tlsext_status_type == -1) || (size > 0))
+			/* The extension MUST be empty and may only sent if
+			 * we've requested a status request message. */
+			if (CBS_len(&extension) != 0)
 				{
-				*al = TLS1_AD_UNSUPPORTED_EXTENSION;
+				*out_alert = SSL_AD_DECODE_ERROR;
 				return 0;
 				}
-			/* Set flag to expect CertificateStatus message */
+			if (s->tlsext_status_type == -1)
+				{
+				*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
+				return 0;
+				}
+			/* Set a flag to expect a CertificateStatus message */
 			s->tlsext_status_expected = 1;
 			}
 #ifndef OPENSSL_NO_NEXTPROTONEG
-		else if (type == TLSEXT_TYPE_next_proto_neg &&
-			 s->s3->tmp.finish_md_len == 0)
+		else if (type == TLSEXT_TYPE_next_proto_neg && s->s3->tmp.finish_md_len == 0) {
+		unsigned char *selected;
+		unsigned char selected_len;
+
+		/* We must have requested it. */
+		if (s->ctx->next_proto_select_cb == NULL)
 			{
-			unsigned char *selected;
-			unsigned char selected_len;
-
-			/* We must have requested it. */
-			if (s->ctx->next_proto_select_cb == NULL)
-				{
-				*al = TLS1_AD_UNSUPPORTED_EXTENSION;
-				return 0;
-				}
-			/* The data must be valid */
-			if (!ssl_next_proto_validate(data, size))
-				{
-				*al = TLS1_AD_DECODE_ERROR;
-				return 0;
-				}
-			if (s->ctx->next_proto_select_cb(s, &selected, &selected_len, data, size, s->ctx->next_proto_select_cb_arg) != SSL_TLSEXT_ERR_OK)
-				{
-				*al = TLS1_AD_INTERNAL_ERROR;
-				return 0;
-				}
-			s->next_proto_negotiated = OPENSSL_malloc(selected_len);
-			if (!s->next_proto_negotiated)
-				{
-				*al = TLS1_AD_INTERNAL_ERROR;
-				return 0;
-				}
-			memcpy(s->next_proto_negotiated, selected, selected_len);
-			s->next_proto_negotiated_len = selected_len;
-			s->s3->next_proto_neg_seen = 1;
+			*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
+			return 0;
 			}
-#endif
 
+		/* The data must be valid. */
+		if (!ssl_next_proto_validate(&extension))
+			{
+			*out_alert = SSL_AD_DECODE_ERROR;
+			return 0;
+			}
+
+		if (s->ctx->next_proto_select_cb(s, &selected, &selected_len,
+				CBS_data(&extension), CBS_len(&extension),
+				s->ctx->next_proto_select_cb_arg) != SSL_TLSEXT_ERR_OK)
+			{
+			*out_alert = SSL_AD_INTERNAL_ERROR;
+			return 0;
+			}
+
+		s->next_proto_negotiated = BUF_memdup(selected, selected_len);
+		if (s->next_proto_negotiated == NULL)
+			{
+			*out_alert = SSL_AD_INTERNAL_ERROR;
+			return 0;
+			}
+		s->next_proto_negotiated_len = selected_len;
+		s->s3->next_proto_neg_seen = 1;
+		}
+#endif
 		else if (type == TLSEXT_TYPE_application_layer_protocol_negotiation)
 			{
-			unsigned len;
+			CBS protocol_name_list, protocol_name;
 
 			/* We must have requested it. */
 			if (s->alpn_client_proto_list == NULL)
 				{
-				*al = TLS1_AD_UNSUPPORTED_EXTENSION;
+				*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
 				return 0;
 				}
-			if (size < 4)
+
+			/* The extension data consists of a ProtocolNameList
+			 * which must have exactly one ProtocolName. Each of
+			 * these is length-prefixed. */
+			if (!CBS_get_u16_length_prefixed(&extension, &protocol_name_list) ||
+				CBS_len(&extension) != 0 ||
+				!CBS_get_u8_length_prefixed(&protocol_name_list, &protocol_name) ||
+				CBS_len(&protocol_name_list) != 0)
 				{
-				*al = TLS1_AD_DECODE_ERROR;
+				*out_alert = SSL_AD_DECODE_ERROR;
 				return 0;
 				}
-			/* The extension data consists of:
-			 *   uint16 list_length
-			 *   uint8 proto_length;
-			 *   uint8 proto[proto_length]; */
-			len = data[0];
-			len <<= 8;
-			len |= data[1];
-			if (len != (unsigned) size - 2)
+
+			if (!CBS_stow(&protocol_name,
+					&s->s3->alpn_selected,
+					&s->s3->alpn_selected_len))
 				{
-				*al = TLS1_AD_DECODE_ERROR;
+				*out_alert = SSL_AD_INTERNAL_ERROR;
 				return 0;
 				}
-			len = data[2];
-			if (len != (unsigned) size - 3)
-				{
-				*al = TLS1_AD_DECODE_ERROR;
-				return 0;
-				}
-			if (s->s3->alpn_selected)
-				OPENSSL_free(s->s3->alpn_selected);
-			s->s3->alpn_selected = OPENSSL_malloc(len);
-			if (!s->s3->alpn_selected)
-				{
-				*al = TLS1_AD_INTERNAL_ERROR;
-				return 0;
-				}
-			memcpy(s->s3->alpn_selected, data + 3, len);
-			s->s3->alpn_selected_len = len;
 			}
 
 		else if (type == TLSEXT_TYPE_channel_id)
+			{
+			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)
 			{
+			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;
 			}
 
 		else if (type == TLSEXT_TYPE_renegotiate)
 			{
-			if(!ssl_parse_serverhello_renegotiate_ext(s, data, size, al))
+			if (!ssl_parse_serverhello_renegotiate_ext(s, &extension, out_alert))
 				return 0;
 			renegotiate_seen = 1;
 			}
 #ifndef OPENSSL_NO_HEARTBEATS
 		else if (type == TLSEXT_TYPE_heartbeat)
 			{
-			switch(data[0])
+			uint8_t heartbeat_mode;
+			if (!CBS_get_u8(&extension, &heartbeat_mode) ||
+				CBS_len(&extension) != 0)
+				{
+				*alert = SSL_AD_DECODE_ERROR;
+				return 0;
+				}
+			switch (heartbeat_mode)
 				{
 				case 0x01:	/* Server allows us to send HB requests */
 							s->tlsext_heartbeat |= SSL_TLSEXT_HB_ENABLED;
@@ -3093,15 +3100,14 @@
 							s->tlsext_heartbeat |= SSL_TLSEXT_HB_ENABLED;
 							s->tlsext_heartbeat |= SSL_TLSEXT_HB_DONT_SEND_REQUESTS;
 							break;
-				default:	*al = SSL_AD_ILLEGAL_PARAMETER;
+				default:	*alert = SSL_AD_ILLEGAL_PARAMETER;
 							return 0;
 				}
 			}
 #endif
 		else if (type == TLSEXT_TYPE_use_srtp)
                         {
-                        if(ssl_parse_serverhello_use_srtp_ext(s, data, size,
-							      al))
+                        if (!ssl_parse_serverhello_use_srtp_ext(s, &extension, out_alert))
                                 return 0;
                         }
 
@@ -3110,34 +3116,29 @@
 			/* We only support audit proofs. It's an error to send
 			 * an authz hello extension if the client
 			 * didn't request a proof. */
-			unsigned char *sdata = data;
-			unsigned char server_authz_dataformatlist_length;
+			CBS authz_data_formats;
+			uint8_t authz_data_format;
 
 			if (!s->ctx->tlsext_authz_server_audit_proof_cb)
 				{
-				*al = TLS1_AD_UNSUPPORTED_EXTENSION;
+				*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
 				return 0;
 				}
 
-			if (!size)
+			if (!CBS_get_u8_length_prefixed(&extension, &authz_data_formats) ||
+				CBS_len(&extension) != 0)
 				{
-				*al = TLS1_AD_DECODE_ERROR;
-				return 0;
-				}
-
-			server_authz_dataformatlist_length = *(sdata++);
-			if (server_authz_dataformatlist_length != size - 1)
-				{
-				*al = TLS1_AD_DECODE_ERROR;
+				*out_alert = SSL_AD_DECODE_ERROR;
 				return 0;
 				}
 
 			/* We only support audit proofs, so a legal ServerHello
 			 * authz list contains exactly one entry. */
-			if (server_authz_dataformatlist_length != 1 ||
-				sdata[0] != TLSEXT_AUTHZDATAFORMAT_audit_proof)
+			if (!CBS_get_u8(&authz_data_formats, &authz_data_format) ||
+				CBS_len(&authz_data_formats) != 0 ||
+				authz_data_format != TLSEXT_AUTHZDATAFORMAT_audit_proof)
 				{
-				*al = TLS1_AD_UNSUPPORTED_EXTENSION;
+				*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
 				return 0;
 				}
 
@@ -3147,6 +3148,10 @@
 		/* If this extension type was not otherwise handled, but 
 		 * matches a custom_cli_ext_record, then send it to the c
 		 * callback */
+		/* TODO(fork): Can this be removed or transitioned to a
+		 * CBS-based API? It's only used in certificate_transparency to
+		 * parse the signed_certificate_timestamp extension which should
+		 * just be built-in. */
 		else if (s->ctx->custom_cli_ext_records_count)
 			{
 			size_t i;
@@ -3157,20 +3162,13 @@
 				record = &s->ctx->custom_cli_ext_records[i];
 				if (record->ext_type == type)
 					{
-					if (record->fn2 && !record->fn2(s, type, data, size, al, record->arg))
+					if (record->fn2 && !record->fn2(s, type, CBS_data(&extension), CBS_len(&extension), out_alert, record->arg))
 						return 0;
 					break;
 					}
 				}			
 			}
  
-		data += size;
-		}
-
-	if (data != d+n)
-		{
-		*al = SSL_AD_DECODE_ERROR;
-		return 0;
 		}
 
 	if (!s->hit && tlsext_servername == 1)
@@ -3182,20 +3180,18 @@
 				s->session->tlsext_hostname = BUF_strdup(s->tlsext_hostname);	
 				if (!s->session->tlsext_hostname)
 					{
-					*al = SSL_AD_UNRECOGNIZED_NAME;
+					*out_alert = SSL_AD_UNRECOGNIZED_NAME;
 					return 0;
 					}
 				}
 			else 
 				{
-				*al = SSL_AD_DECODE_ERROR;
+				*out_alert = SSL_AD_DECODE_ERROR;
 				return 0;
 				}
 			}
 		}
 
-	*p = data;
-
 	ri_check:
 
 	/* Determine if we need to see RI. Strictly speaking if we want to
@@ -3209,7 +3205,7 @@
 		&& !(s->options & SSL_OP_LEGACY_SERVER_CONNECT)
 		&& !(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;
 		}
@@ -3535,24 +3531,26 @@
 		}
 	}
 
-int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n) 
+int ssl_parse_serverhello_tlsext(SSL *s, CBS *cbs)
 	{
-	int al = -1;
+	int alert = -1;
 	if (s->version < SSL3_VERSION)
 		return 1;
-	if (ssl_scan_serverhello_tlsext(s, p, d, n, &al) <= 0) 
+
+	if (ssl_scan_serverhello_tlsext(s, cbs, &alert) <= 0)
 		{
-		ssl3_send_alert(s,SSL3_AL_FATAL,al); 
+		ssl3_send_alert(s, SSL3_AL_FATAL, alert);
 		return 0;
 		}
 
-	if (ssl_check_serverhello_tlsext(s) <= 0) 
+	if (ssl_check_serverhello_tlsext(s) <= 0)
 		{
 		OPENSSL_PUT_ERROR(SSL, ssl_add_serverhello_tlsext, SSL_R_SERVERHELLO_TLSEXT);
 		return 0;
 		}
+
 	return 1;
-}
+	}
 
 /* Since the server cache lookup is done early on in the processing of the
  * ClientHello, and other operations depend on the result, we need to handle
diff --git a/ssl/t1_reneg.c b/ssl/t1_reneg.c
index e5b181d..208fb47 100644
--- a/ssl/t1_reneg.c
+++ b/ssl/t1_reneg.c
@@ -109,6 +109,7 @@
 #include <stdio.h>
 #include <assert.h>
 
+#include <openssl/bytestring.h>
 #include <openssl/obj.h>
 #include <openssl/err.h>
 
@@ -231,48 +232,40 @@
 
 /* Parse the server's renegotiation binding and abort if it's not
    right */
-int ssl_parse_serverhello_renegotiate_ext(SSL *s, unsigned char *d, int len,
-					  int *al)
+int ssl_parse_serverhello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert)
     {
     int expected_len=s->s3->previous_client_finished_len
 	+ s->s3->previous_server_finished_len;
-    int ilen;
+    CBS renegotiated_connection;
+    const uint8_t *d;
 
     /* Check for logic errors */
     assert(!expected_len || s->s3->previous_client_finished_len);
     assert(!expected_len || s->s3->previous_server_finished_len);
-    
-    /* Parse the length byte */
-    if(len < 1)
-        {
-        OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext, SSL_R_RENEGOTIATION_ENCODING_ERR);
-        *al=SSL_AD_ILLEGAL_PARAMETER;
-        return 0;
-        }
-    ilen = *d;
-    d++;
 
-    /* Consistency check */
-    if(ilen+1 != len)
+    /* Parse out the extension contents. */
+    if (!CBS_get_u8_length_prefixed(cbs, &renegotiated_connection) ||
+        CBS_len(cbs) != 0)
         {
         OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext, SSL_R_RENEGOTIATION_ENCODING_ERR);
-        *al=SSL_AD_ILLEGAL_PARAMETER;
+        *out_alert = SSL_AD_ILLEGAL_PARAMETER;
         return 0;
         }
     
-    /* Check that the extension matches */
-    if(ilen != expected_len)
+    /* Check that the extension matches. */
+    if(CBS_len(&renegotiated_connection) != expected_len)
         {
         OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext, SSL_R_RENEGOTIATION_MISMATCH);
-        *al=SSL_AD_HANDSHAKE_FAILURE;
+        *out_alert = SSL_AD_HANDSHAKE_FAILURE;
         return 0;
         }
 
+    d = CBS_data(&renegotiated_connection);
     if(memcmp(d, s->s3->previous_client_finished,
 	      s->s3->previous_client_finished_len))
         {
         OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext, SSL_R_RENEGOTIATION_MISMATCH);
-        *al=SSL_AD_HANDSHAKE_FAILURE;
+        *out_alert = SSL_AD_HANDSHAKE_FAILURE;
         return 0;
         }
     d += s->s3->previous_client_finished_len;
@@ -281,14 +274,10 @@
 	      s->s3->previous_server_finished_len))
         {
         OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext, SSL_R_RENEGOTIATION_MISMATCH);
-        *al=SSL_AD_ILLEGAL_PARAMETER;
+        *out_alert = SSL_AD_ILLEGAL_PARAMETER;
         return 0;
         }
-#ifdef OPENSSL_RI_DEBUG
-    fprintf(stderr, "%s RI extension received by client\n",
-				ilen ? "Non-empty" : "Empty");
-#endif
-    s->s3->send_connection_binding=1;
+    s->s3->send_connection_binding = 1;
 
     return 1;
     }