Skip to content

Commit d4c8707

Browse files
committed
fix(dataplane): address five critical issues in Rust dataplane
- #1051: IPv4-mapped IPv6 addresses now checked against v4 denylist in SSRF filter; ULA check extended to cover full fc00::/7 range - #1052: Router is now rebuilt on incremental upsert_route/delete_route via a shared rebuild_router helper, not just on full apply_config - #1053: LB cache invalidation in upsert_cluster/delete_cluster changed from try_write() (silently skipped on contention) to async write().await - #1054: PoolGuard now stores Arc<Semaphore> and implements Drop to return the permit automatically, preventing leaks on panic - #1055: Backend selection replaced SystemTime nanos with per-Route AtomicU64 counter for deterministic weighted round-robin
1 parent 9c683ca commit d4c8707

7 files changed

Lines changed: 245 additions & 162 deletions

File tree

dataplane/novaedge-dataplane/src/config.rs

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -272,23 +272,18 @@ impl RuntimeConfig {
272272
///
273273
/// Also invalidates the cached LB instance for this cluster so that
274274
/// endpoint or algorithm changes take effect on the next request.
275-
pub fn upsert_cluster(&self, cluster: ClusterState) {
275+
pub async fn upsert_cluster(&self, cluster: ClusterState) {
276276
let name = cluster.name.clone();
277277
self.clusters.insert(name.clone(), cluster);
278-
// Invalidate the LB cache entry for this cluster (best-effort; uses try_write
279-
// to avoid blocking the caller if the lock is held by an async task).
280-
if let Ok(mut cache) = self.lb_cache.try_write() {
281-
cache.remove(&name);
282-
}
278+
// Invalidate the LB cache entry for this cluster.
279+
self.lb_cache.write().await.remove(&name);
283280
self.bump();
284281
}
285282

286283
/// Remove a cluster by name.
287-
pub fn delete_cluster(&self, name: &str) {
284+
pub async fn delete_cluster(&self, name: &str) {
288285
self.clusters.remove(name);
289-
if let Ok(mut cache) = self.lb_cache.try_write() {
290-
cache.remove(name);
291-
}
286+
self.lb_cache.write().await.remove(name);
292287
self.bump();
293288
}
294289

@@ -659,34 +654,34 @@ mod tests {
659654
assert_eq!(snap.routes["route-1"].backend_refs[0].0, "backend-1");
660655
}
661656

662-
#[test]
663-
fn test_upsert_and_get_cluster() {
657+
#[tokio::test]
658+
async fn test_upsert_and_get_cluster() {
664659
let cfg = RuntimeConfig::new();
665-
cfg.upsert_cluster(test_cluster("cluster-1"));
660+
cfg.upsert_cluster(test_cluster("cluster-1")).await;
666661
let snap = cfg.snapshot();
667662
assert_eq!(snap.clusters.len(), 1);
668663
assert_eq!(snap.clusters["cluster-1"].endpoints.len(), 1);
669664
}
670665

671-
#[test]
672-
fn test_get_cluster() {
666+
#[tokio::test]
667+
async fn test_get_cluster() {
673668
let cfg = RuntimeConfig::new();
674669
assert!(cfg.get_cluster("missing").is_none());
675-
cfg.upsert_cluster(test_cluster("cluster-1"));
670+
cfg.upsert_cluster(test_cluster("cluster-1")).await;
676671
let c = cfg.get_cluster("cluster-1").unwrap();
677672
assert_eq!(c.endpoints.len(), 1);
678673
}
679674

680-
#[test]
681-
fn test_delete_removes_entry() {
675+
#[tokio::test]
676+
async fn test_delete_removes_entry() {
682677
let cfg = RuntimeConfig::new();
683678
cfg.upsert_gateway(test_gateway("gw-1", 8080));
684679
cfg.upsert_route(test_route("route-1", "b"));
685-
cfg.upsert_cluster(test_cluster("cluster-1"));
680+
cfg.upsert_cluster(test_cluster("cluster-1")).await;
686681

687682
cfg.delete_gateway("gw-1");
688683
cfg.delete_route("route-1");
689-
cfg.delete_cluster("cluster-1");
684+
cfg.delete_cluster("cluster-1").await;
690685

691686
let snap = cfg.snapshot();
692687
assert!(snap.gateways.is_empty());

dataplane/novaedge-dataplane/src/listener.rs

Lines changed: 50 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -615,54 +615,56 @@ mod tests {
615615
hedge_delay_ms: 0,
616616
hedge_max_requests: 0,
617617
});
618-
config.upsert_cluster(ClusterState {
619-
name: "tcp-cluster".into(),
620-
endpoints: vec![EndpointState {
621-
address: backend_addr.ip().to_string(),
622-
port: backend_addr.port() as u32,
623-
weight: 1,
624-
healthy: true,
625-
zone: None,
626-
priority: 0,
627-
draining: false,
628-
drain_start: None,
629-
}],
630-
lb_algorithm: "round-robin".into(),
631-
health_check_path: String::new(),
632-
session_affinity_type: String::new(),
633-
session_affinity_cookie: String::new(),
634-
session_affinity_header: String::new(),
635-
outlier_consecutive_5xx: 0,
636-
outlier_ejection_duration_ms: 0,
637-
outlier_max_ejection_pct: 0,
638-
outlier_interval_ms: 0,
639-
outlier_sr_min_hosts: 0,
640-
outlier_sr_min_requests: 0,
641-
outlier_sr_stdev_factor: 0.0,
642-
slow_start_window_ms: 0,
643-
slow_start_aggression: 1.0,
644-
protocol: String::new(),
645-
upstream_proxy_protocol_enabled: false,
646-
upstream_proxy_protocol_version: 0,
647-
tls_enabled: false,
648-
tls_insecure_skip_verify: false,
649-
tls_ca_pem: Vec::new(),
650-
tls_server_name: String::new(),
651-
cb_failure_threshold: 0,
652-
cb_success_threshold: 0,
653-
cb_open_duration_ms: 0,
654-
cb_half_open_max_requests: 0,
655-
pool_max_connections: 0,
656-
pool_max_idle: 0,
657-
pool_idle_timeout_ms: 0,
658-
pool_connect_timeout_ms: 0,
659-
panic_threshold_percent: 0,
660-
max_requests_per_connection: 0,
661-
request_queue_depth: 0,
662-
request_queue_timeout_ms: 0,
663-
subset_size: 0,
664-
remote_endpoints: Vec::new(),
665-
});
618+
config
619+
.upsert_cluster(ClusterState {
620+
name: "tcp-cluster".into(),
621+
endpoints: vec![EndpointState {
622+
address: backend_addr.ip().to_string(),
623+
port: backend_addr.port() as u32,
624+
weight: 1,
625+
healthy: true,
626+
zone: None,
627+
priority: 0,
628+
draining: false,
629+
drain_start: None,
630+
}],
631+
lb_algorithm: "round-robin".into(),
632+
health_check_path: String::new(),
633+
session_affinity_type: String::new(),
634+
session_affinity_cookie: String::new(),
635+
session_affinity_header: String::new(),
636+
outlier_consecutive_5xx: 0,
637+
outlier_ejection_duration_ms: 0,
638+
outlier_max_ejection_pct: 0,
639+
outlier_interval_ms: 0,
640+
outlier_sr_min_hosts: 0,
641+
outlier_sr_min_requests: 0,
642+
outlier_sr_stdev_factor: 0.0,
643+
slow_start_window_ms: 0,
644+
slow_start_aggression: 1.0,
645+
protocol: String::new(),
646+
upstream_proxy_protocol_enabled: false,
647+
upstream_proxy_protocol_version: 0,
648+
tls_enabled: false,
649+
tls_insecure_skip_verify: false,
650+
tls_ca_pem: Vec::new(),
651+
tls_server_name: String::new(),
652+
cb_failure_threshold: 0,
653+
cb_success_threshold: 0,
654+
cb_open_duration_ms: 0,
655+
cb_half_open_max_requests: 0,
656+
pool_max_connections: 0,
657+
pool_max_idle: 0,
658+
pool_idle_timeout_ms: 0,
659+
pool_connect_timeout_ms: 0,
660+
panic_threshold_percent: 0,
661+
max_requests_per_connection: 0,
662+
request_queue_depth: 0,
663+
request_queue_timeout_ms: 0,
664+
subset_size: 0,
665+
remote_endpoints: Vec::new(),
666+
})
667+
.await;
666668

667669
let handler = make_handler(config.clone());
668670
let mgr = ListenerManager::new(config.clone(), handler);

dataplane/novaedge-dataplane/src/middleware/auth/forward.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,12 @@ fn is_denied_ip(ip: &std::net::IpAddr) -> bool {
155155
|| (v4.octets()[0] == 169 && v4.octets()[1] == 254) // 169.254.0.0/16 (link-local / cloud metadata)
156156
}
157157
std::net::IpAddr::V6(v6) => {
158+
// Detect IPv4-mapped IPv6 (::ffff:x.x.x.x) and check the inner v4 address.
159+
if let Some(v4) = v6.to_ipv4_mapped() {
160+
return is_denied_ip(&std::net::IpAddr::V4(v4));
161+
}
158162
v6.is_loopback() // ::1
159-
|| v6.octets()[0] == 0xfd // fd00::/8 (ULA)
163+
|| (v6.octets()[0] & 0xfe) == 0xfc // fc00::/7 (ULA)
160164
|| v6.octets()[0] == 0xfe && (v6.octets()[1] & 0xc0) == 0x80 // fe80::/10 (link-local)
161165
}
162166
}
@@ -264,6 +268,17 @@ mod tests {
264268
assert!(is_denied_ip(&IpAddr::V6(Ipv6Addr::new(
265269
0xfd00, 0, 0, 0, 0, 0, 0, 1
266270
))));
271+
// fc00::/8 (ULA — the other half of fc00::/7)
272+
assert!(is_denied_ip(&IpAddr::V6(Ipv6Addr::new(
273+
0xfc00, 0, 0, 0, 0, 0, 0, 1
274+
))));
275+
// IPv4-mapped IPv6 addresses must be checked against the v4 denylist
276+
assert!(is_denied_ip(&IpAddr::V6(Ipv6Addr::new(
277+
0, 0, 0, 0, 0, 0xffff, 0x7f00, 0x0001 // ::ffff:127.0.0.1
278+
))));
279+
assert!(is_denied_ip(&IpAddr::V6(Ipv6Addr::new(
280+
0, 0, 0, 0, 0, 0xffff, 0xa9fe, 0xa9fe // ::ffff:169.254.169.254
281+
))));
267282

268283
// Allowed ranges
269284
assert!(!is_denied_ip(&IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8))));

dataplane/novaedge-dataplane/src/proxy/handler.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ impl ProxyHandler {
351351
rewrite_path: None,
352352
add_headers: HashMap::new(),
353353
middleware_refs: Vec::new(),
354+
rr_counter: std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)),
354355
}
355356
} else {
356357
debug!("No route matched for {method} {host}{path}");
@@ -682,7 +683,7 @@ impl ProxyHandler {
682683
let backend_addr = SocketAddr::new(selected.addr, selected.port);
683684

684685
// Pool acquire.
685-
let _pool_guard = match self.connection_pool.acquire(backend_addr).await {
686+
let mut pool_guard = match self.connection_pool.acquire(backend_addr).await {
686687
Ok(guard) => guard,
687688
Err(e) => {
688689
warn!(
@@ -841,6 +842,7 @@ impl ProxyHandler {
841842
cb.record_failure();
842843
outlier_detector.record_failure(backend_addr);
843844
balancer.report(backend_idx, upstream_start.elapsed(), false);
845+
pool_guard.mark_released();
844846
self.connection_pool.release(backend_addr).await;
845847
if attempt < max_attempts - 1
846848
&& is_retryable_condition("reset", &retry_on)
@@ -857,6 +859,7 @@ impl ProxyHandler {
857859
}
858860
};
859861

862+
pool_guard.mark_released();
860863
self.connection_pool.release(backend_addr).await;
861864

862865
final_status = status;
@@ -881,6 +884,7 @@ impl ProxyHandler {
881884
cb.record_failure();
882885
outlier_detector.record_failure(backend_addr);
883886
balancer.report(backend_idx, upstream_latency, false);
887+
pool_guard.mark_released();
884888
self.connection_pool.release(backend_addr).await;
885889
self.connection_pool
886890
.record_connection_failure(backend_addr)
@@ -1116,6 +1120,7 @@ impl ProxyHandler {
11161120
rewrite_path: None,
11171121
add_headers: HashMap::new(),
11181122
middleware_refs: Vec::new(),
1123+
rr_counter: std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)),
11191124
}
11201125
} else {
11211126
return Err(hyper::Response::builder()
@@ -1544,6 +1549,7 @@ mod tests {
15441549
rewrite_path: None,
15451550
add_headers: HashMap::new(),
15461551
middleware_refs: Vec::new(),
1552+
rr_counter: std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)),
15471553
}
15481554
}
15491555

@@ -1623,7 +1629,8 @@ mod tests {
16231629
request_queue_timeout_ms: 0,
16241630
subset_size: 0,
16251631
remote_endpoints: Vec::new(),
1626-
});
1632+
})
1633+
.await;
16271634

16281635
let _handler = test_handler(router, cfg.clone());
16291636
let cluster = cfg.get_cluster("test-cluster").unwrap();
@@ -1715,7 +1722,8 @@ mod tests {
17151722
request_queue_timeout_ms: 0,
17161723
subset_size: 0,
17171724
remote_endpoints: Vec::new(),
1718-
});
1725+
})
1726+
.await;
17191727

