diff --git a/Cargo.lock b/Cargo.lock index a84ec70..212a679 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -24,7 +24,7 @@ dependencies = [ [[package]] name = "barbacane-waf" -version = "0.1.0" +version = "0.1.1" dependencies = [ "aho-corasick", "barbacane-libinjection", diff --git a/crates/parapet/Cargo.toml b/crates/parapet/Cargo.toml index d81fd3c..f3a9bdc 100644 --- a/crates/parapet/Cargo.toml +++ b/crates/parapet/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "barbacane-waf" -version = "0.1.0" +version = "0.1.1" description = "A SecLang rule engine in pure Rust, compatible with the OWASP Core Rule Set" keywords = ["waf", "seclang", "modsecurity", "security", "firewall"] categories = ["network-programming", "web-programming"] diff --git a/crates/parapet/src/transaction.rs b/crates/parapet/src/transaction.rs index b7e4569..69d2ce0 100644 --- a/crates/parapet/src/transaction.rs +++ b/crates/parapet/src/transaction.rs @@ -349,8 +349,10 @@ impl<'r> Transaction<'r> { } fn run_phase(&mut self, phase: Phase) -> Verdict { - // A disruptive action already fired; later phases do not run. - if self.verdict != Verdict::Allow { + // A disruptive verdict stops later phases, except phase 5 (logging), + // which runs on every transaction so a blocked transaction is still + // logged and its correlation rules run, matching ModSecurity. + if self.verdict != Verdict::Allow && phase != Phase::Logging { return self.verdict.clone(); } @@ -390,7 +392,9 @@ impl<'r> Transaction<'r> { | Some(Disruptive::Block) | Some(Disruptive::Drop) | Some(Disruptive::Redirect) => { - if self.mode == EngineMode::Blocking { + // Phase 5 cannot intercept: a disruptive action there never + // changes the verdict, so logging preserves an earlier block. + if self.mode == EngineMode::Blocking && phase != Phase::Logging { self.verdict = Verdict::Deny { status: rule.status.unwrap_or(403), rule_id: rule.id.unwrap_or(0), @@ -1432,6 +1436,72 @@ SecRule ARGS "@rx attack" "id:1,phase:1,block" assert_eq!(tx.vars.tx_get("logged"), Some(&b"1"[..])); } + #[test] + fn phase_five_logging_runs_after_a_block() { + // A phase-2 block must not stop phase 5: the logging phase runs on a + // blocked transaction too, and its verdict is unchanged. + let rs = rules( + r#" +SecRule ARGS "@rx attack" "id:1,phase:2,deny,status:403" +SecAction "id:2,phase:5,pass,setvar:'tx.logged=1'" +"#, + ); + let mut tx = get(&rs, "/?q=attack"); + tx.process_request_headers(); + assert!(matches!( + tx.process_request_body(), + Verdict::Deny { rule_id: 1, .. } + )); + assert!(matches!( + tx.process_logging(), + Verdict::Deny { rule_id: 1, .. } + )); + assert_eq!( + tx.vars.tx_get("logged"), + Some(&b"1"[..]), + "phase 5 must run and apply its setvar even after a block" + ); + assert!(tx.matched_ids().contains(&2), "the phase-5 rule must match"); + } + + #[test] + fn a_disruptive_action_in_phase_five_does_not_change_the_verdict() { + // Phase 5 cannot intercept. A (malformed) deny there is ignored and an + // allowed transaction stays allowed. + let rs = rules(r#"SecRule REMOTE_ADDR "@rx ." "id:1,phase:5,deny,status:403""#); + let mut tx = get(&rs, "/"); + tx.set_remote_addr("203.0.113.7"); + tx.process_request_headers(); + assert_eq!(tx.process_logging(), Verdict::Allow); + } + + #[test] + fn a_phase_three_block_still_skips_phase_four() { + // Only phase 5 is exempt from the stop-after-block rule. + let rs = rules( + r#" +SecRule RESPONSE_HEADERS:X-Leak "@streq yes" "id:1,phase:3,deny,status:403" +SecRule RESPONSE_BODY "@rx secret" "id:2,phase:4,pass,setvar:'tx.body_ran=1'" +"#, + ); + let mut tx = get(&rs, "/"); + tx.process_request_headers(); + tx.process_request_body(); + tx.set_response_status(200); + tx.add_response_header("X-Leak", "yes"); + assert!(matches!( + tx.process_response_headers(), + Verdict::Deny { rule_id: 1, .. } + )); + tx.set_response_body(b"this leaks a secret"); + tx.process_response_body(); + assert_eq!( + tx.vars.tx_get("body_ran"), + None, + "phase 4 must not run after a phase-3 block" + ); + } + #[test] fn ctl_rule_remove_by_id_disables_a_later_rule() { // A range form covers the id, so rule 2 is switched off before it runs.