DaLaw2/NetGuardia
1
0
Fork 0
mirror of https://github.com/DaLaw2/NetGuardia.git synced 2026-08-24 14:10:28 +09:00
Code Issues Packages Projects Releases Wiki Activity

fix: address Agent Team review — security, perf, correctness

Browse Source 
Security:
- S1: Add RBAC permission check for /api/logs/ and /api/audit/ endpoints
  (previously any authenticated user could access)
- S2/S3: Remove report_dir and log_dir from configurable settings to
  prevent arbitrary directory write via config API
- A2: Pin DNS-resolved IPs in webhook reqwest client to prevent DNS
  rebinding TOCTOU attack (resolve() instead of re-resolving)

Performance:
- P7: Add 50K key cap to FrequencyTracker to prevent unbounded growth
  under DDoS (was unbounded, worst case 1.6GB)
- P9: Increase ML alert broadcast capacity 100 → 1024 to prevent lost
  alerts during DDoS spikes (3 subscribers contend on 100-slot buffer)
- P2: Reduce FLOW_MAX_PERIODS 10000 → 1000 (saves 144KB/flow, feature
  extraction only uses aggregate stats)
- P1: Remove unnecessary FlowKey clone on hot path (~1.9MB/s saved)
- P5: Beaconing detector: split analyze_and_alert into read-lock scan
  + selective write-lock update (reduces DashMap contention)

Correctness:
- A4: Capture correlation counts inside DashMap guard before dropping,
  eliminating TOCTOU in logged values (botnet, scan, lateral)
- A6: Log warning when SOAR playbook action params JSON is malformed
  instead of silently replacing with empty object

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
DaLaw2 2026-04-03 15:33:11 +08:00
parent 4bdf051969
commit 4167fc1e2a
10 changed files with 91 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
net-guardia/src/core/auth/middleware.rs
View File