17201728
let handler = Arc::new(test_handler(router, cfg));
17211729

dataplane/novaedge-dataplane/src/proxy/router.rs

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use std::collections::HashMap;
2+
use std::sync::atomic::{AtomicU64, Ordering};
3+
use std::sync::Arc;
24

35
/// A route configuration for the HTTP proxy.
4-
#[derive(Debug, Clone)]
6+
#[derive(Clone)]
57
pub struct Route {
68
pub name: String,
79
pub hostnames: Vec<HostMatch>,
@@ -15,13 +17,35 @@ pub struct Route {
1517
pub rewrite_path: Option<String>,
1618
pub add_headers: HashMap<String, String>,
1719
pub middleware_refs: Vec<String>,
20+
/// Per-route counter for deterministic weighted round-robin backend selection.
21+
pub rr_counter: Arc<AtomicU64>,
22+
}
23+
24+
impl std::fmt::Debug for Route {
25+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
26+
f.debug_struct("Route")
27+
.field("name", &self.name)
28+
.field("hostnames", &self.hostnames)
29+
.field("paths", &self.paths)
30+
.field("methods", &self.methods)
31+
.field("headers", &self.headers)
32+
.field("query_params", &self.query_params)
33+
.field("backend_refs", &self.backend_refs)
34+
.field("priority", &self.priority)
35+
.field("rewrite_path", &self.rewrite_path)
36+
.field("add_headers", &self.add_headers)
37+
.field("middleware_refs", &self.middleware_refs)
38+
.field("rr_counter", &self.rr_counter.load(Ordering::Relaxed))
39+
.finish()
40+
}
1841
}
1942

