TLS KeyShare dissection boundary, remove conf.debug_dissector, and import test infrastructure

scapy/contrib/automotive/uds.py

@@ -41,8 +41,6 @@
     #                 "The default value is False.")
     conf.contribs['UDS'] = {'treat-response-pending-as-answer': False}
 
-conf.debug_dissector = True
-
 
 class UDS(ISOTP):
     services = ObservableDict(

scapy/layers/tls/keyexchange_tls13.py

@@ -15,7 +15,6 @@
 from scapy.fields import (
     FieldLenField,
     IntField,
-    PacketField,
     PacketLenField,
     PacketListField,
     ShortEnumField,
@@ -157,7 +156,8 @@ class TLS_Ext_KeyShare_SH(TLS_Ext_Unknown):
     name = "TLS Extension - Key Share (for ServerHello)"
     fields_desc = [ShortEnumField("type", 0x33, _tls_ext),
                    ShortField("len", None),
-                   PacketField("server_share", None, KeyShareEntry)]
+                   PacketLenField("server_share", None,
+                                  KeyShareEntry, length_from=lambda pkt: pkt.len)]
 
     def post_build(self, pkt, pay):
         if not self.tls_session.frozen and self.server_share.privkey:

test/imports.uts

@@ -10,8 +10,8 @@ import os
 import glob
 import subprocess
 import re
-import time
 import sys
+from concurrent.futures import ThreadPoolExecutor, as_completed
 from scapy.consts import WINDOWS, OPENBSD
 
 # DEV: to add your file to this list, make sure you have
@@ -49,46 +49,19 @@ ALL_FILES = [
 
 NB_PROC = 1 if WINDOWS or OPENBSD else 4
 
-def append_processes(processes, filename):
-    processes.append(
-        (subprocess.Popen(
-            [sys.executable, "-c", "import %s" % filename],
-            stderr=subprocess.PIPE, encoding="utf8"),
-         time.time(),
-         filename))
-
-def check_processes(processes):
-    for i, tup in enumerate(processes):
-        proc, start_ts, file = tup
-        errs = ""
-        try:
-            _, errs = proc.communicate(timeout=0.5)
-        except subprocess.TimeoutExpired:
-            if time.time() - start_ts > 30:
-                proc.kill()
-                errs = "Timed out (>30s)!"
-        if proc.returncode is None:
-            continue
-        else:
-            print("Finished %s with %d after %f sec" %
-                  (file, proc.returncode, time.time() - start_ts))
-            if proc.returncode != 0:
-                for p in processes:
-                    p[0].kill()
-                raise Exception(
-                    "Importing the file '%s' failed !\\n%s" % (file, errs))
-            del processes[i]
-            return
-
-
 def import_all(FILES):
-    processes = list()
-    while len(processes) == NB_PROC:
-        check_processes(processes)
-    for filename in FILES:
-        check_processes(processes)
-        if len(processes) < NB_PROC:
-            append_processes(processes, filename)
+    with ThreadPoolExecutor(max_workers=NB_PROC) as executor:
+        futures = {
+            executor.submit(subprocess.run,
+                [sys.executable, "-c", "import %s" % f],
+                stderr=subprocess.PIPE, encoding="utf8", timeout=30): f
+            for f in FILES
+        }
+        for future in as_completed(futures):
+            result = future.result()
+            if result.returncode != 0:
+                raise Exception(
+                    "Importing the file '%s' failed !\n%s" % (futures[future], result.stderr))
 
 
 = Try importing all core separately
@@ -102,3 +75,20 @@ import_all(x for x in ALL_FILES if "layers" in x)
 = Try importing all contribs separately
 
 import_all(x for x in ALL_FILES if "contrib" in x)
+
+= Verify no module modifies conf.debug_dissector on import
+
+with ThreadPoolExecutor(max_workers=NB_PROC) as _executor:
+    _futures = {
+        _executor.submit(subprocess.run,
+            [sys.executable, "-c",
+             "import %s; from scapy.config import conf; "
+             "assert not conf.debug_dissector, "
+             "'%s set conf.debug_dissector as a side-effect'" % (f, f)],
+            stderr=subprocess.PIPE, encoding="utf8", timeout=30): f
+        for f in ALL_FILES
+    }
+    for _future in as_completed(_futures):
+        _result = _future.result()
+        assert _result.returncode == 0, \
+            "%s set conf.debug_dissector as a side-effect\n%s" % (_futures[_future], _result.stderr)

test/scapy/layers/tls/tls13.uts

@@ -1212,6 +1212,38 @@ assert ch.len == 103
 assert ch.client_shares[0].kxlen == 97
 assert len(ch.client_shares[0].key_exchange) == 97
 
+= TLS_Ext_KeyShare_SH - dissect raw bytes with PacketLenField
+
+from scapy.layers.tls.keyexchange_tls13 import TLS_Ext_KeyShare_SH
+from scapy.packet import Raw
+
+# Raw bytes of TLS_Ext_KeyShare_SH with an x25519 server key (RFC 8448 test vector)
+x25519_pub_rfc8448 = 'c9828876112095fe66762bdbf7c672e156d6cc253b833df1dd69b1b04e751f0f'
+raw_ks = bytes.fromhex(
+    '0033'  # type = key_share (0x0033)
+    '0024'  # len = 36 (4 header + 32 key bytes)
+    '001d'  # group = x25519 (29)
+    '0020'  # kxlen = 32
+    + x25519_pub_rfc8448
+)
+pkt = TLS_Ext_KeyShare_SH(raw_ks)
+assert pkt.type == 0x0033
+assert pkt.len == 36
+assert pkt.server_share.group == 29
+assert pkt.server_share.kxlen == 32
+assert pkt.server_share.key_exchange == bytes.fromhex(x25519_pub_rfc8448)
+
+# Trailing bytes (simulating a subsequent extension) must not be consumed by server_share
+trailing = bytes.fromhex('002b00020304')  # TLS_Ext_SupportedVersion_SH (TLS 1.3)
+pkt2 = TLS_Ext_KeyShare_SH(raw_ks + trailing)
+assert pkt2.server_share.group == 29
+assert pkt2.server_share.kxlen == 32
+assert len(pkt2.server_share.key_exchange) == 32
+assert pkt2.server_share.key_exchange == bytes.fromhex(x25519_pub_rfc8448)
+# Trailing bytes must be preserved as payload, not silently consumed by server_share
+assert Raw in pkt2
+assert pkt2[Raw].load == trailing
+
 = Parse TLS 1.3 Client Hello with non-rfc 5077 ticket
 
 from scapy.layers.tls.keyexchange_tls13 import TLS_Ext_PreSharedKey_CH