Refactor ClientHello extension parsing

1) Simplify code with better PACKET methods.

2) Make broken SNI parsing explicit. SNI was intended to be extensible
to new name types but RFC 4366 defined the syntax inextensibly, and
OpenSSL has never parsed SNI in a way that would allow adding a new name
type. RFC 6066 fixed the definition but due to broken implementations
being widespread, it appears impossible to ever extend SNI.

3) Annotate resumption behaviour. OpenSSL doesn't currently handle all
extensions correctly upon resumption. Annotate for further clean-up.

4) Send an alert on ALPN protocol mismatch.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>

2 files changed, 102 insertions, 4 deletions

diff --git a/test/packettest.c b/test/packettest.c
index 0555c7b..57ef51b 100644
--- a/test/packettest.c
+++ b/test/packettest.c
@@ -77,6 +77,24 @@ static int test_PACKET_remaining(unsigned char buf[BUF_LEN])
     return 1;
 }
 
+static int test_PACKET_end(unsigned char buf[BUF_LEN])
+{
+    PACKET pkt;
+
+    if (       !PACKET_buf_init(&pkt, buf, BUF_LEN)
+            ||  PACKET_remaining(&pkt) != BUF_LEN
+            ||  PACKET_end(&pkt) != buf + BUF_LEN
+            || !PACKET_forward(&pkt, BUF_LEN - 1)
+            || PACKET_end(&pkt) != buf + BUF_LEN
+            || !PACKET_forward(&pkt, 1)
+            || PACKET_end(&pkt) != buf + BUF_LEN) {
+        fprintf(stderr, "test_PACKET_end() failed\n");
+        return 0;
+    }
+
+    return 1;
+}
+
 static int test_PACKET_get_1(unsigned char buf[BUF_LEN])
 {
     unsigned int i;
@@ -308,6 +326,26 @@ static int test_PACKET_strndup()
     return 1;
 }
 
+static int test_PACKET_contains_zero_byte()
+{
+    char buf[10], buf2[10];
+    PACKET pkt;
+
+    memset(buf, 'x', 10);
+    memset(buf2, 'y', 10);
+    buf2[5] = '\0';
+
+    if (       !PACKET_buf_init(&pkt, (unsigned char*)buf, 10)
+            ||  PACKET_contains_zero_byte(&pkt)
+            || !PACKET_buf_init(&pkt, (unsigned char*)buf2, 10)
+            || !PACKET_contains_zero_byte(&pkt)) {
+        fprintf(stderr, "test_PACKET_contains_zero_byte failed\n");
+        return 0;
+    }
+
+    return 1;
+}
+
 static int test_PACKET_forward(unsigned char buf[BUF_LEN])
 {
     const unsigned char *byte;
@@ -457,6 +495,57 @@ static int test_PACKET_get_length_prefixed_3()
     return 1;
 }
 
