From c3c65eef6792101b31603f77f6b54cfffd9fca3b Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Fri, 3 Apr 2026 19:08:49 +0200 Subject: [PATCH 1/2] Add files via upload --- doc/mbedtls4_crypto_primitive_selection.md | 93 ++++++++++ doc/mbedtls4_security_review.md | 201 +++++++++++++++++++++ 2 files changed, 294 insertions(+) create mode 100644 doc/mbedtls4_crypto_primitive_selection.md create mode 100644 doc/mbedtls4_security_review.md diff --git a/doc/mbedtls4_crypto_primitive_selection.md b/doc/mbedtls4_crypto_primitive_selection.md new file mode 100644 index 0000000000..761f32906b --- /dev/null +++ b/doc/mbedtls4_crypto_primitive_selection.md @@ -0,0 +1,93 @@ +# ModSecurity v3 – Auswahl kryptografischer Primitive (SHA-256 vs HMAC vs SHA-512 vs Verschlüsselung) + +Datum: 2026-04-02 + +## 1) Bestandsaufnahme (verifizierte Fundstellen) + +### A. Regel-Transformationen (angreiferkontrollierter Input möglich) +- `t:md5` und `t:sha1` sind aktiv registriert und ausführbar. +- Dateien: + - `src/actions/transformations/transformation.cc` + - `src/actions/transformations/md5.cc` + - `src/actions/transformations/sha1.cc` + - Parser-Tokens: `src/parser/seclang-scanner.ll` +- Zweck: Regel-Transformation / Datennormalisierung, **kein** verschlüsselter Speicher und **keine** Authentizitätsgarantie. + +### B. Digest-Wrapper +- Datei: `src/utils/sha1.h` + `src/utils/md5.h`. +- Zweck: allgemeine Digest-Helfer für unkeyed Hashes. + +### C. Audit-Index (Legacy-Format) +- Datei: `src/audit_log/writer/parallel.cc` und `src/transaction.cc` (`md5:`). +- Zweck: altes Indexformat/Referenz, stark formatgebunden. + +### D. Machine Unique ID +- Datei: `src/unique_id.cc`. +- Zweck: stabiler technischer Identifier aus Hostmerkmalen (`SHA-1(mac+hostname)`), **kein** Auth-Token. + +### E. TLS/Remote Rules +- Datei: `src/utils/https_client.cc`. +- Zweck: Transportschutz via TLS für Remote Rules Download. +- In den geprüften Pfaden gibt es keinen lokalen HMAC- oder Verschlüsselungs-Workflow für Regelinhalte. + +## 2) Sicherheitsziel je Fundstelle + +- **Transformationen (`t:md5`, `t:sha1`)** + - Ziel: unkeyed Digest-Transformation (Kategorie 1) + Legacy-Kompatibilität (Kategorie 4). +- **Digest-Wrapper** + - Ziel: unkeyed Digest (Kategorie 1). +- **Audit-Index** + - Ziel: Formatkompatibilität / Referenz (Kategorie 4), nicht Authentizität. +- **Unique-ID** + - Ziel: technischer Identifier (Kategorie 5), nicht kryptografische Authentizität. +- **TLS Remote Rules** + - Ziel: Transportintegrität und Vertraulichkeit via TLS, nicht Payload-HMAC im lokalen Codepfad. + +## 3) Bewertung alternativer Verfahren + +### SHA-256 +**Sinnvoll und ausreichend** für alle Stellen, die heute unkeyed MD5/SHA-1 als Digest verwenden und nicht formatgebunden sind. + +### HMAC-SHA-256 +**Nur sinnvoll, wenn keyed Authentizität fachlich gefordert ist.** +In den geprüften Pfaden wird aktuell kein lokaler geheimer Schlüssel für Hash-Verifikation verwendet; daher ist HMAC **nicht** der direkte Ersatz für bestehende unkeyed Transformations-/Indexpfade. + +### SHA-512 / SHA-384 +Kein belegbarer Zusatznutzen in den aktuellen ModSecurity-v3-Pfaden: +- keine gezeigten Anforderungen, die SHA-256 übersteigen, +- höhere Kosten/Komplexität ohne klares Sicherheitsziel. + +### Echte Verschlüsselung (z. B. AES-GCM/ChaCha20-Poly1305) +In den geprüften MD5/SHA-1-Stellen ist **kein Vertraulichkeitsziel** erkennbar. +Daher ist „Hashing durch Verschlüsselung ersetzen“ fachlich falsch. +Transport-Vertraulichkeit für Remote Rules wird bereits über TLS adressiert. + +## 4) Antwort auf die Hauptfrage + +### Wo SHA-256 allein die richtige Wahl ist +- Neue unkeyed Digest-Pfade als Ersatz für MD5/SHA-1 (insbesondere künftige nicht-legacy Erweiterungen). + +### Wo HMAC-SHA-256 besser wäre +- Nur bei zukünftigen Features mit expliziter keyed Authentizität (z. B. signierte/geschützte Remote-Policy-Protokolle). +- Für die aktuell gefundenen Pfade **nicht** als Drop-in-Ersatz belegt. + +### Wo SHA-512/SHA-384 keinen Zusatznutzen bringt +- Bei Transformationen, Audit-Index-Referenzen und technischen Identifiern im aktuellen Design. + +### Wo Verschlüsselung relevant wäre +- Nur wenn Anforderungen an Vertraulichkeit der gespeicherten/transportierten Daten über TLS hinaus gefordert werden; in den untersuchten Hash-Pfaden nicht ersichtlich. + +### Wo Legacy unverändert bleiben sollte +- `t:md5`, `t:sha1` und altes Audit-Indexformat nicht still brechen; nur additiv migrieren. + +## 5) Konkrete, nicht-brechende nächste Schritte + +1. **SHA-256-first additiv einführen** (z. B. neue `t:sha256` Transformation). +2. **MD5/SHA-1 als Legacy markieren** (Doku + Hinweise), nicht sofort entfernen. +3. Audit-Index mittelfristig additiv um SHA-256 erweitern, Legacy-`md5:` zunächst behalten. +4. HMAC-SHA-256 nur bei neuem, klar spezifiziertem keyed-Authentizitätsbedarf einführen. + +## 6) Offene Punkte + +- Ob externe Parser zwingend das exakte `md5:`-Format benötigen, ist ohne Integrationsinventar nicht vollständig belegbar. +- Ob ein zukünftiges Remote-Rules-Protokoll Payload-Authentizität (zusätzlich zu TLS) fordert, ist eine Produktentscheidung außerhalb der aktuellen Codepfade. diff --git a/doc/mbedtls4_security_review.md b/doc/mbedtls4_security_review.md new file mode 100644 index 0000000000..968b48e2f1 --- /dev/null +++ b/doc/mbedtls4_security_review.md @@ -0,0 +1,201 @@ +# OWASP ModSecurity v3 – Deep Security Review (Crypto/TLS, libcurl, Mbed TLS 4.x) + +Date: 2026-04-02 + +## A) Executive Summary + +### Gesamtbewertung +ModSecurity v3 hat in den untersuchten Pfaden **keine triviale RCE-Klasse**, aber es gibt mehrere **reale security-relevante Schwachstellen bzw. riskante Defaults**: + +1. **SecRemoteRules kann bei Download-Fehlern effektiv fail-open laufen**, wenn `SecRemoteRulesFailAction` nicht explizit auf `Abort` gesetzt ist. +2. **MD5/SHA-1 sind aktiv in Laufzeitpfaden**, teils nur Legacy/Kompatibilität, teils aber weiterhin als Integritäts-/Indexwert genutzt. +3. **Nicht-kryptographische Zufallsquellen (`rand()/srand`)** werden für Boundary/ID-nahe Werte genutzt (vorhersagbar). + +Die geplante Mbed TLS 4.x-Migration ist buildseitig schon vorbereitet (TF-PSA-Crypto Layout), wird aber derzeit primär für Kompatibilität genutzt, nicht für konsequente Security-Modernisierung. + +### Top 3 echte Risiken +1. Remote Rules Fail-Open (konfigurationsabhängig, aber praktisch ausnutzbar in realen Störszenarien). +2. Legacy-Hashes in aktivem Produktpfad (MD5/SHA-1) bleiben ohne klare Policy aktiv. +3. Schwache RNG-Nutzung in Token-/Boundary-Nahbereich. + +### Top 3 wichtigste Fixes +1. `SecRemoteRulesFailAction` Default auf `Abort` (oder PropertyNotSet wie Abort behandeln). +2. SHA-256-first: neue sichere Transformation/Indexoption + Legacy klar markieren. +3. `rand()/srand` in Boundary/ID-Pfaden ersetzen (OS RNG/CSPRNG). + +--- + +## B) Verifizierte Schwachstellen + +## 1. SecRemoteRules – verifiziertes Fail-Open-Verhalten + +### Codepfad +1. `RulesSet::loadFromUri()` ruft `Driver::parseFile()` auf. Fehler nur, wenn Parser fehl schlägt (`parseFile()==0`). +2. Scanner-Regel für `SecRemoteRules` ruft `HttpsClient::download(url)`. +3. Bei `ret == false` wird **nur dann** abgebrochen, wenn `m_remoteRulesActionOnFailed == Abort...`. +4. Für `PropertyNotSetRemoteRulesAction` gibt es **keinen Abort** und auch keinen implementierten Warn-Log-Pfad (TODO). +5. Danach läuft `yy_scan_string(c.content.c_str())`; bei leerem Content wird Parsing fortgesetzt. + +### Evidenz +- Default: `m_remoteRulesActionOnFailed(PropertyNotSetRemoteRulesAction)` in `RulesSetProperties` ctor. +- Branching in Scanner: Abort nur bei explizitem `Abort...`. +- Parse-Rückgabe in Driver: Erfolg, wenn Parser syntaktisch durchläuft. + +### Warum unsicher +Wenn ein Deployment auf Remote Rules vertraut und der Download fehlschlägt (DNS/Netz/Timeout/TLS-Handshake), werden Regeln ggf. nicht geladen, ohne harten Fail-stop. + +### Realistische Ausnutzung +- **DNS Manipulation / Resolver-Ausfall / egress blockiert / künstliche Timeouts** → Remote Rule Fetch schlägt fehl. +- Ergebnis: Instanz läuft ggf. ohne erwartete Regelbasis weiter (Policy-Lücke). + +### Einstufung +**Hoch (konfigurationsabhängig, aber real).** + +--- + +## 2. MD5/SHA-1 in aktiven Pfaden + +### Fundstellen und Einordnung +1. **Transformationen `t:md5`, `t:sha1`** + - Zweck: regelbasierte Transformation. + - Risiko: Kollisionsresistenz schwach; Angreifer kann Input oft kontrollieren. + - Einstufung: **Mittel (Legacy-Kompatibilität, aber veraltet)**. + +2. **Audit-Indexformat `md5:`** + - Zweck: altes Audit-Indexformat (`toOldAuditLogFormatIndex`). + - Risiko: primär Integritäts-/Referenzcharakter; kein direkter Auth-Mechanismus. + - Einstufung: **Mittel (technischer Debt mit sicherheitsrelevantem Beigeschmack)**. + +3. **UniqueId via SHA-1(MAC+Hostname)** + - Zweck: stabiler Host-Identifier. + - Risiko: kein Signatur-/Auth-Kontext; Kollision praktisch weniger kritisch. + - Einstufung: **Niedrig bis Mittel (eher Debt)**. + +### Urteil +- **Nicht alles ist „kritische Schwachstelle“**. +- Aber: aktive Verfügbarkeit schwacher Hashes ohne harte Leitplanken ist für Security-Produktionsumgebungen nicht mehr zeitgemäß. + +--- + +## 3. RNG (`rand`/`srand`) in sicherheitsnahen Pfaden + +### Fundstellen +- Globales Seeding: `srand(time(NULL))` im `ModSecurity`-Konstruktor. +- Boundary-Generierung nutzt `rand() % ...`. + +### Risiko +- Vorhersagbarkeit bei bekannter Startzeit und PRNG-Zustand möglich. +- Für Audit-Boundary eher Integritäts-/Robustheits- als direkter Kryptobruch. + +### Einstufung +**Mittel (Hardening mit realem Missbrauchspotenzial).** + +--- + +## 4. `assert()` in Crypto-relevantem Digestpfad + +### Fundstellen +- `assert(mdInfo != nullptr)` und `assert(ret == 0)` in `DigestImpl`. + +### Risikoanalyse +- In Release (`NDEBUG`) entfallen asserts. +- Fehlerfälle werden dann nicht kontrolliert behandelt; definierte Fehlerpropagation fehlt. +- Das ist eher Robustheits-/Sicherheits-Härtung als unmittelbare exploitable Memory-Corruption. + +### Einstufung +**Mittel (sicherheitsrelevante Robustheitslücke).** + +--- + +## 5. TLS/libcurl Integration + +### Positiv verifiziert +- TLS minimum explizit auf 1.2 gesetzt. +- Peer- und Host-Verification gesetzt. + +### Einschränkungen +- `CURLOPT_SSL_VERIFYHOST` ist auf `1` gesetzt (historisch uneinheitlich; explizit `2L` ist der robuste Wert). +- Kein explizites Pinning-/Trust-Override-Konzept. +- Build-Makro `WITH_CURL_SSLVERSION_TLSv1_2` wird zwar in `build/curl.m4` gesetzt, im Code aber nicht genutzt (Kompatibilitäts-/Wartungsrisiko). + +### Einstufung +**Niedrig bis Mittel** (kein offensichtlicher MITM-default, aber Härtung sinnvoll). + +--- + +## C) False Positives / Entwarnung + +1. **„ModSecurity implementiert seinen eigenen TLS-Stack“** → Nein. In den geprüften Pfaden erfolgt TLS über libcurl für Remote Rules. +2. **„MD5/SHA-1 überall sofort kritisch“** → Nein. Mehrere Verwendungen sind Legacy-/Format-/ID-bezogen, nicht direkt Signatur-/Auth-Kontrolle. +3. **„Mbed TLS 4.x komplett unvorbereitet“** → Nein. Buildsystem nutzt bereits TF-PSA-Crypto-Pfade. + +--- + +## D) Technischer Debt vs. echte Risiken + +### Echte Risiken +- SecRemoteRules Fail-Open bei nicht gesetzter FailAction. +- Vorhersagbare RNG in Boundary/ID-nahen Werten (missbrauchbar je nach Integrationskontext). + +### Technischer Debt (mit Security-Relevanz) +- MD5/SHA-1 weiterhin aktiv ohne moderne Default-Policy. +- assert-basierte Digest-Fehlerbehandlung. +- curl-Optionen nicht maximal explizit gehärtet. + +--- + +## E) Konkrete Verbesserungen (priorisiert) + +### Kurzfristig (must-fix) +1. **Fail-Closed für SecRemoteRules** + - `PropertyNotSetRemoteRulesAction` wie `Abort` behandeln. +2. **`CURLOPT_SSL_VERIFYHOST` auf `2L` setzen**. +3. **Digest-asserts ersetzen** durch kontrollierte Fehlerpfade (Return-Fehler/Exception mit sauberer Behandlung). + +### Mittelfristig +1. SHA-256 Transformation einführen; MD5/SHA-1 als legacy kennzeichnen. +2. Audit-Index zusätzlich mit SHA-256 ausgeben (parallel zum alten Format zur Kompatibilität). +3. RNG-Ablösung (`rand/srand`) durch CSPRNG/OS RNG. + +### Optional +1. Optionales Certificate Pinning für SecRemoteRules. +2. Sicherheits-Baseline zwischen Autotools und Win32 CMake harmonisieren. + +--- + +## F) Patch-Level Vorschläge + +## 1. SecRemoteRules fail-closed +- Datei: `src/parser/seclang-scanner.ll` +- Änderung: + - bei `ret == false` zusätzlich auf `PropertyNotSetRemoteRulesAction` prüfen und aborten. + +## 2. Digest wrapper runtime-sicher +- Datei: `src/utils/sha1.h` +- Änderung: + - `assert` entfernen, stattdessen: + - `if (mdInfo == nullptr) { ... }` + - `if (ret != 0) { ... }` + - klaren Fehlervertrag definieren. + +## 3. RNG-Härtung +- Dateien: `src/modsecurity.cc`, `src/audit_log/writer/writer.cc` +- Änderung: + - `srand/rand` entfernen. + - Ersatz über CSPRNG-nahe Quelle (z. B. `std::random_device` + starke Engine / OS-RNG Wrapper). + +## 4. TLS-Härtung +- Datei: `src/utils/https_client.cc` +- Änderung: + - `CURLOPT_SSL_VERIFYHOST` => `2L`. + - optional: Zeitouts (`CONNECTTIMEOUT`, `TIMEOUT`) und restriktive Protokoll-/Redirect-Policy. + +--- + +## G) Offene Unsicherheiten / was zusätzlich zu testen wäre + +1. Ob `SecRemoteRules` in produktiven Connector-Deployments standardmäßig genutzt wird (Impact variiert stark). +2. Wie externe Parser/SIEM auf das alte `md5:`-Auditindex-Format angewiesen sind. +3. Ob es Deployments mit sehr alter libcurl gibt, bei denen TLSv1.2-Setzen Build-/Runtime-Sonderfälle auslöst. +4. Ob organisatorisch FIPS/PSA-only Vorgaben existieren (würde Priorisierung der Hash-Migration erhöhen). + From a3c5e85ae13e30671c8a84e98e1da553ef89fcfa Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Tue, 7 Apr 2026 20:26:17 +0200 Subject: [PATCH 2/2] Add bilingual deep JSON library fit analysis reports --- ANALYSE_DE.md | 235 +++++++++++++++++++++++++++++++++++++++++++++++++ ANALYSIS_EN.md | 235 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 470 insertions(+) create mode 100644 ANALYSE_DE.md create mode 100644 ANALYSIS_EN.md diff --git a/ANALYSE_DE.md b/ANALYSE_DE.md new file mode 100644 index 0000000000..06c3943cb6 --- /dev/null +++ b/ANALYSE_DE.md @@ -0,0 +1,235 @@ +# 1. Kurzfazit + +## Kurze Antwort +Auf Basis dieses Repositories **im aktuellen Zustand** ist die einzige tatsächlich integrierte und benchmark-relevante JSON-Bibliothek **YAJL**. Die anderen in deinem Briefing genannten Bibliotheken (RapidJSON, nlohmann/json, json-c, Jansson, cJSON, JsonCpp, jsoncons, simdjson, yyjson, Glaze) sind **im Codebestand nicht vorhanden**; daher kann man sie ohne zusätzliche Integrationsarbeit nicht seriös als „besten Fit für dieses Repository“ auswählen. + +## Praktische Entscheidung +- **Beste Gesamtwahl für dieses Repository heute:** **YAJL** (weil produktiv integriert, nicht wegen theoretischer Rohgeschwindigkeit). +- **Beste Wahl für maximale Performance:** **Unklar in diesem Repo-Zustand** (kein in-repo, fairer Vergleich aller Bibliotheken). +- **Beste Wahl für Robustheit hier:** **YAJL**, da echte Request-Body-Verarbeitung, Fehlerpfade und Tiefenlimit bereits angebunden sind. +- **Beste Wahl für modernes C++-API:** allgemein eher **nlohmann/json / Glaze / simdjson**, aber **hier nicht durch Code belegt**. +- **Beste Wahl für minimale Abhängigkeiten in diesem Repo:** YAJL ist bereits Pflicht und integriert. +- **Beste Wahl für strikte JSON-Compliance:** allgemein eher simdjson/yyjson/RapidJSON im Strict-Mode; **hier nicht nachweisbar**. +- **Beste Wahl für tolerantes Parsing:** YAJL wirkt gemäß deiner externen Beobachtung tolerant (test05), aber **diese Datei fehlt im Repo** und ist hier nicht verifizierbar. + +--- + +# 2. Überblick über das Repository + +## Was hier tatsächlich gebenchmarkt wird +Der Benchmark unter `test/benchmark/benchmark.cc` misst **ModSecurity-Transaktionsdurchsatz**, nicht den direkten JSON-Parservergleich. Es wird eine vollständige Transaktion pro Iteration ausgeführt (Connection/URI/Header/Body/Response/Logging). JSON-Parsing ist dabei nur ein möglicher Teilkostenfaktor. + +### Fakten aus dem Code +- Die Benchmark-Schleife erzeugt `Transaction`, durchläuft Phasen und ruft danach `processLogging()` auf. +- Eingaben sind größtenteils fixe HTTP-Metadaten plus ein fixer XML-Response-Body. +- Standard-Regeldatei ist `test/benchmark/basic_rules.conf` mit nur `modsecurity.conf-recommended`. + +### Wichtige Implikation +Dieser Benchmark passt zu **End-to-End-Laufzeit von ModSecurity**, aber nicht zu direkten Aussagen „JSON-Library X ist schneller als Y“. + +--- + +# 3. Nutzung der JSON-Bibliotheken + +## JSON im produktiven Pfad (Fakten) +1. **Request-Body-JSON-Parsing** läuft über YAJL-Callbacks (`yajl_alloc`, `yajl_parse`, Callbacks für map/array/string/number etc.). +2. **JSON-Serialisierung/Erzeugung** nutzt YAJL-Generator-API in Core-Pfaden. +3. **JSON-Dateien in Unit/Regression-Tests** werden ebenfalls via YAJL-Tree-API verarbeitet. +4. Configure/Build bindet YAJL explizit ein (`configure.ac`, `build/yajl.m4`, `test/benchmark/Makefile.am`). + +## Verwendetes Parsing-Modell +- **Streaming/Event-basiert (SAX-ähnlich)** im Request-Body-Prozessor. +- Kein DOM-zentrischer Hot Path. +- Numerische Werte werden bewusst als Strings weitergereicht (laut Kommentar im Code). +- Es gibt ein explizites Tiefenlimit für verschachtelte JSON-Strukturen. + +## Bibliotheken aus deiner Liste +Alle Nicht-YAJL-Bibliotheken aus deiner Liste sind in dieser Revision **nicht eingebunden**. + +--- + +# 4. Validität des Benchmarks + +## Stärken +- Lange, wiederholte Transaktionsschleife mit konfigurierbarer Iterationszahl. +- Realistische Phasenreihenfolge inkl. Intervention-Checks. +- Mit OWASP CRS per Skript erweiterbar. + +## Schwächen / Fairness-Probleme +1. **Kein JSON-Library-Benchmark**: Es wird die komplette WAF-Pipeline gemessen. +2. **Default-Payload ist kein JSON-Stresstest** (fixe Requestdaten + XML-Response). +3. **Keine bibliotheksübergreifende Implementierungsparität** (nur YAJL vorhanden). +4. **Keine Methodik-Kontrollen gefunden** für CPU-Pinning, Warmup, Governor, NUMA, Varianzmetriken. +5. **Doku-Drift möglich**: README sagt, die letzte Logging-Phase werde nicht aufgerufen; der aktuelle Code ruft `processLogging()` auf. + +## Konsequenz +Aussagen wie „YYJSON gewinnt“ oder „SIMDJSON gewinnt nicht immer“ sind aus diesem Repository **allein nicht reproduzierbar**. + +--- + +# 5. Analyse pro Bibliothek + +Ich trenne je Bibliothek in: +- **Code-basierter Fakt (Repo)** +- **Inference (allgemeines Ökosystemwissen)** +- **Unsicherheit** + +## YAJL +- Fakt: Voll integriert für Parsing, Generierung, Tests, Configure/Build und Benchmark-Linking. +- Fakt: Request-Body-Parser nutzt Callback-Stil mit Tiefenlimit/Fehlerbehandlung. +- Inference: Sehr guter Fit zur bestehenden Architektur, weil die Logik auf Streaming-Callbacks aufgebaut ist. +- Schwäche: C-API, manuelles State-Management, weniger modernes C++-Erlebnis. +- Empfehlung für dieses Repo: **Ja**. + +## RapidJSON +- Fakt: Nicht integriert. +- Inference: Könnte SAX/DOM abdecken und schnell sein; Migrationsaufwand mittel/hoch wegen notwendigem Re-Design der Callback/Argument-Extraktion. +- Unsicherheit: Kein Benchmark-Nachweis im Repo. +- Empfehlung: **Eher nein (aktuell)**. + +## nlohmann/json +- Fakt: Nicht integriert. +- Inference: Sehr hohe Entwicklerproduktivität/Lesbarkeit, aber meist langsamer/allokationsintensiver für Streaming-WAF-Hot-Path. +- Unsicherheit: Keine in-repo Messung. +- Empfehlung: **Eher nein** für Hot Path, **eher ja** für Tooling/Control-Plane-Code. + +## json-c +- Fakt: Nicht integriert. +- Inference: Solide C-Bibliothek, oft DOM-lastig; kann je Integration mehr Speicher erzeugen als Streaming-Ansatz. +- Empfehlung: **Eher nein**. + +## Jansson +- Fakt: Nicht integriert. +- Inference: Saubere C-API, robust, aber nicht ohne Adapter-Aufwand passend zum aktuellen Streaming-Pfad. +- Empfehlung: **Eher nein**. + +## cJSON +- Fakt: Nicht integriert. +- Inference: Klein/einfach, aber für komplexes Security-Parsing oft zu limitiert bei Features/Fehlerbehandlung. +- Empfehlung: **Nein**. + +## JsonCpp +- Fakt: Nicht integriert. +- Inference: Älterer C++-DOM-Stil; für High-Performance-Streaming tendenziell weniger attraktiv. +- Empfehlung: **Eher nein**. + +## jsoncons +- Fakt: Nicht integriert. +- Inference: Moderne C++-Features, aber zusätzliche Komplexität/Abhängigkeiten plus Integrationsaufwand. +- Empfehlung: **Eher nein**. + +## simdjson +- Fakt: Nicht integriert. +- Inference: Sehr hoher Durchsatz bei großen validen JSON-Daten; reale Vorteile brauchen idiomatische Nutzung (Padding, ondemand, wenig Konvertierungsoverhead). +- Risiko: DOM-Umwege oder viele String-Kopien können Vorteile stark reduzieren. +- Empfehlung: **Eher ja** nur bei gezielter Integrationsneugestaltung und sauberer Benchmark-Methodik. + +## yyjson +- Fakt: Nicht integriert. +- Inference: Häufig Top-Performance bei Parse/Write; C-API passt grundsätzlich zu low-level Performance-Zielen. +- Risiko: Aktuelle Repo-Semantik (Callback-basierte Extraktion) kann naive Portierung ausbremsen. +- Empfehlung: **Eher ja** für Performance-Experimente, **kein sofortiger Drop-in**. + +## Glaze +- Fakt: Nicht integriert. +- Inference: Stark für C++-Reflexion/Serialization; weniger natürlicher Fit bei dynamischem, potenziell untrusted JSON in Security-Pfaden. +- Empfehlung: **Eher nein** für diesen spezifischen Hot Path. + +--- + +# 6. Analyse von test05.json + +## Was sich aus diesem Repository verifizieren lässt +- `test05.json` ist im Tree **nicht vorhanden**. +- Es gibt hier **kein** Multi-Library-JSON-Benchmark-Harness. + +## Daher +Die Aussage „nur YAJL besteht test05.json“ ist aus dem Repository-Code allein nicht verifizierbar. + +## Wahrscheinliche Ursachen (Inference) +Wenn YAJL besteht und strengere Parser scheitern, sind typische Ursachen: +- nicht-standardkonforme Zahlenformate, +- trailing commas / Kommentare (JSON5-ähnlich), +- UTF-8-/Control-Character-Kantenfälle, +- Unterschiede bei Duplicate Keys, Tiefenlimits oder unvollständigen Tokens, +- unterschiedliche Toleranz-/Quirk-Modi. + +## Striktes vs. tolerantes Parsing für dieses Projekt +Für eine Security Engine ist als Default meist sinnvoll: +- **striktes Parsing für Vorhersagbarkeit/Korrektheit**, plus +- optional toleranter Modus nur, wenn explizit für Kompatibilität benötigt. + +--- + +# 7. Bewertung nach Kriterien + +## Entscheidungs-Matrix (Kontext: aktueller Repo-Zustand) + +| Library | Language | Strengths | Weaknesses | Repo Fit | Recommendation | +|---|---|---|---|---|---| +| YAJL | C | Bereits end-to-end integriert; Streaming-Callbacks; Build/Test vorhanden | Weniger ergonomische C-API; manuelles State-Handling | Exzellent (aktuelle Architektur) | Yes | +| RapidJSON | C++ | Schnell, SAX+DOM, reif | Nicht integriert; Migrationsaufwand | Aktuell gering | Probably no | +| nlohmann/json | C++ | Sehr produktiv/lesbar | Meist langsamer; nicht integriert | Für Hot Path gering | Probably no | +| json-c | C | Solide C-Bibliothek | Oft DOM-lastig; nicht integriert | Gering | Probably no | +| Jansson | C | Saubere API, robust | Nicht integriert; Adapteraufwand | Gering | Probably no | +| cJSON | C | Minimal/einfach | Begrenzte Robustheit für komplexe Security-Fälle | Sehr gering | No | +| JsonCpp | C++ | Bekanntes Legacy-API | Älterer Stil/Perf-Trade-offs; nicht integriert | Gering | Probably no | +| jsoncons | C++ | Reiches modernes Feature-Set | Nicht integriert; mehr Komplexität | Gering | Probably no | +| simdjson | C++ | Sehr hoher Durchsatz bei großen Dateien | Integrations-Neudesign nötig; Fehlbenutzungsrisiko | Mittel (bei Invest) | Probably yes | +| yyjson | C | Sehr hohe Parse/Write-Performance in vielen Workloads | Nicht integriert; Semantik-Mismatch möglich | Mittel (bei Invest) | Probably yes | +| Glaze | C++ | Modern, stark bei compile-time Serialization | Weniger natürlicher Fit für dynamisches/untrusted Parsing | Gering | Probably no | + +--- + +# 8. Konkrete Empfehlung + +## Für dieses Repository jetzt +1. **YAJL als Produktionsparser beibehalten**, solange keine architektonische Integrationsinitiative gestartet wird. +2. Bei Performance-Fokus: **separates Parser-Mikrobenchmark-Harness** + **End-to-End-ModSecurity-Benchmark** mit identischer Semantik. +3. Erste Challengers für Prototyping: **yyjson** (C, low-level speed) und **simdjson** (stark bei großen validen JSON-Daten bei korrekter Nutzung). + +## Kategorien (mit aktueller Evidenzqualität) +- Beste Gesamtwahl für DIESES Repo: **YAJL**. +- Höchstes Performance-Potenzial: **yyjson / simdjson** (Inference, hier nicht validiert). +- Höchste Robustheit in DIESEM Repo: **YAJL** (reife Integration). +- Bestes modernes C++-API: **nlohmann/json** (Ergonomie), **simdjson** (performance-orientiertes modernes API). +- Minimale Abhängigkeiten in DIESEM Repo: **YAJL**. +- Strikte Compliance: **Unklar aus Repo; benötigt Conformance-Testsuite**. +- Tolerantes Parsing: **Möglicherweise YAJL laut externer Beobachtung; hier nicht verifizierbar**. + +--- + +# 9. Verbesserungen / To-dos + +## Benchmark-Design +1. Dediziertes JSON-Parser-Benchmark-Binary unabhängig von Transaktionsphasen ergänzen. +2. Semantische Gleichheit je Bibliothek erzwingen: + - identische Input-Bytes, + - identische Output-Extraktion (Pfade/Werte), + - identische Fehlerpolitik. +3. Umgebungsparameter dokumentieren: + - CPU-Modell, Governor, Core-Pinning, SMT, + - Compiler + Flags, + - Anzahl Läufe, Median/p95/Stdabw. +4. Zusätzliche Metriken: + - Parse-Latenz/Throughput, + - Allokationen (Anzahl/Bytes), + - Peak RSS, + - Fehlerdiagnostik. + +## Korrektheits-Korpus +- RFC-8259-valid, invalid, UTF-8-Edge-Cases, tiefe Verschachtelung, Duplicate Keys, Zahlen-Extrema sowie `test05.json` mit klarer Expected-Policy. + +## Integrationshygiene +- In Reports strikt zwischen Fakten und Inference trennen. +- README/Code-Mismatch zur Logging-Phase korrigieren. +- CI-Target für reproduzierbare JSON-Benchmarks ergänzen. + +--- + +# 10. Unsicherheiten + +1. Die in deinem Kontext genannten Bibliotheken sind in dieser Repo-Revision nicht enthalten; deren Bewertung bleibt inferenziell. +2. `test05.json` und Python-Validierungsbefunde sind extern zu diesem Tree. +3. Compiler-Optimierungsflags und Laufzeit-Pinning früherer Benchmarks sind hier nicht dokumentiert. +4. Jede Gewinner-Aussage jenseits von YAJL-Fit braucht erst gleichwertige Adapter-Implementierung + Validierung. diff --git a/ANALYSIS_EN.md b/ANALYSIS_EN.md new file mode 100644 index 0000000000..878738d2ce --- /dev/null +++ b/ANALYSIS_EN.md @@ -0,0 +1,235 @@ +# 1. Executive summary + +## Short answer +Based on this repository **as it exists**, the only JSON library that is actually integrated and benchmark-relevant is **YAJL**. The other libraries listed in your brief (RapidJSON, nlohmann/json, json-c, Jansson, cJSON, JsonCpp, jsoncons, simdjson, yyjson, Glaze) are **not present in the codebase** and therefore cannot be selected as the “best fit for this repository” without adding significant new integration code. + +## Practical decision +- **Best overall choice for this repository today:** **YAJL** (because it is the implemented production path, not because it is fastest in synthetic parser microbenchmarks). +- **Best for maximum performance:** **Unknown in this repository state** (no in-repo apples-to-apples benchmark exists across those libraries). +- **Best for robustness here:** **YAJL**, because real request-body handling, error propagation, and depth-limit behavior are already wired. +- **Best for modern C++ ergonomics:** likely **nlohmann/json / Glaze / simdjson** in general, but **not evidenced in this repo**. +- **Best for minimal dependencies in this repo:** YAJL already required and wired. +- **Best for strict JSON compliance:** likely simdjson/yyjson/RapidJSON in strict mode in general; **not test-proven here**. +- **Best for tolerant parsing:** YAJL appears to be tolerant in your external observation (test05), but **that file is absent in this repo** and cannot be verified from code here. + +--- + +# 2. Repository overview + +## What this repository actually benchmarks +The benchmark under `test/benchmark/benchmark.cc` measures **ModSecurity transaction processing throughput**, not JSON parser shootout throughput. It creates a full transaction, processes connection/URI/headers/body/response/logging phases, and repeats. JSON parsing is only one potential sub-cost depending on payload/rules. + +### Facts from code +- Benchmark loop creates a `Transaction`, runs request/response processing phases, then calls `processLogging()`. +- Input is mostly fixed HTTP metadata and a fixed XML response body. +- Default ruleset file used is `test/benchmark/basic_rules.conf`, which includes only `modsecurity.conf-recommended`. + +### Important implication +This benchmark is suitable for **end-to-end ModSecurity runtime** comparisons, not direct “library X vs Y JSON parse” claims. + +--- + +# 3. Library usage + +## JSON in production path (facts) +1. **Request-body JSON parsing**: implemented via YAJL callback API (`yajl_alloc`, `yajl_parse`, callbacks for map/array/string/number/etc.). +2. **JSON serialization/generation**: YAJL generator API is used in core files (e.g., transaction/reporting paths). +3. **JSON test file parsing** in unit/regression harnesses also uses YAJL tree APIs. +4. Build/configure path explicitly checks and wires YAJL (`configure.ac`, `build/yajl.m4`, `test/benchmark/Makefile.am`). + +## Parsing model used +- **Streaming/event (SAX-like) callbacks** for request-body processor. +- Not DOM-centric in hot path. +- Numeric values are handled as strings for argument extraction (intentional in code comments). +- There is explicit maximum nesting depth handling in the JSON body processor. + +## Libraries in your list +All non-YAJL libraries from your list are **absent** from includes/build files in this repo revision. + +--- + +# 4. Benchmark validity + +## Strengths +- Repeated long-run transaction loop with configurable iteration count. +- Includes realistic phase ordering and intervention checks. +- Can be rerun with OWASP CRS via provided scripts. + +## Weaknesses / fairness issues +1. **Not a JSON-library benchmark**: measures whole WAF transaction pipeline. +2. **Payload does not target JSON stress** by default (fixed request metadata + XML response body). +3. **No cross-library implementation parity** present (only YAJL code path exists). +4. **No methodology controls** found for CPU pinning, warmup policy, freq governor, NUMA, or variance reporting. +5. **Potential documentation drift**: README says benchmark does not call last logging phase, but current code does call `processLogging()`. + +## Consequence +Any conclusion like “YYJSON wins” or “SIMDJSON not always winning” is **not reproducible from this repository alone** without external code/scripts. + +--- + +# 5. Per-library analysis + +Below, I split each item into: +- **Code-grounded fact (repo)** +- **Inference (general ecosystem knowledge)** +- **Uncertainty** + +## YAJL +- Fact: Fully integrated in parsing, generation, tests, configure/build, benchmark linkage. +- Fact: Request-body parser uses callback style and has depth-limit/error handling. +- Inference: Strong fit for current architecture because existing logic depends on callback streaming semantics. +- Weakness: C-style API, manual state management, less modern C++ ergonomics. +- Recommendation for this repo: **Yes**. + +## RapidJSON +- Fact: Not integrated. +- Inference: Could fit SAX or DOM needs and be fast; migration effort moderate/high because request-body processor callbacks and argument-path logic would need reimplementation. +- Uncertainty: No benchmark evidence in this repo. +- Recommendation: **Probably no (for now)**. + +## nlohmann/json +- Fact: Not integrated. +- Inference: Best ergonomics/readability for modern C++, but likely slower and more allocation-heavy for streaming WAF body parsing. +- Uncertainty: No in-repo measurements. +- Recommendation: **Probably no** for hot-path parsing, **Probably yes** for tooling/control-plane code if added separately. + +## json-c +- Fact: Not integrated. +- Inference: C library, pragmatic, stable; mostly DOM-style usage, could increase memory pressure vs streaming callback model depending integration design. +- Recommendation: **Probably no**. + +## Jansson +- Fact: Not integrated. +- Inference: Clean C API, robust, but similarly not aligned to existing streaming callback path without adapter effort. +- Recommendation: **Probably no**. + +## cJSON +- Fact: Not integrated. +- Inference: Small/simple but feature/error-handling limitations for complex high-throughput security parsing. +- Recommendation: **No**. + +## JsonCpp +- Fact: Not integrated. +- Inference: Older C++ DOM-style approach; generally less compelling for high-performance streaming parse workloads. +- Recommendation: **Probably no**. + +## jsoncons +- Fact: Not integrated. +- Inference: Modern C++ and feature-rich; viable but introduces new dependency surface and migration work. +- Recommendation: **Probably no** for current repo priorities. + +## simdjson +- Fact: Not integrated. +- Inference: Very strong throughput on large valid JSON; best results require idiomatic usage (padded buffers, ondemand/document iteration patterns, avoiding conversion overhead). +- Risk: If wrapped into DOM conversions or copied strings aggressively, real gains can disappear. +- Recommendation: **Probably yes** only if you commit to a dedicated integration redesign and benchmark methodology. + +## yyjson +- Fact: Not integrated. +- Inference: Often top-tier parse/write speed; C API can align with low-level performance goals. +- Risk: Current repo logic is callback-path extraction into transaction args; naive port may negate raw parser gains. +- Recommendation: **Probably yes** for performance experiments, **not immediate drop-in**. + +## Glaze +- Fact: Not integrated. +- Inference: Excellent for C++ reflection/serialization workflows; less natural fit when input schema is unknown/untrusted and handling is event-driven security parsing. +- Recommendation: **Probably no** for this specific hot path. + +--- + +# 6. Analysis of test05.json + +## What can be verified from this repository +- `test05.json` is **not present** in this repo tree. +- No multi-library JSON benchmark harness exists here. + +## Therefore +Your statement “YAJL is the only library that succeeds on test05.json” cannot be validated from repository code alone. + +## Likely explanations (inference) +If YAJL passes and stricter libraries fail, likely causes include: +- non-standard numeric format, +- trailing commas/comments (JSON5-like features), +- control-character/UTF-8 edge case, +- duplicate keys / depth / unterminated token behavior differences, +- tolerance/quirk mode differences. + +## Strict vs tolerant parsing for this project +Given this is a security engine, default should usually be: +- **strict parsing for correctness and predictability**, plus +- optional tolerant mode only if explicitly required for compatibility with upstream traffic ecosystems. + +--- + +# 7. Criteria-based evaluation + +## Decision matrix (context: current repository state) + +| Library | Language | Strengths | Weaknesses | Repo Fit | Recommendation | +|---|---|---|---|---|---| +| YAJL | C | Already integrated end-to-end; streaming callbacks; build/test wiring exists | Less ergonomic C API; manual state handling | Excellent (current architecture) | Yes | +| RapidJSON | C++ | Fast, SAX+DOM options, mature | Not integrated; migration effort | Low currently | Probably no | +| nlohmann/json | C++ | Excellent developer productivity/readability | Usually slower; not integrated | Low for hot path | Probably no | +| json-c | C | Stable C ecosystem library | Mostly DOM usage patterns; not integrated | Low | Probably no | +| Jansson | C | Clean C API, robust | Not integrated; adapter work | Low | Probably no | +| cJSON | C | Minimal/simple dependency | Limited robustness for advanced/security parsing | Very low | No | +| JsonCpp | C++ | Familiar legacy C++ JSON API | Older style/perf trade-offs; not integrated | Low | Probably no | +| jsoncons | C++ | Rich modern feature set | Not integrated; complexity/dependency increase | Low | Probably no | +| simdjson | C++ | Excellent large-file throughput when used idiomatically | Integration redesign needed; misuse risk | Medium (if investing) | Probably yes | +| yyjson | C | Very high speed parse/write in many workloads | Not integrated; semantics mismatch risk | Medium (if investing) | Probably yes | +| Glaze | C++ | Modern compile-time serialization strength | Less natural for untrusted dynamic JSON parsing | Low | Probably no | + +--- + +# 8. Final recommendation + +## For this repository now +1. **Keep YAJL as production parser** unless you are willing to perform architecture-level integration work. +2. If performance R&D is desired, run a **separate parser microbenchmark harness** plus **end-to-end ModSecurity benchmark** with identical semantics. +3. Candidate challengers worth prototyping first: **yyjson** (C, low-level speed) and **simdjson** (best-in-class large valid JSON throughput when used correctly). + +## Category winners (with current evidence quality) +- Best overall for THIS repo: **YAJL**. +- Best max performance potential: **yyjson / simdjson** (inference; not validated here). +- Best robustness in THIS repo: **YAJL** (existing mature integration). +- Best modern C++ API: **nlohmann/json** (ergonomics), **simdjson** (performance-oriented modern API). +- Best minimal dependencies in THIS repo: **YAJL**. +- Best strict compliance: **Unknown from repo; must test with conformance corpus**. +- Best tolerant parsing: **Possibly YAJL in your external data; not verifiable here**. + +--- + +# 9. Improvements / to-dos + +## Benchmark design +1. Add dedicated JSON parser benchmark executable independent of transaction phases. +2. Use identical workload semantics per library: + - same input bytes, + - same output extraction (paths/values), + - same error policy. +3. Record environment controls: + - CPU model, governor, core pinning, SMT status, + - compiler + exact flags, + - run count, median/p95/stdev. +4. Separate metrics: + - parse latency, throughput, + - allocation counts/bytes, + - peak RSS, + - failure diagnostics. + +## Correctness corpus +- Include RFC 8259-valid suite, invalid suite, UTF-8 edge cases, deep nesting, duplicate-key cases, number extremes, and your `test05.json` with a precise expected result policy. + +## Integration hygiene +- Keep fact/inference labels in benchmark reports. +- Fix README/code mismatch regarding logging phase. +- Add CI target for JSON benchmark reproducibility. + +--- + +# 10. Uncertainties + +1. The libraries in your context list are not present in this repo revision; analysis for them is necessarily inferential. +2. `test05.json` and Python-validation evidence are external to this tree. +3. Compiler optimization flags and runtime pinning policy for your previous benchmark runs are not documented here. +4. Any claim about “winner” beyond YAJL fit would require implementing and validating comparable adapters first.