When parsing extensions by calling the parse functions directly, ensure

that we have consumed all of the extension data. This catches the ALPN
extension handling issue that was just fixed.

1 files changed, 82 insertions, 2 deletions

diff --git a/regress/lib/libssl/tlsext/tlsexttest.c b/regress/lib/libssl/tlsext/tlsexttest.c
index 1b2820d78be..63c64e94116 100644
--- a/regress/lib/libssl/tlsext/tlsexttest.c
+++ b/regress/lib/libssl/tlsext/tlsexttest.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tlsexttest.c,v 1.15 2017/08/27 02:58:04 doug Exp $ */
+/* $OpenBSD: tlsexttest.c,v 1.16 2017/08/29 17:24:59 jsing Exp $ */
 /*
  * Copyright (c) 2017 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -198,6 +198,10 @@ test_tlsext_alpn_clienthello(void)
 		FAIL("failed to parse ALPN");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (ssl->internal->alpn_client_proto_list_len !=
 	    sizeof(tlsext_alpn_single_proto_val)) {
@@ -261,6 +265,10 @@ test_tlsext_alpn_clienthello(void)
 		FAIL("failed to parse ALPN");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (ssl->internal->alpn_client_proto_list_len !=
 	    sizeof(tlsext_alpn_multiple_protos_val)) {
@@ -398,6 +406,10 @@ test_tlsext_alpn_serverhello(void)
 		FAIL("Should be able to parse serverhello when we request it");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (S3I(ssl)->alpn_selected_len !=
 	    sizeof(tlsext_alpn_single_proto_name)) {
@@ -597,6 +609,10 @@ test_tlsext_ec_clienthello(void)
 		FAIL("failed to parse clienthello Ellipticcurves\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (SSI(ssl)->tlsext_supportedgroups_length !=
 	    sizeof(tlsext_ec_clienthello_secp384r1_val) / sizeof(uint16_t)) {
@@ -693,6 +709,10 @@ test_tlsext_ec_clienthello(void)
 		FAIL("failed to parse clienthello Ellipticcurves\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (SSI(ssl)->tlsext_supportedgroups_length !=
 	    sizeof(tlsext_ec_clienthello_nistp192and224_val) / sizeof(uint16_t)) {
@@ -895,6 +915,10 @@ test_tlsext_ecpf_clienthello(void)
 		FAIL("failed to parse clienthello ECPointFormats\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (SSI(ssl)->tlsext_ecpointformatlist_length !=
 	    sizeof(tlsext_ecpf_hello_uncompressed_val)) {
@@ -984,6 +1008,10 @@ test_tlsext_ecpf_clienthello(void)
 		FAIL("failed to parse clienthello ECPointFormats\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (SSI(ssl)->tlsext_ecpointformatlist_length !=
 	    sizeof(tlsext_ecpf_hello_prefer_order_val)) {
@@ -1100,6 +1128,10 @@ test_tlsext_ecpf_serverhello(void)
 		FAIL("must include uncompressed in serverhello ECPointFormats\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	/*
 	 * Test with a custom order that replaces the default uncompressed.
@@ -1184,6 +1216,10 @@ test_tlsext_ecpf_serverhello(void)
 		FAIL("failed to parse serverhello ECPointFormats\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (SSI(ssl)->tlsext_ecpointformatlist_length !=
 	    sizeof(tlsext_ecpf_hello_prefer_order_val)) {
@@ -1308,6 +1344,10 @@ test_tlsext_ri_clienthello(void)
 		FAIL("failed to parse clienthello RI\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (S3I(ssl)->renegotiate_seen != 1) {
 		FAIL("renegotiate seen not set\n");
@@ -1417,6 +1457,10 @@ test_tlsext_ri_serverhello(void)
 		FAIL("failed to parse serverhello RI\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (S3I(ssl)->renegotiate_seen != 1) {
 		FAIL("renegotiate seen not set\n");
@@ -1534,6 +1578,10 @@ test_tlsext_sigalgs_clienthello(void)
 		failure = 1;
 		goto done;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto done;
+	}
 
 	if (ssl->cert->pkeys[SSL_PKEY_RSA_SIGN].digest != EVP_sha512()) {
 		fprintf(stderr, "FAIL: RSA sign digest mismatch\n");
@@ -1601,7 +1649,7 @@ test_tlsext_sigalgs_serverhello(void)
 
 	CBS_init(&cbs, tlsext_sigalgs_clienthello, sizeof(tlsext_sigalgs_clienthello));
 	if (tlsext_sigalgs_serverhello_parse(ssl, &cbs, &alert)) {
-		fprintf(stderr, "FAIL: failed to parse serverhello sigalgs\n");
+		fprintf(stderr, "FAIL: serverhello should not parse sigalgs\n");
 		failure = 1;
 		goto done;
 	}
@@ -1699,6 +1747,10 @@ test_tlsext_sni_clienthello(void)
 		FAIL("failed to parse clienthello SNI\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (ssl->session->tlsext_hostname == NULL) {
 		FAIL("no tlsext_hostname from clienthello SNI\n");
@@ -1810,6 +1862,10 @@ test_tlsext_sni_serverhello(void)
 		FAIL("failed to parse serverhello SNI\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if (ssl->session->tlsext_hostname == NULL) {
 		FAIL("no tlsext_hostname after serverhello SNI\n");
@@ -1898,6 +1954,10 @@ test_tlsext_ocsp_clienthello(void)
 		FAIL("failed to parse ocsp clienthello\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	failure = 0;
 
@@ -2385,6 +2445,10 @@ test_tlsext_srtp_clienthello(void)
 		FAIL("failed to parse SRTP\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if ((prof = SSL_get_selected_srtp_profile(ssl)) == NULL) {
 		FAIL("SRTP profile should be set now\n");
@@ -2448,6 +2512,10 @@ test_tlsext_srtp_clienthello(void)
 		FAIL("failed to parse SRTP\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if ((prof = SSL_get_selected_srtp_profile(ssl)) == NULL) {
 		FAIL("SRTP profile should be set now\n");
@@ -2475,6 +2543,10 @@ test_tlsext_srtp_clienthello(void)
 		FAIL("failed to parse SRTP\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if ((prof = SSL_get_selected_srtp_profile(ssl)) == NULL) {
 		FAIL("SRTP profile should be set now\n");
@@ -2500,6 +2572,10 @@ test_tlsext_srtp_clienthello(void)
 		FAIL("should be able to fall back to negotiated\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	/* If we fallback, the server should NOT send the extension. */
 	if (SSL_get_selected_srtp_profile(ssl) != NULL) {
@@ -2609,6 +2685,10 @@ test_tlsext_srtp_serverhello(void)
 		FAIL("failed to parse SRTP\n");
 		goto err;
 	}
+	if (CBS_len(&cbs) != 0) {
+		FAIL("extension data remaining");
+		goto err;
+	}
 
 	if ((prof = SSL_get_selected_srtp_profile(ssl)) == NULL) {
 		FAIL("SRTP profile should be set now\n");