+static int test_PACKET_as_length_prefixed_1()
+{
+    unsigned char buf[BUF_LEN];
+    const size_t len = 16;
+    unsigned int i;
+    PACKET pkt, exact_pkt, subpkt;
+
+    buf[0] = len;
+    for (i = 1; i < BUF_LEN; i++) {
+        buf[i] = (i * 2) & 0xff;
+    }
+
+    if (       !PACKET_buf_init(&pkt, buf, BUF_LEN)
+            || !PACKET_buf_init(&exact_pkt, buf, len + 1)
+            ||  PACKET_as_length_prefixed_1(&pkt, &subpkt)
+            ||  PACKET_remaining(&pkt) != BUF_LEN
+            || !PACKET_as_length_prefixed_1(&exact_pkt, &subpkt)
+            ||  PACKET_remaining(&exact_pkt) != 0
+            ||  PACKET_remaining(&subpkt) != len) {
+        fprintf(stderr, "test_PACKET_as_length_prefixed_1() failed\n");
+        return 0;
+    }
+
+    return 1;
+}
+
+static int test_PACKET_as_length_prefixed_2()
+{
+    unsigned char buf[1024];
+    const size_t len = 516;  /* 0x0204 */
+    unsigned int i;
+    PACKET pkt, exact_pkt, subpkt;
+
+    for (i = 1; i <= 1024; i++) {
+        buf[i-1] = (i * 2) & 0xff;
+    }
+
+    if (       !PACKET_buf_init(&pkt, buf, 1024)
+            || !PACKET_buf_init(&exact_pkt, buf, len + 2)
+            ||  PACKET_as_length_prefixed_2(&pkt, &subpkt)
+            ||  PACKET_remaining(&pkt) != 1024
+            || !PACKET_as_length_prefixed_2(&exact_pkt, &subpkt)
+            ||  PACKET_remaining(&exact_pkt) != 0
+            ||  PACKET_remaining(&subpkt) != len) {
+        fprintf(stderr, "test_PACKET_as_length_prefixed_2() failed\n");
+        return 0;
+    }
+
+    return 1;
+}
+
 int main(int argc, char **argv)
 {
     unsigned char buf[BUF_LEN];
@@ -470,6 +559,7 @@ int main(int argc, char **argv)
     if (       !test_PACKET_buf_init()
             || !test_PACKET_null_init()
             || !test_PACKET_remaining(buf)
+            || !test_PACKET_end(buf)
             || !test_PACKET_equal(buf)
             || !test_PACKET_get_1(buf)
             || !test_PACKET_get_4(buf)
@@ -482,10 +572,13 @@ int main(int argc, char **argv)
             || !test_PACKET_copy_all(buf)
             || !test_PACKET_memdup(buf)
             || !test_PACKET_strndup()
+            || !test_PACKET_contains_zero_byte()
             || !test_PACKET_forward(buf)
             || !test_PACKET_get_length_prefixed_1()
             || !test_PACKET_get_length_prefixed_2()
-            || !test_PACKET_get_length_prefixed_3()) {
+            || !test_PACKET_get_length_prefixed_3()
+            || !test_PACKET_as_length_prefixed_1()
+            || !test_PACKET_as_length_prefixed_2()) {
         return 1;
     }
     printf("PASS\n");
diff --git a/test/recipes/80-test_ssl.t b/test/recipes/80-test_ssl.t
index e0f2fc5..bcab4b5 100644
--- a/test/recipes/80-test_ssl.t
+++ b/test/recipes/80-test_ssl.t
@@ -606,20 +606,25 @@ sub testssl {
     subtest 'ALPN tests' => sub {
 	######################################################################
 
-	plan tests => 12;
+	plan tests => 14;
 
       SKIP: {
 	  skip "TLSv1.0 is not supported by this OpenSSL build", 12
 	      if $no_tls1;
 
-	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo", "-alpn_server", "bar"])));
+	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo"])));
+	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_server", "foo"])));
 	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo", "-alpn_server", "foo", "-alpn_expected", "foo"])));
 	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo,bar", "-alpn_server", "foo", "-alpn_expected", "foo"])));
 	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "bar,foo", "-alpn_server", "foo", "-alpn_expected", "foo"])));
 	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "bar,foo", "-alpn_server", "foo,bar", "-alpn_expected", "foo"])));
 	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "bar,foo", "-alpn_server", "bar,foo", "-alpn_expected", "bar"])));
 	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo,bar", "-alpn_server", "bar,foo", "-alpn_expected", "bar"])));
-	  ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "baz", "-alpn_server", "bar,foo"])));
+
+	  is(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo", "-alpn_server", "bar"])), 0,
+             "Testing ALPN with protocol mismatch, expecting failure");
+	  is(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "baz", "-alpn_server", "bar,foo"])), 0,
+             "Testing ALPN with protocol mismatch, expecting failure");
 
 	SKIP: {
 	    skip "skipping SRP tests", 4