commit aedf303cc2edbe3fc404b73186477cbb614b65ad
author David Benjamin <davidben@google.com> Thu Dec 01 16:47:56 2016 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu Dec 01 21:53:13 2016 +0000
tree c37966ea49c04f7cc02ed34ffc3ae036f4d7d328
parent bf48364a8c2ccbb9b3609727b628c2e1f1ed12bd

Parse the entire PSK extension.

Although we ignore all but the first identity, keep clients honest by
parsing the whole thing. Also explicitly check that the binder and
identity counts match.

Change-Id: Ib9c4caae18398360f3b80f8db1b22d4549bd5746
Reviewed-on: https://boringssl-review.googlesource.com/12469
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/t1_lib.c

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 21005f0..19c36d7 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1977,11 +1977,12 @@
                                              CBS *contents) {
   /* We only process the first PSK identity since we don't support pure PSK. */
   uint32_t obfuscated_ticket_age;
-  CBS identity, ticket, binders;
-  if (!CBS_get_u16_length_prefixed(contents, &identity) ||
-      !CBS_get_u16_length_prefixed(&identity, &ticket) ||
-      !CBS_get_u32(&identity, &obfuscated_ticket_age) ||
+  CBS identities, ticket, binders;
+  if (!CBS_get_u16_length_prefixed(contents, &identities) ||
+      !CBS_get_u16_length_prefixed(&identities, &ticket) ||
+      !CBS_get_u32(&identities, &obfuscated_ticket_age) ||
       !CBS_get_u16_length_prefixed(contents, &binders) ||
+      CBS_len(&binders) == 0 ||
       CBS_len(contents) != 0) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
     *out_alert = SSL_AD_DECODE_ERROR;
@@ -1990,11 +1991,38 @@
 
   *out_binders = binders;
 
-  /* The PSK identity must have a corresponding binder. */
-  CBS binder;
-  if (!CBS_get_u8_length_prefixed(&binders, &binder)) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
-    *out_alert = SSL_AD_DECODE_ERROR;
+  /* Check the syntax of the remaining identities, but do not process them. */
+  size_t num_identities = 1;
+  while (CBS_len(&identities) != 0) {
+    CBS unused_ticket;
+    uint32_t unused_obfuscated_ticket_age;
+    if (!CBS_get_u16_length_prefixed(&identities, &unused_ticket) ||
+        !CBS_get_u32(&identities, &unused_obfuscated_ticket_age)) {
+      OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+      *out_alert = SSL_AD_DECODE_ERROR;
+      return 0;
+    }
+
+    num_identities++;
+  }
+
+  /* Check the syntax of the binders. The value will be checked later if
+   * resuming. */
+  size_t num_binders = 0;
+  while (CBS_len(&binders) != 0) {
+    CBS binder;
+    if (!CBS_get_u8_length_prefixed(&binders, &binder)) {
+      OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+      *out_alert = SSL_AD_DECODE_ERROR;
+      return 0;
+    }
+
+    num_binders++;
+  }
+
+  if (num_identities != num_binders) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_PSK_IDENTITY_BINDER_COUNT_MISMATCH);
+    *out_alert = SSL_AD_ILLEGAL_PARAMETER;
     return 0;
   }