added more tests with code refactoring.

src/ssl.c

@@ -568,7 +568,6 @@ word16 CyaSSL_SNI_GetRequest(CYASSL* ssl, byte type, void** data)
     return 0;
 }
 
-
 int CyaSSL_SNI_GetFromBuffer(const byte* buffer, word32 bufferSz, byte type,
                                                      byte* sni, word32* inOutSz)
 {

src/tls.c

@@ -863,6 +863,32 @@ void TLSX_SNI_SetOptions(TLSX* extensions, byte type, byte options)
         sni->options = options;
 }
 
+#define BYTE_CHECK(buffer, offset, op, expected) do { \
+    if (buffer[offset++] op expected)                 \
+        return BUFFER_ERROR;                          \
+} while (0)
+
+#define SAFE_READ_16(buffer, offset, max, len) do { \
+    ato16(buffer + offset, &len); offset += 2;      \
+                                                    \
+    if (offset + len > max)                         \
+        return INCOMPLETE_DATA;                     \
+} while (0)
+
+#define SAFE_READ_32(buffer, offset, max, len) do { \
+    c24to32(buffer + offset, &len); offset += 3;    \
+                                                    \
+    if (offset + len > max)                         \
+        return INCOMPLETE_DATA;                     \
+} while (0)
+
+#define SKIP_LEN8(buffer, offset, max) do { \
+    if (offset + buffer[offset] > max)      \
+        return INCOMPLETE_DATA;             \
+                                            \
+    offset += ENUM_LEN + buffer[offset];    \
+} while (0)
+
 int TLSX_SNI_GetFromBuffer(const byte* buffer, word32 bufferSz,
                            byte type, byte* sni, word32* inOutSz)
 {
@@ -874,57 +900,37 @@ int TLSX_SNI_GetFromBuffer(const byte* buffer, word32 bufferSz,
         return INCOMPLETE_DATA;
 
     /* TLS record header */
-    if ((enum ContentType) buffer[offset++] != handshake)
+    BYTE_CHECK(buffer, offset, !=, handshake);
-        return BUFFER_ERROR;
+    BYTE_CHECK(buffer, offset, !=, SSLv3_MAJOR);
-
+    BYTE_CHECK(buffer, offset,  <, TLSv1_MINOR);
-    if (buffer[offset++] != SSLv3_MAJOR)
+    SAFE_READ_16(buffer, offset, bufferSz, len16);
-        return BUFFER_ERROR;
-
-    if (buffer[offset++] < TLSv1_MINOR)
-        return BUFFER_ERROR;
-
-    ato16(buffer + offset, &len16);
-    offset += OPAQUE16_LEN;
-
-    if (offset + len16 > bufferSz)
-        return INCOMPLETE_DATA;
 
     /* Handshake header */
-    if ((enum HandShakeType) buffer[offset] != client_hello)
+    BYTE_CHECK(buffer, offset, !=, client_hello);
-        return BUFFER_ERROR;
+    SAFE_READ_32(buffer, offset, bufferSz, len32);
-
-    c24to32(buffer + offset + 1, &len32);
-    offset += HANDSHAKE_HEADER_SZ;
-
-    if (offset + len32 > bufferSz)
-        return INCOMPLETE_DATA;
 
     /* client hello */
     offset += VERSION_SZ + RAN_LEN;      /* version, random */
+    SKIP_LEN8(buffer, offset, bufferSz); /* session id      */
 
-    if (offset + buffer[offset] > bufferSz)
+    /* cypher suites */
+    if (bufferSz < offset + 2)
         return INCOMPLETE_DATA;
 
-    offset += ENUM_LEN + buffer[offset]; /* session id */
+    SAFE_READ_16(buffer, offset, bufferSz, len16);
+    offset += len16;
 
-    ato16(buffer + offset, &len16);
+    /* compression methods */
-    offset += OPAQUE16_LEN; /* cypher suites len */
+    if (bufferSz < offset + 1)
-
-    if (offset + len16 > bufferSz)
         return INCOMPLETE_DATA;
 
-    offset += len16; /* cypher suites */
+    SKIP_LEN8(buffer, offset, bufferSz);
 
-    if (offset + buffer[offset] > bufferSz)
+    /* extensions */
-        return INCOMPLETE_DATA;
+    if (bufferSz < offset + 2)
+        return 0; /* no extensions in client hello. */
 
-    offset += ENUM_LEN + buffer[offset]; /* compression methods */
+    SAFE_READ_16(buffer, offset, bufferSz, len16);
-
-    ato16(buffer + offset, &len16);
-    offset += OPAQUE16_LEN; /* EXTENSIONS LEN */
-
-    if (offset + len16 > bufferSz)
-        return INCOMPLETE_DATA;
 
     while (len16 > OPAQUE16_LEN + OPAQUE16_LEN) {
         word16 extType;
@@ -933,11 +939,7 @@ int TLSX_SNI_GetFromBuffer(const byte* buffer, word32 bufferSz,
         ato16(buffer + offset, &extType);
         offset += OPAQUE16_LEN;
 
-        ato16(buffer + offset, &extLen);
+        SAFE_READ_16(buffer, offset, bufferSz, extLen);
-        offset += OPAQUE16_LEN;
-
-        if (offset + extLen > bufferSz)
-            return INCOMPLETE_DATA;
 
         if (extType != SERVER_NAME_INDICATION) {
             offset += extLen;
@@ -945,21 +947,13 @@ int TLSX_SNI_GetFromBuffer(const byte* buffer, word32 bufferSz,
         } else {
             word16 listLen;
 
-            ato16(buffer + offset, &listLen);
+            SAFE_READ_16(buffer, offset, bufferSz, listLen);
-            offset += OPAQUE16_LEN;
-
-            if (offset + listLen > bufferSz)
-                return INCOMPLETE_DATA;
 
             while (listLen > ENUM_LEN + OPAQUE16_LEN) {
                 byte   sniType = buffer[offset++];
                 word16 sniLen;
 
-                ato16(buffer + offset, &sniLen);
+                SAFE_READ_16(buffer, offset, bufferSz, sniLen);
-                offset += OPAQUE16_LEN;
-
-                if (offset + sniLen > bufferSz)
-                    return INCOMPLETE_DATA;
 
                 if (sniType != type) {
                     offset += sniLen;
@@ -969,15 +963,19 @@ int TLSX_SNI_GetFromBuffer(const byte* buffer, word32 bufferSz,
                 *inOutSz = min(sniLen, *inOutSz);
                 XMEMCPY(sni, buffer + offset, *inOutSz);
 
-                break;
+                return SSL_SUCCESS;
             }
-
-            break;
         }
     }
 
-    return 0;
+    return len16 ? BUFFER_ERROR : 0;
 }
+
+#undef SAFE_READ_32
+#undef SAFE_READ_16
+#undef BYTE_CHECK
+#undef SKIP_LEN8
+
 #endif
 
 #define SNI_FREE_ALL TLSX_SNI_FreeAll

tests/api.c

@@ -334,7 +334,19 @@ static void verify_SNI_fake_matching(CYASSL* ssl)
 
 static void test_CyaSSL_SNI_GetFromBuffer(void)
 {
-    byte buffer[] = { /* api.textmate.org */
+    byte buffer[] = { /* www.paypal.com */
+        0x00, 0x00, 0x00, 0x00, 0xff, 0x01, 0x00, 0x00, 0x60, 0x03, 0x03, 0x5c,
+        0xc4, 0xb3, 0x8c, 0x87, 0xef, 0xa4, 0x09, 0xe0, 0x02, 0xab, 0x86, 0xca,
+        0x76, 0xf0, 0x9e, 0x01, 0x65, 0xf6, 0xa6, 0x06, 0x13, 0x1d, 0x0f, 0xa5,
+        0x79, 0xb0, 0xd4, 0x77, 0x22, 0xeb, 0x1a, 0x00, 0x00, 0x16, 0x00, 0x6b,
+        0x00, 0x67, 0x00, 0x39, 0x00, 0x33, 0x00, 0x3d, 0x00, 0x3c, 0x00, 0x35,
+        0x00, 0x2f, 0x00, 0x05, 0x00, 0x04, 0x00, 0x0a, 0x01, 0x00, 0x00, 0x21,
+        0x00, 0x00, 0x00, 0x13, 0x00, 0x11, 0x00, 0x00, 0x0e, 0x77, 0x77, 0x77,
+        0x2e, 0x70, 0x61, 0x79, 0x70, 0x61, 0x6c, 0x2e, 0x63, 0x6f, 0x6d, 0x00,
+        0x0d, 0x00, 0x06, 0x00, 0x04, 0x04, 0x01, 0x02, 0x01
+    };
+
+    byte buffer2[] = { /* api.textmate.org */
         0x16, 0x03, 0x01, 0x00, 0xc6, 0x01, 0x00, 0x00, 0xc2, 0x03, 0x03, 0x52,
         0x8b, 0x7b, 0xca, 0x69, 0xec, 0x97, 0xd5, 0x08, 0x03, 0x50, 0xfe, 0x3b,
         0x99, 0xc3, 0x20, 0xce, 0xa5, 0xf6, 0x99, 0xa5, 0x71, 0xf9, 0x57, 0x7f,
@@ -354,33 +366,41 @@ static void test_CyaSSL_SNI_GetFromBuffer(void)
         0x0a, 0x05, 0x01, 0x04, 0x01, 0x02, 0x01, 0x04, 0x03, 0x02, 0x03
     };
 
-    byte buffer2[] = { /* www.paypal.com */
-        0x16, 0x03, 0x03, 0x00, 0x64, 0x01, 0x00, 0x00, 0x60, 0x03, 0x03, 0x5c,
-        0xc4, 0xb3, 0x8c, 0x87, 0xef, 0xa4, 0x09, 0xe0, 0x02, 0xab, 0x86, 0xca,
-        0x76, 0xf0, 0x9e, 0x01, 0x65, 0xf6, 0xa6, 0x06, 0x13, 0x1d, 0x0f, 0xa5,
-        0x79, 0xb0, 0xd4, 0x77, 0x22, 0xeb, 0x1a, 0x00, 0x00, 0x16, 0x00, 0x6b,
-        0x00, 0x67, 0x00, 0x39, 0x00, 0x33, 0x00, 0x3d, 0x00, 0x3c, 0x00, 0x35,
-        0x00, 0x2f, 0x00, 0x05, 0x00, 0x04, 0x00, 0x0a, 0x01, 0x00, 0x00, 0x21,
-        0x00, 0x00, 0x00, 0x13, 0x00, 0x11, 0x00, 0x00, 0x0e, 0x77, 0x77, 0x77,
-        0x2e, 0x70, 0x61, 0x79, 0x70, 0x61, 0x6c, 0x2e, 0x63, 0x6f, 0x6d, 0x00,
-        0x0d, 0x00, 0x06, 0x00, 0x04, 0x04, 0x01, 0x02, 0x01
-    };
-
     byte result[32] = {0};
     word32 length   = 32;
 
-//    AssertIntEQ(-228, CyaSSL_SNI_GetFromBuffer((const byte*) "\x16\x03\x00\x00\x01", 5, 0,
+    AssertIntEQ(-228, CyaSSL_SNI_GetFromBuffer(buffer, sizeof(buffer), 0,
-//                                                              result, &length));
+                                                              result, &length));
 
-    AssertIntEQ(0, CyaSSL_SNI_GetFromBuffer(buffer, sizeof(buffer), 0, result,
+    buffer[0] = 0x16;
-                                                                      &length));
+
-    result[length] = 0;
+    AssertIntEQ(-228, CyaSSL_SNI_GetFromBuffer(buffer, sizeof(buffer), 0,
-    AssertStrEQ("api.textmate.org", (const char*) result);
+                                                                  result, &length));
+
+    buffer[1] = 0x03;
+
+    AssertIntEQ(-228, CyaSSL_SNI_GetFromBuffer(buffer, sizeof(buffer), 0,
+                                                                  result, &length));
+
+    buffer[2] = 0x03;
+
+    AssertIntEQ(-210, CyaSSL_SNI_GetFromBuffer(buffer, sizeof(buffer), 0,
+                                                                  result, &length));
+
+    buffer[4] = 0x64;
+
+    AssertIntEQ(1, CyaSSL_SNI_GetFromBuffer(buffer, sizeof(buffer), 0,
+                                                              result, &length));
 
-    AssertIntEQ(0, CyaSSL_SNI_GetFromBuffer(buffer2, sizeof(buffer2), 0, result,
-                                                                      &length));
     result[length] = 0;
     AssertStrEQ("www.paypal.com", (const char*) result);
+
+    length = 32;
+
+    AssertIntEQ(1, CyaSSL_SNI_GetFromBuffer(buffer2, sizeof(buffer2), 0,
+                                                              result, &length));
+    result[length] = 0;
+    AssertStrEQ("api.textmate.org", (const char*) result);
 }
 
 void test_CyaSSL_UseSNI(void)