LibTLS: Support empty SNI data in ServerHello

According to RFC6066, empty extension_data for an SNI extension is
absolutely one of the possibilities - so let's support this instead of
spamming the debug log.

Userland/Libraries/LibTLS/ClientHandshake.cpp

@@ -117,27 +117,24 @@ ssize_t TLSv12::handle_hello(ReadonlyBytes buffer, WritePacketStage& write_packe
     // The handshake hash function is _always_ SHA256
     m_context.handshake_hash.initialize(Crypto::Hash::HashKind::SHA256);
 
-    if (buffer.size() - res < 1) {
+    // Compression method
-        dbgln("not enough data for compression spec");
+    if (buffer.size() - res < 1)
         return (i8)Error::NeedMoreData;
-    }
     u8 compression = buffer[res++];
-    if (compression != 0) {
+    if (compression != 0)
-        dbgln("Server told us to compress, we will not!");
         return (i8)Error::CompressionNotSupported;
-    }
 
-    if (res > 0) {
     if (m_context.connection_status != ConnectionStatus::Renegotiating)
         m_context.connection_status = ConnectionStatus::Negotiating;
     if (m_context.is_server) {
         dbgln("unsupported: server mode");
         write_packets = WritePacketStage::ServerHandshake;
     }
-    }
 
-    if (res > 2) {
+    // Presence of extensions is determined by availability of bytes after compression_method
-        res += 2;
+    if (buffer.size() - res >= 2) {
+        auto extensions_bytes_total = AK::convert_between_host_and_network_endian(*(const u16*)buffer.offset_pointer(res += 2));
+        dbgln_if(TLS_DEBUG, "Extensions bytes total: {}", extensions_bytes_total);
     }
 
     while ((ssize_t)buffer.size() - res >= 4) {
@@ -146,26 +143,38 @@ ssize_t TLSv12::handle_hello(ReadonlyBytes buffer, WritePacketStage& write_packe
         u16 extension_length = AK::convert_between_host_and_network_endian(*(const u16*)buffer.offset_pointer(res));
         res += 2;
 
-        dbgln_if(TLS_DEBUG, "extension {} with length {}", (u16)extension_type, extension_length);
+        dbgln_if(TLS_DEBUG, "Extension {} with length {}", (u16)extension_type, extension_length);
 
-        if (extension_length) {
+        if (buffer.size() - res < extension_length)
-            if (buffer.size() - res < extension_length) {
-                dbgln("not enough data for extension");
             return (i8)Error::NeedMoreData;
-            }
 
-            dbgln("Encountered extension {} with length {}", (u16)extension_type, extension_length);
-            // SNI
         if (extension_type == HandshakeExtension::ServerName) {
-                u16 sni_host_length = AK::convert_between_host_and_network_endian(*(const u16*)buffer.offset_pointer(res + 3));
+            // RFC6066 section 3: SNI extension_data can be empty in the server hello
-                if (buffer.size() - res - 5 < sni_host_length) {
+            if (extension_length > 0) {
-                    dbgln("Not enough data for sni {} < {}", (buffer.size() - res - 5), sni_host_length);
+                // ServerNameList total size
+                if (buffer.size() - res < 2)
                     return (i8)Error::NeedMoreData;
-                }
+                auto sni_name_list_bytes = AK::convert_between_host_and_network_endian(*(const u16*)buffer.offset_pointer(res += 2));
+                dbgln_if(TLS_DEBUG, "SNI: expecting ServerNameList of {} bytes", sni_name_list_bytes);
 
-                if (sni_host_length) {
+                // Exactly one ServerName should be present
-                    m_context.extensions.SNI = String { (const char*)buffer.offset_pointer(res + 5), sni_host_length };
+                if (buffer.size() - res < 3)
-                    dbgln("server name indicator: {}", m_context.extensions.SNI);
+                    return (i8)Error::NeedMoreData;
+                auto sni_name_type = (NameType)(*(const u8*)buffer.offset_pointer(res++));
+                auto sni_name_length = AK::convert_between_host_and_network_endian(*(const u16*)buffer.offset_pointer(res += 2));
+
+                if (sni_name_type != NameType::HostName)
+                    return (i8)Error::NotUnderstood;
+
+                if (sizeof(sni_name_type) + sizeof(sni_name_length) + sni_name_length != sni_name_list_bytes)
+                    return (i8)Error::BrokenPacket;
+
+                // Read out the host_name
+                if (buffer.size() - res < sni_name_length)
+                    return (i8)Error::NeedMoreData;
+                m_context.extensions.SNI = String { (const char*)buffer.offset_pointer(res), sni_name_length };
+                res += sni_name_length;
+                dbgln("SNI host_name: {}", m_context.extensions.SNI);
             }
         } else if (extension_type == HandshakeExtension::ApplicationLayerProtocolNegotiation && m_context.alpn.size()) {
             if (buffer.size() - res > 2) {
@@ -189,17 +198,15 @@ ssize_t TLSv12::handle_hello(ReadonlyBytes buffer, WritePacketStage& write_packe
                     }
                 }
             }
+            res += extension_length;
         } else if (extension_type == HandshakeExtension::SignatureAlgorithms) {
             dbgln("supported signatures: ");
             print_buffer(buffer.slice(res, extension_length));
+            res += extension_length;
             // FIXME: what are we supposed to do here?
         } else {
             dbgln("Encountered unknown extension {} with length {}", (u16)extension_type, extension_length);
-            }
             res += extension_length;
-        } else {
-            // Zero-length extensions.
-            dbgln("Encountered unknown extension {} with length {}", (u16)extension_type, extension_length);
         }
     }
 
@@ -372,9 +379,8 @@ ssize_t TLSv12::handle_payload(ReadonlyBytes vbuffer)
             if (m_context.is_server) {
                 dbgln("unsupported: server mode");
                 VERIFY_NOT_REACHED();
-            } else {
-                payload_res = handle_hello(buffer.slice(1, payload_size), write_packets);
             }
+            payload_res = handle_hello(buffer.slice(1, payload_size), write_packets);
             break;
         case HelloVerifyRequest:
             dbgln("unsupported: DTLS");
@@ -585,6 +591,7 @@ ssize_t TLSv12::handle_payload(ReadonlyBytes vbuffer)
             }
             case Error::NeedMoreData:
                 // Ignore this, as it's not an "error"
+                dbgln_if(TLS_DEBUG, "More data needed");
                 break;
             default:
                 dbgln("Unknown TLS::Error with value {}", payload_res);

Userland/Libraries/LibTLS/TLSv12.h

@@ -175,6 +175,10 @@ enum class HandshakeExtension : u16 {
     SignatureAlgorithms = 0x0d,
 };
 
+enum class NameType : u8 {
+    HostName = 0x00,
+};
+
 enum class WritePacketStage {
     Initial = 0,
     ClientHandshake = 1,