@ -66,6 +66,8 @@ fn required_permission(path: &str, method: &actix_web::http::Method) -> Option<S
|| path.starts_with("/api/notifications/")
|| path.starts_with("/api/report/")
|| path.starts_with("/api/api-keys/")
|| path.starts_with("/api/logs/")
|| path.starts_with("/api/audit/")
{
"system"
} else {

6
net-guardia/src/core/config_service.rs
View File

@ -49,7 +49,9 @@ const SETTINGS_MAP: &[(&str, &[&str])] = &[
"models",
&["deep_autoencoder_name", "classifier_name", "models_config_name"],
),
("misc", &["geoip_db_name", "report_dir", "log_dir"]),
// report_dir and log_dir intentionally NOT configurable via API to prevent
// arbitrary directory write/read. They use hardcoded safe defaults.
("misc", &["geoip_db_name"]),
("soar", &["soar_max_auto_block_cap", "soar_max_ttl_secs"]),
("ml", &["ml_drift_window_secs"]),
("telegram", &["telegram_max_messages_per_minute"]),
@ -116,8 +118,6 @@ impl ConfigService {
},
"misc": {
"geoip_db_name": get("geoip_db_name"),
"report_dir": get("report_dir"),
"log_dir": get("log_dir"),
},
"soar": {
"soar_max_auto_block_cap": get("soar_max_auto_block_cap"),

10
net-guardia/src/core/correlation/botnet.rs
View File

@ -66,12 +66,14 @@ impl BotnetDetector {
set.sources.insert(alert.src_ip.clone());
set.last_alert = alert.clone();
set.sources.len() >= self.threshold
if set.sources.len() >= self.threshold {
Some(set.sources.len())
} else {
None
}
};
if should_alert {
let unique_sources = self.state.get(&key).map(|e| e.sources.len()).unwrap_or(0);
if let Some(unique_sources) = should_alert {
log!(DetectionLog::BotnetDetected {
dst_ip: key.clone(),
unique_sources,

11
net-guardia/src/core/correlation/lateral.rs
View File

@ -65,12 +65,15 @@ impl LateralMovementDetector {
}
set.dests.insert(alert.dst_ip.clone());
set.dests.len() >= self.threshold
if set.dests.len() >= self.threshold {
Some(set.dests.len())
} else {
None
}
};
if should_alert {
let unique_dests = self.state.get(&key).map(|e| e.dests.len()).unwrap_or(0);
if let Some(unique_dests) = should_alert {
log!(DetectionLog::LateralMovementDetected {
src_ip: key.clone(),
unique_dests,

14
net-guardia/src/core/correlation/scan.rs
View File

@ -64,16 +64,14 @@ impl ScanDetector {
set.ports.insert(alert.dst_port);
set.last_dst_ip = alert.dst_ip.clone();
set.ports.len() >= self.threshold
if set.ports.len() >= self.threshold {
Some((set.ports.len(), set.last_dst_ip.clone()))
} else {
None
}
};
if should_alert {
let (unique_ports, last_dst_ip) = self
.state
.get(&key)
.map(|e| (e.ports.len(), e.last_dst_ip.clone()))
.unwrap_or((0, String::new()));
if let Some((unique_ports, last_dst_ip)) = should_alert {
log!(DetectionLog::ScanDetected {
src_ip: key.clone(),
unique_ports,

60
net-guardia/src/core/detection/beaconing.rs
View File

@ -102,46 +102,50 @@ impl BeaconingDetector {
let now = Instant::now();
let cooldown = Duration::from_secs(ALERT_COOLDOWN_SECS);
for mut entry in self.flow_cache.iter_mut() {
let src_ip = entry.key().0.clone();
let dst_ip = entry.key().1.clone();
let dst_port = entry.key().2;
let flow = entry.value_mut();
// Phase 1: read-lock scan to find beaconing candidates (avoids holding write locks
// across the entire 50K-entry iteration, reducing contention with record_flow).
let mut alerts: Vec<(FlowTuple, f64, usize)> = Vec::new();
for entry in self.flow_cache.iter() {
let flow = entry.value();
if flow.timestamps.len() < MIN_OBSERVATIONS {
continue;
}
// Skip if recently alerted
if let Some(last) = flow.last_alerted
&& now.duration_since(last) < cooldown
{
continue;
}
let cv = compute_cv(&flow.timestamps);
if cv < CV_THRESHOLD {
log!(DetectionLog::BeaconingDetected {
src_ip: src_ip.clone(),
dst_ip: dst_ip.clone(),
dst_port,
cv,
count: flow.timestamps.len(),
});
alerts.push((entry.key().clone(), cv, flow.timestamps.len()));
}
}
let event = DetectionEvent {
source: DetectionSource::Beaconing,
attack_type: "c2_communication".to_string(),
confidence: (1.0 - cv / CV_THRESHOLD) as f32 * 0.5 + 0.5, // 0.5–1.0 range
source_ip: src_ip,
dest_ip: dst_ip,
protocol: 6, // Most C2 is TCP
packet_count: flow.timestamps.len() as u64,
flow_duration_us: 0,
};
// Phase 2: selective write-lock only for entries that need last_alerted update.
for (key, cv, count) in alerts {
let (src_ip, dst_ip, dst_port) = &key;
log!(DetectionLog::BeaconingDetected {
src_ip: src_ip.clone(),
dst_ip: dst_ip.clone(),
dst_port: *dst_port,
cv,
count,
});
let _ = self.detection_tx.try_send(event);
flow.last_alerted = Some(now);
let event = DetectionEvent {
source: DetectionSource::Beaconing,
attack_type: "c2_communication".to_string(),
confidence: (1.0 - cv / CV_THRESHOLD) as f32 * 0.5 + 0.5,
source_ip: src_ip.clone(),
dest_ip: dst_ip.clone(),
protocol: 6,
packet_count: count as u64,
flow_duration_us: 0,
};
let _ = self.detection_tx.try_send(event);
if let Some(mut entry) = self.flow_cache.get_mut(&key) {
entry.last_alerted = Some(now);
}
}
}

2
net-guardia/src/core/ml/flow_tracker.rs
View File

@ -223,7 +223,7 @@ impl FlowTracker {
pub fn process_packet(&mut self, mut packet: UserPacket, is_ingress: bool) {
let packet_key = FlowKey::from_packet(&packet);
let reversed_key = packet_key.clone().reverse();
let reversed_key = packet_key.reverse();
// Try to match an existing flow first (canonical key already established).
// Use peek() to avoid promoting — we'll promote via get_mut() below.

26
net-guardia/src/core/soar/engine.rs
View File

@ -127,7 +127,13 @@ impl SoarEngine {
pb.actions.push(PlaybookAction {
action_order: order,
action_type: atype,
params: serde_json::from_str(&params_str).unwrap_or(serde_json::Value::Object(Default::default())),
params: serde_json::from_str(&params_str).unwrap_or_else(|e| {
log!(SoarLog::PlaybookError {
name: pb.name.clone(),
error: format!("Malformed action params JSON: {}", e),
});
serde_json::Value::Object(Default::default())
}),
});
}
}
@ -839,13 +845,17 @@ impl SoarEngine {
"timestamp": chrono::Utc::now().format("%Y-%m-%d %H:%M:%S UTC").to_string(),
});
let client = reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(timeout_secs))
.build()
.map_err(|e| SoarError::ActionFailed {
action_type: "webhook".to_string(),
reason: format!("HTTP client error: {}", e),
})?;
// Pin resolved IPs to prevent DNS rebinding: the DNS check above verified
// all resolved addresses are public, so we force reqwest to use those same
// addresses instead of re-resolving (which could return a private IP on TTL expiry).
let mut client_builder = reqwest::Client::builder().timeout(std::time::Duration::from_secs(timeout_secs));
for addr in &addrs {
client_builder = client_builder.resolve(host, *addr);
}
let client = client_builder.build().map_err(|e| SoarError::ActionFailed {
action_type: "webhook".to_string(),
reason: format!("HTTP client error: {}", e),
})?;
let resp = client
.post(url_str)

14
net-guardia/src/core/soar/frequency.rs
View File

@ -6,6 +6,9 @@ use dashmap::DashMap;
/// Key for frequency tracking: (playbook_id, source_ip).
type FreqKey = (i64, String);
/// Maximum tracked keys to bound memory under DDoS.
const MAX_TRACKED_KEYS: usize = 50_000;
/// Lock-free frequency tracker using DashMap for concurrent per-IP event counting.
pub struct FrequencyTracker {
events: DashMap<FreqKey, VecDeque<Instant>>,
@ -68,6 +71,17 @@ impl FrequencyTracker {
}
true
});
// Enforce max key cap to prevent unbounded growth under DDoS
if self.events.len() > MAX_TRACKED_KEYS {
let excess = self.events.len() - MAX_TRACKED_KEYS;
let keys_to_remove: Vec<FreqKey> = self.events.iter().take(excess).map(|e| e.key().clone()).collect();
for key in keys_to_remove {
self.events.remove(&key);
removed += 1;
}
}
removed
}
}

4
net-guardia/src/model/config/constants.rs
View File

@ -5,9 +5,9 @@
pub const MAX_PENDING_UNBLOCK_RETRIES: i64 = 5;
// ── ML Engine ──────────────────────────────────────────────────────
pub const ML_ALERT_CHANNEL_CAPACITY: usize = 100;
pub const ML_ALERT_CHANNEL_CAPACITY: usize = 1024;
pub const FLOW_MAX_PACKETS_PER_DIRECTION: usize = 1000;
pub const FLOW_MAX_PERIODS: usize = 10000;
pub const FLOW_MAX_PERIODS: usize = 1000;
pub const FLOW_IDLE_THRESHOLD_US: u64 = 1_000_000;
pub const FLOW_BULK_MIN_PACKETS: u64 = 4;
pub const FLOW_BULK_MIN_BYTES: u64 = 1000;