diff --git a/docs/superpowers/plans/2026-04-06-http-parse-error.md b/docs/superpowers/plans/2026-04-06-http-parse-error.md new file mode 100644 index 0000000..39cde41 --- /dev/null +++ b/docs/superpowers/plans/2026-04-06-http-parse-error.md @@ -0,0 +1,405 @@ +# HTTP Parse Error Counter Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add an HTTP parse error counter and TooManyHeaders security finding to the HTTP analyzer. + +**Architecture:** Change `parse_one_request`/`parse_one_response` return types to preserve `httparse::Error`, add aggregate counter to `HttpAnalyzer`, generate a `Finding` for `TooManyHeaders`, surface counter in `summarize()`. + +**Tech Stack:** Rust 2024, httparse 1.10.1, serde_json + +--- + +### Task 1: Change Return Types and Add Counter + Accessor + +**Files:** +- Modify: `src/analyzer/http.rs:22-55` (parse functions), `src/analyzer/http.rs:86-115` (struct + new) + +- [ ] **Step 1: Change `parse_one_request` return type** + +In `src/analyzer/http.rs`, change the function signature and error arm: + +```rust +fn parse_one_request(buf: &[u8]) -> Result, httparse::Error> { + let mut headers = [httparse::EMPTY_HEADER; MAX_HEADERS]; + let mut req = httparse::Request::new(&mut headers); + match req.parse(buf) { + Ok(httparse::Status::Complete(n)) => Ok(Some(ParsedRequest { + bytes_consumed: n, + method: req.method.unwrap_or("").to_string(), + uri: req.path.unwrap_or("").to_string(), + version: req.version.unwrap_or(1), + host: find_header(req.headers, "host"), + user_agent: find_header(req.headers, "user-agent"), + })), + Ok(httparse::Status::Partial) => Ok(None), + Err(e) => Err(e), + } +} +``` + +- [ ] **Step 2: Change `parse_one_response` return type** + +Same pattern: + +```rust +fn parse_one_response(buf: &[u8]) -> Result, httparse::Error> { + let mut headers = [httparse::EMPTY_HEADER; MAX_HEADERS]; + let mut resp = httparse::Response::new(&mut headers); + match resp.parse(buf) { + Ok(httparse::Status::Complete(n)) => Ok(Some(ParsedResponse { + bytes_consumed: n, + status_code: resp.code.unwrap_or(0), + })), + Ok(httparse::Status::Partial) => Ok(None), + Err(e) => Err(e), + } +} +``` + +- [ ] **Step 3: Add `parse_errors` field to `HttpAnalyzer` struct** + +Add after the `all_findings` field: + +```rust +pub struct HttpAnalyzer { + flows: HashMap, + methods: HashMap, + status_codes: HashMap, + hosts: HashMap, + user_agents: HashMap, + uris: Vec, + transactions: u64, + all_findings: Vec, + parse_errors: u64, +} +``` + +- [ ] **Step 4: Initialize `parse_errors` in `new()`** + +Add `parse_errors: 0` to the constructor: + +```rust + pub fn new() -> Self { + HttpAnalyzer { + flows: HashMap::new(), + methods: HashMap::new(), + status_codes: HashMap::new(), + hosts: HashMap::new(), + user_agents: HashMap::new(), + uris: Vec::new(), + transactions: 0, + all_findings: Vec::new(), + parse_errors: 0, + } + } +``` + +- [ ] **Step 5: Add `parse_error_count()` accessor** + +Add after `status_code_counts()` (around line 139): + +```rust + pub fn parse_error_count(&self) -> u64 { + self.parse_errors + } +``` + +- [ ] **Step 6: Verify compilation** + +Run: `cargo check 2>&1` + +Expected: Compiler errors in `try_parse_requests` and `try_parse_responses` because the match arms still expect `Err(())` but now receive `Err(httparse::Error)`. This is expected — Task 2 will fix these. + +- [ ] **Step 7: Commit** + +```bash +git add src/analyzer/http.rs +git commit -m "refactor: change parse helpers to return httparse::Error, add parse_errors counter" +``` + +### Task 2: Update Callers with Error Handling and TooManyHeaders Finding + +**Files:** +- Modify: `src/analyzer/http.rs:264-339` (try_parse_requests, try_parse_responses) + +- [ ] **Step 1: Write the failing test for parse error counter** + +Add to `tests/http_analyzer_tests.rs`: + +```rust +#[test] +fn test_parse_error_increments_counter() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // "NOT_HTTP\r\n\r\n" triggers httparse::Error::Token + analyzer.on_data(&fk, Direction::ClientToServer, b"NOT_HTTP\r\n\r\n", 0); + + assert_eq!(analyzer.parse_error_count(), 1); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test --test http_analyzer_tests test_parse_error_increments_counter 2>&1` + +Expected: FAIL — `parse_error_count()` returns 0 because `try_parse_requests` doesn't increment it yet. + +- [ ] **Step 3: Update `try_parse_requests` error arm** + +Replace the `Some(Err(()))` arm (lines 301-306) with: + +```rust + Some(Err(e)) => { + self.parse_errors += 1; + if e == httparse::Error::TooManyHeaders { + self.all_findings.push(Finding { + category: ThreatCategory::Anomaly, + verdict: Verdict::Inconclusive, + confidence: Confidence::Medium, + summary: "Excessive HTTP headers exceeded parser limit (possible DoS or header-based attack)".to_string(), + evidence: vec!["Direction: request".to_string()], + mitre_technique: Some("T1499.002".to_string()), + source_ip: None, + timestamp: None, + }); + } + if let Some(state) = self.flows.get_mut(flow_key) { + state.request_buf.clear(); + } + return; + } +``` + +- [ ] **Step 4: Update `try_parse_responses` error arm** + +Replace the `Some(Err(()))` arm (lines 330-335) with: + +```rust + Some(Err(e)) => { + self.parse_errors += 1; + if e == httparse::Error::TooManyHeaders { + self.all_findings.push(Finding { + category: ThreatCategory::Anomaly, + verdict: Verdict::Inconclusive, + confidence: Confidence::Medium, + summary: "Excessive HTTP headers exceeded parser limit (possible DoS or header-based attack)".to_string(), + evidence: vec!["Direction: response".to_string()], + mitre_technique: Some("T1499.002".to_string()), + source_ip: None, + timestamp: None, + }); + } + if let Some(state) = self.flows.get_mut(flow_key) { + state.response_buf.clear(); + } + return; + } +``` + +- [ ] **Step 5: Run the parse error test** + +Run: `cargo test --test http_analyzer_tests test_parse_error_increments_counter 2>&1` + +Expected: PASS + +- [ ] **Step 6: Run all existing tests to verify no regressions** + +Run: `cargo test --test http_analyzer_tests 2>&1` + +Expected: All 15 tests pass (14 existing + 1 new). + +- [ ] **Step 7: Commit** + +```bash +git add src/analyzer/http.rs tests/http_analyzer_tests.rs +git commit -m "feat: increment parse_errors counter and generate TooManyHeaders finding" +``` + +### Task 3: Add `parse_errors` to `summarize()` Output + +**Files:** +- Modify: `src/analyzer/http.rs:384-420` (summarize method) + +- [ ] **Step 1: Write the failing test for summarize** + +Add to `tests/http_analyzer_tests.rs`: + +```rust +#[test] +fn test_parse_error_in_summarize() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + analyzer.on_data(&fk, Direction::ClientToServer, b"NOT_HTTP\r\n\r\n", 0); + + let summary = analyzer.summarize(); + assert_eq!(summary.detail["parse_errors"], 1); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test --test http_analyzer_tests test_parse_error_in_summarize 2>&1` + +Expected: FAIL — `parse_errors` key not in detail map. + +- [ ] **Step 3: Add `parse_errors` to `summarize()`** + +In `summarize()`, add after the `user_agents` insert (around line 413): + +```rust + detail.insert( + "parse_errors".to_string(), + serde_json::json!(self.parse_errors), + ); +``` + +- [ ] **Step 4: Run the summarize test** + +Run: `cargo test --test http_analyzer_tests test_parse_error_in_summarize 2>&1` + +Expected: PASS + +- [ ] **Step 5: Run all tests** + +Run: `cargo test --test http_analyzer_tests 2>&1` + +Expected: All 16 tests pass. + +- [ ] **Step 6: Commit** + +```bash +git add src/analyzer/http.rs tests/http_analyzer_tests.rs +git commit -m "feat: surface parse_errors in summarize() output" +``` + +### Task 4: Add Remaining Tests (TooManyHeaders Finding, Response Error, Buffer Recovery, No-Error Baseline) + +**Files:** +- Modify: `tests/http_analyzer_tests.rs` + +- [ ] **Step 1: Write `test_too_many_headers_generates_finding`** + +This test programmatically builds a request with 97 headers to exceed `MAX_HEADERS=96`: + +```rust +#[test] +fn test_too_many_headers_generates_finding() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // Build a request with 97 headers to exceed MAX_HEADERS (96) + let mut request = b"GET / HTTP/1.1\r\n".to_vec(); + for i in 0..97 { + request.extend_from_slice(format!("X-Header-{i}: value\r\n").as_bytes()); + } + request.extend_from_slice(b"\r\n"); + + analyzer.on_data(&fk, Direction::ClientToServer, &request, 0); + + assert_eq!(analyzer.parse_error_count(), 1); + let findings = analyzer.findings(); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].category, ThreatCategory::Anomaly); + assert_eq!(findings[0].verdict, Verdict::Inconclusive); + assert_eq!(findings[0].confidence, Confidence::Medium); + assert_eq!( + findings[0].mitre_technique.as_deref(), + Some("T1499.002") + ); + assert!(findings[0].summary.contains("Excessive HTTP headers")); + assert!(findings[0].evidence[0].contains("request")); +} +``` + +- [ ] **Step 2: Write `test_parse_error_in_response`** + +```rust +#[test] +fn test_parse_error_in_response() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // Send valid request first + let request = b"GET / HTTP/1.1\r\nHost: example.com\r\n\r\n"; + analyzer.on_data(&fk, Direction::ClientToServer, request, 0); + + // Send malformed response + analyzer.on_data(&fk, Direction::ServerToClient, b"NOT_HTTP\r\n\r\n", 0); + + assert_eq!(analyzer.parse_error_count(), 1); +} +``` + +- [ ] **Step 3: Write `test_parse_error_clears_buffer_and_continues`** + +```rust +#[test] +fn test_parse_error_clears_buffer_and_continues() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // First: malformed request (triggers error, clears buffer) + analyzer.on_data(&fk, Direction::ClientToServer, b"GARBAGE\r\n\r\n", 0); + assert_eq!(analyzer.parse_error_count(), 1); + + // Second: valid request (should parse successfully on fresh buffer) + let valid = b"GET /index.html HTTP/1.1\r\nHost: example.com\r\n\r\n"; + analyzer.on_data(&fk, Direction::ClientToServer, valid, 0); + + assert_eq!(analyzer.parse_error_count(), 1); // no new errors + assert_eq!(*analyzer.method_counts().get("GET").unwrap(), 1); +} +``` + +- [ ] **Step 4: Write `test_normal_request_no_parse_errors`** + +```rust +#[test] +fn test_normal_request_no_parse_errors() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + let request = + b"GET /index.html HTTP/1.1\r\nHost: example.com\r\nUser-Agent: Mozilla/5.0\r\n\r\n"; + analyzer.on_data(&fk, Direction::ClientToServer, request, 0); + + assert_eq!(analyzer.parse_error_count(), 0); + assert!(analyzer.findings().is_empty()); +} +``` + +- [ ] **Step 5: Run all tests** + +Run: `cargo test --test http_analyzer_tests 2>&1` + +Expected: All 20 tests pass (14 existing + 6 new). + +- [ ] **Step 6: Run clippy and fmt** + +Run: `cargo clippy --tests 2>&1 && cargo fmt --check 2>&1` + +Expected: No warnings, no formatting issues. + +- [ ] **Step 7: Commit** + +```bash +git add tests/http_analyzer_tests.rs +git commit -m "test: add parse error counter, TooManyHeaders finding, and recovery tests" +``` + +## Files Modified + +| File | Change | +|------|--------| +| `src/analyzer/http.rs` | Return type change, counter field, error handling, summarize, accessor | +| `tests/http_analyzer_tests.rs` | 6 new tests | + +## Self-Review Checklist + +- [x] Spec coverage: all 5 spec items mapped to tasks (return type, counter, summarize, finding, accessor) +- [x] No placeholders: all code blocks contain complete implementation +- [x] Type consistency: `httparse::Error` used consistently, `parse_errors: u64` matches accessor return type +- [x] Test coverage: counter increment, response error, TooManyHeaders finding, buffer recovery, summarize output, no-error baseline diff --git a/docs/superpowers/specs/2026-04-06-http-parse-error-design.md b/docs/superpowers/specs/2026-04-06-http-parse-error-design.md new file mode 100644 index 0000000..9030589 --- /dev/null +++ b/docs/superpowers/specs/2026-04-06-http-parse-error-design.md @@ -0,0 +1,83 @@ +# HTTP Parse Error Counter and Finding Design + +**Issue:** #17 — fix: add HTTP parse error counter and surface in summary +**Scope:** Production changes to `src/analyzer/http.rs` + new tests in `tests/http_analyzer_tests.rs`. + +## Problem + +`httparse` errors in the HTTP analyzer are silently discarded. Both `parse_one_request` (line 35) and `parse_one_response` (line 53) map `Err(_) => Err(())`, losing all error variant information. The callers (`try_parse_requests` line 301, `try_parse_responses` line 330) clear the buffer and return with no counter or diagnostic. The `summarize()` output gives false confidence that no malformed HTTP exists. + +`httparse` 1.10.1 defines 7 error variants: `HeaderName`, `HeaderValue`, `NewLine`, `Status`, `Token`, `TooManyHeaders`, `Version`. The enum is **not** `#[non_exhaustive]`, implements `Display` and `Debug`, and is `Copy + Clone + PartialEq + Eq`. + +## Approach + +### 1. Preserve Error Variant in Return Type + +Change `parse_one_request` and `parse_one_response` return types from `Result<_, ()>` to `Result<_, httparse::Error>`. The `Err(_) => Err(())` arm becomes `Err(e) => Err(e)`, which can be simplified to removing the `Err` match arm entirely and using `?` or just `Err(e) => Err(e)`. + +### 2. Add Aggregate Parse Error Counter + +Add `parse_errors: u64` to `HttpAnalyzer`. Increment on any `httparse::Error` variant, for both requests and responses. Single aggregate counter (not split by direction) — matches Suricata's `http.error` pattern. + +### 3. Surface in `summarize()` + +Add `"parse_errors"` key to the `detail` HashMap in `summarize()`, with the counter value. + +### 4. Generate Finding for `TooManyHeaders` + +When `httparse::Error::TooManyHeaders` is encountered (request or response), generate a `Finding`: + +- **Category:** `ThreatCategory::Anomaly` +- **Verdict:** `Verdict::Inconclusive` — high false positive rate from legitimate cookie jars, proxy chains adding headers +- **Confidence:** `Confidence::Medium` +- **MITRE:** `T1499.002` (Service Exhaustion Flood) — TooManyHeaders maps to resource exhaustion via header flooding, not T1190 (which requires exploiting a software vulnerability) +- **Summary:** `"Excessive HTTP headers exceeded parser limit (possible DoS or header-based attack)"` +- **Evidence:** direction (request/response) indicated in evidence + +Other error variants (`HeaderName`, `HeaderValue`, `NewLine`, `Status`, `Token`, `Version`) increment the counter only — they indicate malformed traffic, not specific attacks at individual occurrence level. + +### 5. Add `parse_error_count()` Accessor + +Public method on `HttpAnalyzer` returning `u64`, for test assertions. + +## Changes + +### `src/analyzer/http.rs` + +| Location | Change | +|----------|--------| +| `parse_one_request` (line 22-37) | Return type `Result<_, httparse::Error>`, change `Err(_) => Err(())` to `Err(e) => Err(e)` | +| `parse_one_response` (line 44-55) | Same return type change | +| `HttpAnalyzer` struct (line 86-95) | Add `parse_errors: u64` field | +| `HttpAnalyzer::new()` (line 104-115) | Initialize `parse_errors: 0` | +| `try_parse_requests` (line 264-310) | Match on `Some(Err(e))`, increment counter and generate finding only if no prior successful parse this call (suppresses body-byte false positives) | +| `try_parse_responses` (line 312-339) | Same error handling change | +| `summarize()` (line 384-420) | Add `parse_errors` to detail map | +| New method | `pub fn parse_error_count(&self) -> u64` | + +### `tests/http_analyzer_tests.rs` + +| Test | Description | +|------|-------------| +| `test_parse_error_increments_counter` | Send malformed HTTP (e.g. `"NOT_HTTP\r\n\r\n"`), assert `parse_error_count() == 1` | +| `test_parse_error_in_response` | Send malformed response data, assert counter increments | +| `test_parse_error_clears_buffer_and_continues` | Send malformed request then valid request, assert counter == 1 and valid request still parsed | +| `test_too_many_headers_generates_finding` | Programmatically build a request with 97 headers (exceeds `MAX_HEADERS=96`), assert finding with `ThreatCategory::Anomaly`, `Confidence::Medium`, MITRE `T1499.002` | +| `test_parse_error_in_summarize` | Send malformed request, assert `summarize().detail["parse_errors"]` reflects count | +| `test_normal_request_no_parse_errors` | Send valid HTTP, assert `parse_error_count() == 0` | + +## Design Decision: Body-Byte Suppression + +The analyzer does not handle HTTP body framing (Content-Length / Transfer-Encoding). After a successful header parse, remaining body bytes stay in the buffer and get re-parsed as HTTP, triggering `httparse::Error::Token`. Without mitigation, this would inflate `parse_errors` on every response with a body. + +**Fix:** A `had_success` flag in `try_parse_requests` / `try_parse_responses` tracks whether headers were successfully parsed this call. Parse errors after a successful parse are suppressed (body-byte-induced), while errors on fresh data (no prior success) are counted. + +**Trade-off:** A pipelined valid request followed by a genuinely malformed request in the same buffer would not be counted. This is acceptable — pipelined malformed traffic is rare, and the counter remains accurate for the primary use case (malformed traffic arriving fresh). + +## Not In Scope + +- Splitting counter by request vs response (Suricata uses aggregate) +- Findings for non-TooManyHeaders error variants (benign malformed traffic at individual level) +- Changes to `Finding` struct +- Changes to reassembly engine or other analyzers +- Content-Length / Transfer-Encoding body handling (would eliminate the need for had_success heuristic) diff --git a/src/analyzer/http.rs b/src/analyzer/http.rs index 79cd45b..fdb8c69 100644 --- a/src/analyzer/http.rs +++ b/src/analyzer/http.rs @@ -19,7 +19,7 @@ struct ParsedRequest { user_agent: Option, } -fn parse_one_request(buf: &[u8]) -> Result, ()> { +fn parse_one_request(buf: &[u8]) -> Result, httparse::Error> { let mut headers = [httparse::EMPTY_HEADER; MAX_HEADERS]; let mut req = httparse::Request::new(&mut headers); match req.parse(buf) { @@ -32,7 +32,7 @@ fn parse_one_request(buf: &[u8]) -> Result, ()> { user_agent: find_header(req.headers, "user-agent"), })), Ok(httparse::Status::Partial) => Ok(None), - Err(_) => Err(()), + Err(e) => Err(e), } } @@ -41,7 +41,7 @@ struct ParsedResponse { status_code: u16, } -fn parse_one_response(buf: &[u8]) -> Result, ()> { +fn parse_one_response(buf: &[u8]) -> Result, httparse::Error> { let mut headers = [httparse::EMPTY_HEADER; MAX_HEADERS]; let mut resp = httparse::Response::new(&mut headers); match resp.parse(buf) { @@ -50,7 +50,7 @@ fn parse_one_response(buf: &[u8]) -> Result, ()> { status_code: resp.code.unwrap_or(0), })), Ok(httparse::Status::Partial) => Ok(None), - Err(_) => Err(()), + Err(e) => Err(e), } } @@ -92,6 +92,7 @@ pub struct HttpAnalyzer { uris: Vec, transactions: u64, all_findings: Vec, + parse_errors: u64, } impl Default for HttpAnalyzer { @@ -111,6 +112,7 @@ impl HttpAnalyzer { uris: Vec::new(), transactions: 0, all_findings: Vec::new(), + parse_errors: 0, } } @@ -138,6 +140,10 @@ impl HttpAnalyzer { &self.status_codes } + pub fn parse_error_count(&self) -> u64 { + self.parse_errors + } + fn check_request_detections(&mut self, parsed: &ParsedRequest, _flow_key: &FlowKey) { let uri_lower = parsed.uri.to_lowercase(); @@ -262,6 +268,11 @@ impl HttpAnalyzer { } fn try_parse_requests(&mut self, flow_key: &FlowKey) { + // Track whether we've successfully parsed headers this call. After a + // successful parse, remaining bytes are likely HTTP body data (we don't + // handle Content-Length/Transfer-Encoding). Suppress error counting for + // body-byte-induced failures to avoid inflating the counter on normal traffic. + let mut had_success = false; loop { let result = self .flows @@ -271,6 +282,7 @@ impl HttpAnalyzer { match result { Some(Ok(Some(parsed))) => { + had_success = true; if self.methods.len() < MAX_MAP_ENTRIES || self.methods.contains_key(&parsed.method) { @@ -298,7 +310,22 @@ impl HttpAnalyzer { } } Some(Ok(None)) => return, // Partial — wait for more data - Some(Err(())) => { + Some(Err(e)) => { + if !had_success { + self.parse_errors += 1; + if e == httparse::Error::TooManyHeaders { + self.all_findings.push(Finding { + category: ThreatCategory::Anomaly, + verdict: Verdict::Inconclusive, + confidence: Confidence::Medium, + summary: "Excessive HTTP headers exceeded parser limit (possible DoS or header-based attack)".to_string(), + evidence: vec!["Direction: request".to_string()], + mitre_technique: Some("T1499.002".to_string()), + source_ip: None, + timestamp: None, + }); + } + } if let Some(state) = self.flows.get_mut(flow_key) { state.request_buf.clear(); } @@ -310,6 +337,7 @@ impl HttpAnalyzer { } fn try_parse_responses(&mut self, flow_key: &FlowKey) { + let mut had_success = false; loop { let result = self .flows @@ -319,6 +347,7 @@ impl HttpAnalyzer { match result { Some(Ok(Some(parsed))) => { + had_success = true; *self.status_codes.entry(parsed.status_code).or_insert(0) += 1; self.transactions += 1; @@ -327,7 +356,22 @@ impl HttpAnalyzer { } } Some(Ok(None)) => return, - Some(Err(())) => { + Some(Err(e)) => { + if !had_success { + self.parse_errors += 1; + if e == httparse::Error::TooManyHeaders { + self.all_findings.push(Finding { + category: ThreatCategory::Anomaly, + verdict: Verdict::Inconclusive, + confidence: Confidence::Medium, + summary: "Excessive HTTP headers exceeded parser limit (possible DoS or header-based attack)".to_string(), + evidence: vec!["Direction: response".to_string()], + mitre_technique: Some("T1499.002".to_string()), + source_ip: None, + timestamp: None, + }); + } + } if let Some(state) = self.flows.get_mut(flow_key) { state.response_buf.clear(); } @@ -411,6 +455,10 @@ impl StreamAnalyzer for HttpAnalyzer { "user_agents".to_string(), serde_json::json!(self.user_agents), ); + detail.insert( + "parse_errors".to_string(), + serde_json::json!(self.parse_errors), + ); AnalysisSummary { analyzer_name: self.name().to_string(), diff --git a/tests/http_analyzer_tests.rs b/tests/http_analyzer_tests.rs index cc4b6fa..1b2a7d4 100644 --- a/tests/http_analyzer_tests.rs +++ b/tests/http_analyzer_tests.rs @@ -223,3 +223,153 @@ fn test_flow_close_cleans_up_state() { assert_eq!(*analyzer.method_counts().get("GET").unwrap(), 2); assert_eq!(*analyzer.host_counts().get("y.com").unwrap(), 1); } + +#[test] +fn test_parse_error_increments_counter() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // "NOT_HTTP\r\n\r\n" triggers httparse::Error::Token + analyzer.on_data(&fk, Direction::ClientToServer, b"NOT_HTTP\r\n\r\n", 0); + + assert_eq!(analyzer.parse_error_count(), 1); + // Token error should NOT generate a finding (only TooManyHeaders does) + assert!(analyzer.findings().is_empty()); +} + +#[test] +fn test_parse_error_in_summarize() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + analyzer.on_data(&fk, Direction::ClientToServer, b"NOT_HTTP\r\n\r\n", 0); + + let summary = analyzer.summarize(); + assert_eq!(summary.detail["parse_errors"], 1); +} + +#[test] +fn test_too_many_headers_generates_finding() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // Build a request with 97 headers to exceed MAX_HEADERS (96) + let mut request = b"GET / HTTP/1.1\r\n".to_vec(); + for i in 0..97 { + request.extend_from_slice(format!("X-Header-{i}: value\r\n").as_bytes()); + } + request.extend_from_slice(b"\r\n"); + + analyzer.on_data(&fk, Direction::ClientToServer, &request, 0); + + assert_eq!(analyzer.parse_error_count(), 1); + let findings = analyzer.findings(); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].category, ThreatCategory::Anomaly); + assert_eq!(findings[0].verdict, Verdict::Inconclusive); + assert_eq!(findings[0].confidence, Confidence::Medium); + assert_eq!(findings[0].mitre_technique.as_deref(), Some("T1499.002")); + assert!(findings[0].summary.contains("Excessive HTTP headers")); + assert!(findings[0].evidence[0].contains("request")); +} + +#[test] +fn test_parse_error_in_response() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // Send valid request first + let request = b"GET / HTTP/1.1\r\nHost: example.com\r\n\r\n"; + analyzer.on_data(&fk, Direction::ClientToServer, request, 0); + + // Send malformed response + analyzer.on_data(&fk, Direction::ServerToClient, b"NOT_HTTP\r\n\r\n", 0); + + assert_eq!(analyzer.parse_error_count(), 1); + // Token error on response should NOT generate a finding + assert!(analyzer.findings().is_empty()); +} + +#[test] +fn test_parse_error_clears_buffer_and_continues() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // First: malformed request (triggers error, clears buffer) + analyzer.on_data(&fk, Direction::ClientToServer, b"GARBAGE\r\n\r\n", 0); + assert_eq!(analyzer.parse_error_count(), 1); + + // Second: valid request (should parse successfully on fresh buffer) + let valid = b"GET /index.html HTTP/1.1\r\nHost: example.com\r\n\r\n"; + analyzer.on_data(&fk, Direction::ClientToServer, valid, 0); + + assert_eq!(analyzer.parse_error_count(), 1); // no new errors + assert_eq!(*analyzer.method_counts().get("GET").unwrap(), 1); +} + +#[test] +fn test_normal_request_no_parse_errors() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + let request = + b"GET /index.html HTTP/1.1\r\nHost: example.com\r\nUser-Agent: Mozilla/5.0\r\n\r\n"; + analyzer.on_data(&fk, Direction::ClientToServer, request, 0); + + assert_eq!(analyzer.parse_error_count(), 0); + assert!(analyzer.findings().is_empty()); +} + +#[test] +fn test_too_many_headers_in_response_generates_finding() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // Build a response with 97 headers to exceed MAX_HEADERS (96) + let mut response = b"HTTP/1.1 200 OK\r\n".to_vec(); + for i in 0..97 { + response.extend_from_slice(format!("X-Header-{i}: value\r\n").as_bytes()); + } + response.extend_from_slice(b"\r\n"); + + analyzer.on_data(&fk, Direction::ServerToClient, &response, 0); + + assert_eq!(analyzer.parse_error_count(), 1); + let findings = analyzer.findings(); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].category, ThreatCategory::Anomaly); + assert!(findings[0].evidence[0].contains("response")); +} + +#[test] +fn test_multiple_parse_errors_accumulate() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // First error: malformed request + analyzer.on_data(&fk, Direction::ClientToServer, b"GARBAGE\r\n\r\n", 0); + assert_eq!(analyzer.parse_error_count(), 1); + + // Second error: malformed response + analyzer.on_data(&fk, Direction::ServerToClient, b"ALSO_BAD\r\n\r\n", 0); + assert_eq!(analyzer.parse_error_count(), 2); +} + +#[test] +fn test_body_bytes_do_not_inflate_parse_errors() { + let mut analyzer = HttpAnalyzer::new(); + let fk = test_flow_key(); + + // Request with no body + let request = b"GET /index.html HTTP/1.1\r\nHost: example.com\r\n\r\n"; + analyzer.on_data(&fk, Direction::ClientToServer, request, 0); + + // Response WITH body "hello" (Content-Length: 5) + // Body bytes remain in buffer after header parse and would previously + // be re-parsed as HTTP, triggering a false parse error. + let response = b"HTTP/1.1 200 OK\r\nContent-Type: text/html\r\nContent-Length: 5\r\n\r\nhello"; + analyzer.on_data(&fk, Direction::ServerToClient, response, 0); + + assert_eq!(analyzer.parse_error_count(), 0); + assert!(analyzer.findings().is_empty()); +}