Revert Workaround TLS fragmented records but keep invalid UTF check (#33

)

Task/Issue URL: https://app.asana.com/0/488551667048375/1206049262682135/f

### Description
See asana.

Revert the workaround for TLS fragmentation but keep the UTF8 check for sni hostname

### Steps to test this PR

- [x] from this branch, publish the library to maven local ie. `./gradlew clean assemble publishToMavenLocal`
- [x] In the DDG android app apply the following path
```diff
Subject: [PATCH] Maven local use
---
Index: build.gradle
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/build.gradle b/build.gradle
--- a/build.gradle	(revision d11f7491d7ab4b27223fd352f83c26be403e79ed)
+++ b/build.gradle	(revision 3b1fe446b5d33e4d8a7f400137134ea0b5a797d7)
@@ -40,6 +40,7 @@
     repositories {
         google()
         mavenCentral()
+        mavenLocal()
     }
     configurations.all {
         resolutionStrategy.force 'org.objenesis:objenesis:2.6'
Index: versions.properties
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>ISO-8859-1
===================================================================
diff --git a/versions.properties b/versions.properties
--- a/versions.properties	(revision d11f7491d7ab4b27223fd352f83c26be403e79ed)
+++ b/versions.properties	(revision 3b1fe446b5d33e4d8a7f400137134ea0b5a797d7)
@@ -55,7 +55,7 @@
 
 version.com.android.installreferrer..installreferrer=2.2
 
-version.com.duckduckgo.netguard..netguard-android=1.6.0
+version.com.duckduckgo.netguard..netguard-android=1.7.0-SNAPSHOT
 
 version.com.duckduckgo.synccrypto..sync-crypto-android=0.3.0
 
```
- [x] build DDG app
- [x] AppTP smoke tests

src/netguard/tls_parser.c