2043
impl Route {
21-
/// Select a backend cluster name using weighted random selection.
44+
/// Select a backend cluster name using deterministic weighted round-robin.
2245
///
23-
/// If there is only one backend, returns it directly. If multiple, picks
24-
/// one randomly proportional to weights.
46+
/// If there is only one backend, returns it directly. If multiple, uses
47+
/// a per-route atomic counter to cycle through backends proportional to
48+
/// their weights. This is deterministic and thread-safe.
2549
pub fn select_backend(&self) -> Option<&str> {
2650
match self.backend_refs.len() {
2751
0 => None,
@@ -31,21 +55,13 @@ impl Route {
3155
if total_weight == 0 {
3256
return Some(&self.backend_refs[0].0);
3357
}
34-
let mut rng_val = {
35-
// Use a fast inline xorshift for weight selection to avoid
36-
// pulling in rand as a dependency just for this.
37-
use std::time::SystemTime;
38-
let seed = SystemTime::now()
39-
.duration_since(SystemTime::UNIX_EPOCH)
40-
.unwrap_or_default()
41-
.subsec_nanos();
42-
seed % total_weight
43-
};
58+
let slot = self.rr_counter.fetch_add(1, Ordering::Relaxed) % total_weight as u64;
59+
let mut remaining = slot as u32;
4460
for (name, weight) in &self.backend_refs {
45-
if rng_val < *weight {
61+
if remaining < *weight {
4662
return Some(name);
4763
}
48-
rng_val -= weight;
64+
remaining -= weight;
4965
}
5066
// Fallback (shouldn't reach here).
5167
Some(&self.backend_refs[0].0)
@@ -303,6 +319,7 @@ mod tests {
303319
rewrite_path: None,
304320
add_headers: HashMap::new(),
305321
middleware_refs: Vec::new(),
322+
rr_counter: Arc::new(AtomicU64::new(0)),
306323
}
307324
}
308325

0 commit comments

Comments
 (0)