@@ -71,17 +71,17 @@ static int parse_tls_server_name(const uint8_t *data, const size_t data_len, cha
 
     /* TLS record length */
     size_t len = ntohs(*((uint16_t *) (data + 3))) + TLS_HEADER_LEN;
-    log_print(PLATFORM_LOG_PRIORITY_INFO, "data len %d, record len %d\n", data_len, len);
+//    data_len = MIN(len, data_len);
     if (data_len < len) {
         // purposely don't return as we have checks later on
         log_print(PLATFORM_LOG_PRIORITY_WARN, "TLS data length smaller than expected, proceed anyways");
     }
 
     /* handshake */
     size_t pos = TLS_HEADER_LEN;
-    if (pos + 1 > data_len) {
-        return -5;
-    }
+//    if (pos + 1 > data_len) {
+//        return -5;
+//    }
 
     if (data[pos] != 0x1) {
         // not a client hello
@@ -98,17 +98,17 @@ static int parse_tls_server_name(const uint8_t *data, const size_t data_len, cha
     pos += 38;
 
     // Session ID
-    if (pos + 1 > data_len) return -7;
+//    if (pos + 1 > data_len) return -7;
     len = (size_t)data[pos];
     pos += 1 + len;
 
     /* Cipher Suites */
-    if (pos + 2 > data_len) return -8;
+//    if (pos + 2 > data_len) return -8;
     len = ntohs(*((uint16_t *) (data + pos)));
     pos += 2 + len;
 
     /* Compression Methods */
-    if (pos + 1 > data_len) return -9;
+//    if (pos + 1 > data_len) return -9;
     len = (size_t)data[pos];
     pos += 1 + len;
 
@@ -118,17 +118,15 @@ static int parse_tls_server_name(const uint8_t *data, const size_t data_len, cha
     }
 
     /* Extensions */
-    if (pos + 2 > data_len) {
-        return -11;
-    }
+//    if (pos + 2 > data_len) {
+//        return -11;
+//    }
     len = ntohs(*((uint16_t *) (data + pos)));
     pos += 2;
 
-    if (pos + len > data_len) {
-        // Possibly a TLS fragmented record, continue anyways to see if we find SNI in the fragment
-        log_print(PLATFORM_LOG_PRIORITY_WARN, "Out of bounds at extensions length, pos(%d) + len(%d) > data_len(%d)", pos, len, data_len);
+//    if (pos + len > data_len) {
 //        return -12;
-    }
+//    }
     return parse_extensions(data + pos, len, server_name);
 }
 
@@ -145,8 +143,8 @@ static int parse_extensions(const uint8_t *data, size_t data_len, char *hostname
         if (data[pos] == 0x00 && data[pos + 1] == 0x00) {
             /* There can be only one extension of each type, so we break
                our state and move p to beinnging of the extension here */
-            if (pos + 4 + len > data_len)
-                return -20;
+//            if (pos + 4 + len > data_len)
+//                return -20;
             return parse_server_name_extension(data + pos + 4, len, hostname);
         }
         pos += 4 + len; /* Advance to the next extension header */
@@ -165,9 +163,9 @@ static int parse_server_name_extension(const uint8_t *data, size_t data_len, cha
     while (pos + 3 < data_len) {
         len = ntohs(*((uint16_t *) (data + pos + 1)));
 
-        if (pos + 3 + len > data_len) {
-            return -30;
-        }
+//        if (pos + 3 + len > data_len) {
+//            return -30;
+//        }
 
         switch (data[pos]) { /* name type */
             case 0x00: /* host_name */

src/test/stubs.c

@@ -1,3 +1,6 @@
+#include <string.h>
+
 int is_valid_utf8(const char *str) {
-    return 1;
+    const char pattern[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0};
+    return strcmp((char*)pattern, str) != 0;
 }

src/test/test_tls.c

@@ -376,14 +376,14 @@ const unsigned char bad_data_3[] = {
         0x16, 0x03, 0x01, 0x00
 };
 
-const unsigned char bad_data_4[] = {
+const unsigned char sni_invalid_utf[] = {
         // TLS record
         0x16, // Content Type: Handshake
         0x03, 0x01, // Version: TLS 1.0
-        0x00, 0x48, // Length
+        0x00, 0x47, // Length
         // Handshake
         0x01, // Handshake Type: Client Hello
-        0x00, 0x00, 0x42, // Length
+        0x00, 0x00, 0x41, // Length
         0x03, 0x03, // Version: TLS 1.2
         // Random
         0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
@@ -396,19 +396,18 @@ const unsigned char bad_data_4[] = {
         0x00, 0xff, // RENEGOTIATION INFO SCSV
         0x01, // Compression Methods
         0x00, // NULL
-        0x00, 0x17, // Extensions Length
+        0x00, 0x16, // Extensions Length
         // Extension
         0x00, 0x00, // Extension Type: Server Name
         0x00, 0x0e, // Length
         0x00, 0x0c, // Server Name Indication Length
         0x00, // Server Name Type: host_name
         0x00, 0x09, // Length
         // "localhost"
-        0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x68, 0x6f, 0x73, 0x74,
+        0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
         // Extension
-        0x00, 0x0f, // Extension Type: Heart Beat
-        0x00, 0x01, // Length
-        0x01 // Mode: Peer allows to send requests
+        0x00, 0x23, // Extension Type: Session Ticket TLS
+        0x00, 0x00, // Length
 };
 
 const unsigned char wrong_sni_length[] = {
@@ -569,7 +568,7 @@ int main() {
     error = get_server_name(pkt, sizeof(bad_data_2), pkt, sn);
     assert(strcmp("localhost", sn) != 0);
     assert(strlen(sn) == 0);
-    assert(error == -30);
+    assert(error == -31);
 
     pkt = (uint8_t *)bad_data_3;
     memset(sn, 0, FQDN_LENGTH);
@@ -585,7 +584,7 @@ int main() {
     error = get_server_name(pkt, sizeof(wrong_sni_length), pkt, sn);
     assert(strcmp("localhost", sn) != 0);
     assert(strlen(sn) == 0);
-    assert(error == -30);
+    assert(error == -33);
 
     pkt = (uint8_t *)fragmentedSNI2;
     memset(sn, 0, FQDN_LENGTH);
@@ -603,5 +602,13 @@ int main() {
     assert(strlen(sn) == 9);
     assert(error == 9);
 
+    pkt = (uint8_t *)sni_invalid_utf;
+    memset(sn, 0, FQDN_LENGTH);
+    *sn = 0;
+    error = get_server_name(pkt, sizeof(sni_invalid_utf), pkt, sn);
+    assert(strcmp("localhost", sn) != 0);
+    assert(strlen(sn) == 0);
+    assert(error == -34);
+
     return 0;
 }